fix(plugin-sharing): carry compileCelToFilter's reason and detail into the sharing-rule seeder's skip WARN

.changeset/sharing-seeder-skip-reason.md

@@ -0,0 +1,5 @@
+---
+'@objectstack/plugin-sharing': patch
+---
+
+The sharing-rule seeder's skip WARN now names WHY a declared rule's CEL `condition` did not translate: `compileCelToFilter`'s `reason` (the aggregatable category) and `detail` (the concrete refused shape, variable path, or parse bound) are carried into the log meta instead of being collapsed to `null` one line before the log that needed them. `celToFilter` keeps its published `Record | null` signature and delegates to the new `celToFilterOutcome` sibling (the `plugin-security` rls-compiler shape from #13942, one seam over). Skip semantics are unchanged — an unlowerable or match-all condition is still never seeded as a permissive match-all rule (ADR-0049).

packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts

@@ -51,6 +51,7 @@
 import type { SharingRuleService } from './sharing-rule-service.js';
 import type { SharingRuleRecipientType, ShareAccessLevel } from '@objectstack/spec/contracts';
 import { compileCelToFilter } from '@objectstack/formula';
+import type { CelFilterFailReason } from '@objectstack/formula';
 import { isMatchAllCriteria } from './rule-criteria.js';
 
 const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const;
@@ -129,8 +130,50 @@ function mapRecipientType(t: unknown): SharingRuleRecipientType | null {
  * never seeding a permissive match-all (ADR-0049).
  */
 export function celToFilter(cel: unknown): Record<string, unknown> | null {
+  return celToFilterOutcome(cel).filter;
+}
+
+/**
+ * Why a declared rule's `condition` produced no criteria — the compiler's OWN
+ * answer, carried instead of discarded. [#13943]
+ *
+ * `compileCelToFilter` already returns `{ reason, detail }` on every refusal;
+ * `celToFilter` used to consume `!ok` and collapse the rest to `null` one line
+ * before the only WARN that could surface it — so an operator whose declared
+ * rule was silently not granting got the fact ("skipped") and the source text
+ * back, but not WHICH shape the compiler refused or why. The extra member is
+ * this FILE's own drop (the ADR-0049 match-all guard at the call site), which
+ * the compiler reports as a success — same skip, same silence, so it joins the
+ * same vocabulary rather than staying unnamed (the `empty-membership`
+ * precedent in `plugin-security/src/rls-compiler.ts`, #13639).
+ */
+type SharingSkipReason = CelFilterFailReason | 'match-all-criteria';
+
+/** A skipped rule's cause: the compiler's `reason` (the aggregatable category) plus its human `detail` (the concrete fault). */
+interface SharingSkipCause {
+  reason: SharingSkipReason;
+  detail: string;
+}
+
+/** {@link celToFilterOutcome}'s answer: the filter, or why there is none. */
+type CelToFilterOutcome =
+  | { filter: Record<string, unknown>; cause?: undefined }
+  | { filter: null; cause: SharingSkipCause };
+
+/**
+ * [#13943] {@link celToFilter}'s answer WITH the reason it refused.
+ *
+ * Same compile, same decision, same returned filter — the only difference is
+ * that the compiler's `{ reason, detail }` survives to the caller instead of
+ * being collapsed into `null` at the `!result.ok` line. `celToFilter` stays
+ * exactly as published (`Record | null`) and delegates here — the
+ * `compileExpressionOutcome` shape from `plugin-security/src/rls-compiler.ts`
+ * (#13942), one seam over.
+ */
+export function celToFilterOutcome(cel: unknown): CelToFilterOutcome {
   const result = compileCelToFilter(cel as string | { source?: string }, { variables: {} });
-  return result.ok ? (result.filter as Record<string, unknown>) : null;
+  if (!result.ok) return { filter: null, cause: { reason: result.reason, detail: result.detail } };
+  return { filter: result.filter as Record<string, unknown> };
 }
 
 /**
@@ -197,12 +240,29 @@ export async function bootstrapDeclaredSharingRules(
     // schema requires `condition`, so reaching here means a hand-crafted
     // `{ dialect, source: '' }` envelope or a stale pre-built package, and
     // neither earns a match-all.
-    const f = celToFilter(r.condition);
-    if (!f || isMatchAllCriteria(f)) {
-      logger?.warn?.('[sharing-rule] skipped (missing or untranslatable CEL condition — never seeded as match-all) [experimental]', { rule: r.name, condition: r.condition });
+    const outcome = celToFilterOutcome(r.condition);
+    if (outcome.filter === null || isMatchAllCriteria(outcome.filter)) {
+      // [#13943] The skip keeps its REASON. `reason` + `detail` are what the
+      // compiler already computed — the shape it refused, the variable path,
+      // the parse bound that was overrun — and discarding them here is what
+      // left an operator with a skipped rule, its source text, and no why.
+      // The skip decision itself is byte-identical to before (ADR-0049: an
+      // unlowerable condition is never seeded as a permissive match-all).
+      const cause: SharingSkipCause = outcome.filter === null
+        ? outcome.cause
+        : {
+            // The compiler answered `ok`, so there is no compiler detail to
+            // carry — this drop is THIS file's match-all guard, and it names
+            // itself rather than being reported as untranslatable.
+            reason: 'match-all-criteria',
+            detail:
+              `the condition lowered to ${JSON.stringify(outcome.filter)}, which constrains nothing — ` +
+              'seeding it would share every record of the object (ADR-0049)',
+          };
+      logger?.warn?.('[sharing-rule] skipped (missing or untranslatable CEL condition — never seeded as match-all) [experimental]', { rule: r.name, condition: r.condition, reason: cause.reason, detail: cause.detail });
       skipped += 1; continue;
     }
-    const criteria: Record<string, unknown> = f;
+    const criteria: Record<string, unknown> = outcome.filter;
     try {
       await ruleService.defineRule({
         name: r.name,

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

@@ -13,7 +13,7 @@ import { BusinessUnitGraphService } from './business-unit-graph.js';
 // ruling can be pinned in the same suite as the positive half: the filter
 // belongs to the sharing CALL SITE, never to the expansion helper.
 import { PositionGraphService } from './position-graph.js';
-import { celToFilter } from './bootstrap-declared-sharing-rules.js';
+import { celToFilter, celToFilterOutcome, bootstrapDeclaredSharingRules } from './bootstrap-declared-sharing-rules.js';
 import { isMatchAllCriteria } from './rule-criteria.js';
 import { bindRuleCriteriaGuard } from './rule-hooks.js';
 
@@ -617,6 +617,113 @@ describe('#1887 — compound sharing condition compiled + enforced (ADR-0058 D3)
   });
 });
 
+// ---------------------------------------------------------------------------
+// #13943 — the seeder's skip WARN carries the compiler's reason AND detail
+//
+// `compileCelToFilter` returns `{ ok: false, reason, detail }` on every
+// refusal; `celToFilter` used to collapse the whole thing to `null` one line
+// before the only WARN that could surface it, so the operator learned THAT
+// the condition did not translate but never WHY. `celToFilterOutcome` is the
+// sibling that keeps the cause (`compileExpressionOutcome` in
+// plugin-security's rls-compiler, #13942, is the same shape one seam over);
+// `celToFilter` stays exactly as published (`Record | null`) and delegates.
+// The skip DECISION is unchanged either way — an unlowerable condition is
+// never seeded as a permissive match-all (ADR-0049).
+// ---------------------------------------------------------------------------
+describe('#13943 — sharing-rule seeder skip WARN names the compiler’s reason and detail', () => {
+  const SKIP_WARN =
+    '[sharing-rule] skipped (missing or untranslatable CEL condition — never seeded as match-all) [experimental]';
+
+  /** Registry-backed seeder harness: declared rules in, defineRule + log lines out. */
+  function seedHarness(declared: any[]) {
+    const engine = { _registry: { listItems: (type: string) => (type === 'sharing_rule' ? declared : []) } };
+    const defineRule = vi.fn(async (input: any) => ({ id: `id_${input.name}` }));
+    const warns: Array<{ msg: string; meta: any }> = [];
+    const logger = {
+      warn: (msg: string, meta?: any) => { warns.push({ msg, meta }); },
+      info: () => {},
+    };
+    return { engine, ruleService: { defineRule } as any, logger, warns, defineRule };
+  }
+
+  const RULE_BASE = {
+    object: 'opportunity',
+    sharedWith: { type: 'user', value: 'alice' },
+    accessLevel: 'read',
+  };
+
+  it('celToFilterOutcome carries the compiler’s { reason, detail } instead of collapsing to null', () => {
+    // Refusal: a function call is not pushdown-able — the cause survives.
+    const refused = celToFilterOutcome('size(record.tags) > 0');
+    expect(refused.filter).toBeNull();
+    expect(refused.cause?.reason).toBe('unsupported');
+    expect(refused.cause?.detail).toEqual(expect.any(String));
+    expect(refused.cause?.detail.length).toBeGreaterThan(0);
+    // Missing / empty condition: the compiler's own empty-expression refusal.
+    const missing = celToFilterOutcome(undefined);
+    expect(missing).toEqual({ filter: null, cause: { reason: 'parse-error', detail: 'empty expression' } });
+    // Success: same filter celToFilter returns, and no cause at all.
+    const ok = celToFilterOutcome('record.amount >= 100000');
+    expect(ok.filter).toEqual({ amount: { $gte: 100000 } });
+    expect(ok.cause).toBeUndefined();
+    // The published wrapper delegates: byte-identical answers on both paths.
+    expect(celToFilter('size(record.tags) > 0')).toBeNull();
+    expect(celToFilter('record.amount >= 100000')).toEqual(ok.filter);
+  });
+
+  it('an untranslatable condition is skipped WITH the why: reason + detail land in the WARN meta', async () => {
+    const { engine, ruleService, logger, warns, defineRule } = seedHarness([
+      { ...RULE_BASE, name: 'r_unsupported', condition: 'size(record.tags) > 0' },
+    ]);
+    const res = await bootstrapDeclaredSharingRules(ruleService, null, engine, logger);
+    expect(res).toEqual({ seeded: 0, skipped: 1 });
+    expect(defineRule).not.toHaveBeenCalled();
+    const skips = warns.filter((w) => w.msg === SKIP_WARN);
+    expect(skips).toHaveLength(1);
+    // The fact (rule, condition) is still there; the why (reason, detail) now is too.
+    expect(skips[0].meta).toMatchObject({
+      rule: 'r_unsupported',
+      condition: 'size(record.tags) > 0',
+      reason: 'unsupported',
+    });
+    expect(skips[0].meta.detail).toEqual(expect.any(String));
+    expect(skips[0].meta.detail.length).toBeGreaterThan(0);
+  });
+
+  it('NEGATIVE: a rule whose condition lowers cleanly emits no skip line and seeds as before', async () => {
+    const { engine, ruleService, logger, warns, defineRule } = seedHarness([
+      { ...RULE_BASE, name: 'r_clean', condition: 'record.amount >= 100000' },
+    ]);
+    const res = await bootstrapDeclaredSharingRules(ruleService, null, engine, logger);
+    expect(res).toEqual({ seeded: 1, skipped: 0 });
+    // No new log line of any kind on the clean path — not just "no skip WARN".
+    expect(warns).toHaveLength(0);
+    expect(defineRule).toHaveBeenCalledTimes(1);
+    expect(defineRule.mock.calls[0][0]).toMatchObject({
+      name: 'r_clean',
+      criteria: { amount: { $gte: 100000 } },
+    });
+  });
+
+  it('NEGATIVE: a MISSING condition takes the path it takes today — skipped via the same WARN, never seeded', async () => {
+    const { engine, ruleService, logger, warns, defineRule } = seedHarness([
+      { ...RULE_BASE, name: 'r_missing' /* no condition at all */ },
+    ]);
+    const res = await bootstrapDeclaredSharingRules(ruleService, null, engine, logger);
+    expect(res).toEqual({ seeded: 0, skipped: 1 });
+    expect(defineRule).not.toHaveBeenCalled();
+    const skips = warns.filter((w) => w.msg === SKIP_WARN);
+    expect(skips).toHaveLength(1);
+    // Same branch, same message string as before; the cause names the
+    // compiler's own empty-expression refusal rather than being absent.
+    expect(skips[0].meta).toMatchObject({
+      rule: 'r_missing',
+      reason: 'parse-error',
+      detail: 'empty expression',
+    });
+  });
+});
+
 // ---------------------------------------------------------------------------
 // #3896 — a rule with no criteria must share NOTHING, never everything
 //