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

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