Skip to content

Commit 7e2469f

Browse files
committed
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
1 parent 596ee7a commit 7e2469f

5 files changed

Lines changed: 281 additions & 35 deletions

File tree

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

Lines changed: 135 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111

1212
import { describe, it, expect, beforeEach, vi } from 'vitest';
1313
import { APPROVAL_REVISE_NODE_TYPE, templateSlotSource } from '@objectstack/spec/automation';
14-
import { ApprovalService, REMIND_COOLDOWN_MS, ESCALATION_ENABLED_FLIP_CUTOFF_MS, SLA_ACTOR_ID } from './approval-service.js';
14+
import {
15+
ApprovalService, ApproverDirectoryUnreadableError, REMIND_COOLDOWN_MS, ESCALATION_ENABLED_FLIP_CUTOFF_MS, SLA_ACTOR_ID,
16+
} from './approval-service.js';
1517
import { bindApprovalLockHook, bindDelegationWriteGuard, unbindAllHooks } from './lifecycle-hooks.js';
1618
import { celEngine } from '@objectstack/formula';
1719
import { SysApprovalRequest } from './sys-approval-request.object.js';
@@ -4207,6 +4209,138 @@ describe('ApprovalService — a queue approver routes to the queue\'s members (A
42074209
});
42084210
});
42094211

4212+
// [ADR-0142 D6 as amended; ruling record 6599731698 on #29604, letter B] A
4213+
// directory read behind a `team`, `position` or `queue` approver that FAILS is
4214+
// not an empty slate: the node refuses to open under every `onEmptyApprovers`
4215+
// policy, on its own retryable error class, naming the read — and writes
4216+
// nothing. Before, the slot fell back to its literal and the policy acted on a
4217+
// slate missing the people who could not be read.
4218+
describe('ApprovalService — a team, position or queue approver whose directory read fails refuses the node (ADR-0142 D6)', () => {
4219+
const POLICIES = ['admin_rescue', 'auto_approve', 'fail', 'fallback'] as const;
4220+
4221+
/** A service over `engine` whose reads of `failing` throw, as a driver outage does. */
4222+
const outage = (failing: string[], queues?: () => any) => {
4223+
const engine = makeFakeEngine();
4224+
engine._tables['sys_team_member'] = [{ id: 'tm1', team_id: 't_ops', user_id: 'u5' }];
4225+
engine._tables['sys_user_position'] = [{ id: 'up1', position: 'cfo', user_id: 'u9', organization_id: 't1' }];
4226+
const real = engine.find.bind(engine);
4227+
engine.find = async (object: string, options?: any) => {
4228+
if (failing.includes(object)) throw new Error(`SQLITE_IOERR: disk I/O error reading ${object}`);
4229+
return real(object, options);
4230+
};
4231+
const warnings: any[] = [];
4232+
let n = 0;
4233+
const svc = new ApprovalService({
4234+
engine,
4235+
clock: { now: () => new Date(1757000000000 + (n++) * 1000) },
4236+
logger: { warn: (msg: any, meta: any) => warnings.push([msg, meta]) },
4237+
...(queues ? { ownershipQueues: queues } : {}),
4238+
});
4239+
return { engine, svc, warnings };
4240+
};
4241+
const node = (approvers: any[], configExtra: Record<string, any> = {}) => ({
4242+
...openInput([]),
4243+
config: { approvers, behavior: 'first_response' as const, lockRecord: false, ...configExtra },
4244+
});
4245+
const policy = (p: (typeof POLICIES)[number]) => (
4246+
p === 'fallback' ? { onEmptyApprovers: p, fallbackApprovers: [{ type: 'user', value: 'u_backstop' }] } : { onEmptyApprovers: p }
4247+
);
4248+
const refusalOf = (p: Promise<unknown>) => p.then(() => null, (e: unknown) => e);
4249+
const nothingWritten = (engine: ReturnType<typeof makeFakeEngine>) => {
4250+
expect(engine._tables['sys_approval_request'] ?? []).toEqual([]);
4251+
expect(engine._tables['sys_approval_approver'] ?? []).toEqual([]);
4252+
expect(engine._tables['sys_approval_action'] ?? []).toEqual([]);
4253+
};
4254+
4255+
for (const [type, value, table] of [['team', 't_ops', 'sys_team_member'], ['position', 'cfo', 'sys_user_position']] as const) {
4256+
for (const p of POLICIES) {
4257+
it(`a ${type} approver whose ${table} read fails: onEmptyApprovers '${p}' opens nothing and approves nothing`, async () => {
4258+
const { engine, svc } = outage([table]);
4259+
const refusal = await refusalOf(svc.openNodeRequest(node([{ type, value }], policy(p)), CTX));
4260+
4261+
expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError);
4262+
expect(refusal).toMatchObject({ approverType: type, approverValue: value, retryable: true });
4263+
expect((refusal as Error).message).toMatch(new RegExp(`^APPROVER_DIRECTORY_UNREADABLE: approver '${type}:${value}'`));
4264+
expect((refusal as ApproverDirectoryUnreadableError).failedRead).toMatch(/SQLITE_IOERR/);
4265+
expect((refusal as Error).message).not.toMatch(/NO_APPROVERS|expanded to nobody/);
4266+
nothingWritten(engine);
4267+
});
4268+
}
4269+
}
4270+
4271+
it('beside a staffed person, the node still refuses: no slate shortened to the people who could be read', async () => {
4272+
const { engine, svc } = outage(['sys_team_member']);
4273+
const refusal = await refusalOf(svc.openNodeRequest(
4274+
node([{ type: 'user', value: 'u2' }, { type: 'team', value: 't_ops' }]), CTX,
4275+
));
4276+
4277+
expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError);
4278+
nothingWritten(engine);
4279+
});
4280+
4281+
it('a fallback slate whose directory read fails refuses too, rather than degrade to the administrators', async () => {
4282+
// The primary slate reads fine and is empty (an unstaffed position); the
4283+
// declared fallback names a team whose read fails.
4284+
const { engine, svc } = outage(['sys_team_member']);
4285+
const refusal = await refusalOf(svc.openNodeRequest(node([{ type: 'position', value: 'nobody_holds_this' }], {
4286+
onEmptyApprovers: 'fallback', fallbackApprovers: [{ type: 'team', value: 't_ops' }],
4287+
}), CTX));
4288+
4289+
expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError);
4290+
expect(refusal).toMatchObject({ approverType: 'team', approverValue: 't_ops' });
4291+
nothingWritten(engine);
4292+
});
4293+
4294+
it('a queue whose expansion fails refuses before its "expanded to nobody" answer', async () => {
4295+
const { engine, svc, warnings } = outage([], () => ({
4296+
expandMembers: async () => { throw new Error('ownership-queue read failed'); },
4297+
}));
4298+
const refusal = await refusalOf(svc.openNodeRequest(node([{ type: 'queue', value: 'billing' }]), CTX));
4299+
4300+
expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError);
4301+
expect(refusal).toMatchObject({ approverType: 'queue', approverValue: 'billing' });
4302+
expect(warnings.some(([msg]) => String(msg).includes("approver 'queue:billing' expanded to nobody"))).toBe(false);
4303+
nothingWritten(engine);
4304+
});
4305+
4306+
it('control — the readable slate opens as before', async () => {
4307+
const { svc } = outage([]);
4308+
const req = await svc.openNodeRequest(node([{ type: 'team', value: 't_ops' }, { type: 'position', value: 'cfo' }]), CTX) as any;
4309+
4310+
expect(req.status).toBe('pending');
4311+
expect([...req.pending_approvers].sort()).toEqual(['u5', 'u9']);
4312+
});
4313+
4314+
it('control — a department read that fails is not named by the ruling: the node still opens, as before', async () => {
4315+
const { engine, svc } = outage(['sys_business_unit', 'sys_business_unit_member']);
4316+
const outcome = await refusalOf(svc.openNodeRequest(
4317+
node([{ type: 'department', value: 'bu_x' }], { onEmptyApprovers: 'fallback', fallbackApprovers: [{ type: 'user', value: 'u_backstop' }] }),
4318+
CTX,
4319+
));
4320+
4321+
expect(outcome).toBeNull();
4322+
expect(engine._tables['sys_approval_request']).toHaveLength(1);
4323+
});
4324+
4325+
it("decide's pre-snapshot re-resolution keeps the literal: a recorded decision is never failed by the outage", async () => {
4326+
// A unanimous request opened while the directory read, then stripped of
4327+
// its open-time snapshot as a request opened before the snapshot existed.
4328+
const failing: string[] = [];
4329+
const { engine, svc } = outage(failing);
4330+
const req = await svc.openNodeRequest(
4331+
node([{ type: 'user', value: 'u2' }, { type: 'team', value: 't_ops' }], { behavior: 'unanimous' }), CTX,
4332+
) as any;
4333+
const row = engine._tables['sys_approval_request'].find((r: any) => r.id === req.id);
4334+
const config = JSON.parse(row.node_config_json);
4335+
delete config.__approverGroups;
4336+
row.node_config_json = JSON.stringify(config);
4337+
4338+
failing.push('sys_team_member');
4339+
const decided = await svc.decideNode(req.id, { decision: 'approve', actorId: 'u2' }, SYS);
4340+
expect(decided.decision).toBe('approve');
4341+
});
4342+
});
4343+
42104344
// #3807 follow-up: `queue` was not the only way to end up with a slot nobody
42114345
// can act on — every GRAPH approver type falls back to the same literal when
42124346
// its lookup finds no one, and that fallback used to happen in total silence.

‎packages/plugins/plugin-approvals/src/business-unit-holders-complete.integration.test.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import { SqlDriver } from '@objectstack/driver-sql';
3030
import {
3131
SysBusinessUnit, SysBusinessUnitMember, SysMember, SysOrganization, SysTeamMember,
3232
} from '@objectstack/platform-objects/identity';
33-
import { ApprovalService } from './approval-service.js';
33+
import { ApprovalService, ApproverDirectoryUnreadableError } from './approval-service.js';
3434

3535
const ORG = 'org_large';
3636
const ELSEWHERE = 'org_elsewhere';
@@ -129,8 +129,8 @@ const department = (rig: Rig, value = ROOT) =>
129129
(rig.svc as any).expandBusinessUnitUsers(value, ORG) as Promise<string[]>;
130130
const resolveSpec = (rig: Rig, a: { type: string; value: string }) =>
131131
(rig.svc as any).resolveApproverSpec(a, undefined, ORG, Date.now()) as Promise<string[]>;
132-
const resolveExpression = (rig: Rig, a: { type: string; value: string; resolveAs: string }) =>
133-
(rig.svc as any).resolveExpressionApprovers(a, {}, ORG, Date.now()) as Promise<{
132+
const resolveExpression = (rig: Rig, a: { type: string; value: string; resolveAs: string }, onDirectoryFailure?: 'refuse' | 'literal') =>
133+
(rig.svc as any).resolveExpressionApprovers(a, {}, ORG, Date.now(), undefined, undefined, onDirectoryFailure) as Promise<{
134134
slots: Array<{ id: string; subGroup?: string }>;
135135
raw: string[];
136136
}>;
@@ -220,10 +220,18 @@ describe('[#29606] item 3 — an expression approver whose resolveAs expansion f
220220
expect(failed[0][1].error).toMatch(/SQLITE_IOERR/);
221221
});
222222

223-
it('⛔ a failed team read through resolveAs warns the same way', async () => {
223+
it('⛔ a failed team read through resolveAs refuses the node as a team approver does; the literal reader warns the same way', async () => {
224224
const rig = await boot(0);
225225
rig.failReadsOf('sys_team_member');
226-
const { slots } = await resolveExpression(rig, { type: 'expression', value: `'team_x'`, resolveAs: 'team' });
226+
// [ADR-0142 D6 as amended, #29604] The value is read through the team
227+
// expander, so its failure refuses the node like a static `team` approver's.
228+
const refusal = await resolveExpression(rig, { type: 'expression', value: `'team_x'`, resolveAs: 'team' })
229+
.then(() => null, (e: unknown) => e);
230+
expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError);
231+
expect(refusal).toMatchObject({ approverType: 'team', approverValue: 'team_x', retryable: true });
232+
expect((refusal as Error).message).toMatch(/^APPROVER_DIRECTORY_UNREADABLE: approver 'expression' \(resolveAs 'team', value 'team_x'\)/);
233+
234+
const { slots } = await resolveExpression(rig, { type: 'expression', value: `'team_x'`, resolveAs: 'team' }, 'literal');
227235

228236
expect(slots).toEqual([{ id: 'team:team_x', subGroup: 'team_x' }]);
229237
const failed = failureWarnings(rig);

‎packages/plugins/plugin-approvals/src/member-tier-holders-complete.integration.test.ts‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,15 @@
1717
* and an `org_membership_level` approver logged "expanded to nobody",
1818
* indistinguishable from a tier nobody holds.
1919
*
20-
* What a caller sees now: a failed read throws out of the expansion, the
21-
* approver-spec resolver's directory catch takes it, and the slot falls back
22-
* to its literal (`position:<p>`, acted on by whoever holds the position at
23-
* decision time; `org_membership_level:<t>`, decided by the override) with a
24-
* warning that names the failed read and carries its `error` — never a slate
25-
* cut down to the part that happened to read.
20+
* What a caller sees now: a failed read throws out of the expansion and the
21+
* approver-spec resolver's directory catch takes it. An `org_membership_level`
22+
* slot falls back to its literal (`org_membership_level:<t>`, decided by the
23+
* override) with a warning that names the failed read and carries its `error`.
24+
* [ADR-0142 D6 as amended, #29604] A `position` approver, whose transition limb
25+
* this read is, refuses the approval node instead
26+
* (`ApproverDirectoryUnreadableError`); only the `'literal'` reader keeps its
27+
* `position:<p>` literal. Never a slate cut down to the part that happened to
28+
* read.
2629
*
2730
* Measured on a REAL engine (ObjectQL + SqlDriver over in-memory SQLite)
2831
* carrying the shipped `SysMember`, because the completion is a keyset read
@@ -33,7 +36,7 @@ import { describe, it, expect, afterEach, vi } from 'vitest';
3336
import { ObjectQL } from '@objectstack/objectql';
3437
import { SqlDriver } from '@objectstack/driver-sql';
3538
import { SysMember, SysOrganization } from '@objectstack/platform-objects/identity';
36-
import { ApprovalService } from './approval-service.js';
39+
import { ApprovalService, ApproverDirectoryUnreadableError } from './approval-service.js';
3740

3841
const ORG = 'org_large';
3942
const ELSEWHERE = 'org_elsewhere';
@@ -112,8 +115,8 @@ async function boot(n: number, opts: { registerMember?: boolean } = {}): Promise
112115
/** The private expansions, as the approver-spec resolver calls them. */
113116
const tier = (rig: Rig, value: string) => (rig.svc as any).expandMembershipTierUsers(value, ORG) as Promise<string[]>;
114117
const position = (rig: Rig, value: string) => (rig.svc as any).expandPositionUsers(value, ORG) as Promise<string[]>;
115-
const resolveSpec = (rig: Rig, a: { type: string; value: string }) =>
116-
(rig.svc as any).resolveApproverSpec(a, undefined, ORG, Date.now()) as Promise<string[]>;
118+
const resolveSpec = (rig: Rig, a: { type: string; value: string }, onDirectoryFailure?: 'refuse' | 'literal') =>
119+
(rig.svc as any).resolveApproverSpec(a, undefined, ORG, Date.now(), undefined, onDirectoryFailure) as Promise<string[]>;
117120

118121
const failMemberReads = (rig: Rig, fault: Error) => {
119122
const memberDriver = (rig.engine as any).getDriverForObject('sys_member');
@@ -168,11 +171,18 @@ describe('[#29362] approvals membership-tier expansion reads every holder (drive
168171
expect(rig.warn.mock.calls.some(([, meta]) => typeof meta?.error === 'string')).toBe(false);
169172
});
170173

171-
it('⛔ what the caller sees: the slot falls back to its literal, and the warning names the failed read', async () => {
174+
it('⛔ what the caller sees: a position refuses the node on its sys_member limb; the tier slot keeps its literal', async () => {
172175
const rig = await boot(3);
173176
failMemberReads(rig, new Error('SQLITE_IOERR: disk I/O error'));
174177

175-
expect(await resolveSpec(rig, { type: 'position', value: TIER })).toEqual([`position:${TIER}`]);
178+
// [#29604] The ADR-0057 D4 transition limb is a read behind the position:
179+
// its failure is a failed read, and the node refuses on it.
180+
const refusal = await resolveSpec(rig, { type: 'position', value: TIER }).then(() => null, (e: unknown) => e);
181+
expect(refusal).toBeInstanceOf(ApproverDirectoryUnreadableError);
182+
expect(refusal).toMatchObject({ approverType: 'position', approverValue: TIER, retryable: true });
183+
expect((refusal as ApproverDirectoryUnreadableError).failedRead).toMatch(/SQLITE_IOERR/);
184+
185+
expect(await resolveSpec(rig, { type: 'position', value: TIER }, 'literal')).toEqual([`position:${TIER}`]);
176186
expect(await resolveSpec(rig, { type: 'org_membership_level', value: TIER })).toEqual([`org_membership_level:${TIER}`]);
177187
const failed = rig.warn.mock.calls.filter(([, meta]) => typeof meta?.error === 'string');
178188
expect(failed.map(([, meta]) => meta.type)).toEqual(['position', 'org_membership_level']);

0 commit comments

Comments
 (0)