From 596ee7a0c027749e169f7e19182f9f9521d3fdf3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 03:39:41 +0000 Subject: [PATCH 01/14] fix(plugin-approvals)!: an approval node refuses to open when a team, position or queue directory read fails; queue reads page to completion Part 1 of #29604 (seat ruling 6648946388, route A). - plugin-approvals: a failed directory read behind a team, position or queue approver (and an expression's resolveAs team/position value) throws ApproverDirectoryUnreadableError, so openNodeRequest opens on no slate under any onEmptyApprovers policy, the fallback slate included. decide's pre-snapshot re-resolution keeps the literal slot. - plugin-sharing: the queue service's authored sys_queue / sys_queue_member reads go through the shared keyset walk instead of a 10,000-row cut; a failed read keeps today's answer until part 2. Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh Co-authored-by: Claude --- .../plugin-approvals/src/approval-service.ts | 142 +++++++++++++++++- .../src/ownership-queue-service.ts | 65 +++++--- 2 files changed, 178 insertions(+), 29 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 6fb82eb5804..4f6ecd0084e 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -1163,6 +1163,73 @@ function holderReadRefusal(scope: string, err: unknown): Error { ), { cause: err }); } +/** + * [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] The + * approver types whose failed directory read refuses the approval node. A + * `queue`, `team` or `position` approver names a SET of people read from the + * directory at node entry; when that read fails, the slate it would build is + * missing people nobody can name, and every `onEmptyApprovers` policy would act + * on that wrong slate — open on the rest of it, open on the administrators or on + * a fallback, or approve. So the node opens on none of them. + * + * An `expression` approver's `resolveAs: 'team' | 'position'` values are read + * through the same expanders, and refuse the same way ({@link + * ApprovalService.resolveExpressionApprovers}). The other graph kinds + * (`department` and its dialects, `org_membership_level`) are not named by the + * ruling and keep their literal slot. + */ +const DIRECTORY_REFUSING_APPROVER_TYPES: ReadonlySet = new Set(['queue', 'team', 'position']); + +/** + * [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] The + * approval node refuses to open because a directory read behind one of its + * approvers failed ({@link DIRECTORY_REFUSING_APPROVER_TYPES}). + * + * Its own class, and never `NO_APPROVERS` or "expanded to nobody": the slate + * was not found empty, it could not be read. The message names the approver + * and the read that failed, and starts with `APPROVER_DIRECTORY_UNREADABLE:` in + * the message-prefix family this service's refusals use. Retryable: nothing is + * written before it is thrown, so the run can be started again once the + * directory reads. No parked or auto-retry state is part of it. + */ +export class ApproverDirectoryUnreadableError extends Error { + override readonly name = 'ApproverDirectoryUnreadableError'; + /** The run can be re-triggered once the directory reads again. */ + readonly retryable = true; + /** The approver whose directory read failed: its type and value as authored. */ + readonly approverType: string; + readonly approverValue: string; + /** What could not be read, as the read reported it. */ + readonly failedRead: string; + + constructor(input: { + type: string; + value: unknown; + organizationId: string | null | undefined; + failedRead: string; + /** The CEL source, when the approver is an `expression` resolved as `type`. */ + expression?: string; + cause?: unknown; + }) { + const approver = input.expression === undefined + ? `approver '${input.type}:${input.value}'` + : `approver 'expression' (resolveAs '${input.type}', value '${input.value}')`; + const inOrg = input.organizationId ? ` in organization '${input.organizationId}'` : ''; + const read = input.failedRead.trim(); + super( + `APPROVER_DIRECTORY_UNREADABLE: ${approver} could not be expanded${inOrg}, because a read behind it failed: ` + + `${read}${/[.!?]$/.test(read) ? '' : '.'} The approval node does not open: the people who could not be ` + + `read would be routed past, so the request is not opened on the rest of the slate, on the declared ` + + `fallbackApprovers or on the organization's administrators, and is not auto-approved, whatever the node's ` + + `onEmptyApprovers says. Run the flow again once the directory can be read.`, + ); + if (input.cause !== undefined) Object.assign(this, { cause: input.cause }); + this.approverType = input.type; + this.approverValue = String(input.value); + this.failedRead = input.failedRead; + } +} + /** * Approver types resolved by QUERYING a graph rather than by taking `value` * literally (#3807). Each can legitimately come back empty — an unstaffed @@ -2432,9 +2499,19 @@ export class ApprovalService implements IApprovalService { * as `__resolvedFrom` so "why these people" stays answerable later. */ resolvedFrom?: Record; + /** + * [ADR-0142 D6 as amended] What a failed directory read behind a + * `queue` / `team` / `position` approver does: `'refuse'` (the default) + * throws {@link ApproverDirectoryUnreadableError}, so a node never opens on + * a slate it could not read; `'literal'` keeps the slot's `type:value` + * literal, for the one reader that re-resolves a slate AFTER a decision + * was recorded and must not fail it (`decide`'s pre-snapshot tally). + */ + onDirectoryFailure?: 'refuse' | 'literal'; }, ): Promise { if (!step || !Array.isArray(step.approvers)) return []; + const onDirectoryFailure = opts?.onDirectoryFailure ?? 'refuse'; const now = opts?.now ?? this.clock.now().getTime(); const out: string[] = []; // A person is a member of a per_group group once (#28684): the tally sizes @@ -2468,7 +2545,7 @@ export class ApprovalService implements IApprovalService { // contract of resolveApproverSpec cannot carry. if (canonicalApproverType(String(a.type)) === 'expression') { const resolved = await this.resolveExpressionApprovers( - a, record, organizationId, now, opts?.substitutions, opts?.exprCtx, + a, record, organizationId, now, opts?.substitutions, opts?.exprCtx, onDirectoryFailure, ); if (opts?.resolvedFrom) opts.resolvedFrom[`expression#${idx}`] = resolved.raw; for (const entry of resolved.slots) { @@ -2484,7 +2561,7 @@ export class ApprovalService implements IApprovalService { if (opts?.resolvedFrom && canonicalApproverType(String(a.type)) === 'field' && a.value != null) { opts.resolvedFrom[`field:${a.value}`] = (record as any)?.[a.value] ?? null; } - const ids = await this.resolveApproverSpec(a, record, organizationId, now, opts?.substitutions); + const ids = await this.resolveApproverSpec(a, record, organizationId, now, opts?.substitutions, onDirectoryFailure); // per_group (#3266): tag each resolved id with this spec's group. for (const u of ids) { if (!u) continue; @@ -2513,6 +2590,7 @@ export class ApprovalService implements IApprovalService { organizationId: string | null | undefined, now: number, substitutions?: OooSubstitution[], + onDirectoryFailure: 'refuse' | 'literal' = 'refuse', ): Promise { // ADR-0090 D3: `role` is the deprecated spelling of `org_membership_level`. // Resolve on the canonical type, and write the canonical type in the @@ -2602,14 +2680,25 @@ export class ApprovalService implements IApprovalService { // // A cross-org slate is filtered to the people who can actually READ the // request (D2 union); same-org routing is untouched and does no extra read. - const bounded = async (users: string[]): Promise => ( - crossOrg - ? filterApproversWhoCanRead(this.orgScopeDeps, users, organizationId, { + // + // [#29604] A failure of the read filter itself is named as that read, not + // as the directory's, so the warning and the node's refusal below say which + // read could not be done. + const bounded = async (users: string[]): Promise => { + if (!crossOrg) return users; + try { + return await filterApproversWhoCanRead(this.orgScopeDeps, users, organizationId, { approverType: type, value: a.value != null ? String(a.value) : undefined, directoryOrgId: directoryOrg, - }) - : users - ); + }); + } catch (err) { + throw Object.assign(new Error( + `the cross-organization read filter (who among the people of organization '${directoryOrg}' can read ` + + `this request of organization '${organizationId}') could not be read: ` + + `${err instanceof Error ? err.message : String(err)}.`, + ), { cause: err }); + } + }; // The user a `manager` approver resolved FROM, kept for the warning below // when no manager comes back. @@ -2659,6 +2748,17 @@ export class ApprovalService implements IApprovalService { // A directory lookup failed → handled below as resolving to nobody, and // the graph-type warning names the failure (#29362). directoryFailure = err instanceof Error ? err.message : String(err); + // [ADR-0142 D6 as amended; ruling record 6599731698, letter B] …unless + // the approver names a directory SET: then the node refuses to open, + // before any `onEmptyApprovers` policy can act on a slate missing the + // people who could not be read. Ahead of the `queue` branch's "expanded to + // nobody" return below, which would otherwise answer an unreadable queue + // as an empty one. + if (onDirectoryFailure === 'refuse' && DIRECTORY_REFUSING_APPROVER_TYPES.has(type)) { + throw new ApproverDirectoryUnreadableError({ + type, value: a.value, organizationId: directoryOrg, failedRead: directoryFailure, cause: err, + }); + } } // #22558: a PERSON type that resolved to nobody contributes NO slot — see // PERSON_APPROVER_TYPES for why its `type:value` literal could never be @@ -2695,7 +2795,19 @@ export class ApprovalService implements IApprovalService { // are read at node entry, so a member who joins later is not owed this // request. A `queue:` slot a stored request already holds stays as stored, // decidable by the override (the ADR-0090 D3 `role:` precedent). + // + // A queue whose read FAILED reaches here only on the `'literal'` reader (a + // refusing one threw above); it still writes no slot, and its warning says + // the read failed rather than that the queue is empty. if (type === 'queue') { + if (directoryFailure !== undefined) { + this.logger?.warn?.( + `[approvals] approver 'queue:${a.value}' could not be expanded — a directory or read-filter lookup failed. ` + + `It adds no slot to this slate, as an empty queue adds none; fix the read (see error).`, + { type, value: a.value ?? null, organizationId: directoryOrg ?? null, error: directoryFailure }, + ); + return []; + } this.logger?.warn?.( `[approvals] approver 'queue:${a.value}' expanded to nobody — ` + (this.queueSource() @@ -2797,6 +2909,7 @@ export class ApprovalService implements IApprovalService { now: number, substitutions?: OooSubstitution[], exprCtx?: ApproverExpressionContext, + onDirectoryFailure: 'refuse' | 'literal' = 'refuse', ): Promise<{ slots: Array<{ id: string; subGroup?: string }>; raw: string[] }> { const source = String(a.value ?? '').trim(); if (!source) { @@ -2916,6 +3029,15 @@ export class ApprovalService implements IApprovalService { } } catch (err: any) { if (String(err?.message ?? '').startsWith('VALIDATION_FAILED')) throw err; + // [ADR-0142 D6 as amended; ruling record 6599731698, letter B] A + // `team` or `position` value is read through the same expanders as the + // static types, and refuses the node the same way. + if (onDirectoryFailure === 'refuse' && DIRECTORY_REFUSING_APPROVER_TYPES.has(resolveAs)) { + throw new ApproverDirectoryUnreadableError({ + type: resolveAs, value: key, organizationId: directoryOrg, + failedRead: err instanceof Error ? err.message : String(err), expression: source, cause: err, + }); + } // [#29606] The literal slot below is the same fallback the static graph // types take, and it says why exactly as they do: a failed read is not // an unstaffed value, so an operator is pointed at the read rather than @@ -4985,8 +5107,12 @@ export class ApprovalService implements IApprovalService { groupMap = snapshotGroups; original = Object.keys(snapshotGroups); } else { + // The decision is already recorded above, so a directory outage here + // must not fail it: the legacy re-resolution keeps the literal slot it + // always kept (ADR-0142 D6's refusal is the node's, at open time). original = await this.expandApprovers( { approvers: config.approvers }, parseJson(raw.payload_json, undefined), org, + { onDirectoryFailure: 'literal' }, ); groupMap = {}; } diff --git a/packages/plugins/plugin-sharing/src/ownership-queue-service.ts b/packages/plugins/plugin-sharing/src/ownership-queue-service.ts index 2fad5375487..93809415a47 100644 --- a/packages/plugins/plugin-sharing/src/ownership-queue-service.ts +++ b/packages/plugins/plugin-sharing/src/ownership-queue-service.ts @@ -37,6 +37,18 @@ * read of the activation ledger leaves the declared default — on — for that * call and is logged, the posture the sharing-rule service takes for the same * ledger. + * + * ## Read to completion + * + * [#29604] The authored reads — an organization's `sys_queue` rows and its + * `sys_queue_member` rows — are read to completion through the keyset walk the + * team, position and business-unit member reads share + * (`readMembershipToCompletion`), never cut at a row bound: a queue past the + * bound lost its later members, a member past it lost every queue it sits in, + * and nothing said so. A read that fails, or a walk that cannot vouch for + * completion, takes the failed-read answer above. ADR-0142 D2 as amended has + * the recompute consumers (D6, D7) refuse a failed read instead; that answer + * waits on the contract carrying the caller's call-time signal. */ import type { @@ -49,7 +61,7 @@ import { QueueMemberType } from '@objectstack/spec/security'; import { METADATA_ACTIVATION_TABLE } from '@objectstack/core'; import type { OptionalSharingLogger } from './logger-shapes.js'; import type { SharingEngine } from './sharing-service.js'; -import { TeamGraphService, teamHasMember } from './team-graph.js'; +import { TeamGraphService, readMembershipToCompletion, teamHasMember } from './team-graph.js'; import { PositionGraphService } from './position-graph.js'; import { BusinessUnitGraphService } from './business-unit-graph.js'; @@ -62,9 +74,6 @@ export const QUEUE_MEMBER_OBJECT = 'sys_queue_member'; /** The `sys_metadata_activation.metadata_type` a declared queue is switched by. */ export const QUEUE_ACTIVATION_TYPE = 'queue'; -/** Upper bound on one organization's authored queues / member rows read in one call. */ -const QUEUE_READ_LIMIT = 10000; - /** The member kinds a specification may carry, as a set for the row guard. */ const MEMBER_KINDS: ReadonlySet = new Set(QueueMemberType.options); @@ -108,6 +117,19 @@ function isRecord(v: unknown): v is Record { return v !== null && typeof v === 'object' && !Array.isArray(v); } +/** + * The reason behind a failed authored read, for a warning that answers nobody. + * A walk's refusal words its own consequence ("so the expansion refuses"), + * which is not this service's answer yet, so the driver's reason it carries as + * `cause` is logged instead; a walk that could not vouch for completion has no + * cause and logs its own sentence. + */ +function readFailureReason(err: unknown): string { + const cause = (err as { cause?: unknown } | null)?.cause; + if (cause instanceof Error) return cause.message; + return err instanceof Error ? err.message : String(err); +} + /** * One call's memo of the reads several specifications share: a position's * activation and holding, a team's membership, a unit's expansion. Scoped to a @@ -268,22 +290,22 @@ export class OwnershipQueueService implements IOwnershipQueueService { } } try { - const rows = await engine.find(QUEUE_MEMBER_OBJECT, { + const rows = await readMembershipToCompletion(engine, { + object: QUEUE_MEMBER_OBJECT, where: { recipient_type: kind, ...(organizationId ? { organization_id: organizationId } : {}), }, fields: ['queue', 'recipient_id'], - limit: QUEUE_READ_LIMIT, - context: SYSTEM_CTX, + subject: `the '${kind}' member rows of the queues${organizationId ? ` in organization '${organizationId}'` : ''}`, }); - for (const r of (Array.isArray(rows) ? rows : []) as Array>) { + for (const r of rows ?? []) { const queue = str(r.queue); const value = str(r.recipient_id); if (queue && value && (BU_MEMBER_KINDS.has(kind) || values.has(value))) out.add(queue); } } catch (err: any) { - this.warnOnce(`naming::${kind}`, '[queue] member read failed while looking through queues — the rules naming those queues are re-evaluated on their next write', { kind, error: err?.message }); + this.warnOnce(`naming::${kind}`, '[queue] member read failed while looking through queues — the rules naming those queues are re-evaluated on their next write', { kind, error: readFailureReason(err) }); } return [...out].sort(); } @@ -388,8 +410,9 @@ export class OwnershipQueueService implements IOwnershipQueueService { * * The checks themselves (`teamHasMember`, `PositionGraphService.holdsPosition`) * now refuse a failed read instead of answering `false`; this keeps the - * queue's consumers where they were. Whether a queue should refuse too is not - * decided by ADR-0142 and is left to a ruling rather than narrowed here. + * queue's consumers where they were. ADR-0142 D2 as amended (#29604) has the + * recompute consumers refuse instead; the call-time signal they say so with + * is the contract's to carry, and until it does every caller gets this answer. */ private async holderCheck(spec: QueueMemberSpec, check: () => Promise): Promise { try { @@ -442,14 +465,14 @@ export class OwnershipQueueService implements IOwnershipQueueService { declaredNames: ReadonlySet, ): Promise { try { - const rows = await engine.find(QUEUE_OBJECT, { + const rows = await readMembershipToCompletion(engine, { + object: QUEUE_OBJECT, where: organizationId ? { organization_id: organizationId } : {}, fields: ['name'], - limit: QUEUE_READ_LIMIT, - context: SYSTEM_CTX, + subject: `the authored queues${organizationId ? ` of organization '${organizationId}'` : ''}`, }); const out: string[] = []; - for (const r of (Array.isArray(rows) ? rows : []) as Array>) { + for (const r of rows ?? []) { const name = str(r.name); // A row under a declared name is not a second queue (the guard refuses // the write; a row that predates the declaration is reported there). @@ -457,7 +480,7 @@ export class OwnershipQueueService implements IOwnershipQueueService { } return out; } catch (err: any) { - this.warnOnce('read::sys_queue', '[queue] sys_queue read failed — authored queues resolve nowhere until it succeeds', { error: err?.message }); + this.warnOnce('read::sys_queue', '[queue] sys_queue read failed — authored queues resolve nowhere until it succeeds', { error: readFailureReason(err) }); return []; } } @@ -471,16 +494,16 @@ export class OwnershipQueueService implements IOwnershipQueueService { const out = new Map(); if (names.length === 0) return out; try { - const rows = await engine.find(QUEUE_MEMBER_OBJECT, { + const rows = await readMembershipToCompletion(engine, { + object: QUEUE_MEMBER_OBJECT, where: { queue: names.length === 1 ? names[0] : { $in: [...names] }, ...(organizationId ? { organization_id: organizationId } : {}), }, fields: ['queue', 'recipient_type', 'recipient_id'], - limit: QUEUE_READ_LIMIT, - context: SYSTEM_CTX, + subject: `the member rows of ${names.length === 1 ? `queue '${names[0]}'` : `${names.length} queues`}${organizationId ? ` in organization '${organizationId}'` : ''}`, }); - for (const r of (Array.isArray(rows) ? rows : []) as Array>) { + for (const r of rows ?? []) { const queue = str(r.queue); const type = str(r.recipient_type); const value = str(r.recipient_id); @@ -490,7 +513,7 @@ export class OwnershipQueueService implements IOwnershipQueueService { out.set(queue, list); } } catch (err: any) { - this.warnOnce('read::sys_queue_member', '[queue] sys_queue_member read failed — authored members count as nobody until it succeeds', { error: err?.message }); + this.warnOnce('read::sys_queue_member', '[queue] sys_queue_member read failed — authored members count as nobody until it succeeds', { error: readFailureReason(err) }); } return out; } From 7e2469fa5d5cf1862bc74ed478995fb1e45a7a04 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 03:55:37 +0000 Subject: [PATCH 02/14] test(plugin-approvals, plugin-sharing): pin the node refusal under every policy and the queue reads past one page The approval node refuses to open, and writes nothing, when a team, position or queue approver's directory read fails, under admin_rescue, auto_approve, fail and fallback alike, beside a staffed person, and on a fallback slate. A department failure and decide's legacy re-resolution keep their answers. The queue reads find the 10,001st member, queue and naming row, and a read that fails on its second page answers as a failed read does today. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- .../src/approval-service.test.ts | 136 +++++++++++++++++- ...-unit-holders-complete.integration.test.ts | 18 ++- ...-tier-holders-complete.integration.test.ts | 32 +++-- ...ition-holders-complete.integration.test.ts | 37 +++-- .../src/ownership-queue.test.ts | 93 +++++++++++- 5 files changed, 281 insertions(+), 35 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 4862830550c..377b666c36d 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -11,7 +11,9 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { APPROVAL_REVISE_NODE_TYPE, templateSlotSource } from '@objectstack/spec/automation'; -import { ApprovalService, REMIND_COOLDOWN_MS, ESCALATION_ENABLED_FLIP_CUTOFF_MS, SLA_ACTOR_ID } from './approval-service.js'; +import { + ApprovalService, ApproverDirectoryUnreadableError, REMIND_COOLDOWN_MS, ESCALATION_ENABLED_FLIP_CUTOFF_MS, SLA_ACTOR_ID, +} from './approval-service.js'; import { bindApprovalLockHook, bindDelegationWriteGuard, unbindAllHooks } from './lifecycle-hooks.js'; import { celEngine } from '@objectstack/formula'; import { SysApprovalRequest } from './sys-approval-request.object.js'; @@ -4207,6 +4209,138 @@ describe('ApprovalService — a queue approver routes to the queue\'s members (A }); }); +// [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] A +// directory read behind a `team`, `position` or `queue` approver that FAILS is +// not an empty slate: the node refuses to open under every `onEmptyApprovers` +// policy, on its own retryable error class, naming the read — and writes +// nothing. Before, the slot fell back to its literal and the policy acted on a +// slate missing the people who could not be read. +describe('ApprovalService — a team, position or queue approver whose directory read fails refuses the node (ADR-0142 D6)', () => { + const POLICIES = ['admin_rescue', 'auto_approve', 'fail', 'fallback'] as const; + + /** A service over `engine` whose reads of `failing` throw, as a driver outage does. */ + const outage = (failing: string[], queues?: () => any) => { + const engine = makeFakeEngine(); + engine._tables['sys_team_member'] = [{ id: 'tm1', team_id: 't_ops', user_id: 'u5' }]; + engine._tables['sys_user_position'] = [{ id: 'up1', position: 'cfo', user_id: 'u9', organization_id: 't1' }]; + const real = engine.find.bind(engine); + engine.find = async (object: string, options?: any) => { + if (failing.includes(object)) throw new Error(`SQLITE_IOERR: disk I/O error reading ${object}`); + return real(object, options); + }; + const warnings: any[] = []; + let n = 0; + const svc = new ApprovalService({ + engine, + clock: { now: () => new Date(1757000000000 + (n++) * 1000) }, + logger: { warn: (msg: any, meta: any) => warnings.push([msg, meta]) }, + ...(queues ? { ownershipQueues: queues } : {}), + }); + return { engine, svc, warnings }; + }; + const node = (approvers: any[], configExtra: Record = {}) => ({ + ...openInput([]), + config: { approvers, behavior: 'first_response' as const, lockRecord: false, ...configExtra }, + }); + const policy = (p: (typeof POLICIES)[number]) => ( + p === 'fallback' ? { onEmptyApprovers: p, fallbackApprovers: [{ type: 'user', value: 'u_backstop' }] } : { onEmptyApprovers: p } + ); + const refusalOf = (p: Promise) => p.then(() => null, (e: unknown) => e); + const nothingWritten = (engine: ReturnType) => { + expect(engine._tables['sys_approval_request'] ?? []).toEqual([]); + expect(engine._tables['sys_approval_approver'] ?? []).toEqual([]); + expect(engine._tables['sys_approval_action'] ?? []).toEqual([]); + }; + + for (const [type, value, table] of [['team', 't_ops', 'sys_team_member'], ['position', 'cfo', 'sys_user_position']] as const) { + for (const p of POLICIES) { + it(`a ${type} approver whose ${table} read fails: onEmptyApprovers '${p}' opens nothing and approves nothing`, async () => { + const { engine, svc } = outage([table]); + const refusal = await refusalOf(svc.openNodeRequest(node([{ type, value }], policy(p)), CTX)); + + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: type, approverValue: value, retryable: true }); + expect((refusal as Error).message).toMatch(new RegExp(`^APPROVER_DIRECTORY_UNREADABLE: approver '${type}:${value}'`)); + expect((refusal as ApproverDirectoryUnreadableError).failedRead).toMatch(/SQLITE_IOERR/); + expect((refusal as Error).message).not.toMatch(/NO_APPROVERS|expanded to nobody/); + nothingWritten(engine); + }); + } + } + + it('beside a staffed person, the node still refuses: no slate shortened to the people who could be read', async () => { + const { engine, svc } = outage(['sys_team_member']); + const refusal = await refusalOf(svc.openNodeRequest( + node([{ type: 'user', value: 'u2' }, { type: 'team', value: 't_ops' }]), CTX, + )); + + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + nothingWritten(engine); + }); + + it('a fallback slate whose directory read fails refuses too, rather than degrade to the administrators', async () => { + // The primary slate reads fine and is empty (an unstaffed position); the + // declared fallback names a team whose read fails. + const { engine, svc } = outage(['sys_team_member']); + const refusal = await refusalOf(svc.openNodeRequest(node([{ type: 'position', value: 'nobody_holds_this' }], { + onEmptyApprovers: 'fallback', fallbackApprovers: [{ type: 'team', value: 't_ops' }], + }), CTX)); + + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: 'team', approverValue: 't_ops' }); + nothingWritten(engine); + }); + + it('a queue whose expansion fails refuses before its "expanded to nobody" answer', async () => { + const { engine, svc, warnings } = outage([], () => ({ + expandMembers: async () => { throw new Error('ownership-queue read failed'); }, + })); + const refusal = await refusalOf(svc.openNodeRequest(node([{ type: 'queue', value: 'billing' }]), CTX)); + + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: 'queue', approverValue: 'billing' }); + expect(warnings.some(([msg]) => String(msg).includes("approver 'queue:billing' expanded to nobody"))).toBe(false); + nothingWritten(engine); + }); + + it('control — the readable slate opens as before', async () => { + const { svc } = outage([]); + const req = await svc.openNodeRequest(node([{ type: 'team', value: 't_ops' }, { type: 'position', value: 'cfo' }]), CTX) as any; + + expect(req.status).toBe('pending'); + expect([...req.pending_approvers].sort()).toEqual(['u5', 'u9']); + }); + + it('control — a department read that fails is not named by the ruling: the node still opens, as before', async () => { + const { engine, svc } = outage(['sys_business_unit', 'sys_business_unit_member']); + const outcome = await refusalOf(svc.openNodeRequest( + node([{ type: 'department', value: 'bu_x' }], { onEmptyApprovers: 'fallback', fallbackApprovers: [{ type: 'user', value: 'u_backstop' }] }), + CTX, + )); + + expect(outcome).toBeNull(); + expect(engine._tables['sys_approval_request']).toHaveLength(1); + }); + + it("decide's pre-snapshot re-resolution keeps the literal: a recorded decision is never failed by the outage", async () => { + // A unanimous request opened while the directory read, then stripped of + // its open-time snapshot as a request opened before the snapshot existed. + const failing: string[] = []; + const { engine, svc } = outage(failing); + const req = await svc.openNodeRequest( + node([{ type: 'user', value: 'u2' }, { type: 'team', value: 't_ops' }], { behavior: 'unanimous' }), CTX, + ) as any; + const row = engine._tables['sys_approval_request'].find((r: any) => r.id === req.id); + const config = JSON.parse(row.node_config_json); + delete config.__approverGroups; + row.node_config_json = JSON.stringify(config); + + failing.push('sys_team_member'); + const decided = await svc.decideNode(req.id, { decision: 'approve', actorId: 'u2' }, SYS); + expect(decided.decision).toBe('approve'); + }); +}); + // #3807 follow-up: `queue` was not the only way to end up with a slot nobody // can act on — every GRAPH approver type falls back to the same literal when // its lookup finds no one, and that fallback used to happen in total silence. diff --git a/packages/plugins/plugin-approvals/src/business-unit-holders-complete.integration.test.ts b/packages/plugins/plugin-approvals/src/business-unit-holders-complete.integration.test.ts index 45da10bbfd0..b8896b9dd88 100644 --- a/packages/plugins/plugin-approvals/src/business-unit-holders-complete.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/business-unit-holders-complete.integration.test.ts @@ -30,7 +30,7 @@ import { SqlDriver } from '@objectstack/driver-sql'; import { SysBusinessUnit, SysBusinessUnitMember, SysMember, SysOrganization, SysTeamMember, } from '@objectstack/platform-objects/identity'; -import { ApprovalService } from './approval-service.js'; +import { ApprovalService, ApproverDirectoryUnreadableError } from './approval-service.js'; const ORG = 'org_large'; const ELSEWHERE = 'org_elsewhere'; @@ -129,8 +129,8 @@ const department = (rig: Rig, value = ROOT) => (rig.svc as any).expandBusinessUnitUsers(value, ORG) as Promise; const resolveSpec = (rig: Rig, a: { type: string; value: string }) => (rig.svc as any).resolveApproverSpec(a, undefined, ORG, Date.now()) as Promise; -const resolveExpression = (rig: Rig, a: { type: string; value: string; resolveAs: string }) => - (rig.svc as any).resolveExpressionApprovers(a, {}, ORG, Date.now()) as Promise<{ +const resolveExpression = (rig: Rig, a: { type: string; value: string; resolveAs: string }, onDirectoryFailure?: 'refuse' | 'literal') => + (rig.svc as any).resolveExpressionApprovers(a, {}, ORG, Date.now(), undefined, undefined, onDirectoryFailure) as Promise<{ slots: Array<{ id: string; subGroup?: string }>; raw: string[]; }>; @@ -220,10 +220,18 @@ describe('[#29606] item 3 — an expression approver whose resolveAs expansion f expect(failed[0][1].error).toMatch(/SQLITE_IOERR/); }); - it('⛔ a failed team read through resolveAs warns the same way', async () => { + it('⛔ a failed team read through resolveAs refuses the node as a team approver does; the literal reader warns the same way', async () => { const rig = await boot(0); rig.failReadsOf('sys_team_member'); - const { slots } = await resolveExpression(rig, { type: 'expression', value: `'team_x'`, resolveAs: 'team' }); + // [ADR-0142 D6 as amended, #29604] The value is read through the team + // expander, so its failure refuses the node like a static `team` approver's. + const refusal = await resolveExpression(rig, { type: 'expression', value: `'team_x'`, resolveAs: 'team' }) + .then(() => null, (e: unknown) => e); + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: 'team', approverValue: 'team_x', retryable: true }); + expect((refusal as Error).message).toMatch(/^APPROVER_DIRECTORY_UNREADABLE: approver 'expression' \(resolveAs 'team', value 'team_x'\)/); + + const { slots } = await resolveExpression(rig, { type: 'expression', value: `'team_x'`, resolveAs: 'team' }, 'literal'); expect(slots).toEqual([{ id: 'team:team_x', subGroup: 'team_x' }]); const failed = failureWarnings(rig); diff --git a/packages/plugins/plugin-approvals/src/member-tier-holders-complete.integration.test.ts b/packages/plugins/plugin-approvals/src/member-tier-holders-complete.integration.test.ts index 47d16e816ef..a73e1788d5e 100644 --- a/packages/plugins/plugin-approvals/src/member-tier-holders-complete.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/member-tier-holders-complete.integration.test.ts @@ -17,12 +17,15 @@ * and an `org_membership_level` approver logged "expanded to nobody", * indistinguishable from a tier nobody holds. * - * What a caller sees now: a failed read throws out of the expansion, the - * approver-spec resolver's directory catch takes it, and the slot falls back - * to its literal (`position:

`, acted on by whoever holds the position at - * decision time; `org_membership_level:`, decided by the override) with a - * warning that names the failed read and carries its `error` — never a slate - * cut down to the part that happened to read. + * What a caller sees now: a failed read throws out of the expansion and the + * approver-spec resolver's directory catch takes it. An `org_membership_level` + * slot falls back to its literal (`org_membership_level:`, decided by the + * override) with a warning that names the failed read and carries its `error`. + * [ADR-0142 D6 as amended, #29604] A `position` approver, whose transition limb + * this read is, refuses the approval node instead + * (`ApproverDirectoryUnreadableError`); only the `'literal'` reader keeps its + * `position:

` literal. Never a slate cut down to the part that happened to + * read. * * Measured on a REAL engine (ObjectQL + SqlDriver over in-memory SQLite) * carrying the shipped `SysMember`, because the completion is a keyset read @@ -33,7 +36,7 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { SysMember, SysOrganization } from '@objectstack/platform-objects/identity'; -import { ApprovalService } from './approval-service.js'; +import { ApprovalService, ApproverDirectoryUnreadableError } from './approval-service.js'; const ORG = 'org_large'; const ELSEWHERE = 'org_elsewhere'; @@ -112,8 +115,8 @@ async function boot(n: number, opts: { registerMember?: boolean } = {}): Promise /** The private expansions, as the approver-spec resolver calls them. */ const tier = (rig: Rig, value: string) => (rig.svc as any).expandMembershipTierUsers(value, ORG) as Promise; const position = (rig: Rig, value: string) => (rig.svc as any).expandPositionUsers(value, ORG) as Promise; -const resolveSpec = (rig: Rig, a: { type: string; value: string }) => - (rig.svc as any).resolveApproverSpec(a, undefined, ORG, Date.now()) as Promise; +const resolveSpec = (rig: Rig, a: { type: string; value: string }, onDirectoryFailure?: 'refuse' | 'literal') => + (rig.svc as any).resolveApproverSpec(a, undefined, ORG, Date.now(), undefined, onDirectoryFailure) as Promise; const failMemberReads = (rig: Rig, fault: Error) => { const memberDriver = (rig.engine as any).getDriverForObject('sys_member'); @@ -168,11 +171,18 @@ describe('[#29362] approvals membership-tier expansion reads every holder (drive expect(rig.warn.mock.calls.some(([, meta]) => typeof meta?.error === 'string')).toBe(false); }); - it('⛔ what the caller sees: the slot falls back to its literal, and the warning names the failed read', async () => { + it('⛔ what the caller sees: a position refuses the node on its sys_member limb; the tier slot keeps its literal', async () => { const rig = await boot(3); failMemberReads(rig, new Error('SQLITE_IOERR: disk I/O error')); - expect(await resolveSpec(rig, { type: 'position', value: TIER })).toEqual([`position:${TIER}`]); + // [#29604] The ADR-0057 D4 transition limb is a read behind the position: + // its failure is a failed read, and the node refuses on it. + const refusal = await resolveSpec(rig, { type: 'position', value: TIER }).then(() => null, (e: unknown) => e); + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: 'position', approverValue: TIER, retryable: true }); + expect((refusal as ApproverDirectoryUnreadableError).failedRead).toMatch(/SQLITE_IOERR/); + + expect(await resolveSpec(rig, { type: 'position', value: TIER }, 'literal')).toEqual([`position:${TIER}`]); expect(await resolveSpec(rig, { type: 'org_membership_level', value: TIER })).toEqual([`org_membership_level:${TIER}`]); const failed = rig.warn.mock.calls.filter(([, meta]) => typeof meta?.error === 'string'); expect(failed.map(([, meta]) => meta.type)).toEqual(['position', 'org_membership_level']); diff --git a/packages/plugins/plugin-approvals/src/team-position-holders-complete.integration.test.ts b/packages/plugins/plugin-approvals/src/team-position-holders-complete.integration.test.ts index a1374a3cc7b..aa86ee56e00 100644 --- a/packages/plugins/plugin-approvals/src/team-position-holders-complete.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/team-position-holders-complete.integration.test.ts @@ -16,10 +16,13 @@ * - a failed `sys_user_position` read routed a `position` approver to its * `sys_member.role` holders ALONE — a shrunk slate that looks complete. * - * What a caller sees now: the expansion throws, the approver-spec resolver's - * directory catch takes it, and the slot falls back to its literal with the - * warning that names the failed read and carries its `error`. A membership - * object that does not exist is still no holders. + * What a caller sees now: the expansion throws and the approver-spec + * resolver's directory catch takes it. [ADR-0142 D6 as amended, #29604] The + * resolver then refuses the approval node (`ApproverDirectoryUnreadableError`, + * naming the read), so no `onEmptyApprovers` policy acts on the slate; only + * `decide`'s pre-snapshot re-resolution (the `'literal'` reader) still falls + * back to the literal, with the warning that names the failed read and carries + * its `error`. A membership object that does not exist is still no holders. * * Measured on a REAL engine (ObjectQL + SqlDriver over in-memory SQLite) * carrying the shipped `SysTeamMember` and `SysMember`; `sys_user_position` is @@ -31,7 +34,7 @@ import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { ObjectSchema, Field } from '@objectstack/spec/data'; import { SysMember, SysOrganization, SysTeamMember } from '@objectstack/platform-objects/identity'; -import { ApprovalService } from './approval-service.js'; +import { ApprovalService, ApproverDirectoryUnreadableError } from './approval-service.js'; const ORG = 'org_large'; const ELSEWHERE = 'org_elsewhere'; @@ -137,8 +140,8 @@ async function boot(seeded: 'team' | 'position', n: number, opts: { omit?: strin /** The private expansions and the resolver, as the request opener calls them. */ const team = (rig: Rig, value = TEAM) => (rig.svc as any).expandTeamUsers(value, ORG) as Promise; const position = (rig: Rig, value = POSITION) => (rig.svc as any).expandPositionUsers(value, ORG) as Promise; -const resolveSpec = (rig: Rig, a: { type: string; value: string }) => - (rig.svc as any).resolveApproverSpec(a, undefined, ORG, Date.now()) as Promise; +const resolveSpec = (rig: Rig, a: { type: string; value: string }, onDirectoryFailure?: 'refuse' | 'literal') => + (rig.svc as any).resolveApproverSpec(a, undefined, ORG, Date.now(), undefined, onDirectoryFailure) as Promise; const failureWarnings = (rig: Rig) => rig.warn.mock.calls.filter(([, meta]) => typeof meta?.error === 'string'); describe('[#29503] approvals team expansion reads every member (driver-sql, shipped SysTeamMember)', () => { @@ -173,11 +176,18 @@ describe('[#29503] approvals team expansion reads every member (driver-sql, ship await expect(team(rig)).rejects.toThrow(/members of team 'team_large'.*could not be read/); }); - it('⛔ what the caller sees: the team slot falls back to its literal, and the warning names the failed read', async () => { + it('⛔ what the caller sees: the node refuses, naming the failed read; the literal reader keeps the literal and warns', async () => { const rig = await boot('team', 3); rig.failReadsOf('sys_team_member'); - expect(await resolveSpec(rig, { type: 'team', value: TEAM })).toEqual([`team:${TEAM}`]); + const refusal = await resolveSpec(rig, { type: 'team', value: TEAM }).then(() => null, (e: unknown) => e); + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: 'team', approverValue: TEAM, retryable: true }); + expect((refusal as Error).message).toMatch(/^APPROVER_DIRECTORY_UNREADABLE: approver 'team:team_large'/); + expect((refusal as ApproverDirectoryUnreadableError).failedRead).toMatch(/members of team 'team_large'.*could not be read/); + expect(failureWarnings(rig)).toEqual([]); + + expect(await resolveSpec(rig, { type: 'team', value: TEAM }, 'literal')).toEqual([`team:${TEAM}`]); const failed = failureWarnings(rig); expect(failed.map(([, meta]) => meta.type)).toEqual(['team']); expect(failed[0][0]).toMatch(/could not be expanded — a directory or read-filter lookup failed/); @@ -221,14 +231,19 @@ describe('[#29503] approvals position expansion reads every assignment (driver-s await expect(position(rig)).rejects.toThrow(/assignments of position 'cfo_office'.*could not be read/); }); - it('⛔ what the caller sees: the position slot falls back to its literal, never the tier holders alone', async () => { + it('⛔ what the caller sees: the node refuses, never the tier holders alone; the literal reader keeps the literal', async () => { const rig = await boot('position', 3); await (rig.engine as any).getDriverForObject('sys_member').bulkCreate('sys_member', [ { id: 'mem_tier', user_id: 'usr_tier', organization_id: ORG, role: POSITION }, ]); rig.failReadsOf('sys_user_position'); - expect(await resolveSpec(rig, { type: 'position', value: POSITION })).toEqual([`position:${POSITION}`]); + const refusal = await resolveSpec(rig, { type: 'position', value: POSITION }).then(() => null, (e: unknown) => e); + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: 'position', approverValue: POSITION, retryable: true }); + expect((refusal as ApproverDirectoryUnreadableError).failedRead).toMatch(/assignments of position 'cfo_office'.*could not be read/); + + expect(await resolveSpec(rig, { type: 'position', value: POSITION }, 'literal')).toEqual([`position:${POSITION}`]); const failed = failureWarnings(rig); expect(failed.map(([, meta]) => meta.type)).toEqual(['position']); expect(failed[0][1].error).toMatch(/SQLITE_IOERR/); diff --git a/packages/plugins/plugin-sharing/src/ownership-queue.test.ts b/packages/plugins/plugin-sharing/src/ownership-queue.test.ts index 5370315e3f6..a148b580518 100644 --- a/packages/plugins/plugin-sharing/src/ownership-queue.test.ts +++ b/packages/plugins/plugin-sharing/src/ownership-queue.test.ts @@ -25,8 +25,9 @@ import { ruleRegrantQueue } from './rule-hooks.js'; type Row = Record; /** - * Equality, `$in`, `$or` and `$and` — every predicate shape the queue reads - * send. Any other combinator or operator is refused rather than misread. + * Equality, `$in`, `$gt`, `$or` and `$and` — every predicate shape the queue + * reads send (`$gt` is the keyset cursor of a paged read, #29604). Any other + * combinator or operator is refused rather than misread. */ function matches(row: Row, filter: any): boolean { if (!filter || typeof filter !== 'object') return true; @@ -43,6 +44,10 @@ function matches(row: Row, filter: any): boolean { const rv = row[k] ?? null; if (v != null && typeof v === 'object') { const ops = Object.keys(v as object); + if (ops.length === 1 && ops[0] === '$gt') { + if (!(rv != null && String(rv) > String((v as any).$gt))) return false; + continue; + } if (ops.length !== 1 || ops[0] !== '$in') throw new Error(`test double: operator ${ops.join(',')} on '${k}' is not implemented`); if (!(v as any).$in.includes(rv)) return false; continue; @@ -72,6 +77,9 @@ function makeEngine(opts: { if (opts.failOn?.includes(object)) throw new Error(`${object} unavailable`); const filter = options?.filter ?? options?.where; const rows = (tables[object] ?? []).filter((r) => matches(r, filter)); + for (const { field } of [...(options?.orderBy ?? [])].reverse()) { + rows.sort((a, b) => (String(a[field]) < String(b[field]) ? -1 : String(a[field]) > String(b[field]) ? 1 : 0)); + } return typeof options?.limit === 'number' ? rows.slice(0, options.limit) : rows; }, async insert(object: string, data: any) { @@ -117,11 +125,12 @@ const BASE_TABLES = (): Record => ({ { id: 'q1', name: 'billing', label: 'Billing', organization_id: 'org_a' }, { id: 'q2', name: 'billing', label: 'Billing B', organization_id: 'org_b' }, ], + // [#29604] Carrying their `id` too: the authored queue reads page by it. sys_queue_member: [ - { queue: 'billing', recipient_type: 'position', recipient_id: 'billing_clerk', organization_id: 'org_a' }, - { queue: 'billing', recipient_type: 'user', recipient_id: 'u_e', organization_id: 'org_a' }, - { queue: 'support_tier1', recipient_type: 'team', recipient_id: 't_night', organization_id: 'org_a' }, - { queue: 'billing', recipient_type: 'user', recipient_id: 'u_b_only', organization_id: 'org_b' }, + { id: 'qm_1', queue: 'billing', recipient_type: 'position', recipient_id: 'billing_clerk', organization_id: 'org_a' }, + { id: 'qm_2', queue: 'billing', recipient_type: 'user', recipient_id: 'u_e', organization_id: 'org_a' }, + { id: 'qm_3', queue: 'support_tier1', recipient_type: 'team', recipient_id: 't_night', organization_id: 'org_a' }, + { id: 'qm_4', queue: 'billing', recipient_type: 'user', recipient_id: 'u_b_only', organization_id: 'org_b' }, ], [METADATA_ACTIVATION_TABLE]: [], }); @@ -235,7 +244,7 @@ describe('[ADR-0142 D2] OwnershipQueueService — the one expander', () => { it('queuesOf lists every queue the user is in, declared and authored, ordered by name', async () => { const tables = BASE_TABLES(); - tables.sys_queue_member.push({ queue: 'billing', recipient_type: 'user', recipient_id: 'u_a', organization_id: 'org_a' }); + tables.sys_queue_member.push({ id: 'qm_5', queue: 'billing', recipient_type: 'user', recipient_id: 'u_a', organization_id: 'org_a' }); const svc = service(makeEngine({ declared: SUPPORT_DECLARED, tables })); expect(await svc.queuesOf('u_a', 'org_a')).toEqual(['billing', 'support_tier1']); expect(await svc.queuesOf('u_c', 'org_a')).toEqual(['billing']); @@ -293,6 +302,76 @@ describe('[ADR-0142 D2] OwnershipQueueService — the one expander', () => { // ─── "Does this organization have any queue at all?" ─────────────────────── +// [#29604, review record 6591670782] The queue reads paged by id to completion, +// where they read one page of 10,000 rows and answered on it: a queue's +// 10,001st member, an organization's 10,001st queue, and the queues naming a +// recipient past that bound were silently not there. A read that fails partway +// keeps the answer a failed read gives today (nobody, logged once) until the +// contract can carry a refusal (#30251) — never the first page as if complete. +describe('[ADR-0142 D2] OwnershipQueueService — the queue reads page to completion (#29604)', () => { + const PAST_ONE_PAGE = 10_001; + const pad = (i: number) => String(i).padStart(5, '0'); + const LAST = pad(PAST_ONE_PAGE - 1); + + /** `count` user members of queue `big` in org_a, keyed so the last sorts last. */ + const bigQueue = (count = PAST_ONE_PAGE): Record => ({ + ...BASE_TABLES(), + sys_queue: [{ id: 'q_big', name: 'big', label: 'Big', organization_id: 'org_a' }], + sys_queue_member: Array.from({ length: count }, (_, i) => ({ + id: `qm_${pad(i)}`, queue: 'big', recipient_type: 'user', recipient_id: `u_${pad(i)}`, organization_id: 'org_a', + })), + }); + + it('a queue past one page expands to every member, and its last member is one', async () => { + const svc = service(makeEngine({ tables: bigQueue() })); + const members = await svc.expandMembers('big', 'org_a'); + expect(members).toHaveLength(PAST_ONE_PAGE); + expect(members.at(-1)).toBe(`u_${LAST}`); + expect(await svc.isMember(`u_${LAST}`, 'big', 'org_a')).toBe(true); + expect(await svc.queuesOf(`u_${LAST}`, 'org_a')).toEqual(['big']); + }); + + it('an organization past one page of queues lists its last queue for that queue\'s member', async () => { + const tables = BASE_TABLES(); + tables.sys_queue = Array.from({ length: PAST_ONE_PAGE }, (_, i) => ({ + id: `q_${pad(i)}`, name: `queue_${pad(i)}`, label: `Queue ${i}`, organization_id: 'org_a', + })); + tables.sys_queue_member = [ + { id: 'qm_last', queue: `queue_${LAST}`, recipient_type: 'user', recipient_id: 'u_z', organization_id: 'org_a' }, + ]; + const svc = service(makeEngine({ tables })); + expect(await svc.queuesOf('u_z', 'org_a')).toEqual([`queue_${LAST}`]); + expect(await svc.hasAnyQueue('org_a')).toBe(true); + }); + + it('the queues naming a recipient are found past one page of member rows', async () => { + const tables = BASE_TABLES(); + tables.sys_queue_member = Array.from({ length: PAST_ONE_PAGE }, (_, i) => ({ + id: `qm_${pad(i)}`, queue: `queue_${pad(i)}`, recipient_type: 'position', + recipient_id: i === PAST_ONE_PAGE - 1 ? 'closer' : 'filler', organization_id: 'org_a', + })); + const svc = service(makeEngine({ tables })); + expect(await svc.queuesNaming('position', new Set(['closer']), 'org_a')).toEqual([`queue_${LAST}`]); + }); + + it('a read that fails on its second page answers as a failed read does today, never with the first page', async () => { + const engine = makeEngine({ tables: bigQueue() }); + const find = engine.find.bind(engine); + engine.find = async (object: string, options?: any) => { + if (object === 'sys_queue_member' && options?.where?.$and) throw new Error('sys_queue_member page 2 unavailable'); + return find(object, options); + }; + const warn = vi.fn(); + const svc = service(engine, { warn }); + + expect(await svc.expandMembers('big', 'org_a')).toEqual([]); + expect(await svc.isMember('u_00000', 'big', 'org_a')).toBe(false); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toMatch(/sys_queue_member read failed/); + expect(warn.mock.calls[0][1]).toEqual({ error: 'sys_queue_member page 2 unavailable' }); + }); +}); + describe('[ADR-0131 D4] OwnershipQueueService.hasAnyQueue — the session payload\'s one boolean', () => { const ROW_ONLY = (): Record => ({ sys_queue: [{ id: 'q1', name: 'billing', label: 'Billing', organization_id: 'org_a' }], From 98d62fd6cacef2d6489674e725d65c5a72d458ed Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 04:00:44 +0000 Subject: [PATCH 03/14] chore(changeset): plugin-approvals refuses a node on an unreadable approver directory, plugin-sharing pages its queue reads Also states the request row the legacy-decide pin reads as present, which the test-layer type-check ledger counts. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- ...approver-directory-refusal-queue-paging.md | 19 +++++++++++++++++++ .../src/approval-service.test.ts | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 .changeset/29604-approver-directory-refusal-queue-paging.md diff --git a/.changeset/29604-approver-directory-refusal-queue-paging.md b/.changeset/29604-approver-directory-refusal-queue-paging.md new file mode 100644 index 00000000000..fc0c9ed3007 --- /dev/null +++ b/.changeset/29604-approver-directory-refusal-queue-paging.md @@ -0,0 +1,19 @@ +--- +'@objectstack/plugin-approvals': patch +'@objectstack/plugin-sharing': patch +--- + +fix(plugin-approvals)!: an approval node refuses to open when a team, position or queue approver's directory read fails, and the ownership-queue reads page to completion + +Clause-②: yes (narrowing) — an approval node whose `team`, `position` or `queue` approver (static, or an `expression` approver with `resolveAs` `team` / `position`) cannot be expanded because a directory read failed no longer opens: under every `onEmptyApprovers` policy (`admin_rescue`, `auto_approve`, `fail`, `fallback`, the `fallbackApprovers` slate included) it now fails the node with a retryable `APPROVER_DIRECTORY_UNREADABLE:` error naming the read, where it used to open on the rest of the slate, on the fallback slate or on the administrators, or auto-approve. Every other input answers as before. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the approval node (`@objectstack/plugin-approvals`): a failed directory read behind a `team`, `position` or `queue` approver refuses the node instead of opening it on a slate that is missing people (ADR-0142 D6 as amended, ruling record 6599731698 on #29604). + +- **Before.** A `team` or `position` approver whose `sys_team_member`, `sys_user_position` or `sys_member` read failed kept its `type:value` literal slot, which nobody can act on, and logged the failure. A `queue` approver whose expansion threw answered "expanded to nobody" and added no slot. Either way the node went on as if those people did not exist: beside other approvers it opened on them alone, and on an otherwise empty slate the node's `onEmptyApprovers` policy acted, opening on the `fallbackApprovers` or the organization's administrators, or auto-approving. +- **Now.** The node does not open and writes no request, approver or action row. It fails with an `ApproverDirectoryUnreadableError` whose message starts `APPROVER_DIRECTORY_UNREADABLE:`, names the approver, the organization and the read that failed (a failure of the cross-organization read filter is named as that filter's read), and is retryable: the flow node's failure is a runtime one, so the run can be resumed once the directory can be read. A `fallbackApprovers` slate whose own `team` or `position` read fails refuses the same way. `department` / `business_unit`, `org_membership_level` and `manager` approvers are not named by the ruling and answer exactly as before. +- **Unchanged.** A decision on a request opened before the approver snapshot existed still re-reads its slate on the old path, which keeps the literal slot on a failed read, because the decision has already been recorded when that read happens. + +`@objectstack/plugin-sharing`: the ownership-queue service reads its `sys_queue` and `sys_queue_member` rows by id, page after page, to the end, where it read one page of 10,000 rows and answered on it. A queue's 10,001st member, an organization's 10,001st queue and the queues naming a recipient past that bound are now found. A read that fails partway answers as a failed read did before (that read contributes nobody, logged once), never with the pages it had read. diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 377b666c36d..c06b73646cb 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -4330,7 +4330,7 @@ describe('ApprovalService — a team, position or queue approver whose directory const req = await svc.openNodeRequest( node([{ type: 'user', value: 'u2' }, { type: 'team', value: 't_ops' }], { behavior: 'unanimous' }), CTX, ) as any; - const row = engine._tables['sys_approval_request'].find((r: any) => r.id === req.id); + const row = engine._tables['sys_approval_request'].find((r: any) => r.id === req.id)!; const config = JSON.parse(row.node_config_json); delete config.__approverGroups; row.node_config_json = JSON.stringify(config); From c18905922b673d1f7c2cd66ef30287c7eb36a202 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:42:56 +0000 Subject: [PATCH 04/14] fix(plugin-approvals): an expression approver's failed cross-organization read filter refuses the node, named as that read The ADR-0105 D9 read filter on an expression approver's team and position values ran outside the refusing try, so its failure failed the node with the raw error. It now refuses with ApproverDirectoryUnreadableError naming the filter's read, as the static resolver's filter does; one helper words that read for both. Pinned on both paths with the filter made to reject. The ownership-queue warning for a walk that could not vouch for completion logs the reason only, without the walk's "so the expansion refuses instead", since the queue still answers nobody. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- .../plugin-approvals/src/approval-service.ts | 37 +++-- ...prover-directory-refusal-cross-org.test.ts | 133 ++++++++++++++++++ .../src/ownership-queue-service.ts | 12 +- .../src/ownership-queue.test.ts | 15 ++ 4 files changed, 184 insertions(+), 13 deletions(-) create mode 100644 packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 4f6ecd0084e..b369ec2a059 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -1180,6 +1180,17 @@ function holderReadRefusal(scope: string, err: unknown): Error { */ const DIRECTORY_REFUSING_APPROVER_TYPES: ReadonlySet = new Set(['queue', 'team', 'position']); +/** + * [#29604] The read a failed ADR-0105 D9 cross-organization read filter names: + * that filter's, not the directory's. One wording for the static resolver and + * the `expression` one, so a refusal from either says the same thing. + */ +function crossOrgFilterReadFailure(directoryOrg: unknown, organizationId: unknown, err: unknown): string { + return `the cross-organization read filter (who among the people of organization '${directoryOrg}' can read ` + + `this request of organization '${organizationId}') could not be read: ` + + `${err instanceof Error ? err.message : String(err)}.`; +} + /** * [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] The * approval node refuses to open because a directory read behind one of its @@ -2692,11 +2703,7 @@ export class ApprovalService implements IApprovalService { directoryOrgId: directoryOrg, }); } catch (err) { - throw Object.assign(new Error( - `the cross-organization read filter (who among the people of organization '${directoryOrg}' can read ` - + `this request of organization '${organizationId}') could not be read: ` - + `${err instanceof Error ? err.message : String(err)}.`, - ), { cause: err }); + throw Object.assign(new Error(crossOrgFilterReadFailure(directoryOrg, organizationId, err)), { cause: err }); } }; @@ -3046,9 +3053,23 @@ export class ApprovalService implements IApprovalService { users = []; } if (crossOrg && users.length) { - users = await filterApproversWhoCanRead(this.orgScopeDeps, users, organizationId, { - approverType: resolveAs, value: key, directoryOrgId: directoryOrg, - }); + try { + users = await filterApproversWhoCanRead(this.orgScopeDeps, users, organizationId, { + approverType: resolveAs, value: key, directoryOrgId: directoryOrg, + }); + } catch (err) { + // [#29604] The read filter is a read behind a `team` or `position` + // value as much as the expansion is: it refuses the node the same + // way, naming the filter's read as the static resolver's `bounded` + // does. Every other case fails as it always did. + if (onDirectoryFailure === 'refuse' && DIRECTORY_REFUSING_APPROVER_TYPES.has(resolveAs)) { + throw new ApproverDirectoryUnreadableError({ + type: resolveAs, value: key, organizationId: directoryOrg, + failedRead: crossOrgFilterReadFailure(directoryOrg, organizationId, err), expression: source, cause: err, + }); + } + throw err; + } } if (!users.length) { slots.push({ id: `${resolveAs}:${key}`, subGroup: key }); diff --git a/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts b/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts new file mode 100644 index 00000000000..d629936d93a --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts @@ -0,0 +1,133 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] The + * ADR-0105 D9 cross-organization read filter is a read behind a `position` or + * `team` approver as much as the directory expansion is. When it fails, the + * node refuses to open with `ApproverDirectoryUnreadableError`, naming THAT + * read — on the static resolver (its `bounded` filter) and on an `expression` + * approver's `resolveAs` values alike. + * + * `filterApproversWhoCanRead` answers its own failed `sys_member` read by + * keeping the slate, so the only way to make it fail is to make it reject: the + * module is wrapped here with a pass-through spy, and each case says when it + * rejects. Same scenario as `approver-cross-org.integration.test.ts`: a Plant A + * request routed to the group CFO. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { ApprovalService, ApproverDirectoryUnreadableError } from './approval-service.js'; +import { filterApproversWhoCanRead } from './approver-org-scope.js'; + +vi.mock('./approver-org-scope.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, filterApproversWhoCanRead: vi.fn(actual.filterApproversWhoCanRead) }; +}); + +interface Row { [k: string]: any } + +function makeEngine(seed: Record) { + const tables: Record = { ...seed }; + const ensure = (n: string) => (tables[n] ??= []); + const matches = (row: Row, filter: any): boolean => { + if (!filter || typeof filter !== 'object') return true; + for (const [k, v] of Object.entries(filter)) { + if (k === '$or') { + if (!(v as any[]).some((sub) => matches(row, sub))) return false; + continue; + } + if (k.startsWith('$')) throw new Error(`fake engine: unsupported filter operator ${k}`); + const rv = row[k]; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + if (v != null && typeof v === 'object' && '$ne' in (v as any)) { + if (rv === (v as any).$ne) return false; + continue; + } + if (rv !== v) return false; + } + return true; + }; + return { + _tables: tables, + async find(object: string, options?: any) { + return ensure(object).filter((r) => matches(r, options?.filter ?? options?.where)); + }, + async insert(object: string, data: Row) { ensure(object).push({ ...data }); return { ...data }; }, + async update() { return {}; }, + async delete() { return {}; }, + async count(object: string) { return ensure(object).length; }, + registerHook() { /* no-op */ }, + unregisterHooksByPackage() { /* no-op */ }, + }; +} + +const SEED = () => ({ + sys_user: [{ id: 'u_cfo' }], + sys_organization: [ + { id: 'o_group', slug: 'acme-group', parent_organization_id: null }, + { id: 'o_plant', slug: 'acme-plant-a', parent_organization_id: 'o_group' }, + ], + sys_user_position: [{ id: 'up1', user_id: 'u_cfo', position: 'cfo', organization_id: 'o_group' }], + sys_member: [{ id: 'm1', user_id: 'u_cfo', organization_id: 'o_plant', role: 'member' }], +}); + +const CTX = { userId: 'u_submitter', organizationId: 'o_plant', positions: [], permissions: [] } as any; + +const node = (approver: Record) => ({ + object: 'purchase_order', + recordId: 'po1', + runId: 'run_1', + nodeId: 'group_signoff', + flowName: 'po_approval', + config: { approvers: [approver], behavior: 'unanimous' as const, lockRecord: false, onEmptyApprovers: 'admin_rescue' as const }, + record: { id: 'po1', signoff_position: 'cfo' }, +}); + +const STATIC_CFO = { type: 'position', value: 'cfo', organization: '$root' }; +const EXPRESSION_CFO = { type: 'expression', value: 'trigger.signoff_position', resolveAs: 'position', organization: '$root' }; + +const FILTER_READ = /^the cross-organization read filter \(who among the people of organization 'o_group' can read this request of organization 'o_plant'\) could not be read: membership read filter down\.$/; + +describe('ADR-0142 D6 — a failed cross-organization read filter refuses the node, named as that read (#29604)', () => { + let engine: ReturnType; + let svc: ApprovalService; + let n = 0; + + beforeEach(() => { + vi.mocked(filterApproversWhoCanRead).mockClear(); + engine = makeEngine(SEED()); + n = 0; + svc = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(1767000000000 + (n++) * 1000) }, + tenancyPosture: () => 'group', + }); + }); + + const refusalOf = (p: Promise) => p.then(() => null, (e: unknown) => e); + const filterDown = () => vi.mocked(filterApproversWhoCanRead).mockRejectedValueOnce(new Error('membership read filter down')); + + for (const [label, approver, type, value] of [ + ['a static position approver', STATIC_CFO, 'position', 'cfo'], + ['an expression approver resolving as position', EXPRESSION_CFO, 'position', 'cfo'], + ] as const) { + it(`control — ${label} reaches the group CFO through the filter`, async () => { + const req = await svc.openNodeRequest(node(approver), CTX) as any; + expect(req.pending_approvers).toEqual(['u_cfo']); + expect(filterApproversWhoCanRead).toHaveBeenCalledTimes(1); + }); + + it(`${label}: the filter's failure refuses the node and names the filter's read`, async () => { + filterDown(); + const refusal = await refusalOf(svc.openNodeRequest(node(approver), CTX)); + + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: type, approverValue: value, retryable: true }); + expect((refusal as ApproverDirectoryUnreadableError).failedRead).toMatch(FILTER_READ); + expect(engine._tables['sys_approval_request'] ?? []).toEqual([]); + }); + } +}); diff --git a/packages/plugins/plugin-sharing/src/ownership-queue-service.ts b/packages/plugins/plugin-sharing/src/ownership-queue-service.ts index 93809415a47..8f03d2573a2 100644 --- a/packages/plugins/plugin-sharing/src/ownership-queue-service.ts +++ b/packages/plugins/plugin-sharing/src/ownership-queue-service.ts @@ -119,15 +119,17 @@ function isRecord(v: unknown): v is Record { /** * The reason behind a failed authored read, for a warning that answers nobody. - * A walk's refusal words its own consequence ("so the expansion refuses"), - * which is not this service's answer yet, so the driver's reason it carries as - * `cause` is logged instead; a walk that could not vouch for completion has no - * cause and logs its own sentence. + * A walk's refusal ends on its own consequence ("…, so the expansion refuses + * instead."), which is not this service's answer until #29604 part 2, so only + * the reason is logged: the driver's error the refusal carries as `cause`, or, + * for a walk that could not vouch for completion (no cause), the refusal's + * sentence with that closing clause cut. */ function readFailureReason(err: unknown): string { const cause = (err as { cause?: unknown } | null)?.cause; if (cause instanceof Error) return cause.message; - return err instanceof Error ? err.message : String(err); + const message = err instanceof Error ? err.message : String(err); + return message.replace(/ [^.]*\brefuses instead\.$/, ''); } /** diff --git a/packages/plugins/plugin-sharing/src/ownership-queue.test.ts b/packages/plugins/plugin-sharing/src/ownership-queue.test.ts index a148b580518..f5d8402d6cb 100644 --- a/packages/plugins/plugin-sharing/src/ownership-queue.test.ts +++ b/packages/plugins/plugin-sharing/src/ownership-queue.test.ts @@ -370,6 +370,21 @@ describe('[ADR-0142 D2] OwnershipQueueService — the queue reads page to comple expect(warn.mock.calls[0][0]).toMatch(/sys_queue_member read failed/); expect(warn.mock.calls[0][1]).toEqual({ error: 'sys_queue_member page 2 unavailable' }); }); + + it('a read that cannot vouch for completion logs its reason, not the walk\'s "refuses instead" — the queue answers nobody', async () => { + // A full page whose last row carries no id: the walk cannot resume after it. + const tables = bigQueue(10_000); + delete tables.sys_queue_member.at(-1)!.id; + const warn = vi.fn(); + const svc = service(makeEngine({ tables }), { warn }); + + expect(await svc.expandMembers('big', 'org_a')).toEqual([]); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][1].error).toBe( + "[sharing] the member rows of queue 'big' in organization 'org_a' could not be read: " + + 'the read by id could not continue past a page (a row without an id, or a page that repeated the cursor).', + ); + }); }); describe('[ADR-0131 D4] OwnershipQueueService.hasAnyQueue — the session payload\'s one boolean', () => { From 34e14c1eff24dd69d1c57a359ec351759b86e387 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:44:01 +0000 Subject: [PATCH 05/14] chore(changeset): a queue approver refuses only when its expansion rejects until part 2 The ownership-queue service does not reject a failed member read yet, so the narrowing names a queue approver whose expansion rejects, and says the service's own failed reads still answer nobody until the contract can carry the refusal. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- .../29604-approver-directory-refusal-queue-paging.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.changeset/29604-approver-directory-refusal-queue-paging.md b/.changeset/29604-approver-directory-refusal-queue-paging.md index fc0c9ed3007..f9b0dac2830 100644 --- a/.changeset/29604-approver-directory-refusal-queue-paging.md +++ b/.changeset/29604-approver-directory-refusal-queue-paging.md @@ -3,17 +3,18 @@ '@objectstack/plugin-sharing': patch --- -fix(plugin-approvals)!: an approval node refuses to open when a team, position or queue approver's directory read fails, and the ownership-queue reads page to completion +fix(plugin-approvals)!: an approval node refuses to open when a team or position approver's directory read fails, or a queue approver's expansion rejects, and the ownership-queue reads page to completion -Clause-②: yes (narrowing) — an approval node whose `team`, `position` or `queue` approver (static, or an `expression` approver with `resolveAs` `team` / `position`) cannot be expanded because a directory read failed no longer opens: under every `onEmptyApprovers` policy (`admin_rescue`, `auto_approve`, `fail`, `fallback`, the `fallbackApprovers` slate included) it now fails the node with a retryable `APPROVER_DIRECTORY_UNREADABLE:` error naming the read, where it used to open on the rest of the slate, on the fallback slate or on the administrators, or auto-approve. Every other input answers as before. +Clause-②: yes (narrowing) — an approval node no longer opens when a `team` or `position` approver (static, or an `expression` approver with `resolveAs` `team` / `position`) cannot be expanded because a directory read or the cross-organization read filter failed, or when a `queue` approver's expansion rejects. Under every `onEmptyApprovers` policy, the `fallbackApprovers` slate included, it now fails the node with a retryable `APPROVER_DIRECTORY_UNREADABLE:` error naming the read, where it used to open on the rest of the slate, on the fallback slate or on the administrators, or auto-approve. The registered ownership-queue service does not reject yet: a failed member read inside it still answers nobody until #29604 part 2 (#30251), so a `queue` approver refuses here only when `expandMembers` itself rejects. Every other input answers as before. Metadata-protocol: none -**BREAKING** accept-set narrowing at the approval node (`@objectstack/plugin-approvals`): a failed directory read behind a `team`, `position` or `queue` approver refuses the node instead of opening it on a slate that is missing people (ADR-0142 D6 as amended, ruling record 6599731698 on #29604). +**BREAKING** accept-set narrowing at the approval node (`@objectstack/plugin-approvals`): a failed directory read behind a `team` or `position` approver, or a `queue` approver whose expansion rejects, refuses the node instead of opening it on a slate that is missing people (ADR-0142 D6 as amended, ruling record 6599731698 on #29604). - **Before.** A `team` or `position` approver whose `sys_team_member`, `sys_user_position` or `sys_member` read failed kept its `type:value` literal slot, which nobody can act on, and logged the failure. A `queue` approver whose expansion threw answered "expanded to nobody" and added no slot. Either way the node went on as if those people did not exist: beside other approvers it opened on them alone, and on an otherwise empty slate the node's `onEmptyApprovers` policy acted, opening on the `fallbackApprovers` or the organization's administrators, or auto-approving. -- **Now.** The node does not open and writes no request, approver or action row. It fails with an `ApproverDirectoryUnreadableError` whose message starts `APPROVER_DIRECTORY_UNREADABLE:`, names the approver, the organization and the read that failed (a failure of the cross-organization read filter is named as that filter's read), and is retryable: the flow node's failure is a runtime one, so the run can be resumed once the directory can be read. A `fallbackApprovers` slate whose own `team` or `position` read fails refuses the same way. `department` / `business_unit`, `org_membership_level` and `manager` approvers are not named by the ruling and answer exactly as before. +- **Now.** The node does not open and writes no request, approver or action row. It fails with an `ApproverDirectoryUnreadableError` whose message starts `APPROVER_DIRECTORY_UNREADABLE:`, names the approver, the organization and the read that failed (a failure of the cross-organization read filter is named as that filter's read, on a static approver and on an `expression` one alike), and is retryable: the flow node's failure is a runtime one, so the run can be resumed once the directory can be read. A `fallbackApprovers` slate whose own `team` or `position` read fails refuses the same way. `department` / `business_unit`, `org_membership_level` and `manager` approvers are not named by the ruling and answer exactly as before. - **Unchanged.** A decision on a request opened before the approver snapshot existed still re-reads its slate on the old path, which keeps the literal slot on a failed read, because the decision has already been recorded when that read happens. +- **Not yet.** The ownership-queue service shipped in `@objectstack/plugin-sharing` answers a failed `sys_queue` / `sys_queue_member`, team or position read with nobody (logged once) rather than rejecting, so a `queue` approver backed by it still adds no slot on such a failure. Its refusal arrives with #29604 part 2, once the contract can carry it (#30251). -`@objectstack/plugin-sharing`: the ownership-queue service reads its `sys_queue` and `sys_queue_member` rows by id, page after page, to the end, where it read one page of 10,000 rows and answered on it. A queue's 10,001st member, an organization's 10,001st queue and the queues naming a recipient past that bound are now found. A read that fails partway answers as a failed read did before (that read contributes nobody, logged once), never with the pages it had read. +`@objectstack/plugin-sharing`: the ownership-queue service reads its `sys_queue` and `sys_queue_member` rows by id, page after page, to the end, where it read one page of 10,000 rows and answered on it. A queue's 10,001st member, an organization's 10,001st queue and the queues naming a recipient past that bound are now found. A read that fails partway answers as a failed read did before (that read contributes nobody, logged once, with the read's reason and without the walk's own "refuses instead" clause), never with the pages it had read. From 50deed3e8fd17ec50152140ff5d73cf1a870c313 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:49:25 +0000 Subject: [PATCH 06/14] test(plugin-approvals): type the cross-org refusal pins' approver as the node config takes it The test-layer type-check program admits no new file with errors. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- .../src/approver-directory-refusal-cross-org.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts b/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts index d629936d93a..475cf086ac6 100644 --- a/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts +++ b/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts @@ -82,7 +82,7 @@ const node = (approver: Record) => ({ runId: 'run_1', nodeId: 'group_signoff', flowName: 'po_approval', - config: { approvers: [approver], behavior: 'unanimous' as const, lockRecord: false, onEmptyApprovers: 'admin_rescue' as const }, + config: { approvers: [approver as any], behavior: 'unanimous' as const, lockRecord: false, onEmptyApprovers: 'admin_rescue' as const }, record: { id: 'po1', signoff_position: 'cfo' }, }); From c4c64fdeec4c995324b0fff29eef614c5f984024 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:59:05 +0000 Subject: [PATCH 07/14] test(plugin-approvals): the cross-org refusal pins' engine double honours limit and carries no write verbs it is never sent The node under these pins never updates or deletes through the engine, so the double declares neither, and its find cuts at the caller's limit as the engine does. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- .../src/approver-directory-refusal-cross-org.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts b/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts index 475cf086ac6..6c4caf02cb7 100644 --- a/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts +++ b/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts @@ -53,11 +53,10 @@ function makeEngine(seed: Record) { return { _tables: tables, async find(object: string, options?: any) { - return ensure(object).filter((r) => matches(r, options?.filter ?? options?.where)); + const rows = ensure(object).filter((r) => matches(r, options?.filter ?? options?.where)); + return typeof options?.limit === 'number' ? rows.slice(0, options.limit) : rows; }, async insert(object: string, data: Row) { ensure(object).push({ ...data }); return { ...data }; }, - async update() { return {}; }, - async delete() { return {}; }, async count(object: string) { return ensure(object).length; }, registerHook() { /* no-op */ }, unregisterHooksByPackage() { /* no-op */ }, From 1a3854d288f1e5e2bac081863ee5281783bfe012 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 06:34:46 +0000 Subject: [PATCH 08/14] test(plugin-approvals): the cross-org read-filter refusal pins live in the shared approval-service harness Moved as one top-level describe into approval-service.test.ts, on the shared fake engine, with the pass-through spy on the read filter declared there; the separate file is gone. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- .../src/approval-service.test.ts | 75 ++++++++++ ...prover-directory-refusal-cross-org.test.ts | 132 ------------------ 2 files changed, 75 insertions(+), 132 deletions(-) delete mode 100644 packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index c06b73646cb..b656a179d72 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -15,9 +15,17 @@ import { ApprovalService, ApproverDirectoryUnreadableError, REMIND_COOLDOWN_MS, ESCALATION_ENABLED_FLIP_CUTOFF_MS, SLA_ACTOR_ID, } from './approval-service.js'; import { bindApprovalLockHook, bindDelegationWriteGuard, unbindAllHooks } from './lifecycle-hooks.js'; +import { filterApproversWhoCanRead } from './approver-org-scope.js'; import { celEngine } from '@objectstack/formula'; import { SysApprovalRequest } from './sys-approval-request.object.js'; +// [#29604] A pass-through spy on the ADR-0105 D9 read filter, so one describe +// below can make it reject. Every other case runs the real filter through it. +vi.mock('./approver-org-scope.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, filterApproversWhoCanRead: vi.fn(actual.filterApproversWhoCanRead) }; +}); + interface FakeRow { [k: string]: any } /** @@ -4341,6 +4349,73 @@ describe('ApprovalService — a team, position or queue approver whose directory }); }); +// [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] The +// ADR-0105 D9 cross-organization read filter is a read behind a `position` or +// `team` approver as much as the directory expansion is. When it fails, the +// node refuses, naming THAT read, on the static resolver (its `bounded` +// filter) and on an `expression` approver's `resolveAs` values alike. +// `filterApproversWhoCanRead` answers its own failed `sys_member` read by +// keeping the slate, so the pass-through spy above is how it is made to fail. +// The scenario is `approver-cross-org.integration.test.ts`'s: a Plant A +// request routed to the group CFO. +describe('ApprovalService — a failed cross-organization read filter refuses the node, named as that read (ADR-0142 D6)', () => { + const PLANT_CTX = { userId: 'u_submitter', organizationId: 'o_plant', positions: [], permissions: [] } as any; + const FILTER_READ = /^the cross-organization read filter \(who among the people of organization 'o_group' can read this request of organization 'o_plant'\) could not be read: membership read filter down\.$/; + let engine: ReturnType; + let svc: ApprovalService; + let n = 0; + + beforeEach(() => { + vi.mocked(filterApproversWhoCanRead).mockClear(); + engine = makeFakeEngine(); + engine._tables['sys_user'].push({ id: 'u_cfo' }); + engine._tables['sys_organization'] = [ + { id: 'o_group', slug: 'acme-group', parent_organization_id: null }, + { id: 'o_plant', slug: 'acme-plant-a', parent_organization_id: 'o_group' }, + ]; + engine._tables['sys_user_position'] = [{ id: 'up1', user_id: 'u_cfo', position: 'cfo', organization_id: 'o_group' }]; + engine._tables['sys_member'] = [{ id: 'm1', user_id: 'u_cfo', organization_id: 'o_plant', role: 'member' }]; + n = 0; + svc = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(1767000000000 + (n++) * 1000) }, + tenancyPosture: () => 'group', + }); + }); + + const node = (approver: Record) => ({ + object: 'purchase_order', + recordId: 'po1', + runId: 'run_1', + nodeId: 'group_signoff', + flowName: 'po_approval', + config: { approvers: [approver as any], behavior: 'unanimous' as const, lockRecord: false, onEmptyApprovers: 'admin_rescue' as const }, + record: { id: 'po1', signoff_position: 'cfo' }, + }); + const refusalOf = (p: Promise) => p.then(() => null, (e: unknown) => e); + + for (const [label, approver] of [ + ['a static position approver', { type: 'position', value: 'cfo', organization: '$root' }], + ['an expression approver resolving as position', { type: 'expression', value: 'trigger.signoff_position', resolveAs: 'position', organization: '$root' }], + ] as const) { + it(`control — ${label} reaches the group CFO through the filter`, async () => { + const req = await svc.openNodeRequest(node(approver), PLANT_CTX) as any; + expect(req.pending_approvers).toEqual(['u_cfo']); + expect(filterApproversWhoCanRead).toHaveBeenCalledTimes(1); + }); + + it(`${label}: the filter's failure refuses the node and names the filter's read`, async () => { + vi.mocked(filterApproversWhoCanRead).mockRejectedValueOnce(new Error('membership read filter down')); + const refusal = await refusalOf(svc.openNodeRequest(node(approver), PLANT_CTX)); + + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: 'position', approverValue: 'cfo', retryable: true }); + expect((refusal as ApproverDirectoryUnreadableError).failedRead).toMatch(FILTER_READ); + expect(engine._tables['sys_approval_request'] ?? []).toEqual([]); + }); + } +}); + // #3807 follow-up: `queue` was not the only way to end up with a slot nobody // can act on — every GRAPH approver type falls back to the same literal when // its lookup finds no one, and that fallback used to happen in total silence. diff --git a/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts b/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts deleted file mode 100644 index 6c4caf02cb7..00000000000 --- a/packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts +++ /dev/null @@ -1,132 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] The - * ADR-0105 D9 cross-organization read filter is a read behind a `position` or - * `team` approver as much as the directory expansion is. When it fails, the - * node refuses to open with `ApproverDirectoryUnreadableError`, naming THAT - * read — on the static resolver (its `bounded` filter) and on an `expression` - * approver's `resolveAs` values alike. - * - * `filterApproversWhoCanRead` answers its own failed `sys_member` read by - * keeping the slate, so the only way to make it fail is to make it reject: the - * module is wrapped here with a pass-through spy, and each case says when it - * rejects. Same scenario as `approver-cross-org.integration.test.ts`: a Plant A - * request routed to the group CFO. - */ - -import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { ApprovalService, ApproverDirectoryUnreadableError } from './approval-service.js'; -import { filterApproversWhoCanRead } from './approver-org-scope.js'; - -vi.mock('./approver-org-scope.js', async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, filterApproversWhoCanRead: vi.fn(actual.filterApproversWhoCanRead) }; -}); - -interface Row { [k: string]: any } - -function makeEngine(seed: Record) { - const tables: Record = { ...seed }; - const ensure = (n: string) => (tables[n] ??= []); - const matches = (row: Row, filter: any): boolean => { - if (!filter || typeof filter !== 'object') return true; - for (const [k, v] of Object.entries(filter)) { - if (k === '$or') { - if (!(v as any[]).some((sub) => matches(row, sub))) return false; - continue; - } - if (k.startsWith('$')) throw new Error(`fake engine: unsupported filter operator ${k}`); - const rv = row[k]; - if (v != null && typeof v === 'object' && '$in' in (v as any)) { - if (!(v as any).$in.includes(rv)) return false; - continue; - } - if (v != null && typeof v === 'object' && '$ne' in (v as any)) { - if (rv === (v as any).$ne) return false; - continue; - } - if (rv !== v) return false; - } - return true; - }; - return { - _tables: tables, - async find(object: string, options?: any) { - const rows = ensure(object).filter((r) => matches(r, options?.filter ?? options?.where)); - return typeof options?.limit === 'number' ? rows.slice(0, options.limit) : rows; - }, - async insert(object: string, data: Row) { ensure(object).push({ ...data }); return { ...data }; }, - async count(object: string) { return ensure(object).length; }, - registerHook() { /* no-op */ }, - unregisterHooksByPackage() { /* no-op */ }, - }; -} - -const SEED = () => ({ - sys_user: [{ id: 'u_cfo' }], - sys_organization: [ - { id: 'o_group', slug: 'acme-group', parent_organization_id: null }, - { id: 'o_plant', slug: 'acme-plant-a', parent_organization_id: 'o_group' }, - ], - sys_user_position: [{ id: 'up1', user_id: 'u_cfo', position: 'cfo', organization_id: 'o_group' }], - sys_member: [{ id: 'm1', user_id: 'u_cfo', organization_id: 'o_plant', role: 'member' }], -}); - -const CTX = { userId: 'u_submitter', organizationId: 'o_plant', positions: [], permissions: [] } as any; - -const node = (approver: Record) => ({ - object: 'purchase_order', - recordId: 'po1', - runId: 'run_1', - nodeId: 'group_signoff', - flowName: 'po_approval', - config: { approvers: [approver as any], behavior: 'unanimous' as const, lockRecord: false, onEmptyApprovers: 'admin_rescue' as const }, - record: { id: 'po1', signoff_position: 'cfo' }, -}); - -const STATIC_CFO = { type: 'position', value: 'cfo', organization: '$root' }; -const EXPRESSION_CFO = { type: 'expression', value: 'trigger.signoff_position', resolveAs: 'position', organization: '$root' }; - -const FILTER_READ = /^the cross-organization read filter \(who among the people of organization 'o_group' can read this request of organization 'o_plant'\) could not be read: membership read filter down\.$/; - -describe('ADR-0142 D6 — a failed cross-organization read filter refuses the node, named as that read (#29604)', () => { - let engine: ReturnType; - let svc: ApprovalService; - let n = 0; - - beforeEach(() => { - vi.mocked(filterApproversWhoCanRead).mockClear(); - engine = makeEngine(SEED()); - n = 0; - svc = new ApprovalService({ - engine: engine as any, - clock: { now: () => new Date(1767000000000 + (n++) * 1000) }, - tenancyPosture: () => 'group', - }); - }); - - const refusalOf = (p: Promise) => p.then(() => null, (e: unknown) => e); - const filterDown = () => vi.mocked(filterApproversWhoCanRead).mockRejectedValueOnce(new Error('membership read filter down')); - - for (const [label, approver, type, value] of [ - ['a static position approver', STATIC_CFO, 'position', 'cfo'], - ['an expression approver resolving as position', EXPRESSION_CFO, 'position', 'cfo'], - ] as const) { - it(`control — ${label} reaches the group CFO through the filter`, async () => { - const req = await svc.openNodeRequest(node(approver), CTX) as any; - expect(req.pending_approvers).toEqual(['u_cfo']); - expect(filterApproversWhoCanRead).toHaveBeenCalledTimes(1); - }); - - it(`${label}: the filter's failure refuses the node and names the filter's read`, async () => { - filterDown(); - const refusal = await refusalOf(svc.openNodeRequest(node(approver), CTX)); - - expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); - expect(refusal).toMatchObject({ approverType: type, approverValue: value, retryable: true }); - expect((refusal as ApproverDirectoryUnreadableError).failedRead).toMatch(FILTER_READ); - expect(engine._tables['sys_approval_request'] ?? []).toEqual([]); - }); - } -}); From c46708ba8c8a781f39cd1f6b45da36391924244c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 07:41:47 +0000 Subject: [PATCH 09/14] fix(plugin-sharing)!: the ownership-queue service refuses a failed member read when the caller asks, and the approver slate and the sharing-recipient recompute ask expandMembers and isMember honour the contract's onFailedRead option. Under 'refuse' a failed sys_queue, sys_queue_member, team, position, sys_member or business-unit read rejects with a failure naming the queue, its organization and the read, never [] / a partial set / false; isMember still answers true when a read that succeeded reaches the user. The default 'nobody' answers as before, so the admission consumers (access path, claim, assignment pool) are unchanged. The activation-ledger read keeps "switched on" under both. The approver slate's queue arm (D6) and the sharing-rule recompute's queue recipient (D7: whole-rule pass, recipient-axis revoke, per-user pass, record pass) pass 'refuse': the node refuses with ApproverDirectoryUnreadableError, and each recompute's caller keeps the rule's grants and warns. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- .../src/approval-service.test.ts | 58 ++++- .../plugin-approvals/src/approval-service.ts | 8 +- .../src/ownership-queue-service.ts | 237 ++++++++++++++---- .../src/ownership-queue.test.ts | 116 ++++++++- .../src/queue-recipient.test.ts | 99 +++++++- .../src/sharing-rule-service.ts | 18 +- 6 files changed, 471 insertions(+), 65 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index b656a179d72..db63151a8d1 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -4176,7 +4176,7 @@ describe('ApprovalService — a queue approver routes to the queue\'s members (A const req = await svc.openNodeRequest(queueInput('billing'), CTX) as any; expect(req.pending_approvers).toEqual(['u7', 'u8']); - expect(asked).toEqual([['billing', 't1', { purpose: 'addressing' }]]); + expect(asked).toEqual([['billing', 't1', { purpose: 'addressing', onFailedRead: 'refuse' }]]); expect(warnings.filter(([msg]) => String(msg).includes('expanded to nobody'))).toEqual([]); // A member decides it under their own id. const decided = await svc.decideNode(req.id, { decision: 'approve', actorId: 'u8' }, SYS); @@ -4349,6 +4349,62 @@ describe('ApprovalService — a team, position or queue approver whose directory }); }); +// [ADR-0142 D2 as amended, D6; #29604] The queue arm asks the ownership-queue +// service for its refusal (`onFailedRead: 'refuse'`), beside the ADDRESSING +// purpose. The registered service (`@objectstack/plugin-sharing`) answers a +// failed member read with nobody unless asked, so before the arm asked, a +// queue whose team or position read failed added no slot and the node's +// `onEmptyApprovers` policy opened, rescued or approved on a slate missing its +// people. The double below gives the contract's two answers as that service +// does: a rejection naming the read under `'refuse'`, `[]` under `'nobody'`. +describe('ApprovalService — a queue approver asks the queue service to refuse a failed read, and the node refuses with it (ADR-0142 D6)', () => { + const POLICIES = ['admin_rescue', 'auto_approve', 'fail', 'fallback'] as const; + const READ = "[queue] queue 'billing' in organization 't1' does not answer its members, because a read behind it failed: " + + "the members of team 't_night' could not be read: SQLITE_IOERR: disk I/O error."; + + const withFailingQueue = () => { + const engine = makeFakeEngine(); + const asked: unknown[] = []; + const warnings: any[] = []; + let n = 0; + const svc = new ApprovalService({ + engine, + clock: { now: () => new Date(1757000000000 + (n++) * 1000) }, + logger: { warn: (msg: any, meta: any) => warnings.push([msg, meta]) }, + ownershipQueues: () => ({ + expandMembers: async (name: string, org: string | null, options?: { purpose?: string; onFailedRead?: string }) => { + asked.push([name, org, options]); + if (options?.onFailedRead === 'refuse') throw new Error(READ); + return []; + }, + }), + }); + return { engine, svc, asked, warnings }; + }; + const node = (configExtra: Record) => ({ + ...openInput([]), + config: { approvers: [{ type: 'queue' as const, value: 'billing' }], behavior: 'first_response' as const, lockRecord: false, ...configExtra }, + }); + + for (const p of POLICIES) { + it(`a queue whose member read fails: onEmptyApprovers '${p}' opens nothing and approves nothing`, async () => { + const { engine, svc, asked, warnings } = withFailingQueue(); + const refusal = await svc.openNodeRequest(node( + p === 'fallback' ? { onEmptyApprovers: p, fallbackApprovers: [{ type: 'user', value: 'u_backstop' }] } : { onEmptyApprovers: p }, + ), CTX).then(() => null, (e: unknown) => e); + + expect(asked).toEqual([['billing', 't1', { purpose: 'addressing', onFailedRead: 'refuse' }]]); + expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError); + expect(refusal).toMatchObject({ approverType: 'queue', approverValue: 'billing', retryable: true, failedRead: READ }); + expect((refusal as Error).message).toMatch(/^APPROVER_DIRECTORY_UNREADABLE: approver 'queue:billing' could not be expanded in organization 't1'/); + expect(warnings.some(([msg]) => String(msg).includes("approver 'queue:billing' expanded to nobody"))).toBe(false); + expect(engine._tables['sys_approval_request'] ?? []).toEqual([]); + expect(engine._tables['sys_approval_approver'] ?? []).toEqual([]); + expect(engine._tables['sys_approval_action'] ?? []).toEqual([]); + }); + } +}); + // [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] The // ADR-0105 D9 cross-organization read filter is a read behind a `position` or // `team` approver as much as the directory expansion is. When it fails, the diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index b369ec2a059..c24c61d69ef 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -3482,12 +3482,18 @@ export class ApprovalService implements IApprovalService { * fail-open). The service resolves the name registry-first, then the * organization's own `sys_queue` row, and answers `[]` for a name that * resolves nowhere. With no service registered there is nothing to expand. + * + * [#29604] `onFailedRead: 'refuse'` (D2 as amended): a failed read behind the + * queue rejects instead of answering "nobody", and {@link resolveApproverSpec}'s + * catch turns that rejection into the node's {@link ApproverDirectoryUnreadableError}, + * so the slate is never opened without the people who could not be read. An + * axis of its own beside `purpose`. */ private async expandQueueUsers(queueName: string, organizationId?: string | null): Promise { if (!queueName) return []; const queues = this.queueSource(); if (!queues) return []; - return queues.expandMembers(queueName, organizationId ?? null, { purpose: 'addressing' }); + return queues.expandMembers(queueName, organizationId ?? null, { purpose: 'addressing', onFailedRead: 'refuse' }); } /** diff --git a/packages/plugins/plugin-sharing/src/ownership-queue-service.ts b/packages/plugins/plugin-sharing/src/ownership-queue-service.ts index 8f03d2573a2..f7a8ee4961e 100644 --- a/packages/plugins/plugin-sharing/src/ownership-queue-service.ts +++ b/packages/plugins/plugin-sharing/src/ownership-queue-service.ts @@ -29,14 +29,36 @@ * the union de-duplicated and ordered by user id ascending — so round-robin and * the least-loaded tie-break see one stable order wherever they run. * - * ## Absence is not a fault + * ## Absence is not a fault; a failed read is * * A name that resolves nowhere answers `null` / `[]` / `false`, and an empty - * queue answers `[]`. A failed read of a member table contributes nobody for - * that specification (fail closed for every admission that reads it); a failed - * read of the activation ledger leaves the declared default — on — for that - * call and is logged, the posture the sharing-rule service takes for the same - * ledger. + * queue answers `[]`, under every option: those are facts. + * + * A failed read of a table behind the queue — its `sys_queue` row, its + * `sys_queue_member` rows, or a member table a specification reads (team, + * position, `sys_member`, business unit) — is a fault, and what the call + * answers then is the caller's to say (ADR-0142 D2 as amended, the contract's + * `OwnershipQueueFailedReadAnswer`): + * + * - `onFailedRead: 'nobody'`, the default — what sat behind the failed read + * contributes nobody, logged once. The admission consumers take it, because + * for them nobody denies: the sitting-record access path (D3), the claim + * (D4) and the assignment pool (D5). {@link OwnershipQueueService.queuesOf}, + * which the access path reads, has no option and always answers this way. + * - `onFailedRead: 'refuse'` (#29604) — the call rejects with a failure that + * names the queue, the organization and the read ({@link queueReadRefusal}), + * never `[]`, a partial set or `false`. The recompute consumers take it, + * because for them nobody revokes: the approver slate (D6) and the + * sharing-recipient grant recompute (D7). `isMember` under it still answers + * `true` when a read that succeeded reaches the user — a failed read beside + * it cannot take that membership away — and `false` only when every read + * behind the answer succeeded. + * + * A failed read of the activation ledger is neither: under either answer it + * leaves the declared default — on — for that call and is logged, the posture + * the sharing-rule service takes for the same ledger on its own position + * recipients, so a queue's position and a rule's position never disagree on + * an outage (#29604 keeps it; the contract leaves it to this service). * * ## Read to completion * @@ -46,13 +68,13 @@ * (`readMembershipToCompletion`), never cut at a row bound: a queue past the * bound lost its later members, a member past it lost every queue it sits in, * and nothing said so. A read that fails, or a walk that cannot vouch for - * completion, takes the failed-read answer above. ADR-0142 D2 as amended has - * the recompute consumers (D6, D7) refuse a failed read instead; that answer - * waits on the contract carrying the caller's call-time signal. + * completion, is a failed read above, never an answer on the pages already + * read. */ import type { IOwnershipQueueService, + OwnershipQueueFailedReadAnswer, QueueExpansionPurpose, QueueMemberSpec, ResolvedQueue, @@ -117,19 +139,47 @@ function isRecord(v: unknown): v is Record { return v !== null && typeof v === 'object' && !Array.isArray(v); } +/** A walk's or a holder check's closing consequence ("…, so the expansion refuses instead."). */ +const WALK_CONSEQUENCE = / [^.]*\brefuses instead\.$/; + /** * The reason behind a failed authored read, for a warning that answers nobody. * A walk's refusal ends on its own consequence ("…, so the expansion refuses - * instead."), which is not this service's answer until #29604 part 2, so only - * the reason is logged: the driver's error the refusal carries as `cause`, or, - * for a walk that could not vouch for completion (no cause), the refusal's - * sentence with that closing clause cut. + * instead."), which is not the answer a call under `onFailedRead: 'nobody'` + * gives, so only the reason is logged: the driver's error the refusal carries + * as `cause`, or, for a walk that could not vouch for completion (no cause), + * the refusal's sentence with that closing clause cut. */ function readFailureReason(err: unknown): string { const cause = (err as { cause?: unknown } | null)?.cause; if (cause instanceof Error) return cause.message; const message = err instanceof Error ? err.message : String(err); - return message.replace(/ [^.]*\brefuses instead\.$/, ''); + return message.replace(WALK_CONSEQUENCE, ''); +} + +/** + * [#29604] The failure a call under `onFailedRead: 'refuse'` rejects with: the + * queue, its organization, and the read that failed, in the read's own words — + * a membership walk's or a holder check's refusal with its `[sharing]` tag and + * its closing consequence cut (the consequence here is this sentence's), or + * this service's own sentence for the `sys_queue` row read. The failed read's + * error rides along as `cause`. + */ +function queueReadRefusal(name: string, organizationId: string | null, err: unknown): Error { + const read = (err instanceof Error ? err.message : String(err)) + .replace(/^\[sharing\] /, '') + .replace(WALK_CONSEQUENCE, '') + .trim(); + const inOrg = organizationId ? ` in organization '${organizationId}'` : ''; + return Object.assign( + new Error( + `[queue] queue '${name}'${inOrg} does not answer its members, because a read behind it failed: ` + + `${read}${/[.!?]$/.test(read) ? '' : '.'} The caller asked for a refusal over a partial answer ` + + '(onFailedRead \'refuse\'): answering with the members that could be read would revoke or route past ' + + 'the ones that could not.', + ), + { cause: err }, + ); } /** @@ -140,12 +190,21 @@ function readFailureReason(err: unknown): string { interface ExpansionPass { /** `'addressing'` keeps a switched-off position's holders (see the contract's `expandMembers`). */ purpose: QueueExpansionPurpose; + /** + * [#29604] `onFailedRead: 'refuse'`: a failed member-table read rejects the + * call instead of contributing nobody. An axis of its own, never read off + * {@link purpose}. + */ + refuse: boolean; positionActive: Map; expansions: Map>; } -function newPass(purpose: QueueExpansionPurpose = 'access'): ExpansionPass { - return { purpose, positionActive: new Map(), expansions: new Map() }; +function newPass( + purpose: QueueExpansionPurpose = 'access', + onFailedRead: OwnershipQueueFailedReadAnswer = 'nobody', +): ExpansionPass { + return { purpose, refuse: onFailedRead === 'refuse', positionActive: new Map(), expansions: new Map() }; } export class OwnershipQueueService implements IOwnershipQueueService { @@ -161,11 +220,21 @@ export class OwnershipQueueService implements IOwnershipQueueService { // ── The contract ─────────────────────────────────────────────────────────── async resolveQueue(name: string, organizationId: string | null): Promise { + return this.resolve(name, organizationId, false); + } + + /** + * {@link resolveQueue}, with the caller's failed-read answer: under `refuse` + * a failed `sys_queue` or `sys_queue_member` read rejects (a failure naming + * the read, which the public caller wraps in {@link queueReadRefusal}) + * instead of resolving nowhere or to no authored members. + */ + private async resolve(name: string, organizationId: string | null, refuse: boolean): Promise { const engine = this.engine(); if (!engine || !name) return null; const declared = await this.declaredQueueItem(name); if (declared) { - const authoredMembers = await this.authoredMemberSpecs(engine, [name], organizationId); + const authoredMembers = await this.authoredMemberSpecs(engine, [name], organizationId, refuse); return { name, label: declared.label ?? name, @@ -180,9 +249,9 @@ export class OwnershipQueueService implements IOwnershipQueueService { // A declared name switched off in the ledger resolves nowhere — not even to // an authored row, which may not take a declared queue's name. if (this.declaredItems().some((q) => q.name === name)) return null; - const row = await this.authoredQueueRow(engine, name, organizationId); + const row = await this.authoredQueueRow(engine, name, organizationId, refuse); if (!row) return null; - const authoredMembers = await this.authoredMemberSpecs(engine, [name], organizationId); + const authoredMembers = await this.authoredMemberSpecs(engine, [name], organizationId, refuse); return { name, label: str(row.label) ?? name, @@ -195,18 +264,37 @@ export class OwnershipQueueService implements IOwnershipQueueService { async expandMembers( name: string, organizationId: string | null, - options?: { purpose?: QueueExpansionPurpose }, + options?: { purpose?: QueueExpansionPurpose; onFailedRead?: OwnershipQueueFailedReadAnswer }, ): Promise { - const queue = await this.resolveQueue(name, organizationId); - if (!queue) return []; - return this.expandSpecs(queue.members, organizationId, newPass(options?.purpose)); + const pass = newPass(options?.purpose, options?.onFailedRead); + try { + const queue = await this.resolve(name, organizationId, pass.refuse); + if (!queue) return []; + return await this.expandSpecs(queue.members, organizationId, pass); + } catch (err) { + // Only a call under 'refuse' gets here: under 'nobody' every read + // behind the queue answers nobody in place (module header). + if (!pass.refuse) throw err; + throw queueReadRefusal(name, organizationId, err); + } } - async isMember(userId: string, name: string, organizationId: string | null): Promise { + async isMember( + userId: string, + name: string, + organizationId: string | null, + options?: { onFailedRead?: OwnershipQueueFailedReadAnswer }, + ): Promise { if (!userId) return false; - const queue = await this.resolveQueue(name, organizationId); - if (!queue) return false; - return this.specsReachUser(queue.members, userId, organizationId, newPass()); + const pass = newPass('access', options?.onFailedRead); + try { + const queue = await this.resolve(name, organizationId, pass.refuse); + if (!queue) return false; + return await this.specsReachUser(queue.members, userId, organizationId, pass); + } catch (err) { + if (!pass.refuse) throw err; + throw queueReadRefusal(name, organizationId, err); + } } async queuesOf(userId: string, organizationId: string | null): Promise { @@ -331,6 +419,9 @@ export class OwnershipQueueService implements IOwnershipQueueService { let pending = pass.expansions.get(key); if (!pending) { pending = this.expandSpecUncached(spec, organizationId, pass).catch((err: any) => { + // [#29604] Under 'refuse' the specification's failed read is the + // call's answer — the pass, and so this rejected memo, ends with it. + if (pass.refuse) throw err; this.warnOnce(`expand::${key}`, '[queue] a member specification could not be expanded — it contributes nobody until it can', { type: spec.type, value: spec.value, error: err?.message, }); @@ -373,6 +464,14 @@ export class OwnershipQueueService implements IOwnershipQueueService { * {@link expandSpecs}: a position or a team is answered by an indexed * holder check rather than a full expansion, the reason * `PositionGraphService.holdsPosition` exists. + * + * [#29604] Under `onFailedRead: 'refuse'` a specification whose read fails + * does not end the walk: a later one that reaches the user still answers + * `true`, because a read that succeeded and found the user is a fact no + * failure beside it can take away. Only when none reaches does the first + * failure become the answer — `false` is given only when every read behind it + * succeeded. Under `'nobody'` no specification rejects (each failed read + * contributes nobody in place), so the walk is the one it always was. */ private async specsReachUser( specs: readonly QueueMemberSpec[], @@ -382,44 +481,56 @@ export class OwnershipQueueService implements IOwnershipQueueService { ): Promise { const engine = this.engine(); if (!engine) return false; + let failed: { err: unknown } | undefined; for (const spec of specs) { if (!spec.value) continue; - switch (spec.type) { - case 'user': - if (spec.value === userId) return true; - break; - case 'team': - if (await this.holderCheck(spec, () => teamHasMember(engine, spec.value, userId))) return true; - break; - case 'position': { - if (!(await this.positionActive(spec.value, pass))) break; - const positions = new PositionGraphService({ engine, organizationId }); - if (await this.holderCheck(spec, () => positions.holdsPosition(userId, spec.value, organizationId ?? undefined))) return true; - break; - } - default: - if ((await this.expandSpec(spec, organizationId, pass)).includes(userId)) return true; + try { + if (await this.specReachesUser(engine, spec, userId, organizationId, pass)) return true; + } catch (err) { + if (!pass.refuse) throw err; + failed ??= { err }; } } + if (failed) throw failed.err; return false; } + private async specReachesUser( + engine: OwnershipQueueEngine, + spec: QueueMemberSpec, + userId: string, + organizationId: string | null, + pass: ExpansionPass, + ): Promise { + switch (spec.type) { + case 'user': + return spec.value === userId; + case 'team': + return this.holderCheck(spec, pass, () => teamHasMember(engine, spec.value, userId)); + case 'position': { + if (!(await this.positionActive(spec.value, pass))) return false; + const positions = new PositionGraphService({ engine, organizationId }); + return this.holderCheck(spec, pass, () => positions.holdsPosition(userId, spec.value, organizationId ?? undefined)); + } + default: + return (await this.expandSpec(spec, organizationId, pass)).includes(userId); + } + } + /** - * [#29503] A team or position holder check, answered in the posture this - * service declares for a failed member-table read (module header): that - * specification contributes nobody, logged once — the same answer - * {@link expandSpec} gives the expansion, so the two twins cannot disagree. - * - * The checks themselves (`teamHasMember`, `PositionGraphService.holdsPosition`) - * now refuse a failed read instead of answering `false`; this keeps the - * queue's consumers where they were. ADR-0142 D2 as amended (#29604) has the - * recompute consumers refuse instead; the call-time signal they say so with - * is the contract's to carry, and until it does every caller gets this answer. + * [#29503] A team or position holder check, answered in the posture the + * call asked for (module header). The checks themselves (`teamHasMember`, + * `PositionGraphService.holdsPosition`) refuse a failed read instead of + * answering `false`. Under `onFailedRead: 'nobody'` that specification + * contributes nobody, logged once — the same answer {@link expandSpec} gives + * the expansion, so the two twins cannot disagree. Under `'refuse'` (#29604) + * the check's refusal is passed on, as the expansion's is. */ - private async holderCheck(spec: QueueMemberSpec, check: () => Promise): Promise { + private async holderCheck(spec: QueueMemberSpec, pass: ExpansionPass, check: () => Promise): Promise { try { return await check(); } catch (err: any) { + if (pass.refuse) throw err; this.warnOnce(`holds::${spec.type}::${spec.value}`, '[queue] a member specification could not be read for a holder check — it contributes nobody until it can', { type: spec.type, value: spec.value, error: err?.message, }); @@ -442,10 +553,16 @@ export class OwnershipQueueService implements IOwnershipQueueService { return items.filter((q) => !off.has(q.name)); } + /** + * The organization's `sys_queue` row named `name`. A failed read resolves + * the name nowhere, logged once — or, under `refuse` (#29604), rejects with + * a sentence naming the read: whether the queue exists is itself unknown. + */ private async authoredQueueRow( engine: OwnershipQueueEngine, name: string, organizationId: string | null, + refuse = false, ): Promise | null> { try { const rows = await engine.find(QUEUE_OBJECT, { @@ -456,6 +573,12 @@ export class OwnershipQueueService implements IOwnershipQueueService { }); return Array.isArray(rows) && isRecord(rows[0]) ? rows[0] : null; } catch (err: any) { + if (refuse) { + throw Object.assign( + new Error(`the ${QUEUE_OBJECT} row of queue '${name}'${organizationId ? ` in organization '${organizationId}'` : ''} could not be read: ${err?.message ?? String(err)}`), + { cause: err }, + ); + } this.warnOnce('read::sys_queue', '[queue] sys_queue read failed — authored queues resolve nowhere until it succeeds', { error: err?.message }); return null; } @@ -487,11 +610,16 @@ export class OwnershipQueueService implements IOwnershipQueueService { } } - /** The organization's `sys_queue_member` rows for `names`, grouped by queue. */ + /** + * The organization's `sys_queue_member` rows for `names`, grouped by queue. + * A failed read counts no authored member, logged once — or, under `refuse` + * (#29604), passes the walk's refusal on, which names the read. + */ private async authoredMemberSpecs( engine: OwnershipQueueEngine, names: readonly string[], organizationId: string | null, + refuse = false, ): Promise> { const out = new Map(); if (names.length === 0) return out; @@ -515,6 +643,7 @@ export class OwnershipQueueService implements IOwnershipQueueService { out.set(queue, list); } } catch (err: any) { + if (refuse) throw err; this.warnOnce('read::sys_queue_member', '[queue] sys_queue_member read failed — authored members count as nobody until it succeeds', { error: readFailureReason(err) }); } return out; diff --git a/packages/plugins/plugin-sharing/src/ownership-queue.test.ts b/packages/plugins/plugin-sharing/src/ownership-queue.test.ts index f5d8402d6cb..134c727e42d 100644 --- a/packages/plugins/plugin-sharing/src/ownership-queue.test.ts +++ b/packages/plugins/plugin-sharing/src/ownership-queue.test.ts @@ -208,11 +208,12 @@ describe('[ADR-0142 D2] OwnershipQueueService — the one expander', () => { }); it('[#29503] a failed team or position holder read keeps the queue\'s declared posture: that specification contributes nobody, logged once', async () => { - // The team and position reads now refuse a failed read instead of answering - // "nobody"; the queue service keeps answering for its own consumers exactly - // as its header declares (a failed member-table read contributes nobody), - // on the expansion and on the holder check alike. Whether the queue should - // refuse too is left to a ruling (ADR-0142 does not declare this posture). + // The team and position reads refuse a failed read instead of answering + // "nobody"; under the default `onFailedRead: 'nobody'` the queue service + // answers its admission consumers as its header declares (a failed + // member-table read contributes nobody), on the expansion and on the + // holder check alike. Under `'refuse'` it rejects instead — pinned in the + // `onFailedRead 'refuse'` describe below (#29604). for (const failed of ['sys_team_member', 'sys_user_position'] as const) { const warn = vi.fn(); const svc = service(makeEngine({ declared: SUPPORT_DECLARED, tables: BASE_TABLES(), failOn: [failed] }), { warn }); @@ -387,6 +388,95 @@ describe('[ADR-0142 D2] OwnershipQueueService — the queue reads page to comple }); }); +describe('[ADR-0142 D2 as amended] OwnershipQueueService — onFailedRead \'refuse\' rejects a failed read, naming it (#29604)', () => { + const REFUSE = { onFailedRead: 'refuse' } as const; + const esc = (text: string) => text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + /** The refusal's shape: the queue, its organization, then the read that failed, never the walk's own consequence. */ + const refusal = (queue: string, read: string) => + new RegExp(`^${esc(`[queue] queue '${queue}' in organization 'org_a' does not answer its members, because a read behind it failed: ${read}`)}`); + + it('⛔ a failed sys_queue row read rejects, naming it — the default still resolves the name nowhere', async () => { + const warn = vi.fn(); + const svc = service(makeEngine({ tables: BASE_TABLES(), failOn: ['sys_queue'] }), { warn }); + const read = "the sys_queue row of queue 'billing' in organization 'org_a' could not be read: sys_queue unavailable."; + await expect(svc.expandMembers('billing', 'org_a', REFUSE)).rejects.toThrow(refusal('billing', read)); + await expect(svc.isMember('u_e', 'billing', 'org_a', REFUSE)).rejects.toThrow(refusal('billing', read)); + expect(warn).not.toHaveBeenCalled(); + // The admission answer, unchanged: nobody, logged once. + expect(await svc.expandMembers('billing', 'org_a')).toEqual([]); + expect(await svc.isMember('u_e', 'billing', 'org_a')).toBe(false); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('⛔ a failed sys_queue_member read rejects — never the declared positions alone', async () => { + const svc = service(makeEngine({ declared: SUPPORT_DECLARED, tables: BASE_TABLES(), failOn: ['sys_queue_member'] }), { warn: vi.fn() }); + await expect(svc.expandMembers('support_tier1', 'org_a', { purpose: 'addressing', ...REFUSE })).rejects.toThrow( + refusal('support_tier1', "the member rows of queue 'support_tier1' in organization 'org_a' could not be read: sys_queue_member unavailable. The caller"), + ); + await expect(svc.isMember('u_d', 'support_tier1', 'org_a', REFUSE)).rejects.toThrow(/sys_queue_member unavailable/); + expect(await svc.expandMembers('support_tier1', 'org_a')).toEqual(['u_a', 'u_b']); + }); + + it('⛔ a failed member-table read behind a specification rejects, naming the read and carrying the driver error as cause', async () => { + // support_tier1 = position support_agent (u_a, u_b; its sys_member limb + // too) + team t_night (u_a, u_d); billing = position billing_clerk (u_c) + user u_e. + for (const [failed, queue, read] of [ + ['sys_team_member', 'support_tier1', "the members of team 't_night' could not be read: sys_team_member unavailable"], + ['sys_user_position', 'support_tier1', "the assignments of position 'support_agent' in organization 'org_a' could not be read: sys_user_position unavailable"], + ['sys_member', 'billing', "the holders of role 'billing_clerk' in organization 'org_a' could not be read: sys_member unavailable"], + ] as const) { + const svc = service(makeEngine({ declared: SUPPORT_DECLARED, tables: BASE_TABLES(), failOn: [failed] }), { warn: vi.fn() }); + const err = await svc.expandMembers(queue, 'org_a', REFUSE).then(() => null, (e: unknown) => e as Error); + expect(err?.message, failed).toMatch(refusal(queue, `${read}. The caller`)); + expect(err?.message, failed).not.toMatch(/refuses instead/); + expect(((err as any)?.cause as Error)?.message, failed).toMatch(new RegExp(`${failed} unavailable`)); + } + }); + + it('isMember under refuse: true when a read that succeeded reaches the user, whichever specification failed first; a rejection when none does', async () => { + // Position first (declared), team second (authored). + const positionDown = service(makeEngine({ declared: SUPPORT_DECLARED, tables: BASE_TABLES(), failOn: ['sys_user_position'] }), { warn: vi.fn() }); + expect(await positionDown.isMember('u_d', 'support_tier1', 'org_a', REFUSE)).toBe(true); // the team reaches u_d after the failure + await expect(positionDown.isMember('u_b', 'support_tier1', 'org_a', REFUSE)).rejects.toThrow(/sys_user_position unavailable/); + await expect(positionDown.isMember('u_z', 'support_tier1', 'org_a', REFUSE)).rejects.toThrow(/sys_user_position unavailable/); + + const teamDown = service(makeEngine({ declared: SUPPORT_DECLARED, tables: BASE_TABLES(), failOn: ['sys_team_member'] }), { warn: vi.fn() }); + expect(await teamDown.isMember('u_b', 'support_tier1', 'org_a', REFUSE)).toBe(true); // the position reaches u_b before it + await expect(teamDown.isMember('u_d', 'support_tier1', 'org_a', REFUSE)).rejects.toThrow( + refusal('support_tier1', "whether user 'u_d' is a member of team 't_night' could not be read: sys_team_member unavailable. The caller"), + ); + // The admission answer for the same user, unchanged. + expect(await teamDown.isMember('u_d', 'support_tier1', 'org_a')).toBe(false); + }); + + it('facts stay facts: an empty queue is [], an unresolved name is [] / false, and healthy reads answer what the default answers', async () => { + const svc = service(makeEngine({ declared: [...SUPPORT_DECLARED, { name: 'empty_q', label: 'Empty', members: [] }], tables: BASE_TABLES() })); + expect(await svc.expandMembers('empty_q', 'org_a', REFUSE)).toEqual([]); + expect(await svc.expandMembers('nowhere', 'org_a', REFUSE)).toEqual([]); + expect(await svc.isMember('u_a', 'nowhere', 'org_a', REFUSE)).toBe(false); + expect(await svc.expandMembers('support_tier1', 'org_a', REFUSE)).toEqual(['u_a', 'u_b', 'u_d']); + expect(await svc.expandMembers('billing', 'org_a', REFUSE)).toEqual(['u_c', 'u_e']); + expect(await svc.isMember('u_z', 'support_tier1', 'org_a', REFUSE)).toBe(false); + expect(await svc.isMember('u_d', 'support_tier1', 'org_a', REFUSE)).toBe(true); + }); + + it('a refusal is not remembered: the next call, once the table reads, answers the members', async () => { + const failOn = ['sys_team_member']; + const svc = service(makeEngine({ declared: SUPPORT_DECLARED, tables: BASE_TABLES(), failOn }), { warn: vi.fn() }); + await expect(svc.expandMembers('support_tier1', 'org_a', REFUSE)).rejects.toThrow(/sys_team_member unavailable/); + failOn.length = 0; + expect(await svc.expandMembers('support_tier1', 'org_a', REFUSE)).toEqual(['u_a', 'u_b', 'u_d']); + }); + + it('a failed activation-ledger read is not a failed read under refuse either: the declared default — on — answers, logged', async () => { + const warn = vi.fn(); + const svc = service(makeEngine({ declared: SUPPORT_DECLARED, tables: BASE_TABLES(), failOn: [METADATA_ACTIVATION_TABLE] }), { warn }); + expect(await svc.expandMembers('support_tier1', 'org_a', REFUSE)).toEqual(['u_a', 'u_b', 'u_d']); + expect(await svc.isMember('u_b', 'support_tier1', 'org_a', REFUSE)).toBe(true); + expect(warn.mock.calls.some(([line]) => /treated as switched ON/.test(String(line)))).toBe(true); + }); +}); + describe('[ADR-0131 D4] OwnershipQueueService.hasAnyQueue — the session payload\'s one boolean', () => { const ROW_ONLY = (): Record => ({ sys_queue: [{ id: 'q1', name: 'billing', label: 'Billing', organization_id: 'org_a' }], @@ -533,6 +623,22 @@ describe('[ADR-0142 D3] SharingService — a record sitting in a queue the calle expect(JSON.stringify(await bare.buildReadFilter('ticket', ctx('u_a')))).not.toContain('"queue"'); }); + it('[#29604] control — a failed member read reaches nobody through the queue and refuses nothing: the access path keeps the default answer', async () => { + const down = makeEngine({ + declared: SUPPORT_DECLARED, + tables: engine._tables, + schemas: { ticket: TICKET_SCHEMA, job: AUTHOR_QUEUE_SCHEMA, idea: OPEN_SCHEMA, sys_record_share: { name: 'sys_record_share' } }, + failOn: ['sys_team_member'], + }); + const queues = service(down, { warn: vi.fn() }); + const s = new SharingService({ engine: down as any, ownershipQueues: () => queues }); + // u_d sits in support_tier1 only through team t_night, whose read fails: no access, and no throw. + expect(await s.checkEdit('ticket', 't1', ctx('u_d'))).toBe('deny'); + expect(JSON.stringify(await s.buildReadFilter('ticket', ctx('u_d')) ?? {})).not.toContain('"queue"'); + // u_b sits in it through position support_agent, which reads. + expect(await s.checkEdit('ticket', 't1', ctx('u_b'))).toBe('allow'); + }); + it('membership is resolved once per request context', async () => { const queuesOf = vi.fn(async () => ['support_tier1']); const s = new SharingService({ engine: engine as any, ownershipQueues: () => ({ queuesOf, isMember: async () => false }) }); diff --git a/packages/plugins/plugin-sharing/src/queue-recipient.test.ts b/packages/plugins/plugin-sharing/src/queue-recipient.test.ts index 85b68de80a9..15a295db85a 100644 --- a/packages/plugins/plugin-sharing/src/queue-recipient.test.ts +++ b/packages/plugins/plugin-sharing/src/queue-recipient.test.ts @@ -10,6 +10,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/objectql'; import { SharingService } from './sharing-service.js'; import { SharingRuleService } from './sharing-rule-service.js'; +import { OwnershipQueueService } from './ownership-queue-service.js'; interface Row { [k: string]: any } @@ -44,12 +45,16 @@ function matches(row: Row, f: any): boolean { function makeEngine() { const tables: Record = {}; const ensure = (n: string) => (tables[n] ??= []); + /** Tables whose reads fail, as an outage of that table does (#29604). */ + const failing = new Set(); let seq = 0; return { _tables: tables, + failing, getSchema() { return undefined; }, seed(object: string, rows: Row[]) { ensure(object).push(...rows.map((r) => ({ ...r }))); }, async find(o: string, opts?: any) { + if (failing.has(o)) throw new Error(`${o} unavailable`); const f = opts?.filter ?? opts?.where; return ensure(o).filter((r) => matches(r, f)).slice(0, opts?.limit ?? 10000); }, @@ -110,7 +115,8 @@ describe('[ADR-0142 D7] the queue recipient', () => { }); await rules.evaluateRule(RULE, SYS); expect(granteesOf('inv1')).toEqual(['u_c', 'u_e']); - expect(expandMembers).toHaveBeenCalledWith('billing', 'org_a'); + // [#29604] On the recompute answer: a failed member read refuses, never "nobody". + expect(expandMembers).toHaveBeenCalledWith('billing', 'org_a', { onFailedRead: 'refuse' }); }); it('a member who left loses the grant on the next recompute', async () => { @@ -141,3 +147,94 @@ describe('[ADR-0142 D7] the queue recipient', () => { } as any, SYS)).rejects.toThrow(/machine name/); }); }); + +/** + * [ADR-0142 D2 as amended, D7; #29604] The grant recompute asks the queue + * service for its refusal, so a member read that fails leaves the rule's + * grants as they were — the whole-rule pass, the recipient-axis revoke and the + * per-user pass alike — where "nobody" revoked every grant behind the failed + * read. Driven through the registered `OwnershipQueueService` over the same + * engine, so the refusal is the service's own, not a stub's. + */ +describe('[ADR-0142 D7] a failed queue-member read keeps the queue-recipient rule\'s grants (#29604)', () => { + let engine: ReturnType; + let warn: ReturnType; + let rules: SharingRuleService; + const REFUSAL = /\[queue\] queue 'billing' in organization 'org_a' does not answer its members, because a read behind it failed/; + const granteesOf = (recordId: string): string[] => + (engine._tables.sys_record_share ?? []) + .filter((r) => r.record_id === recordId && r.source === 'rule') + .map((r) => String(r.recipient_id)) + .sort(); + const theRule = async () => (await rules.listRules({ activeOnly: true }, SYS))[0]; + + beforeEach(async () => { + engine = makeEngine(); + engine.seed('invoice', [{ id: 'inv1', status: 'open', owner_id: 'author', organization_id: 'org_a' }]); + engine.seed('sys_sharing_rule', [{ + id: 'srule_q', organization_id: 'org_a', name: RULE, label: 'Invoices → Billing queue', + object_name: 'invoice', criteria_json: JSON.stringify({ status: 'open' }), + recipient_type: 'queue', recipient_id: 'billing', access_level: 'read', active: true, + }]); + // billing = user u_c + team t_night (u_d). + engine.seed('sys_queue', [{ id: 'q1', name: 'billing', label: 'Billing', organization_id: 'org_a' }]); + engine.seed('sys_queue_member', [ + { id: 'qm_1', queue: 'billing', recipient_type: 'user', recipient_id: 'u_c', organization_id: 'org_a' }, + { id: 'qm_2', queue: 'billing', recipient_type: 'team', recipient_id: 't_night', organization_id: 'org_a' }, + ]); + engine.seed('sys_team_member', [{ id: 'tm_1', team_id: 't_night', user_id: 'u_d' }]); + warn = vi.fn(); + const logger = { warn, info: vi.fn(), debug: vi.fn(), error: vi.fn() }; + const queues = new OwnershipQueueService({ engine: () => engine as any, logger }); + rules = new SharingRuleService({ + engine: engine as any, + sharing: new SharingService({ engine: engine as any }), + ownershipQueues: () => queues, + logger: logger as any, + }); + await rules.evaluateRule(RULE, SYS); + expect(granteesOf('inv1')).toEqual(['u_c', 'u_d']); + }); + + it('⛔ the whole-rule pass refuses and revokes nothing — the queue\'s own member rows or a member table', async () => { + for (const table of ['sys_queue_member', 'sys_team_member']) { + engine.failing.add(table); + await expect(rules.evaluateRule(RULE, SYS), table).rejects.toThrow(REFUSAL); + expect(granteesOf('inv1'), table).toEqual(['u_c', 'u_d']); + engine.failing.delete(table); + } + }); + + it('⛔ the recipient-axis revoke refuses and retires nobody', async () => { + engine.failing.add('sys_team_member'); + await expect(rules.revokeRuleGrantsForRetiredRecipients(await theRule())).rejects.toThrow(REFUSAL); + expect(granteesOf('inv1')).toEqual(['u_c', 'u_d']); + }); + + it('⛔ the per-user pass refuses for a member it cannot read, and still answers the member it can', async () => { + engine.failing.add('sys_team_member'); + const rule = await theRule(); + await expect(rules.reconcileRuleForUser(rule, 'u_d')).rejects.toThrow(REFUSAL); + expect(granteesOf('inv1')).toEqual(['u_c', 'u_d']); + // u_c is reached by a read that succeeded: a fact, answered as one. + await expect(rules.reconcileRuleForUser(rule, 'u_c')).resolves.toMatchObject({ grantsRevoked: 0 }); + expect(granteesOf('inv1')).toEqual(['u_c', 'u_d']); + }); + + it('⛔ the record pass keeps the rule\'s grants and reports it, per rule (#29362)', async () => { + engine.failing.add('sys_team_member'); + await expect(rules.evaluateAllForRecord('invoice', 'inv1', SYS)).resolves.toEqual([]); + expect(granteesOf('inv1')).toEqual(['u_c', 'u_d']); + expect(warn.mock.calls.some(([line, meta]) => + /record reconcile failed for rule/.test(String(line)) && REFUSAL.test(String((meta as any)?.error)))).toBe(true); + }); + + it('control — once the read succeeds, the recompute converges: a member who left loses the grant', async () => { + engine.failing.add('sys_team_member'); + await expect(rules.evaluateRule(RULE, SYS)).rejects.toThrow(REFUSAL); + engine.failing.delete('sys_team_member'); + engine._tables.sys_team_member.splice(0); + await rules.evaluateRule(RULE, SYS); + expect(granteesOf('inv1')).toEqual(['u_c']); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index b9dd282d8a7..3e9f1cb56ae 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -1741,10 +1741,16 @@ export class SharingRuleService implements ISharingRuleService { } if (rule.recipient_type === 'team') return teamHasMember(this.engine, rule.recipient_id, userId); // [ADR-0142 D7] A queue reaches the user its specifications reach — the - // queue service's holder check, not a full expansion. + // queue service's holder check, not a full expansion. Under + // `onFailedRead: 'refuse'` (D2 as amended, #29604): a failed member read + // rejects rather than answering "not a member", which would revoke the + // user's grants; the per-user pass's caller catches it per rule and the + // grants stay, as they do for a team or position recipient (#29503). if (rule.recipient_type === 'queue') { const queues = this.ownershipQueues?.(); - return queues ? queues.isMember(userId, rule.recipient_id, rule.organization_id ?? null) : false; + return queues + ? queues.isMember(userId, rule.recipient_id, rule.organization_id ?? null, { onFailedRead: 'refuse' }) + : false; } return (await this.expandRecipient(rule, pass)).includes(userId); } @@ -2181,9 +2187,15 @@ export class SharingRuleService implements ISharingRuleService { // the two recompute modules' look-through). ⛔ This grants the rule's // matched records to the queue's people; it confers no sitting-record // access, which is the access path's (`SharingService`). + // + // [#29604] `onFailedRead: 'refuse'` (D2 as amended): this set is diffed + // against the grants the rule holds, so a failed member read answering + // "nobody" would revoke every member's grant on an outage. The refusal + // throws before any diff, and each recompute's caller keeps the rule's + // grants and warns (#29362), as for a team or position recipient. const queues = this.ownershipQueues?.(); if (!queues) return []; - return queues.expandMembers(rule.recipient_id, rule.organization_id ?? null); + return queues.expandMembers(rule.recipient_id, rule.organization_id ?? null, { onFailedRead: 'refuse' }); } return []; } From 8617bd9dd61bfeaa3df0a9ab1c1de7ac594d0093 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 07:42:14 +0000 Subject: [PATCH 10/14] test(plugin-sharing): name the convergence case for what it pins Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- packages/plugins/plugin-sharing/src/queue-recipient.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugins/plugin-sharing/src/queue-recipient.test.ts b/packages/plugins/plugin-sharing/src/queue-recipient.test.ts index 15a295db85a..5f7d054feb1 100644 --- a/packages/plugins/plugin-sharing/src/queue-recipient.test.ts +++ b/packages/plugins/plugin-sharing/src/queue-recipient.test.ts @@ -229,7 +229,7 @@ describe('[ADR-0142 D7] a failed queue-member read keeps the queue-recipient rul /record reconcile failed for rule/.test(String(line)) && REFUSAL.test(String((meta as any)?.error)))).toBe(true); }); - it('control — once the read succeeds, the recompute converges: a member who left loses the grant', async () => { + it('after the refusal, once the read succeeds, the next recompute converges: a member who left loses the grant', async () => { engine.failing.add('sys_team_member'); await expect(rules.evaluateRule(RULE, SYS)).rejects.toThrow(REFUSAL); engine.failing.delete('sys_team_member'); From 38e1282ca4a232fd4e8615572062b41e9765c896 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 07:46:11 +0000 Subject: [PATCH 11/14] chore(changeset): the queue service's refusal, and the first part's entry stated as delivered Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- ...approver-directory-refusal-queue-paging.md | 6 +++--- ...29604-queue-service-refuses-failed-read.md | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 .changeset/29604-queue-service-refuses-failed-read.md diff --git a/.changeset/29604-approver-directory-refusal-queue-paging.md b/.changeset/29604-approver-directory-refusal-queue-paging.md index f9b0dac2830..beb009bd565 100644 --- a/.changeset/29604-approver-directory-refusal-queue-paging.md +++ b/.changeset/29604-approver-directory-refusal-queue-paging.md @@ -5,7 +5,7 @@ fix(plugin-approvals)!: an approval node refuses to open when a team or position approver's directory read fails, or a queue approver's expansion rejects, and the ownership-queue reads page to completion -Clause-②: yes (narrowing) — an approval node no longer opens when a `team` or `position` approver (static, or an `expression` approver with `resolveAs` `team` / `position`) cannot be expanded because a directory read or the cross-organization read filter failed, or when a `queue` approver's expansion rejects. Under every `onEmptyApprovers` policy, the `fallbackApprovers` slate included, it now fails the node with a retryable `APPROVER_DIRECTORY_UNREADABLE:` error naming the read, where it used to open on the rest of the slate, on the fallback slate or on the administrators, or auto-approve. The registered ownership-queue service does not reject yet: a failed member read inside it still answers nobody until #29604 part 2 (#30251), so a `queue` approver refuses here only when `expandMembers` itself rejects. Every other input answers as before. +Clause-②: yes (narrowing) — an approval node no longer opens when a `team` or `position` approver (static, or an `expression` approver with `resolveAs` `team` / `position`) cannot be expanded because a directory read or the cross-organization read filter failed, or when a `queue` approver's expansion rejects. Under every `onEmptyApprovers` policy, the `fallbackApprovers` slate included, it now fails the node with a retryable `APPROVER_DIRECTORY_UNREADABLE:` error naming the read, where it used to open on the rest of the slate, on the fallback slate or on the administrators, or auto-approve. A `queue` approver refuses when `expandMembers` rejects, which the registered ownership-queue service does on a failed member read once the slate asks it to (the companion change in this release, `onFailedRead: 'refuse'`). Every other input answers as before. Metadata-protocol: none @@ -15,6 +15,6 @@ Metadata-protocol: none - **Before.** A `team` or `position` approver whose `sys_team_member`, `sys_user_position` or `sys_member` read failed kept its `type:value` literal slot, which nobody can act on, and logged the failure. A `queue` approver whose expansion threw answered "expanded to nobody" and added no slot. Either way the node went on as if those people did not exist: beside other approvers it opened on them alone, and on an otherwise empty slate the node's `onEmptyApprovers` policy acted, opening on the `fallbackApprovers` or the organization's administrators, or auto-approving. - **Now.** The node does not open and writes no request, approver or action row. It fails with an `ApproverDirectoryUnreadableError` whose message starts `APPROVER_DIRECTORY_UNREADABLE:`, names the approver, the organization and the read that failed (a failure of the cross-organization read filter is named as that filter's read, on a static approver and on an `expression` one alike), and is retryable: the flow node's failure is a runtime one, so the run can be resumed once the directory can be read. A `fallbackApprovers` slate whose own `team` or `position` read fails refuses the same way. `department` / `business_unit`, `org_membership_level` and `manager` approvers are not named by the ruling and answer exactly as before. - **Unchanged.** A decision on a request opened before the approver snapshot existed still re-reads its slate on the old path, which keeps the literal slot on a failed read, because the decision has already been recorded when that read happens. -- **Not yet.** The ownership-queue service shipped in `@objectstack/plugin-sharing` answers a failed `sys_queue` / `sys_queue_member`, team or position read with nobody (logged once) rather than rejecting, so a `queue` approver backed by it still adds no slot on such a failure. Its refusal arrives with #29604 part 2, once the contract can carry it (#30251). +- **The queue service's half.** The ownership-queue service shipped in `@objectstack/plugin-sharing` rejects a failed `sys_queue` / `sys_queue_member`, team, position or business-unit read when the caller passes `onFailedRead: 'refuse'`, and the slate passes it, so a `queue` approver backed by it refuses on such a failure too. That half is its own entry in this release ("the ownership-queue service refuses a failed member read when the caller asks"). -`@objectstack/plugin-sharing`: the ownership-queue service reads its `sys_queue` and `sys_queue_member` rows by id, page after page, to the end, where it read one page of 10,000 rows and answered on it. A queue's 10,001st member, an organization's 10,001st queue and the queues naming a recipient past that bound are now found. A read that fails partway answers as a failed read did before (that read contributes nobody, logged once, with the read's reason and without the walk's own "refuses instead" clause), never with the pages it had read. +`@objectstack/plugin-sharing`: the ownership-queue service reads its `sys_queue` and `sys_queue_member` rows by id, page after page, to the end, where it read one page of 10,000 rows and answered on it. A queue's 10,001st member, an organization's 10,001st queue and the queues naming a recipient past that bound are now found. A read that fails partway is a failed read, never an answer on the pages it had read: under the default it contributes nobody, logged once, with the read's reason and without the walk's own "refuses instead" clause, and under `onFailedRead: 'refuse'` it rejects. diff --git a/.changeset/29604-queue-service-refuses-failed-read.md b/.changeset/29604-queue-service-refuses-failed-read.md new file mode 100644 index 00000000000..6f406e7761f --- /dev/null +++ b/.changeset/29604-queue-service-refuses-failed-read.md @@ -0,0 +1,19 @@ +--- +'@objectstack/plugin-sharing': patch +'@objectstack/plugin-approvals': patch +--- + +fix(plugin-sharing)!: the ownership-queue service refuses a failed member read when the caller asks, and the approver slate and the sharing-recipient grant recompute ask + +Clause-②: yes (narrowing) — `expandMembers` and `isMember` of the registered ownership-queue service, called with `onFailedRead: 'refuse'`, now reject when a read behind the queue fails, where they answered nobody. The approver slate asks for that answer, so an approval node whose `queue` approver cannot be read refuses to open under every `onEmptyApprovers` policy, where it opened on the rest of the slate, on the fallback slate or on the administrators, or auto-approved. The sharing-rule recompute asks for it too, so a whole-rule pass (`evaluateRule`, the REST evaluate endpoint, the boot backfill), the recipient-axis revoke and the per-user pass on a `queue`-recipient rule reject on such a failure, where they completed by revoking the grants of the members who could not be read. Every call without the option, and every read that succeeds, answers as before. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the ownership-queue service's `'refuse'` answer and at its two recompute consumers (`@objectstack/plugin-sharing`, `@objectstack/plugin-approvals`): an empty expansion is a fact, a failed read is a fault (ADR-0142 D2 as amended), and the approver slate (D6) and the sharing-recipient grant recompute (D7) now get the fault instead of "nobody". + +- **The service.** `OwnershipQueueService.expandMembers(name, org, { onFailedRead: 'refuse' })` and `isMember(user, name, org, { onFailedRead: 'refuse' })` reject when the queue's `sys_queue` row, its `sys_queue_member` rows, or a member table one of its specifications reads (`sys_team_member`, `sys_user_position`, `sys_member`, `sys_business_unit_member`, `sys_business_unit`) cannot be read, or a paged read cannot vouch for completion. The rejection's message starts `[queue] queue '' in organization '' does not answer its members, because a read behind it failed:` and names that read; the read's own error rides along as `cause`. It never answers `[]`, a partial set or `false` instead. `isMember` still answers `true` when a read that succeeded reaches the user, and answers `false` only when every read behind it succeeded. An empty queue is still `[]`, and a name that resolves nowhere is still `[]` / `false`. +- **Unchanged: the default.** Without the option (`onFailedRead: 'nobody'`), a failed read still contributes nobody, logged once. The sitting-record access path (`queuesOf`), the claim admission and the assignment pool take that answer, so they deny on a failed read exactly as before. A failed read of the activation ledger (`sys_metadata_activation`) is not a failed read in this sense under either answer: it still reads "switched on" for that call, logged, the posture the sharing-rule service takes for its own position recipients. +- **The approver slate (D6).** A `queue` approver asks with `onFailedRead: 'refuse'` beside the addressing purpose. When the registered service rejects, the node fails with the retryable `APPROVER_DIRECTORY_UNREADABLE:` error that a failed team or position read already raises, naming the queue's failed read, and writes no request, approver or action row. Before, the registered service answered nobody, so the queue added no slot and the node's `onEmptyApprovers` policy acted on a slate missing those people. +- **The sharing-recipient grant recompute (D7).** A `queue`-recipient rule's expansion and its per-user membership check ask with `onFailedRead: 'refuse'`. A failed member read now rejects before any grant is diffed, so the rule's existing grants stay as they were until a pass can read the queue, and each recompute's caller logs it: the record pass and the object pass per rule (`record reconcile failed for rule`, `object reconcile failed for rule`), the queue-membership and business-unit write recomputes (`queue-membership revocation failed`, `queue-membership re-grant failed`, `business-unit revocation failed`, `business-unit re-grant failed`), the membership-row recompute (`recipient membership re-evaluation failed`) and the boot backfill (`boot rule backfill failed for rule`). A direct call to `evaluateRule`, `revokeRuleGrantsForRetiredRecipients` or `reconcileRuleForUser`, the REST evaluate endpoint included, rejects with the service's failure. Before, the failed read answered nobody and the pass revoked the grant of every member behind it. This is the answer a `team` or `position` recipient has given since its own reads began to refuse. +- **The gap the approver refusal left, closed.** With the approval node's own refusal (its entry in this release, "an approval node refuses to open when a team or position approver's directory read fails…") alone, a `queue` approver backed by the registered service still added no slot on a failed member read, because the service answered nobody instead of rejecting; it refused only when `expandMembers` itself rejected. The service now rejects when asked and the slate asks, so that approver refuses on a failed `sys_queue`, `sys_queue_member`, team, position or business-unit read, as a `team` or `position` approver does. From fd12db84050f819da3f3ce20098ce8b44c803919 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 07:48:34 +0000 Subject: [PATCH 12/14] test(plugin-sharing): type the recompute pins' logger as the service takes it Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- .../plugins/plugin-sharing/src/queue-recipient.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/plugins/plugin-sharing/src/queue-recipient.test.ts b/packages/plugins/plugin-sharing/src/queue-recipient.test.ts index 5f7d054feb1..1b642f85f7d 100644 --- a/packages/plugins/plugin-sharing/src/queue-recipient.test.ts +++ b/packages/plugins/plugin-sharing/src/queue-recipient.test.ts @@ -158,7 +158,8 @@ describe('[ADR-0142 D7] the queue recipient', () => { */ describe('[ADR-0142 D7] a failed queue-member read keeps the queue-recipient rule\'s grants (#29604)', () => { let engine: ReturnType; - let warn: ReturnType; + type Warn = (msg: string, meta?: Record) => void; + let warn: ReturnType>; let rules: SharingRuleService; const REFUSAL = /\[queue\] queue 'billing' in organization 'org_a' does not answer its members, because a read behind it failed/; const granteesOf = (recordId: string): string[] => @@ -183,14 +184,14 @@ describe('[ADR-0142 D7] a failed queue-member read keeps the queue-recipient rul { id: 'qm_2', queue: 'billing', recipient_type: 'team', recipient_id: 't_night', organization_id: 'org_a' }, ]); engine.seed('sys_team_member', [{ id: 'tm_1', team_id: 't_night', user_id: 'u_d' }]); - warn = vi.fn(); - const logger = { warn, info: vi.fn(), debug: vi.fn(), error: vi.fn() }; + warn = vi.fn(); + const logger = { warn, info: vi.fn() }; const queues = new OwnershipQueueService({ engine: () => engine as any, logger }); rules = new SharingRuleService({ engine: engine as any, sharing: new SharingService({ engine: engine as any }), ownershipQueues: () => queues, - logger: logger as any, + logger, }); await rules.evaluateRule(RULE, SYS); expect(granteesOf('inv1')).toEqual(['u_c', 'u_d']); From 33a64b2be4d764bd279b1d7d97df0907e5569d38 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 07:52:14 +0000 Subject: [PATCH 13/14] docs(approvals): a failed read behind a team, position or queue approver refuses the node Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- content/docs/automation/approvals.mdx | 32 +++++++++++++++++++-------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index cbc111d3419..12d19d94639 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -455,15 +455,29 @@ What an entry that resolves to nobody leaves on the slate depends on its kind: decide it (the admin-override callout under [Acting on requests in the console](#acting-on-requests-in-the-console)). Beside entries that do resolve, a literal is a slot like any other, so a - `behavior` that needs every slot (`unanimous`) waits on it too. A `team`, - `position`, `department` or `org_membership_level` entry whose holder read - failed (`sys_team_member`, `sys_user_position`, `sys_business_unit_member` - or `sys_member`, and for a `department` also the `sys_business_unit` read of - the unit and of its child units) keeps its literal the same way. It logs `could not be - expanded — a directory or read-filter lookup failed` with the `error`, - instead of `expanded to nobody`, and never routes to the holders that - happened to read. An `expression` value whose `resolveAs` read failed keeps - its literal and logs the same line, with the `expression` source. + `behavior` that needs every slot (`unanimous`) waits on it too. A + `department` or `org_membership_level` entry whose holder read failed + (`sys_business_unit_member` or `sys_member`, and for a `department` also the + `sys_business_unit` read of the unit and of its child units) keeps its + literal the same way. It logs `could not be expanded — a directory or + read-filter lookup failed` with the `error`, instead of `expanded to + nobody`, and never routes to the holders that happened to read. An + `expression` value whose `resolveAs: 'department'` read failed keeps its + literal and logs the same line, with the `expression` source. +- **A failed read behind a `team`, `position` or `queue` entry** is not an + empty slate, and the node does not open (ADR-0142 D6). That is a failed + `sys_team_member` or `sys_user_position` read, the `sys_member` read behind a + position, or, for a queue, a failed read of its `sys_queue` row, its + `sys_queue_member` rows or a member table one of its members reads. An + `expression` value resolved as `team` or `position`, and a cross-organization + read filter that fails in front of any of these entries, count the same. + Under every `onEmptyApprovers` value, the `fallbackApprovers` slate included, + the node fails with a retryable error starting `APPROVER_DIRECTORY_UNREADABLE:` + that names the approver and the read, and writes no request. It is not + opened on the rest of the slate, on the fallback or on the administrators, + and it is not auto-approved. Run the flow again once the directory can be + read. A decision on a request opened before approver snapshots existed still + re-reads its slate the old way, which keeps the literal. - **A slate left empty, or holding only literals,** goes to the node's `onEmptyApprovers` policy, whose four values are listed under [Approving across organizations](#cross-org-approvers). Under the default, From b24881883d2b1704cded17f71731d95b609febee Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 08:29:33 +0000 Subject: [PATCH 14/14] chore(changeset): leave the first part's pending note as written, and state in this entry what it supersedes Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh --- .changeset/29604-approver-directory-refusal-queue-paging.md | 6 +++--- .changeset/29604-queue-service-refuses-failed-read.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.changeset/29604-approver-directory-refusal-queue-paging.md b/.changeset/29604-approver-directory-refusal-queue-paging.md index beb009bd565..f9b0dac2830 100644 --- a/.changeset/29604-approver-directory-refusal-queue-paging.md +++ b/.changeset/29604-approver-directory-refusal-queue-paging.md @@ -5,7 +5,7 @@ fix(plugin-approvals)!: an approval node refuses to open when a team or position approver's directory read fails, or a queue approver's expansion rejects, and the ownership-queue reads page to completion -Clause-②: yes (narrowing) — an approval node no longer opens when a `team` or `position` approver (static, or an `expression` approver with `resolveAs` `team` / `position`) cannot be expanded because a directory read or the cross-organization read filter failed, or when a `queue` approver's expansion rejects. Under every `onEmptyApprovers` policy, the `fallbackApprovers` slate included, it now fails the node with a retryable `APPROVER_DIRECTORY_UNREADABLE:` error naming the read, where it used to open on the rest of the slate, on the fallback slate or on the administrators, or auto-approve. A `queue` approver refuses when `expandMembers` rejects, which the registered ownership-queue service does on a failed member read once the slate asks it to (the companion change in this release, `onFailedRead: 'refuse'`). Every other input answers as before. +Clause-②: yes (narrowing) — an approval node no longer opens when a `team` or `position` approver (static, or an `expression` approver with `resolveAs` `team` / `position`) cannot be expanded because a directory read or the cross-organization read filter failed, or when a `queue` approver's expansion rejects. Under every `onEmptyApprovers` policy, the `fallbackApprovers` slate included, it now fails the node with a retryable `APPROVER_DIRECTORY_UNREADABLE:` error naming the read, where it used to open on the rest of the slate, on the fallback slate or on the administrators, or auto-approve. The registered ownership-queue service does not reject yet: a failed member read inside it still answers nobody until #29604 part 2 (#30251), so a `queue` approver refuses here only when `expandMembers` itself rejects. Every other input answers as before. Metadata-protocol: none @@ -15,6 +15,6 @@ Metadata-protocol: none - **Before.** A `team` or `position` approver whose `sys_team_member`, `sys_user_position` or `sys_member` read failed kept its `type:value` literal slot, which nobody can act on, and logged the failure. A `queue` approver whose expansion threw answered "expanded to nobody" and added no slot. Either way the node went on as if those people did not exist: beside other approvers it opened on them alone, and on an otherwise empty slate the node's `onEmptyApprovers` policy acted, opening on the `fallbackApprovers` or the organization's administrators, or auto-approving. - **Now.** The node does not open and writes no request, approver or action row. It fails with an `ApproverDirectoryUnreadableError` whose message starts `APPROVER_DIRECTORY_UNREADABLE:`, names the approver, the organization and the read that failed (a failure of the cross-organization read filter is named as that filter's read, on a static approver and on an `expression` one alike), and is retryable: the flow node's failure is a runtime one, so the run can be resumed once the directory can be read. A `fallbackApprovers` slate whose own `team` or `position` read fails refuses the same way. `department` / `business_unit`, `org_membership_level` and `manager` approvers are not named by the ruling and answer exactly as before. - **Unchanged.** A decision on a request opened before the approver snapshot existed still re-reads its slate on the old path, which keeps the literal slot on a failed read, because the decision has already been recorded when that read happens. -- **The queue service's half.** The ownership-queue service shipped in `@objectstack/plugin-sharing` rejects a failed `sys_queue` / `sys_queue_member`, team, position or business-unit read when the caller passes `onFailedRead: 'refuse'`, and the slate passes it, so a `queue` approver backed by it refuses on such a failure too. That half is its own entry in this release ("the ownership-queue service refuses a failed member read when the caller asks"). +- **Not yet.** The ownership-queue service shipped in `@objectstack/plugin-sharing` answers a failed `sys_queue` / `sys_queue_member`, team or position read with nobody (logged once) rather than rejecting, so a `queue` approver backed by it still adds no slot on such a failure. Its refusal arrives with #29604 part 2, once the contract can carry it (#30251). -`@objectstack/plugin-sharing`: the ownership-queue service reads its `sys_queue` and `sys_queue_member` rows by id, page after page, to the end, where it read one page of 10,000 rows and answered on it. A queue's 10,001st member, an organization's 10,001st queue and the queues naming a recipient past that bound are now found. A read that fails partway is a failed read, never an answer on the pages it had read: under the default it contributes nobody, logged once, with the read's reason and without the walk's own "refuses instead" clause, and under `onFailedRead: 'refuse'` it rejects. +`@objectstack/plugin-sharing`: the ownership-queue service reads its `sys_queue` and `sys_queue_member` rows by id, page after page, to the end, where it read one page of 10,000 rows and answered on it. A queue's 10,001st member, an organization's 10,001st queue and the queues naming a recipient past that bound are now found. A read that fails partway answers as a failed read did before (that read contributes nobody, logged once, with the read's reason and without the walk's own "refuses instead" clause), never with the pages it had read. diff --git a/.changeset/29604-queue-service-refuses-failed-read.md b/.changeset/29604-queue-service-refuses-failed-read.md index 6f406e7761f..012338227ae 100644 --- a/.changeset/29604-queue-service-refuses-failed-read.md +++ b/.changeset/29604-queue-service-refuses-failed-read.md @@ -16,4 +16,4 @@ Metadata-protocol: none - **Unchanged: the default.** Without the option (`onFailedRead: 'nobody'`), a failed read still contributes nobody, logged once. The sitting-record access path (`queuesOf`), the claim admission and the assignment pool take that answer, so they deny on a failed read exactly as before. A failed read of the activation ledger (`sys_metadata_activation`) is not a failed read in this sense under either answer: it still reads "switched on" for that call, logged, the posture the sharing-rule service takes for its own position recipients. - **The approver slate (D6).** A `queue` approver asks with `onFailedRead: 'refuse'` beside the addressing purpose. When the registered service rejects, the node fails with the retryable `APPROVER_DIRECTORY_UNREADABLE:` error that a failed team or position read already raises, naming the queue's failed read, and writes no request, approver or action row. Before, the registered service answered nobody, so the queue added no slot and the node's `onEmptyApprovers` policy acted on a slate missing those people. - **The sharing-recipient grant recompute (D7).** A `queue`-recipient rule's expansion and its per-user membership check ask with `onFailedRead: 'refuse'`. A failed member read now rejects before any grant is diffed, so the rule's existing grants stay as they were until a pass can read the queue, and each recompute's caller logs it: the record pass and the object pass per rule (`record reconcile failed for rule`, `object reconcile failed for rule`), the queue-membership and business-unit write recomputes (`queue-membership revocation failed`, `queue-membership re-grant failed`, `business-unit revocation failed`, `business-unit re-grant failed`), the membership-row recompute (`recipient membership re-evaluation failed`) and the boot backfill (`boot rule backfill failed for rule`). A direct call to `evaluateRule`, `revokeRuleGrantsForRetiredRecipients` or `reconcileRuleForUser`, the REST evaluate endpoint included, rejects with the service's failure. Before, the failed read answered nobody and the pass revoked the grant of every member behind it. This is the answer a `team` or `position` recipient has given since its own reads began to refuse. -- **The gap the approver refusal left, closed.** With the approval node's own refusal (its entry in this release, "an approval node refuses to open when a team or position approver's directory read fails…") alone, a `queue` approver backed by the registered service still added no slot on a failed member read, because the service answered nobody instead of rejecting; it refused only when `expandMembers` itself rejected. The service now rejects when asked and the slate asks, so that approver refuses on a failed `sys_queue`, `sys_queue_member`, team, position or business-unit read, as a `team` or `position` approver does. +- **Supersedes the approval node's "Not yet".** The approval node's own entry in this release ("an approval node refuses to open when a team or position approver's directory read fails…") says, in its `Clause-②:` line and its **Not yet** bullet, that the registered ownership-queue service answers a failed member read with nobody rather than rejecting, so a `queue` approver backed by it refuses only when `expandMembers` itself rejects. That was true when it was written and is not after this change: the service now rejects on that failure when asked, and the slate asks, so a `queue` approver backed by it refuses on a failed `sys_queue`, `sys_queue_member`, team, position or business-unit read, as a `team` or `position` approver does. Read this bullet in place of those two sentences.