Require a guaranteed warn channel on any sink type that declares an optional error (#9754) (#10555)

* feat(devx): a sink declaring an optional `error` must declare a guaranteed `warn` (#9754)

An optional `error` with no declared alternative is a contract that permits
silence. #9657 and #9748 repaired the call-site spellings; this is the type
contract behind them.

- `SweepLogger` (plugin-email) and `ProjectionLogger` (plugin-security) now
  declare `warn` non-optionally, so `{ info }` is no longer a legal sink and
  the durability reports cannot evaporate against one. `error` stays optional
  — hosts do inject reduced sinks (#9754 option C, falsified).
- New producer-side checker `scripts/check-optional-error-sink-contract.mjs`
  (`pnpm check:optional-error-sink`): structural population, both narrowings
  reported as positive counts on every run, 13-case `--self-test`, and a
  shrink-only ledger holding the 15 sinks in `packages/**` that still permit
  silence. Not wired into CI — #9754 forbids more merge-blocking.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt

* test(plugin-security): drop the phantom compile-time pin its tsconfig cannot evaluate

`pnpm check:type-check-coverage` caught it in this PR's own diff: plugin-security's
tsconfig excludes `**/*.test.ts` (the package carries a TEST_DEBT ledger entry),
so a `@ts-expect-error` in that file evaluates NEVER — a pin that reads like proof
and holds nothing (AGENTS.md → "Build & Test").

The runtime half stays: a `{ info }` sink is shown hearing NOTHING — not the
failure, not the count, not the reassuring "reconciled" line. The compile-time
half lives in plugin-email's `outbox-sweep.test.ts`, whose package does compile
its tests (observed red as `error TS2578: Unused '@ts-expect-error' directive`
when the contract is reverted), and the type contract of both sinks is held by
`pnpm check:optional-error-sink`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt

* docs(plugins): unglue the two sink doc comments (`rejected).   *`)

A heredoc concatenation joined the option-C paragraph and the call-site
backstop note onto one line in both interfaces. Comment text only; no code,
type or test changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/optional-error-sink-contract-requires-warn.md

@@ -0,0 +1,16 @@
+---
+"@objectstack/plugin-email": minor
+"@objectstack/plugin-security": minor
+---
+
+`SweepLogger` and `ProjectionLogger` now declare `warn` as a REQUIRED channel, so a sink handed to the boot outbox sweep or to permission-set reconciliation can no longer be one that prints nothing (#9754)
+
+Both interfaces declared every member optional — `info?`, `warn?`, `error?` — which made `{ info }` a legal sink. Against such a sink both durability reports evaporated: each reaches for `error`, finds none, falls back to `warn`, and finds none of that either. For the sweep that is mail the platform accepted and never delivered, summarised to nobody; for reconciliation it is a permission set that will not survive a re-provision, with the `info` "reconciled" line skipped as well, so the sink heard neither the failure nor the reassurance.
+
+#9657 and #9748 repaired the call-site spellings. This is the other half, and the half that cannot regress: an optional `error` with no guaranteed alternative is a contract that permits silence, so an author reading the interface can write a report that never prints and be right about the type. Requiring `warn` makes that unrepresentable at the point of authoring rather than catchable one gate-run later.
+
+`error` deliberately stays optional on both types — hosts do inject reduced sinks, and requiring `error` would foreclose the `{ warn }`-only host the drivers were written for.
+
+If you pass a logger of your own and it declares no `warn`, add one; the kernel `Logger`, `ctx.logger` and `console` all satisfy the tightened shape unchanged. Consumers reach these types through `@objectstack/plugin-security`'s exported `ProjectionDeps`; `SweepLogger` is internal to `@objectstack/plugin-email`.
+
+The rule now has a checker of its own: `pnpm check:optional-error-sink` scans every sink type in `packages/**`, reports the population as a census on every run, and carries a shrink-only ledger of the 15 sinks that still permit silence.

package.json

@@ -79,6 +79,7 @@
     "check:init-service-contract": "node scripts/check-init-service-contract.mjs --self-test && node scripts/check-init-service-contract.mjs",
     "check:kernel-hook-pairs": "node scripts/check-kernel-hook-pairs.mjs --self-test && node scripts/check-kernel-hook-pairs.mjs",
     "check:durability-log-level": "node scripts/check-durability-degradation-log-level.mjs --self-test && node scripts/check-durability-degradation-log-level.mjs",
+    "check:optional-error-sink": "node scripts/check-optional-error-sink-contract.mjs --self-test && node scripts/check-optional-error-sink-contract.mjs",
     "check:startup-registry-verdict": "node scripts/check-startup-registry-verdict.mjs --self-test && node scripts/check-startup-registry-verdict.mjs",
     "check:console-sha": "node scripts/check-console-sha.mjs",
     "check:console-injection": "node scripts/check-console-injection.mjs --self-test && node scripts/check-console-injection.mjs",

packages/plugins/plugin-email/src/outbox-sweep.test.ts

@@ -331,6 +331,46 @@ describe('failures are loud, counted, and never stop the batch', () => {
     expect(lines(logger.warn).filter((l) => l.includes('could NOT be delivered'))).toHaveLength(0);
   });
 
+  it('[#9754] a sink with NO `warn` cannot be spelled at all — the TYPE forbids the silence', async () => {
+    // THE HARM, reproduced before the fix is believed. Until #9754 every member
+    // of `SweepLogger` was optional, so `{ info }` was a legal sink — and
+    // against it BOTH repaired reports print nothing at all: each reaches for
+    // `error`, finds none, falls back to `warn`, and finds none of that either.
+    // Mail the platform accepted and never delivered, reported to nobody. The
+    // cast below buys exactly what the old contract handed out for free.
+    const engine = fakeEngine([{ id: 'row-bad-1', created_at: ago(min(30)) }]);
+    const service = fakeService({
+      deliver: () => { throw new Error('engine exploded'); },
+    });
+    const silent = { info: vi.fn() };
+
+    const res = await sweepStrandedOutbox({
+      engine,
+      service,
+      logger: silent as unknown as NonNullable<Parameters<typeof sweepStrandedOutbox>[0]['logger']>,
+      now: () => NOW,
+    });
+
+    expect(res).toMatchObject({ scanned: 1, failed: 1 });
+    // Not "logged at the wrong level" — not logged AT ALL, on either report.
+    expect(lines(silent.info).filter((l) => l.includes('could NOT be delivered'))).toHaveLength(0);
+    expect(lines(silent.info).filter((l) => l.includes('engine exploded'))).toHaveLength(0);
+
+    // THE CONTRACT. Without that cast the same sink no longer compiles: `warn`
+    // is non-optional on `SweepLogger` (#9754), so a caller cannot hand over a
+    // sink with nowhere to put a durability report. Restore `warn?` in
+    // outbox-sweep.ts and this directive turns into an "Unused
+    // '@ts-expect-error' directive" error — which is how this assertion proves
+    // it is live rather than decorative.
+    await sweepStrandedOutbox({
+      engine,
+      service,
+      // @ts-expect-error — #9754: a SweepLogger MUST declare a `warn` channel
+      logger: { info: vi.fn() },
+      now: () => NOW,
+    });
+  });
+
   it('propagates a failure of the query itself — the sweep did not happen', async () => {
     const engine = { find: vi.fn(async () => { throw new Error('no such table: sys_email'); }) };
     await expect(sweepStrandedOutbox({ engine, service: fakeService(), now: () => NOW }))

packages/plugins/plugin-email/src/outbox-sweep.ts

@@ -92,7 +92,28 @@ function humanMs(ms: number): string {
  */
 interface SweepLogger {
   info?: (msg: string, meta?: any) => void;
-  warn?: (msg: string, meta?: any) => void;
+  /**
+   * The GUARANTEED channel (#9754). `error` below stays optional — hosts do
+   * inject reduced sinks — so `warn` is where a durability report lands when
+   * `error` is absent, and a fallback that may itself be missing is not a
+   * fallback: `{ info }` alone used to be a legal sink here, and against it the
+   * boot sweep's "N stranded row(s) could NOT be delivered" summary printed
+   * nothing at all. Non-optional is what makes that silence unrepresentable
+   * instead of merely discouraged.
+   *
+   * ⛔ Do NOT "simplify" this by making `error` required instead — that
+   * forecloses the reduced sinks hosts legitimately pass (#9754 option C,
+   * measured and rejected).
+   *
+   * ⚠️ Call sites still spell the fallback `logger?.warn?.(…)`. That `?.` is not
+   * doubt about this declaration — it is the backstop for hosts the TYPE cannot
+   * reach (a plain-JS embedder, or a cast). Dropping it was measured: a sink
+   * that lies about its shape then throws `logger?.warn is not a function`
+   * INSIDE the per-row durability catch, aborting the very batch this function
+   * promises never to stop. Silence for a lying host is the lesser failure; the
+   * guarantee this member adds is at AUTHORING time, where it belongs.
+   */
+  warn: (msg: string, meta?: any) => void;
   error?: (msg: string, meta?: any) => void;
 }
 

packages/plugins/plugin-security/src/permission-set-projection.test.ts

@@ -33,6 +33,7 @@ import {
   registerPermissionSetProjection,
   createPermissionSetWriteThrough,
   reconcilePermissionSetProjection,
+  type ProjectionLogger,
 } from './permission-set-projection.js';
 
 /** In-memory ql over sys_permission_set + sys_metadata. */
@@ -1106,6 +1107,48 @@ describe('reconcilePermissionSetProjection', () => {
     expect(summary[0]!.meta?.failedNames).toEqual(['broken_set']);
   });
 
+  it('[#9754] a sink with NO `warn` cannot be spelled at all — the TYPE forbids the silence', async () => {
+    // THE HARM, reproduced before the fix is believed. Until #9754 every member
+    // of `ProjectionLogger` was optional, so `{ info }` was a legal sink — and
+    // against it the reconcile pass reported NOTHING: the first-failure line
+    // and the summary both reach for `error`, fall back to `warn`, and find
+    // neither, while the `else` branch carrying the reassuring "reconciled"
+    // line is skipped because the pass did fail. A permission set that will not
+    // survive a re-provision, and a boot log that says nothing whatsoever.
+    const ql = makeQl();
+    const protocol = makeProtocol(ql);
+    ql.permRows.push({
+      id: 'ps_bad', name: 'broken_set', managed_by: 'admin', active: true,
+      label: 'Broken Set', object_permissions: JSON.stringify({ ticket: { allowRead: 'yes-please' } }),
+    });
+    const heard: string[] = [];
+    const silent = { info: (m: string) => heard.push(m) };
+
+    const out = await reconcilePermissionSetProjection(protocol, {
+      ql,
+      logger: silent as unknown as ProjectionLogger,
+    });
+
+    expect(out.backfillFailed).toBe(1);
+    expect(heard).toEqual([]);   // neither the failure, nor the count, nor the "reconciled" line
+
+    // THE CONTRACT is what removes that cast's subject: `warn` is non-optional
+    // on `ProjectionLogger` (#9754), so no TS caller can build the sink above
+    // without saying `as unknown as` out loud.
+    //
+    // ⚠️ Deliberately NOT pinned here with `@ts-expect-error`. This package's
+    // tsconfig excludes `**/*.test.ts` (it carries a TEST_DEBT ledger entry in
+    // scripts/check-type-check-coverage.mjs), so no tsc program compiles this
+    // file and the directive would evaluate NEVER — a phantom check that reads
+    // like proof, which is the failure AGENTS.md → "Build & Test" names and
+    // `pnpm check:type-check-coverage` refuses. The compile-time half of this
+    // contract is pinned in plugin-email's `outbox-sweep.test.ts`, whose
+    // package DOES compile its tests (observed: reverting `warn` there turns
+    // that directive into `error TS2578: Unused '@ts-expect-error' directive`),
+    // and the type half of BOTH sinks is held by
+    // `pnpm check:optional-error-sink`.
+  });
+
   it('heals a record that drifted from an EXISTING metadata definition (metadata wins)', async () => {
     const ql = makeQl();
     const declared = { member_default: envBody({ name: 'member_default', systemPermissions: ['declared.baseline'] }) };

packages/plugins/plugin-security/src/permission-set-projection.ts

@@ -92,7 +92,29 @@ export async function tryUpdate(ql: any, object: string, data: any): Promise<boo
 
 export interface ProjectionLogger {
   info?: (m: string, meta?: Record<string, any>) => void;
-  warn?: (m: string, meta?: Record<string, any>) => void;
+  /**
+   * The GUARANTEED channel (#9754). `error` below stays optional — hosts do
+   * inject reduced sinks — so `warn` is where a durability report lands when
+   * `error` is absent, and a fallback that may itself be missing is not a
+   * fallback: `{ info }` alone used to be a legal sink here, and against it the
+   * reconcile summary's "N FAILED backfill(s)" printed nothing at all, while
+   * the `info` "reconciled" line was skipped too — the sink heard neither.
+   * Non-optional is what makes that silence unrepresentable instead of merely
+   * discouraged.
+   *
+   * ⛔ Do NOT "simplify" this by making `error` required instead — that
+   * forecloses the reduced sinks hosts legitimately pass (#9754 option C,
+   * measured and rejected).
+   *
+   * ⚠️ Call sites still spell the fallback `logger?.warn?.(…)`. That `?.` is not
+   * doubt about this declaration — it is the backstop for hosts the TYPE cannot
+   * reach (a plain-JS embedder, or a cast). Dropping it was measured: a sink
+   * that lies about its shape then throws `logger?.warn is not a function`
+   * INSIDE the per-row durability catch, aborting the very batch this function
+   * promises never to stop. Silence for a lying host is the lesser failure; the
+   * guarantee this member adds is at AUTHORING time, where it belongs.
+   */
+  warn: (m: string, meta?: Record<string, any>) => void;
   /**
    * Durability-degradation channel (AGENTS.md "Degradation log levels", #4632):
    * a metadata write that was supposed to land and did not is an `error`, not a
@@ -809,6 +831,10 @@ export function createPermissionSetWriteThrough(
           // NOTHING against a sink that has only `warn` — the durability
           // degradation described above would then be reported by nobody at all
           // (#9657). Reach for `error`, fall back to `warn`, never to silence.
+          // The fallback itself is now GUARANTEED BY THE TYPE: `warn` is
+          // non-optional on `ProjectionLogger` (#9754), so no TS caller can
+          // hand over a sink this line evaporates against. The `?.` below is
+          // the backstop for untyped hosts only — see the interface.
           if (logger?.error) logger.error(message, e as Error, { name: row.name });
           else logger?.warn?.(message, { name: row.name, error: String((e as Error)?.message ?? e) });
         }
@@ -1040,6 +1066,10 @@ export async function reconcilePermissionSetProjection(
           // NOTHING against a sink that has only `warn` — the durability
           // degradation described above would then be reported by nobody at all
           // (#9657). Reach for `error`, fall back to `warn`, never to silence.
+          // The fallback itself is now GUARANTEED BY THE TYPE: `warn` is
+          // non-optional on `ProjectionLogger` (#9754), so no TS caller can
+          // hand over a sink this line evaporates against. The `?.` below is
+          // the backstop for untyped hosts only — see the interface.
           if (logger?.error) logger.error(message, e as Error, { name: row.name });
           else logger?.warn?.(message, { name: row.name, error: String((e as Error)?.message ?? e) });
         }
@@ -1076,6 +1106,9 @@ export async function reconcilePermissionSetProjection(
     // the `else` below is skipped too, so such a sink heard neither the count
     // nor the reassuring "reconciled" line, while the first-failure report
     // (repaired by #9657) still arrived. Fall back to `warn`, not silence.
+    // The fallback itself is now GUARANTEED BY THE TYPE: `warn` is
+    // non-optional on `ProjectionLogger` (#9754). The `?.` is the backstop for
+    // untyped hosts only — see the interface.
     if (logger?.error) logger.error(summary, undefined, summaryMeta);
     else logger?.warn?.(summary, summaryMeta);
   } else {