Skip to content

Commit c1c43e2

Browse files
committed
Merge origin/main into claude/project-thread-3koeqm
2 parents da2c28f + fe835b4 commit c1c43e2

8 files changed

Lines changed: 514 additions & 74 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/plugin-sharing': patch
3+
'@objectstack/plugin-approvals': patch
4+
---
5+
6+
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
7+
8+
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.
9+
Metadata-protocol: none
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped: the ownership-queue contract's `onFailedRead` option already exists, and the approval node config and the sharing rule keep their shapes. What moves is the answer two consumers get while a membership table cannot be read, and nothing at rest records that answer, so there is nothing for a migration to rewrite. -->
12+
13+
**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".
14+
15+
- **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 '<name>' in organization '<org>' 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`.
16+
- **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.
17+
- **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.
18+
- **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.
19+
- **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.

‎content/docs/automation/approvals.mdx‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -455,15 +455,29 @@ What an entry that resolves to nobody leaves on the slate depends on its kind:
455455
decide it (the admin-override callout under
456456
[Acting on requests in the console](#acting-on-requests-in-the-console)).
457457
Beside entries that do resolve, a literal is a slot like any other, so a
458-
`behavior` that needs every slot (`unanimous`) waits on it too. A `team`,
459-
`position`, `department` or `org_membership_level` entry whose holder read
460-
failed (`sys_team_member`, `sys_user_position`, `sys_business_unit_member`
461-
or `sys_member`, and for a `department` also the `sys_business_unit` read of
462-
the unit and of its child units) keeps its literal the same way. It logs `could not be
463-
expanded — a directory or read-filter lookup failed` with the `error`,
464-
instead of `expanded to nobody`, and never routes to the holders that
465-
happened to read. An `expression` value whose `resolveAs` read failed keeps
466-
its literal and logs the same line, with the `expression` source.
458+
`behavior` that needs every slot (`unanimous`) waits on it too. A
459+
`department` or `org_membership_level` entry whose holder read failed
460+
(`sys_business_unit_member` or `sys_member`, and for a `department` also the
461+
`sys_business_unit` read of the unit and of its child units) keeps its
462+
literal the same way. It logs `could not be expanded — a directory or
463+
read-filter lookup failed` with the `error`, instead of `expanded to
464+
nobody`, and never routes to the holders that happened to read. An
465+
`expression` value whose `resolveAs: 'department'` read failed keeps its
466+
literal and logs the same line, with the `expression` source.
467+
- **A failed read behind a `team`, `position` or `queue` entry** is not an
468+
empty slate, and the node does not open (ADR-0142 D6). That is a failed
469+
`sys_team_member` or `sys_user_position` read, the `sys_member` read behind a
470+
position, or, for a queue, a failed read of its `sys_queue` row, its
471+
`sys_queue_member` rows or a member table one of its members reads. An
472+
`expression` value resolved as `team` or `position`, and a cross-organization
473+
read filter that fails in front of any of these entries, count the same.
474+
Under every `onEmptyApprovers` value, the `fallbackApprovers` slate included,
475+
the node fails with a retryable error starting `APPROVER_DIRECTORY_UNREADABLE:`
476+
that names the approver and the read, and writes no request. It is not
477+
opened on the rest of the slate, on the fallback or on the administrators,
478+
and it is not auto-approved. Run the flow again once the directory can be
479+
read. A decision on a request opened before approver snapshots existed still
480+
re-reads its slate the old way, which keeps the literal.
467481
- **A slate left empty, or holding only literals,** goes to the node's
468482
`onEmptyApprovers` policy, whose four values are listed under
469483
[Approving across organizations](#cross-org-approvers). Under the default,

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

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4176,7 +4176,7 @@ describe('ApprovalService — a queue approver routes to the queue\'s members (A
41764176
const req = await svc.openNodeRequest(queueInput('billing'), CTX) as any;
41774177

41784178
expect(req.pending_approvers).toEqual(['u7', 'u8']);
4179-
expect(asked).toEqual([['billing', 't1', { purpose: 'addressing' }]]);
4179+
expect(asked).toEqual([['billing', 't1', { purpose: 'addressing', onFailedRead: 'refuse' }]]);
41804180
expect(warnings.filter(([msg]) => String(msg).includes('expanded to nobody'))).toEqual([]);
41814181
// A member decides it under their own id.
41824182
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
43494349
});
43504350
});
43514351

4352+
// [ADR-0142 D2 as amended, D6; #29604] The queue arm asks the ownership-queue
4353+
// service for its refusal (`onFailedRead: 'refuse'`), beside the ADDRESSING
4354+
// purpose. The registered service (`@objectstack/plugin-sharing`) answers a
4355+
// failed member read with nobody unless asked, so before the arm asked, a
4356+
// queue whose team or position read failed added no slot and the node's
4357+
// `onEmptyApprovers` policy opened, rescued or approved on a slate missing its
4358+
// people. The double below gives the contract's two answers as that service
4359+
// does: a rejection naming the read under `'refuse'`, `[]` under `'nobody'`.
4360+
describe('ApprovalService — a queue approver asks the queue service to refuse a failed read, and the node refuses with it (ADR-0142 D6)', () => {
4361+
const POLICIES = ['admin_rescue', 'auto_approve', 'fail', 'fallback'] as const;
4362+
const READ = "[queue] queue 'billing' in organization 't1' does not answer its members, because a read behind it failed: "
4363+
+ "the members of team 't_night' could not be read: SQLITE_IOERR: disk I/O error.";
4364+
4365+
const withFailingQueue = () => {
4366+
const engine = makeFakeEngine();
4367+
const asked: unknown[] = [];
4368+
const warnings: any[] = [];
4369+
let n = 0;
4370+
const svc = new ApprovalService({
4371+
engine,
4372+
clock: { now: () => new Date(1757000000000 + (n++) * 1000) },
4373+
logger: { warn: (msg: any, meta: any) => warnings.push([msg, meta]) },
4374+
ownershipQueues: () => ({
4375+
expandMembers: async (name: string, org: string | null, options?: { purpose?: string; onFailedRead?: string }) => {
4376+
asked.push([name, org, options]);
4377+
if (options?.onFailedRead === 'refuse') throw new Error(READ);
4378+
return [];
4379+
},
4380+
}),
4381+
});
4382+
return { engine, svc, asked, warnings };
4383+
};
4384+
const node = (configExtra: Record<string, any>) => ({
4385+
...openInput([]),
4386+
config: { approvers: [{ type: 'queue' as const, value: 'billing' }], behavior: 'first_response' as const, lockRecord: false, ...configExtra },
4387+
});
4388+
4389+
for (const p of POLICIES) {
4390+
it(`a queue whose member read fails: onEmptyApprovers '${p}' opens nothing and approves nothing`, async () => {
4391+
const { engine, svc, asked, warnings } = withFailingQueue();
4392+
const refusal = await svc.openNodeRequest(node(
4393+
p === 'fallback' ? { onEmptyApprovers: p, fallbackApprovers: [{ type: 'user', value: 'u_backstop' }] } : { onEmptyApprovers: p },
4394+
), CTX).then(() => null, (e: unknown) => e);
4395+
4396+
expect(asked).toEqual([['billing', 't1', { purpose: 'addressing', onFailedRead: 'refuse' }]]);
4397+
expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError);
4398+
expect(refusal).toMatchObject({ approverType: 'queue', approverValue: 'billing', retryable: true, failedRead: READ });
4399+
expect((refusal as Error).message).toMatch(/^APPROVER_DIRECTORY_UNREADABLE: approver 'queue:billing' could not be expanded in organization 't1'/);
4400+
expect(warnings.some(([msg]) => String(msg).includes("approver 'queue:billing' expanded to nobody"))).toBe(false);
4401+
expect(engine._tables['sys_approval_request'] ?? []).toEqual([]);
4402+
expect(engine._tables['sys_approval_approver'] ?? []).toEqual([]);
4403+
expect(engine._tables['sys_approval_action'] ?? []).toEqual([]);
4404+
});
4405+
}
4406+
});
4407+
43524408
// [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] The
43534409
// ADR-0105 D9 cross-organization read filter is a read behind a `position` or
43544410
// `team` approver as much as the directory expansion is. When it fails, the

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3482,12 +3482,18 @@ export class ApprovalService implements IApprovalService {
34823482
* fail-open). The service resolves the name registry-first, then the
34833483
* organization's own `sys_queue` row, and answers `[]` for a name that
34843484
* resolves nowhere. With no service registered there is nothing to expand.
3485+
*
3486+
* [#29604] `onFailedRead: 'refuse'` (D2 as amended): a failed read behind the
3487+
* queue rejects instead of answering "nobody", and {@link resolveApproverSpec}'s
3488+
* catch turns that rejection into the node's {@link ApproverDirectoryUnreadableError},
3489+
* so the slate is never opened without the people who could not be read. An
3490+
* axis of its own beside `purpose`.
34853491
*/
34863492
private async expandQueueUsers(queueName: string, organizationId?: string | null): Promise<string[]> {
34873493
if (!queueName) return [];
34883494
const queues = this.queueSource();
34893495
if (!queues) return [];
3490-
return queues.expandMembers(queueName, organizationId ?? null, { purpose: 'addressing' });
3496+
return queues.expandMembers(queueName, organizationId ?? null, { purpose: 'addressing', onFailedRead: 'refuse' });
34913497
}
34923498

34933499
/**

0 commit comments

Comments
 (0)