Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions .changeset/optional-error-sink-paydown.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
---
"@objectstack/cloud-connection": minor
"@objectstack/metadata-protocol": minor
"@objectstack/plugin-approvals": minor
"@objectstack/plugin-audit": minor
"@objectstack/plugin-auth": minor
"@objectstack/plugin-email": minor
"@objectstack/plugin-reports": minor
"@objectstack/plugin-sharing": minor
"@objectstack/plugin-webhooks": minor
"@objectstack/service-knowledge": minor
---

**BREAKING** (compile-time only): twelve logger sink types that declared an
optional `error` now declare a **non-optional** `warn`, so a durability report
always has somewhere to land (#9754, #10556).

`minor`, not `major`: during the launch window this stack ships breaking changes
as `minor` — every publishable package versions in lockstep, so a `major` would
promote the whole release. `patch` would be wrong in the other direction, because
this *can* break a consumer's build.

`error` stays optional on every one of these types — hosts legitimately inject
reduced sinks, and requiring `error` was measured and rejected as #9754 option C.
What changes is that its *absence* now has a declared, guaranteed destination.
Call sites keep the `logger?.warn?.(…)` spelling as the backstop for hosts the
type cannot reach, so **no runtime behaviour changes**: nothing that printed
before stops printing, and nothing silent starts printing.

### Who has to change, and what to do

Only a caller that hands one of these sinks an object with **no `warn` method** —
for example `{ info }` or `{ error }` alone. Add a `warn` member; there is no
rename, no removal, and no stored value or metadata key to rewrite. Every
construction site inside this repo already supplied one, so the in-repo cost was
zero; the compile error is reserved for the callers that were silently discarding
these reports.

The affected types, by package:

- `@objectstack/cloud-connection` — the internal `PluginContext['logger']`
- `@objectstack/metadata-protocol` — `IndexMigrationLogger`
- `@objectstack/plugin-approvals` — the internal `MinimalLogger` of `lifecycle-hooks`
- `@objectstack/plugin-audit` — `AuthEventAuditLogger`, `ReadAuditLogger`
- `@objectstack/plugin-auth` — `ReconcileMembershipDeps['logger']`, the internal
`LoggerLike` of `member-role-canonical`, and `AuthManagerOptions['logger']`
- `@objectstack/plugin-email` — `ReclaimLogger`, via `ReclaimAttachmentContentOptions`
- `@objectstack/plugin-reports` — `ReportServiceOptions['logger']`
- `@objectstack/plugin-sharing` — the internal `MinimalLogger` of `bulk-recompute`,
`rule-hooks` and `record-share-cascade`
- `@objectstack/plugin-webhooks` — `OptionalLogger`, via `AutoEnqueuerOptions`
- `@objectstack/service-knowledge` — `KnowledgeLogger`

`AuthManagerOptions['logger']` is the one most likely to be reached from outside:
`AuthManager` is public surface, its `logger` option stays optional, and a logger
that *is* supplied must now carry `warn`. The only non-test construction site in
this repo passes the kernel `Logger`, whose `warn` is already required.

`ReportService` and `AutoEnqueuer` additionally stopped defaulting their logger
field to `{}`. The field is now honestly optional rather than holding an empty
object that declared it could report and discarded everything. Behaviour is
unchanged in both directions.

<!-- adr-0087: not-required (runtime-interface-only packages/plugins/plugin-auth/src/auth-manager.ts#AuthManagerOptions, packages/plugins/plugin-auth/src/reconcile-membership.ts#ReconcileMembershipDeps, packages/metadata-protocol/src/migrations/partial-index-probe.ts#IndexMigrationLogger, packages/plugins/plugin-audit/src/auth-event-audit.ts#AuthEventAuditLogger, packages/plugins/plugin-audit/src/read-audit.ts#ReadAuditLogger, packages/plugins/plugin-reports/src/report-service.ts#ReportServiceOptions, packages/services/service-knowledge/src/knowledge-service.ts#KnowledgeLogger) every tightened type is a plain TypeScript logger interface -- no Zod projection, no metadata surface, and none is referenced by one -- so `objectstack migrate meta` has nothing to rewrite. Nothing is removed or renamed and no stored value moves; the only consumer action is adding a `warn` member at a construction site the compiler names. -->
10 changes: 9 additions & 1 deletion packages/cloud-connection/src/cloud-connection-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -56,7 +56,15 @@ interface PluginContext {
getService<T = any>(name: string): T;
logger?: {
info?: (msg: string) => void;
warn?: (msg: string) => void;
/**
* The GUARANTEED fallback channel (#9754). `error` 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. Call sites keep the `logger?.warn?.(…)` spelling as the backstop
* for hosts the TYPE cannot reach; `SweepLogger` in plugin-email's
* `outbox-sweep.ts` carries the full reasoning and the measurement.
*/
warn: (msg: string) => void;
error?: (msg: string, err?: unknown) => void;
};
}
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import {
probeThenReplaceIndex,
type IndexExec,
} from './partial-index-probe.js';
import type { IndexMigrationLogger } from './partial-index-probe.js';

/**
* The probe-first order, tested where it lives (#6418).
Expand DownExpand Up@@ -382,6 +383,18 @@ describe('probe-first partial index replacement (#6418)', () => {
expect(warnOnly.warn).toHaveBeenCalledWith('msg', { detail: 'detail' });

expect(() => logProblem(undefined, 'msg', 'detail')).not.toThrow();
expect(() => logProblem({}, 'msg', 'detail')).not.toThrow();
// `{}` is no longer a legal `IndexMigrationLogger` — #9754 made `warn`
// non-optional precisely so a sink with NEITHER channel cannot be
// written. The cast is deliberate and is the point of the case: it
// forces through the one host the TYPE cannot reach (a plain-JS
// embedder, or a cast at the boundary) and pins that `logProblem`'s
// `?.` backstop still degrades to silence instead of throwing
// `logger.warn is not a function` inside a migration probe. Type-level
// guarantee and runtime backstop are different promises; this asserts
// the second one, and the compile error that used to be impossible here
// is now what proves the first.
expect(() =>
logProblem({} as unknown as IndexMigrationLogger, 'msg', 'detail'),
).not.toThrow();
});
});
15 changes: 13 additions & 2 deletions packages/metadata-protocol/src/migrations/partial-index-probe.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -97,13 +97,24 @@ export function resolveIndexExecForTable(engine: unknown, table: string): IndexE

/**
* Minimal logger surface, structurally compatible with `@objectstack/spec`'s
* `Logger` (every method optional so a bare console or a test double fits).
* `Logger` (a bare console or a test double fits — but see `warn` below, which
* #9754 made non-optional, so a double must now declare it).
* Signatures mirror that contract exactly — notably `error(msg, Error, meta)`
* versus `warn(msg, meta)` — so a host `Logger` is assignable as-is.
*/
export interface IndexMigrationLogger {
info?(message: string, meta?: Record<string, any>): void;
warn?(message: string, meta?: Record<string, any>): void;
/**
* The GUARANTEED fallback channel (#9754). `error` 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. `logProblem` below is exactly that degrade, and until this member
* was required it could reach for two channels and find neither. Call sites
* keep the `logger?.warn?.(…)` spelling as the backstop for hosts the TYPE
* cannot reach; `SweepLogger` in plugin-email's `outbox-sweep.ts` carries the
* full reasoning and the measurement.
*/
warn(message: string, meta?: Record<string, any>): void;
error?(message: string, error?: Error, meta?: Record<string, any>): void;
}

Expand Down
10 changes: 9 additions & 1 deletion packages/plugins/plugin-approvals/src/lifecycle-hooks.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -87,7 +87,15 @@ interface MinimalEngine {
interface MinimalLogger {
debug?: (msg: any, ...rest: any[]) => void;
info?: (msg: any, ...rest: any[]) => void;
warn?: (msg: any, ...rest: any[]) => void;
/**
* The GUARANTEED fallback channel (#9754). `error` 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. Call sites keep the `logger?.warn?.(…)` spelling as the backstop
* for hosts the TYPE cannot reach; `SweepLogger` in plugin-email's
* `outbox-sweep.ts` carries the full reasoning and the measurement.
*/
warn: (msg: any, ...rest: any[]) => void;
error?: (msg: any, ...rest: any[]) => void;
}

Expand Down
9 changes: 8 additions & 1 deletion packages/plugins/plugin-audit/src/auth-event-audit.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -304,13 +304,20 @@ describe('[#8144] createAuthEventAuditSink writes a login row that names its act
throw new Error('no such table: sys_audit_log');
},
};
const logger = { error: vi.fn(), debug: vi.fn() };
// `warn` is required by `AuthEventAuditLogger` (#9754/#10556): a sink that
// declares an optional `error` must be able to degrade. Present here AND
// asserted unused below, which pins the ORDER — `error` first, `warn` only
// as the fallback. Before the contract change this double was `{ error,
// debug }`, a shape the type accepted and the degrade path could not use.
const logger = { error: vi.fn(), warn: vi.fn(), debug: vi.fn() };
const sink = createAuthEventAuditSink({ getEngine: () => broken, logger });

await expect(sink.recordAuthEvent({ action: 'login', userId: 'usr_1' })).resolves.toBeUndefined();
await sink.recordAuthEvent({ action: 'logout', userId: 'usr_1' });

expect(logger.error).toHaveBeenCalledTimes(1);
// The fallback stays untouched while `error` exists.
expect(logger.warn).not.toHaveBeenCalled();
const [msg] = logger.error.mock.calls[0];
// The two things a durability `error` owes, in its first line.
expect(String(msg)).toContain('INCOMPLETE');
Expand Down
18 changes: 12 additions & 6 deletions packages/plugins/plugin-audit/src/auth-event-audit.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -106,13 +106,19 @@ export interface AuthSessionAuditEvent {
export interface AuthEventAuditLogger {
error?(msg: string, err?: Error, meta?: Record<string, any>): void;
/**
* The fallback channel for the durability report below. `error` is optional
* here, so a sink that has none must still have somewhere to put a lost audit
* row — reaching for `error` and finding nothing must degrade to `warn`,
* never to silence (#9657). Signature and optionality mirror
* `ReadAuditLogger` in `read-audit.ts`, which already declared it.
* The GUARANTEED fallback channel for the durability report below. `error` is
* optional here, so a sink that has none must still have somewhere to put a
* lost audit row — reaching for `error` and finding nothing must degrade to
* `warn`, never to silence (#9657).
*
* This is the sink #9754's body calls the sharpest instance: it declared
* `error?` and `debug?` and NO `warn` at all, so the call site below COULD NOT
* have been written correctly against the contract it was given. #9750 added
* `warn?`, which gave it something to reach for and still no guarantee it was
* there; non-optional (#9754) is what makes the silence unrepresentable.
* Signature and optionality mirror `ReadAuditLogger` in `read-audit.ts`.
*/
warn?(msg: string, meta?: Record<string, any>): void;
warn(msg: string, meta?: Record<string, any>): void;
debug?(msg: string, meta?: Record<string, any>): void;
}

Expand Down
10 changes: 9 additions & 1 deletion packages/plugins/plugin-audit/src/read-audit.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -314,10 +314,18 @@ describe('#8992 the write is OFF the request path', () => {
it('a ledger write failure never reaches the read, and reports once at `error`', async () => {
const errors: string[] = [];
const debugs: string[] = [];
// Required by `ReadAuditLogger` (#9754/#10556) — and asserted unused below,
// so the double pins that `error` is reached for FIRST and `warn` is only
// the degrade path.
const warns: string[] = [];
const writer = installReadAuditWriter(engine, {
objects: ['contact'],
timers: makeManualTimers(),
logger: { error: (m: string) => errors.push(m), debug: (m: string) => debugs.push(m) },
logger: {
error: (m: string) => errors.push(m),
warn: (m: string) => warns.push(m),
debug: (m: string) => debugs.push(m),
},
})!;
// Break the ledger AFTER install, so the probe has already run.
(engine as any).insert = async () => { throw new Error('no such table: sys_audit_log'); };
Expand Down
10 changes: 9 additions & 1 deletion packages/plugins/plugin-audit/src/read-audit.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -112,7 +112,15 @@ export const READ_AUDIT_ACTION = 'read';
/** Minimal logger surface — structurally the kernel `ctx.logger` (`ILogger`). */
export interface ReadAuditLogger {
error?(msg: string, err?: Error, meta?: Record<string, any>): void;
warn?(msg: string, meta?: Record<string, any>): void;
/**
* The GUARANTEED fallback channel (#9754). `error` 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. Call sites keep the `logger?.warn?.(…)` spelling as the backstop
* for hosts the TYPE cannot reach; `SweepLogger` in plugin-email's
* `outbox-sweep.ts` carries the full reasoning and the measurement.
*/
warn(msg: string, meta?: Record<string, any>): void;
debug?(msg: string, meta?: Record<string, any>): void;
}

Expand Down
14 changes: 13 additions & 1 deletion packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -631,8 +631,20 @@ export interface AuthManagerOptions extends Partial<AuthConfig> {
* Optional structured logger (the kernel `ctx.logger`) for best-effort
* bookkeeping surfaces such as the ADR-0093 membership reconciler. Omitted →
* those surfaces run silently (they already fail closed to no-op).
*
* The whole field stays OPTIONAL — omitting it is still a supported posture.
* What is no longer representable is supplying a logger that cannot carry a
* durability report: `warn` is non-optional because this value is FORWARDED
* verbatim into `ReconcileMembershipDeps.logger` (reconcile-membership.ts),
* whose sink guarantees that channel under #9754. Measured before tightening:
* the only non-test construction site in this repo is `auth-plugin.ts`, which
* passes `ctx.logger` — the kernel `Logger`, whose `warn` is already
* required — so the in-tree cost is zero. An external embedder handing
* `AuthManager` a reduced `{ info }` sink is the one caller this asks to
* change, and that is the point: it was the caller silently discarding the
* reconciler's reports (#10556).
*/
logger?: { info?: (msg: string, meta?: any) => void; warn?: (msg: string, meta?: any) => void };
logger?: { info?: (msg: string, meta?: any) => void; warn: (msg: string, meta?: any) => void };

/**
* ADR-0069 D2 — account lockout (anti-brute-force). After this many
Expand Down
10 changes: 9 additions & 1 deletion packages/plugins/plugin-auth/src/member-role-canonical.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -198,7 +198,15 @@ export function isCanonicalMemberRole(raw: unknown): boolean {
*/
type LoggerLike = {
info?(msg: string, meta?: Record<string, any>): void;
warn?(msg: string, meta?: Record<string, any>): void;
/**
* The GUARANTEED fallback channel (#9754). `error` 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. Call sites keep the `logger?.warn?.(…)` spelling as the backstop
* for hosts the TYPE cannot reach; `SweepLogger` in plugin-email's
* `outbox-sweep.ts` carries the full reasoning and the measurement.
*/
warn(msg: string, meta?: Record<string, any>): void;
error?(msg: string, error?: Error, meta?: Record<string, any>): void;
debug?(msg: string, meta?: Record<string, any>): void;
};
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -251,7 +251,7 @@ describe('reconcileMembership / backfillMemberships — off-vocabulary policy (#
const res = await reconcileMembership(makeEngine(), 'user-1', {
policy: asPolicy('inviteOnly'),
resolveTargetOrg: async () => 'org_default',
logger: { error },
logger: { error, warn: vi.fn() },
});
// Returned, so the diagnosis survives a caller that passed no logger.
expect(res.error).toContain(`'inviteOnly'`);
Expand DownExpand Up@@ -302,7 +302,7 @@ describe('reconcileMembership / backfillMemberships — off-vocabulary policy (#
// fields that have no business in a log line.
policy: asPolicy({ membershipPolicy: 'invite-only', adminEmail: 'ops@example.com' }),
resolveTargetOrg: async () => 'org_default',
logger: { error },
logger: { error, warn: vi.fn() },
});
expect(res.outcome).toBe('invalid-policy');
expect(res.error).toContain('[object]');
Expand All@@ -315,7 +315,7 @@ describe('reconcileMembership / backfillMemberships — off-vocabulary policy (#
const res = await reconcileMembership(makeEngine(), 'user-1', {
policy: asPolicy('x'.repeat(500)),
resolveTargetOrg: async () => 'org_default',
logger: { error },
logger: { error, warn: vi.fn() },
});
expect(res.error).toContain('(truncated)');
expect(res.error!.length).toBeLessThan(200);
Expand Down
10 changes: 9 additions & 1 deletion packages/plugins/plugin-auth/src/reconcile-membership.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -106,7 +106,15 @@ export interface ReconcileMembershipDeps {
resolveTargetOrg: () => Promise<string | null>;
logger?: {
info?: (msg: string, meta?: any) => void;
warn?: (msg: string, meta?: any) => void;
/**
* The GUARANTEED fallback channel (#9754). `error` 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. Call sites keep the `logger?.warn?.(…)` spelling as the backstop
* for hosts the TYPE cannot reach; `SweepLogger` in plugin-email's
* `outbox-sweep.ts` carries the full reasoning and the measurement.
*/
warn: (msg: string, meta?: any) => void;
error?: (msg: string, meta?: any) => void;
};
}
Expand Down
10 changes: 9 additions & 1 deletion packages/plugins/plugin-email/src/attachment-reclaim.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,7 +111,15 @@ export interface AttachmentReclaimEngine {
/** Structural logger — same shape the outbox sweep uses. */
interface ReclaimLogger {
info?: (msg: string, meta?: any) => void;
warn?: (msg: string, meta?: any) => void;
/**
* The GUARANTEED fallback channel (#9754). `error` 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. Call sites keep the `logger?.warn?.(…)` spelling as the backstop
* for hosts the TYPE cannot reach; `SweepLogger` in plugin-email's
* `outbox-sweep.ts` carries the full reasoning and the measurement.
*/
warn: (msg: string, meta?: any) => void;
error?: (msg: string, meta?: any) => void;
}

Expand Down
Loading
Loading