feat(plugin-sharing): one INFO line when isSystem writes materialise zero sharing grants (#6783)

.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.

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<any>, 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<string>();
+  /** Active rule names per object — the `rules:` field of the #6783 notice. */
+  const activeRuleNames = new Map<string, string[]>();
   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<string>();
+
+  /**
+   * 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);