From a82a5ef8f705fb170b19c81ad2e74b03004cff18 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 06:23:48 +0000 Subject: [PATCH] feat(plugin-sharing): one INFO line when isSystem writes materialise zero sharing grants (#6783) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Demand 3 of #4707, maintainer-ruled 2026-08-06. The sharing-rule record-write hooks skip isSystem sessions, so a seed run lands rows on an object an ACTIVE rule covers and creates no sys_record_share rows. The skip is correct — the kernel:bootstrapped backfill heals it — but it was completely silent, which is indistinguishable from a broken sharing configuration (hotcrm#640). afterInsert and afterUpdate now emit SYSTEM_WRITE_SKIP_NOTICE once per object per hook-binding generation, carrying the ruled wording verbatim plus the object and its active rules. One line per batch, never per row. Deliberately unchanged: the skip itself, the absence of any new switch, and afterDelete's silence — a delete skips revocation, not materialisation, and no re-evaluation or restart can reach a grant whose record is gone. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ --- .../sharing-issystem-zero-grant-info.md | 53 +++ .../plugins/plugin-sharing/src/rule-hooks.ts | 101 ++++- .../src/system-write-skip-notice.test.ts | 375 ++++++++++++++++++ 3 files changed, 526 insertions(+), 3 deletions(-) create mode 100644 .changeset/sharing-issystem-zero-grant-info.md create mode 100644 packages/plugins/plugin-sharing/src/system-write-skip-notice.test.ts diff --git a/.changeset/sharing-issystem-zero-grant-info.md b/.changeset/sharing-issystem-zero-grant-info.md new file mode 100644 index 0000000000..4463c9e943 --- /dev/null +++ b/.changeset/sharing-issystem-zero-grant-info.md @@ -0,0 +1,53 @@ +--- +"@objectstack/plugin-sharing": patch +--- + +feat(plugin-sharing): an `isSystem` write batch that materialises zero sharing grants now says so, once (#6783) + +The sharing-rule record-write hooks skip `isSystem` sessions, so a seed run — or +any internal write batch — lands rows on an object an **active** sharing rule +covers and creates no `sys_record_share` rows at all. The skip is correct: the +`kernel:bootstrapped` backfill reconciles every rule and `evaluateRule` is +idempotent, so the state heals. What was wrong is that nothing said so. + +hotcrm#640 is the specimen: a fresh install with 9 active sharing rules, 9 +accounts matching their criteria, users holding the right positions — and an +empty `sys_record_share`. Every visible layer said "configured". The only way to +learn that the seed path had skipped materialisation was to query the table, +find it empty, and read `plugin-sharing`'s source. + +**What changed.** The two skips that drop grant materialisation — `afterInsert` +and `afterUpdate` — now emit one INFO line naming the behaviour and both +remedies: + +``` +[sharing-rule] sharing materialisation skipped for isSystem writes; re-evaluate rules or restart to backfill +``` + +with the object and the active rules on it as metadata. + +**One line per batch, not per row.** The notice is latched per object per hook +binding generation, so a seed batch writing 500 rows produces exactly one line. +The defect being fixed is silence; a per-row flood would be the same defect with +a different symptom. The latch re-arms with the binding — `bindRuleRebindTriggers` +re-binds the package on every `sys_sharing_rule` write — so a changed rule set +gets its own notice instead of inheriting the previous generation's silence. + +**INFO, not warn or error**, deliberately: the behaviour is correct and +self-healing, and warning about a subsystem working as designed is how operators +learn to ignore it. + +Deliberately unchanged: + +- **The skip itself.** No write now materialises grants that did not before, and + no `sys_record_share` row is created, updated or revoked by this change. +- **No new switch or flag.** The notice is unconditional. +- **`afterDelete` stays silent.** A delete skips *revocation*, not + materialisation, and the remedy the line names cannot repair that class: + `evaluateRule` iterates records that still exist, so neither re-evaluating a + rule nor restarting can reach a grant whose record is gone. That class belongs + to the record-delete share cascade and the boot orphan sweep. + +The line is a statement about the write path, not a claim that grants were owed — +whether a given seeded row satisfies a rule's criteria is exactly the query the +skip exists to avoid, so answering it here would cost the skip its purpose. diff --git a/packages/plugins/plugin-sharing/src/rule-hooks.ts b/packages/plugins/plugin-sharing/src/rule-hooks.ts index 9d42cee84a..0d5c4958e8 100644 --- a/packages/plugins/plugin-sharing/src/rule-hooks.ts +++ b/packages/plugins/plugin-sharing/src/rule-hooks.ts @@ -29,6 +29,33 @@ export const RULE_REBIND_TRIGGER_PACKAGE = 'plugin-sharing:rule-rebind'; */ export const RULE_CRITERIA_GUARD_PACKAGE = 'plugin-sharing:rule-criteria-guard'; +/** + * [#6783] The one INFO line an `isSystem` write batch gets when it lands rows + * on an object that an ACTIVE sharing rule covers and materialises no grants. + * + * The wording after the tag is the maintainer's, verbatim (ruling on #4707, + * 2026-08-06, demand 3): it names the behaviour AND both remedies, because the + * whole defect being fixed is that neither was discoverable. hotcrm#640 is the + * specimen — a fresh install with 9 active rules, 9 matching accounts and an + * empty `sys_record_share`, where every visible layer said "configured" and + * nothing said "inert". The only way to learn the truth was to query the table, + * find it empty, and read this file. + * + * It is a statement about the WRITE PATH, not a claim that grants were owed: + * whether a given seeded row would have matched a rule's criteria is precisely + * the query the skip exists to avoid, so answering it here would cost the skip + * its reason to exist. Worded this way the line is true in both cases — it says + * materialisation did not run, and where the answer comes from when it does. + * + * INFO, deliberately, not `warn`: the behaviour is CORRECT (the + * `kernel:bootstrapped` backfill in `sharing-plugin.ts` reconciles every rule + * and `evaluateRule` is idempotent), so a warning would train operators to + * ignore a subsystem that is working as designed. + */ +export const SYSTEM_WRITE_SKIP_NOTICE = + '[sharing-rule] sharing materialisation skipped for isSystem writes; ' + + 're-evaluate rules or restart to backfill'; + interface MinimalEngine { registerHook(event: string, handler: (ctx: any) => any | Promise, options?: { object?: string | string[]; @@ -99,6 +126,11 @@ export const ruleRegrantQueue = new RuleRegrantQueue(); * skipped recompute entirely and left stale `sys_record_share` rows granting * access the rules no longer imply. * + * [#6783] The two skips that drop GRANT MATERIALISATION (`afterInsert`, + * `afterUpdate`) now emit {@link SYSTEM_WRITE_SKIP_NOTICE} once per object per + * binding generation. The skips themselves are unchanged — the behaviour is + * correct and the boot backfill heals it; only the silence was the defect. + * * Caller is responsible for invoking {@link unbindAllRuleHooks} before * re-binding when the rule set changes. */ @@ -109,10 +141,55 @@ export function bindRuleHooks( logger?: MinimalLogger, ): void { const objects = new Set(); + /** Active rule names per object — the `rules:` field of the #6783 notice. */ + const activeRuleNames = new Map(); for (const r of rules) { if (r.active === false) continue; - if (r.object_name) objects.add(r.object_name); + if (!r.object_name) continue; + objects.add(r.object_name); + const named = activeRuleNames.get(r.object_name) ?? []; + named.push(String(r.name ?? r.id ?? '')); + activeRuleNames.set(r.object_name, named); } + + /** + * [#6783] Objects whose current silent window has already been reported. + * + * Scoped to this binding generation on purpose. The signal being added is + * "materialisation did not run here", which is a property of the OBJECT and + * of the rule set bound to it — not of the row — so a seed batch of N rows + * must produce ONE line, never N. The failure mode being fixed is silence; + * trading it for a per-row flood would replace one defect with another, and + * an operator who scrolls past the line is exactly as uninformed as one who + * was never told. + * + * The latch re-arms with the binding: `bindRuleRebindTriggers` unbinds and + * re-binds this whole package on every `sys_sharing_rule` write, so a rule + * set that changed gets its own notice rather than inheriting the previous + * generation's silence. + */ + const notified = new Set(); + + /** + * Emit {@link SYSTEM_WRITE_SKIP_NOTICE} at most once per object per binding + * generation. Never throws: this runs on the write path ahead of the hooks' + * own `try`, and a logger that throws must not fail an operator's write. The + * latch is claimed BEFORE the log so a throwing logger cannot turn one + * suppressed line into one throw per row. + */ + const noteSystemWriteSkipped = (objectName: string): void => { + if (notified.has(objectName)) return; + notified.add(objectName); + try { + logger?.info?.(SYSTEM_WRITE_SKIP_NOTICE, { + object: objectName, + rules: activeRuleNames.get(objectName) ?? [], + }); + } catch { + /* a logger that throws must not fail the write */ + } + }; + for (const objectName of objects) { const opts = { object: objectName, packageId: SHARING_RULE_HOOK_PACKAGE, priority: 180 }; @@ -162,7 +239,11 @@ export function bindRuleHooks( const affectedFrom = (ctx: any): AffectedRows => readAffectedRows(ctx); engine.registerHook('afterInsert', async (ctx: any) => { - if ((ctx?.session as any)?.isSystem) return; + if ((ctx?.session as any)?.isSystem) { + // [#6783] The skip stays exactly as it was; it just stops being silent. + noteSystemWriteSkipped(objectName); + return; + } try { const data = ctx?.result ?? ctx?.input?.data ?? {}; const id = String((data as any)?.id ?? ctx?.input?.id ?? ''); @@ -177,7 +258,13 @@ export function bindRuleHooks( engine.registerHook('beforeDelete', stashAffectedRows, opts); engine.registerHook('afterUpdate', async (ctx: any) => { - if ((ctx?.session as any)?.isSystem) return; + if ((ctx?.session as any)?.isSystem) { + // [#6783] An `isSystem` update INTO a rule's criteria owes grants the + // same way an insert does, and `evaluateRule` is diff-based, so the + // notice's remedy is true for both directions of an update. + noteSystemWriteSkipped(objectName); + return; + } try { const affected = affectedFrom(ctx); if (affected.kind === 'rows') { @@ -191,6 +278,14 @@ export function bindRuleHooks( }, opts); engine.registerHook('afterDelete', async (ctx: any) => { + // [#6783] Deliberately silent, unlike the insert/update skips above. + // What a delete skips is REVOCATION, not materialisation, and the + // notice's remedy would be false here: `evaluateRule` iterates records + // that still exist, so no re-evaluation and no restart can reach a grant + // whose record is gone (the orphan named at the tail of #4779). That + // class is owned by `record-share-cascade.ts` — which stashes for system + // writes on its own account (#5103) — and by the boot orphan sweep, so + // an INFO line here would point an operator at a repair that cannot run. if ((ctx?.session as any)?.isSystem) return; try { const affected = affectedFrom(ctx); diff --git a/packages/plugins/plugin-sharing/src/system-write-skip-notice.test.ts b/packages/plugins/plugin-sharing/src/system-write-skip-notice.test.ts new file mode 100644 index 0000000000..b1eb492aac --- /dev/null +++ b/packages/plugins/plugin-sharing/src/system-write-skip-notice.test.ts @@ -0,0 +1,375 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6783] "Configured but inert" stops being silent. + * + * Demand 3 of #4707, maintainer-ruled 2026-08-06. An `isSystem` write batch — + * a seed run, a package install, an internal importer — lands rows on an + * object an ACTIVE sharing rule covers, and the record-write hooks skip it, so + * ZERO `sys_record_share` rows are materialised. That skip is correct: the + * `kernel:bootstrapped` backfill reconciles every rule and `evaluateRule` is + * idempotent. What was wrong is that nothing said so. hotcrm#640: a fresh + * install with 9 active rules, 9 matching accounts, and an empty + * `sys_record_share` — every visible layer said "configured", and the only way + * to learn otherwise was to query the table and go read `rule-hooks.ts`. + * + * What this file pins, in both directions: + * + * - ONE line per batch, not per row. That boundary is the whole shape of the + * fix — the defect is silence, and a per-row flood would trade it for + * noise, which is the same defect with a different symptom. + * - INFO, never warn/error: the behaviour is correct and self-healing. + * - The maintainer's wording, verbatim, as a contract. + * - The negative faces: a non-system write (which materialises normally), + * an active rule whose criteria the written row does not satisfy (zero + * grants, but legitimately so), an object whose only rule is inactive, and + * an `isSystem` DELETE — deliberately silent, because the remedy the line + * names cannot repair a grant whose record is gone. + */ + +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 { + bindRuleHooks, + unbindAllRuleHooks, + SHARING_RULE_HOOK_PACKAGE, + SYSTEM_WRITE_SKIP_NOTICE, +} from './rule-hooks.js'; + +interface Row { [k: string]: any } + +const SYS = { isSystem: true, positions: [], permissions: [] } as any; +/** What a seed run / package install / internal importer sends. */ +const SYSTEM_SESSION = { isSystem: true }; +/** What an interactive admin sends — the path that DOES materialise. */ +const ADMIN_SESSION = { isSystem: false, userId: 'admin' }; + +type HookEntry = { event: string; handler: (ctx: any) => any; options: Row }; + +/** + * A fake ObjectQL engine, pinned to the real engine's write dispatch on both + * destructive verbs (#4550 / #5480) so a double looser than the thing it + * replaces cannot turn this suite green on calls production refuses. + */ +function makeEngine() { + const tables: Record = {}; + const hooks: HookEntry[] = []; + const ensure = (n: string) => (tables[n] ??= []); + + function matches(row: Row, f: any): boolean { + if (!f || typeof f !== 'object') return true; + if (Array.isArray(f.$or)) return f.$or.some((x: any) => matches(row, x)); + if (Array.isArray(f.$and)) return f.$and.every((x: any) => matches(row, x)); + for (const [k, v] of Object.entries(f)) { + if (k === '$or' || k === '$and') continue; + 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 (rv !== v) return false; + } + return true; + } + + const engine = { + _tables: tables, + getSchema() { return undefined; }, + async find(o: string, opts?: any) { + const f = opts?.filter ?? opts?.where; + return ensure(o).filter((r) => matches(r, f)).slice(0, opts?.limit ?? 10000); + }, + async insert(o: string, data: any) { const row = { ...data }; ensure(o).push(row); return row; }, + async update(o: string, data: any, options?: any) { + // Pinned to `ObjectQL.update`'s dispatch (#5480): a scalar `data.id` + // outranks `where`/`multi`, and a predicate update without `multi` is + // the shape a real server refuses. + assertEngineUpdateDispatch(data, options); + const t = ensure(o); const i = t.findIndex((r) => r.id === data?.id); + if (i >= 0) t[i] = { ...t[i], ...data }; + return t[i]; + }, + async delete(o: string, opts?: any) { + // Pinned to `ObjectQL.delete`'s dispatch (#4434 / #4550). + assertEngineDeleteDispatch(opts); + const t = ensure(o); const where = opts?.where ?? {}; + for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1); + return { ok: true }; + }, + registerHook(event: string, handler: (ctx: any) => any, options: Row = {}) { + hooks.push({ event, handler, options }); + }, + unregisterHooksByPackage(packageId: string) { + let removed = 0; + for (let i = hooks.length - 1; i >= 0; i--) { + if (hooks[i].options.packageId === packageId) { hooks.splice(i, 1); removed++; } + } + return removed; + }, + boundFor(packageId: string) { return hooks.filter((h) => h.options.packageId === packageId); }, + + async fire(event: string, object: string, ctx: any) { + for (const h of [...hooks]) { + if (h.event === event && h.options.object === object) await h.handler(ctx); + } + }, + + /** One row insert, fired the way the engine fires `afterInsert`. */ + async simulateInsert(object: string, row: Row, session: any = ADMIN_SESSION) { + ensure(object).push({ ...row }); + await engine.fire('afterInsert', object, { + object, event: 'afterInsert', input: { data: row }, result: row, session, + }); + }, + + /** `count` rows in one pass — a seed batch, one hook fire per row. */ + async simulateInsertBatch(object: string, rows: Row[], session: any = SYSTEM_SESSION) { + for (const row of rows) await engine.simulateInsert(object, row, session); + }, + + /** A predicate update: no `input.id`, one shared ctx across before/after. */ + async simulateBulkUpdate(object: string, where: any, data: Row, session: any = ADMIN_SESSION) { + const ctx: any = { + object, event: 'beforeUpdate', + input: { id: undefined, data, options: { where, multi: true } }, + session, + }; + await engine.fire('beforeUpdate', object, ctx); + const t = ensure(object); + for (let i = 0; i < t.length; i++) { + if (where != null && !matches(t[i], where)) continue; + t[i] = { ...t[i], ...data }; + } + ctx.event = 'afterUpdate'; + await engine.fire('afterUpdate', object, ctx); + }, + + async simulateBulkDelete(object: string, where: any, session: any = ADMIN_SESSION) { + const ctx: any = { + object, event: 'beforeDelete', + input: { id: undefined, options: { where, multi: true } }, + session, + }; + await engine.fire('beforeDelete', object, ctx); + const t = ensure(object); + for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1); + ctx.event = 'afterDelete'; + await engine.fire('afterDelete', object, ctx); + }, + }; + return engine; +} + +type Engine = ReturnType; + +/** Every `sys_record_share` row a rule materialised. */ +const ruleShares = (engine: Engine) => + (engine._tables.sys_record_share ?? []).filter((r) => r.source === 'rule'); + +/** The #6783 notices this logger saw — by message, not by call count. */ +const notices = (logger: any) => + logger.info.mock.calls.filter((c: any[]) => c[0] === SYSTEM_WRITE_SKIP_NOTICE); + +const rule = (over: Row = {}): Row => ({ + id: 'srule_east', + name: 'east_to_alice', + label: 'East → Alice', + object_name: 'opportunity', + criteria_json: JSON.stringify({ region: 'east' }), + recipient_type: 'user', + recipient_id: 'alice', + access_level: 'edit', + active: true, + ...over, +}); + +describe('#6783 isSystem writes that materialise zero grants say so, once', () => { + let engine: Engine; + let rules: SharingRuleService; + let logger: any; + + /** Bind the hooks against whatever is currently in `sys_sharing_rule`. */ + const bind = async () => { + const ruleRows = await rules.listRules({ activeOnly: true }, SYS); + unbindAllRuleHooks(engine as any); + bindRuleHooks(engine as any, rules, ruleRows, logger); + }; + + beforeEach(async () => { + logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + engine = makeEngine(); + engine._tables.opportunity = []; + engine._tables.sys_record_share = []; + engine._tables.sys_sharing_rule = [rule()]; + const sharing = new SharingService({ engine: engine as any }); + rules = new SharingRuleService({ engine: engine as any, sharing, logger }); + await bind(); + }); + + // ── the positive face ──────────────────────────────────────────────── + + it('reports the skip, naming the behaviour, the remedy, the object and the rules', async () => { + await engine.simulateInsert('opportunity', { id: 'opp0', region: 'east', owner_id: 'boss' }, SYSTEM_SESSION); + + expect(notices(logger)).toHaveLength(1); + const [message, meta] = notices(logger)[0]; + // The maintainer's wording is the contract, not a paraphrase of it. + expect(message).toContain( + 'sharing materialisation skipped for isSystem writes; re-evaluate rules or restart to backfill', + ); + expect(meta).toEqual({ object: 'opportunity', rules: ['east_to_alice'] }); + // …and the line is only true because nothing WAS materialised. + expect(ruleShares(engine)).toEqual([]); + }); + + it('emits ONE line for a batch of many rows — the boundary this card is about', async () => { + await engine.simulateInsertBatch('opportunity', [ + { id: 'opp0', region: 'east', owner_id: 'boss' }, + { id: 'opp1', region: 'east', owner_id: 'boss' }, + { id: 'opp2', region: 'east', owner_id: 'boss' }, + { id: 'opp3', region: 'east', owner_id: 'boss' }, + { id: 'opp4', region: 'east', owner_id: 'boss' }, + ]); + + expect(engine._tables.opportunity).toHaveLength(5); + expect(notices(logger)).toHaveLength(1); + expect(ruleShares(engine)).toEqual([]); + }); + + it('counts an isSystem UPDATE as the same skip — insert + update on one object is still one line', async () => { + await engine.simulateInsertBatch('opportunity', [ + { id: 'opp0', region: 'west', owner_id: 'boss' }, + { id: 'opp1', region: 'west', owner_id: 'boss' }, + ]); + await engine.simulateBulkUpdate('opportunity', { region: 'west' }, { region: 'east' }, SYSTEM_SESSION); + + // The update moved both rows INTO the criteria and still granted nothing. + expect(ruleShares(engine)).toEqual([]); + expect(notices(logger)).toHaveLength(1); + }); + + it('is INFO — never warn, never error', async () => { + await engine.simulateInsert('opportunity', { id: 'opp0', region: 'east' }, SYSTEM_SESSION); + + expect(notices(logger)).toHaveLength(1); + for (const level of ['warn', 'error'] as const) { + expect( + logger[level].mock.calls.filter((c: any[]) => String(c[0]).includes('materialisation skipped')), + ).toEqual([]); + } + }); + + it('reports each covered object separately — the latch is per object, not global', async () => { + engine._tables.opportunity = []; + engine._tables.contract = []; + engine._tables.sys_sharing_rule.push( + rule({ id: 'srule_contract', name: 'contract_to_alice', object_name: 'contract' }), + ); + await bind(); + + await engine.simulateInsertBatch('opportunity', [{ id: 'opp0', region: 'east' }, { id: 'opp1', region: 'east' }]); + await engine.simulateInsertBatch('contract', [{ id: 'con0', region: 'east' }, { id: 'con1', region: 'east' }]); + + expect(notices(logger).map((c: any[]) => c[1].object).sort()).toEqual(['contract', 'opportunity']); + }); + + it('re-arms on rebind: a rule change gets its own notice instead of inheriting the old silence', async () => { + await engine.simulateInsert('opportunity', { id: 'opp0', region: 'east' }, SYSTEM_SESSION); + expect(notices(logger)).toHaveLength(1); + + // What `bindRuleRebindTriggers` does on every `sys_sharing_rule` write. + await bind(); + await engine.simulateInsert('opportunity', { id: 'opp1', region: 'east' }, SYSTEM_SESSION); + + expect(notices(logger)).toHaveLength(2); + }); + + it('never fails the write, even when the logger itself throws', async () => { + // A log sink that is down must not turn an observability line into a + // failed seed row: the notice runs ahead of the hook's own `try`. + const angry = { + info: vi.fn((msg: string) => { + if (msg === SYSTEM_WRITE_SKIP_NOTICE) throw new Error('log sink down'); + }), + warn: vi.fn(), error: vi.fn(), debug: vi.fn(), + }; + const ruleRows = await rules.listRules({ activeOnly: true }, SYS); + unbindAllRuleHooks(engine as any); + bindRuleHooks(engine as any, rules, ruleRows, angry as any); + + await expect( + engine.simulateInsert('opportunity', { id: 'opp0', region: 'east' }, SYSTEM_SESSION), + ).resolves.toBeUndefined(); + + // It tried exactly once, and the latch closed even though the log failed — + // a broken sink must not become one throw per row. + expect(angry.info.mock.calls.filter((c: any[]) => c[0] === SYSTEM_WRITE_SKIP_NOTICE)).toHaveLength(1); + await engine.simulateInsert('opportunity', { id: 'opp1', region: 'east' }, SYSTEM_SESSION); + expect(angry.info.mock.calls.filter((c: any[]) => c[0] === SYSTEM_WRITE_SKIP_NOTICE)).toHaveLength(1); + }); + + // ── the negative faces: silence when nothing is wrong ──────────────── + + it('stays silent for a non-system write — which materialises normally', async () => { + await engine.simulateInsert('opportunity', { id: 'opp0', region: 'east', owner_id: 'boss' }, ADMIN_SESSION); + + expect(notices(logger)).toEqual([]); + expect(ruleShares(engine).map((r) => r.record_id)).toEqual(['opp0']); + }); + + it('stays silent when an active rule legitimately matches nothing', async () => { + // Rule active, row written, ZERO grants materialised — and correctly so: + // `west` is outside the criteria. Nothing is inert, so nothing is said. + await engine.simulateInsert('opportunity', { id: 'opp0', region: 'west', owner_id: 'boss' }, ADMIN_SESSION); + + expect(ruleShares(engine)).toEqual([]); + expect(notices(logger)).toEqual([]); + }); + + it("stays silent when the object's only rule is inactive — no active rule, no hooks, no line", async () => { + engine._tables.sys_sharing_rule = [rule({ active: false })]; + await bind(); + + expect(engine.boundFor(SHARING_RULE_HOOK_PACKAGE)).toEqual([]); + await engine.simulateInsert('opportunity', { id: 'opp0', region: 'east' }, SYSTEM_SESSION); + + expect(notices(logger)).toEqual([]); + }); + + it("stays silent on an isSystem DELETE — the line's remedy cannot repair that class", async () => { + // A delete skips REVOCATION, not materialisation, and `evaluateRule` + // iterates records that still exist — so neither re-evaluating nor + // restarting can reach a grant whose record is gone (#4779's orphan). + // `record-share-cascade.ts` and the boot orphan sweep own it instead. + await engine.simulateInsert('opportunity', { id: 'opp0', region: 'east', owner_id: 'boss' }, ADMIN_SESSION); + expect(ruleShares(engine)).toHaveLength(1); + logger.info.mockClear(); + + await engine.simulateBulkDelete('opportunity', { region: 'east' }, SYSTEM_SESSION); + + expect(notices(logger)).toEqual([]); + }); + + it('stays silent on an object no active rule covers', async () => { + engine._tables.invoice = []; + await engine.simulateInsert('invoice', { id: 'inv0', region: 'east' }, SYSTEM_SESSION); + + expect(notices(logger)).toEqual([]); + }); +}); + +describe("#6783 the notice text is the maintainer's ruling, verbatim", () => { + it('carries the ruled wording and the package tag every sharing line uses', () => { + expect(SYSTEM_WRITE_SKIP_NOTICE).toBe( + '[sharing-rule] sharing materialisation skipped for isSystem writes; ' + + 're-evaluate rules or restart to backfill', + ); + }); + + it('names BOTH remedies — re-evaluation and restart', () => { + expect(SYSTEM_WRITE_SKIP_NOTICE).toContain('re-evaluate rules'); + expect(SYSTEM_WRITE_SKIP_NOTICE).toContain('restart to backfill'); + }); +});