Skip to content

Commit fe92819

Browse files
committed
test(plugin-sharing): pin that the per-grant catch and the field recipient compose
After merging #14930 into this branch the two changes share reconcile: the whole-rule pass diffs a DesiredGrantSet and attempts each grant individually. Pinned in both directions on the field kind — a refused grant is counted and the pass (with its stale-row revocations) continues on both reconcile paths; the catch stays narrow on an unrelated engine error; the rule-wide switch's refusal of a field rule carries no engine code and is never reached by a production pass. Also tidies the blank line the merge left before reconcile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
1 parent 78cca29 commit fe92819

2 files changed

Lines changed: 108 additions & 1 deletion

File tree

packages/plugins/plugin-sharing/src/field-recipient.test.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,19 @@ function makeEngine() {
114114
const f = opts?.filter ?? opts?.where;
115115
return ensure(o).filter((r) => matches(r, f)).slice(0, opts?.limit ?? 10000);
116116
},
117+
/**
118+
* [#14754] A recipient whose `sys_record_share` INSERT the engine refuses,
119+
* with the organization-rule code the per-grant catch absorbs (or another
120+
* code, to prove the catch stays narrow). Empty = nothing refused.
121+
*/
122+
_refuseGrantFor: '' as string,
123+
_refuseGrantCode: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' as string,
117124
async insert(o: string, data: any) {
125+
if (o === 'sys_record_share' && engine._refuseGrantFor && data?.recipient_id === engine._refuseGrantFor) {
126+
const err: any = new Error(`Insert on '${o}' was refused: the write resolves no organization`);
127+
err.code = engine._refuseGrantCode;
128+
throw err;
129+
}
118130
const row = { id: data.id ?? `${o}_${++seq}`, ...data };
119131
ensure(o).push(row);
120132
return row;
@@ -806,3 +818,99 @@ describe('#15072 authoring seams', () => {
806818
});
807819
});
808820
});
821+
822+
// ─────────────────────────────────────────────────────────────────────────
823+
/**
824+
* [#15072 × #14754] The per-grant catch (#14930, landed while this card was in
825+
* flight) and the per-record recipient kind compose — pinned in both
826+
* directions, because they meet in the same `reconcile`:
827+
*
828+
* - a refused grant inside a FIELD rule's pass is counted and the pass, with
829+
* its stale-row revocations, continues (the #14754 contract holds for the
830+
* new kind on both reconcile paths);
831+
* - the catch is NARROW and does not absorb this card's deliberate refusal:
832+
* the rule-wide `expandRecipient` THROWS for a field rule (never `[]`), that
833+
* error carries no engine code, and no production pass ever reaches it.
834+
*/
835+
describe('#15072 × #14754 — the per-grant catch and the per-record kind compose', () => {
836+
let h: ReturnType<typeof harness>;
837+
838+
/** The organization-refusal lines the pass logged. */
839+
const refusalWarns = () =>
840+
h.warn.mock.calls.filter((c) => String(c[0]).includes('refused by the engine organization rule'));
841+
842+
beforeEach(() => {
843+
h = harness();
844+
h.engine.seed('sys_sharing_rule', [fieldRule()]);
845+
h.engine.seed(OBJECT, [
846+
{ id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] },
847+
{ id: 'req_2', status: 'approved', owner_id: 'boss', assignees: ['u_refused', 'u_c'] },
848+
]);
849+
});
850+
851+
it('whole-rule pass: ONE refused pair is counted, the other pairs land, and the stale row is still revoked', async () => {
852+
h.seedStaleGrant('req_1', 'u_old');
853+
h.engine._refuseGrantFor = 'u_refused';
854+
855+
const result = await h.rules.evaluateRule(RULE, SYS);
856+
857+
expect(h.granteesOf('req_1')).toEqual(['u_a']); // u_old revoked — the security half
858+
expect(h.granteesOf('req_2')).toEqual(['u_c']); // the pair AFTER the refused one landed
859+
expect(result).toMatchObject({ matchedRecords: 2, expandedUsers: 3, grantsCreated: 2, grantsRevoked: 1, grantsRefused: 1 });
860+
const warns = refusalWarns();
861+
expect(warns).toHaveLength(1);
862+
expect(warns[0][1]).toMatchObject({ rule: RULE, object: OBJECT, record: 'req_2', recipient: 'u_refused', code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' });
863+
});
864+
865+
it('per-record pass: the refused user does not stop that record\'s other user, nor its own stale revocation', async () => {
866+
h.seedStaleGrant('req_2', 'u_old');
867+
h.engine._refuseGrantFor = 'u_refused';
868+
869+
const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_2', SYS);
870+
871+
expect(h.granteesOf('req_2')).toEqual(['u_c']);
872+
expect(result).toMatchObject({ matchedRecords: 1, expandedUsers: 2, grantsCreated: 1, grantsRevoked: 1, grantsRefused: 1 });
873+
});
874+
875+
it('the catch stays NARROW on the new kind too: an unrelated engine error still aborts the pass', async () => {
876+
h.engine._refuseGrantFor = 'u_refused';
877+
h.engine._refuseGrantCode = 'SOME_OTHER_ENGINE_ERROR';
878+
await expect(h.rules.evaluateRule(RULE, SYS)).rejects.toMatchObject({ code: 'SOME_OTHER_ENGINE_ERROR' });
879+
expect(refusalWarns()).toEqual([]);
880+
});
881+
882+
it('the rule-wide switch REFUSES a field rule with a code-less error — nothing the per-grant catch could absorb', async () => {
883+
const rule = (await h.rules.getRule('srule_assignees', SYS))!;
884+
let thrown: any;
885+
try {
886+
await (h.rules as any).expandRecipient(rule);
887+
} catch (err) {
888+
thrown = err;
889+
}
890+
expect(thrown).toBeInstanceOf(Error);
891+
expect(String(thrown.message)).toMatch(/field recipient, which expands per RECORD/);
892+
// No `code` at all: `grantOrAbsorbOrganizationRefusal` compares `err.code`
893+
// against the ONE engine code it absorbs, and this error can never equal it.
894+
expect(thrown.code).toBeUndefined();
895+
expect(thrown.code).not.toBe('ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED');
896+
});
897+
898+
it('no production pass reaches that refusal for a field rule — and the rule-wide switch still serves the other kinds', async () => {
899+
h.engine.seed('sys_sharing_rule', [fieldRule({
900+
id: 'srule_alice', name: 'approved_to_alice', recipient_type: 'user', recipient_id: 'alice',
901+
})]);
902+
const spy = vi.spyOn(h.rules as any, 'expandRecipient');
903+
904+
await h.rules.evaluateRule(RULE, SYS);
905+
await h.rules.evaluateAllRulesForObject(OBJECT);
906+
await h.rules.evaluateAllForRecord(OBJECT, 'req_2', SYS);
907+
908+
const kinds = spy.mock.calls.map((c: any[]) => (c[0] as Row).recipient_type);
909+
expect(kinds).not.toContain('field');
910+
expect(kinds.filter((k) => k === 'user').length).toBeGreaterThan(0); // the control kind went through it
911+
// …and every pass completed with the field grants materialised.
912+
expect(h.granteesOf('req_2')).toEqual(['u_c', 'u_refused']);
913+
expect(h.granteesOf('req_1', 'srule_alice')).toEqual(['alice']);
914+
spy.mockRestore();
915+
});
916+
});

packages/plugins/plugin-sharing/src/sharing-rule-service.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1695,7 +1695,6 @@ export class SharingRuleService implements ISharingRuleService {
16951695
}
16961696
}
16971697

1698-
16991698
/**
17001699
* Diff a whole-rule pass's desired pairs against the rule's materialised
17011700
* grants: upsert what is wanted, revoke the remainder.

0 commit comments

Comments
 (0)