From 5eb6636f2b7780b9eb9e390be4e634ebdbf74255 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 14:41:37 +0000 Subject: [PATCH] refactor(plugin-sharing): collapse the two byte-identical MinimalLogger shapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit plugin-sharing/src declared seven module-local interfaces all named MinimalLogger. The duplication was not the defect; divergence under one name was — when #10556 made bulk-recompute.ts's `warn` non-optional, tsc reported "Two different types with this name exist, but they are unrelated." bu-tree-recompute.ts and primary-bu-projection.ts were byte-identical, so they now share one declaration, OptionalSharingLogger in logger-shapes.ts. The name is deliberately different: the next forwarding edge between it and a module still declaring its own MinimalLogger names two types instead of one twice. The other five are left alone with a stated reason each, recorded in logger-shapes.ts. Notably record-orphan-cleanup.ts's bare `Function` members cannot be tightened here — `Function` is not assignable to any concrete signature, and both loggers handed to it are themselves spelled `Function`. check:optional-error-sink (#9754) population is unchanged: 37 declaring `error`, 2 permit silence, 2 baselined, verified before and after. The shared shape declares no `error` and must not grow one. Part of #10692 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .changeset/sharing-logger-shape-dedup.md | 46 ++++++++++++++ .../plugin-sharing/src/bu-tree-recompute.ts | 8 +-- .../plugin-sharing/src/logger-shapes.ts | 61 +++++++++++++++++++ .../src/primary-bu-projection.ts | 13 ++-- 4 files changed, 114 insertions(+), 14 deletions(-) create mode 100644 .changeset/sharing-logger-shape-dedup.md create mode 100644 packages/plugins/plugin-sharing/src/logger-shapes.ts diff --git a/.changeset/sharing-logger-shape-dedup.md b/.changeset/sharing-logger-shape-dedup.md new file mode 100644 index 0000000000..624aa94290 --- /dev/null +++ b/.changeset/sharing-logger-shape-dedup.md @@ -0,0 +1,46 @@ +--- +"@objectstack/plugin-sharing": patch +--- + +Collapse the two byte-identical `MinimalLogger` declarations in `plugin-sharing` +onto one shared `OptionalSharingLogger` (#10692). Internal types only — none of +the seven local `MinimalLogger` interfaces was exported, so no published surface +and no runtime behaviour changes. + +`plugin-sharing/src` declared **seven** module-local interfaces all named +`MinimalLogger`. The duplication was not the defect; divergence under one name +was. When #10556 made `bulk-recompute.ts`'s `warn` non-optional, `tsc` reported +the forwarding modules as: + +``` +Type 'MinimalLogger' is not assignable to type 'MinimalLogger'. + Two different types with this name exist, but they are unrelated. +``` + +`bu-tree-recompute.ts` and `primary-bu-projection.ts` were byte-identical, so +they now share one declaration in `logger-shapes.ts`. The new type is +deliberately given a DIFFERENT name: the next forwarding edge added between it +and a module that still declares its own `MinimalLogger` produces a diagnostic +naming two different types, instead of the same name twice. + +The other five declarations are left alone, each for a stated reason recorded in +`logger-shapes.ts`. Three are genuinely different contracts (`bulk-recompute.ts` +is the guaranteed sink; `rule-hooks.ts` and `record-share-cascade.ts` require +`warn` because they forward into it). Two are *not* the cheap unification the +card assumed: + +- `sharing-rule-provenance.ts` is `{ info?, warn? }` by optionality but carries a + stricter member signature, `(msg: string, meta?: Record)`. + Folding it onto the `(msg: any, ...rest: any[])` spelling would delete real + checking; folding the others onto its spelling would tighten two modules. +- `record-orphan-cleanup.ts`'s bare `Function` members **cannot** be tightened + here: `Function` is not assignable to any concrete signature ("Type 'Function' + provides no match for the signature"), and the two loggers handed to it — + `SharingServiceOptions['logger']` and `ShareLinkServiceOptions['logger']` — + are themselves spelled with bare `Function`. + +`check:optional-error-sink` (#9754) membership is unchanged and was verified +before and after: 37 sinks declare `error`, 2 permit silence, 2 baselined. The +shared shape declares no `error` and must not grow one — that would enrol every +module using it into that gate's population, which is a contract decision for +the #10556 family rather than a side effect of de-duplication. diff --git a/packages/plugins/plugin-sharing/src/bu-tree-recompute.ts b/packages/plugins/plugin-sharing/src/bu-tree-recompute.ts index b00aa1c414..d78c6b69f0 100644 --- a/packages/plugins/plugin-sharing/src/bu-tree-recompute.ts +++ b/packages/plugins/plugin-sharing/src/bu-tree-recompute.ts @@ -91,6 +91,7 @@ import type { SharingRuleRow, SharingRuleRecipientType } from '@objectstack/spec/contracts'; import { ruleRegrantQueue } from './rule-hooks.js'; +import type { OptionalSharingLogger } from './logger-shapes.js'; const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; @@ -151,11 +152,6 @@ interface MinimalEngine { unregisterHooksByPackage(packageId: string): number; } -interface MinimalLogger { - info?: (msg: any, ...rest: any[]) => void; - warn?: (msg: any, ...rest: any[]) => void; -} - /** The slice of {@link SharingRuleService} this module drives. */ export interface BuTreeRecomputeRuleService { listRules( @@ -220,7 +216,7 @@ export function writeCanChangeExpansion(objectName: string, event: string, hookC export function bindBusinessUnitTreeRecompute( engine: MinimalEngine, service: BuTreeRecomputeRuleService, - logger?: MinimalLogger, + logger?: OptionalSharingLogger, ): void { if (typeof engine.registerHook !== 'function') return; if (typeof engine.unregisterHooksByPackage === 'function') { diff --git a/packages/plugins/plugin-sharing/src/logger-shapes.ts b/packages/plugins/plugin-sharing/src/logger-shapes.ts new file mode 100644 index 0000000000..2533d6dad5 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/logger-shapes.ts @@ -0,0 +1,61 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The one `{ info?, warn? }` logger shape this package's fire-and-forget + * projection modules share (#10692). + * + * ## Why this file exists + * + * `plugin-sharing` declared SEVEN module-local interfaces all named + * `MinimalLogger`. Duplication was not the defect; DIVERGENCE under one name + * was. When #10556 made `bulk-recompute.ts`'s `warn` non-optional, `tsc` + * reported the two modules that forward into it with the unreadable form: + * + * Type 'MinimalLogger' is not assignable to type 'MinimalLogger'. + * Two different types with this name exist, but they are unrelated. + * + * A shared declaration removes that seam for the modules that can share one, + * and the DISTINCT NAME is deliberate: when a future forwarding edge is added + * between this shape and a module that still declares its own, the diagnostic + * names two different types instead of the same one twice. + * + * ## ⛔ Why this shape declares no `error`, and must not grow one + * + * `check:optional-error-sink` (#9754) draws its population STRUCTURALLY: a sink + * is in scope only if it DECLARES an `error` member. A shape with no `error` is + * out of the population — there is no optional fallback for the rule to + * guarantee. Adding `error?` here would enrol every module that uses this type + * into that gate's scope at once, and that ledger is being paid DOWN + * deliberately (#10556: 15 → 3 → 2, shrink-only). Enlarging it is a contract + * decision for the #10556 family, never a side effect of de-duplication. + * + * ## ⛔ Why the other four modules do NOT use this type + * + * They are not the same contract, and collapsing them would change meaning: + * + * - `bulk-recompute.ts` — `{ info?, warn, error? }`. It IS the guaranteed sink; + * its `warn` is required and it declares `error?`, so it is in the gate's + * population by design. + * - `rule-hooks.ts`, `record-share-cascade.ts` — `{ info?, warn }`. Their `warn` + * is required because they FORWARD into `bulk-recompute.ts`'s guaranteed sink; + * a `warn?` there re-opens the silence one module downstream of where #10556 + * closed it. + * - `record-orphan-cleanup.ts` — `{ info?: Function, warn?: Function }`. Bare + * `Function` documents nothing, but it cannot be tightened to this shape: + * `Function` is NOT assignable to a concrete signature ("Type 'Function' + * provides no match for the signature"), and the two loggers handed to it — + * `SharingServiceOptions['logger']` and `ShareLinkServiceOptions['logger']` — + * are themselves spelled with bare `Function`. Tightening it requires + * tightening those producers first, which is a gate-population change, not a + * refactor. Recorded on #10692 rather than done quietly here. + * - `sharing-rule-provenance.ts` — `{ info?, warn? }` by OPTIONALITY but with a + * stricter member signature, `(msg: string, meta?: Record)`. + * Folding it onto the `(msg: any, ...rest: any[])` spelling below would DELETE + * real checking at its call sites; folding the others onto ITS spelling would + * tighten two modules. Either direction changes meaning, so neither is a + * de-duplication — see #10692 for the open contract question. + */ +export interface OptionalSharingLogger { + info?: (msg: any, ...rest: any[]) => void; + warn?: (msg: any, ...rest: any[]) => void; +} diff --git a/packages/plugins/plugin-sharing/src/primary-bu-projection.ts b/packages/plugins/plugin-sharing/src/primary-bu-projection.ts index 27f23acde6..cfc1250cba 100644 --- a/packages/plugins/plugin-sharing/src/primary-bu-projection.ts +++ b/packages/plugins/plugin-sharing/src/primary-bu-projection.ts @@ -23,6 +23,8 @@ * membership is usable single-tenant too). */ +import type { OptionalSharingLogger } from './logger-shapes.js'; + const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; export const PRIMARY_BU_HOOK_PACKAGE = 'plugin-sharing:primary-bu'; @@ -42,14 +44,9 @@ interface MinimalEngine { update(object: string, data: any, options?: any): Promise; } -interface MinimalLogger { - info?: (msg: any, ...rest: any[]) => void; - warn?: (msg: any, ...rest: any[]) => void; -} - /** Recompute one user's primary_business_unit_id from their `is_primary` member * row (null when they have none). Idempotent. */ -async function recompute(engine: MinimalEngine, userId: string, logger?: MinimalLogger): Promise { +async function recompute(engine: MinimalEngine, userId: string, logger?: OptionalSharingLogger): Promise { if (!userId) return; let buId: string | null = null; try { @@ -94,7 +91,7 @@ function collectUserIds(ctx: any): string[] { * projection must stay correct regardless of who mutates membership (seeds, * HRIS sync, admin UI). */ -export function bindPrimaryBuHooks(engine: MinimalEngine, logger?: MinimalLogger): void { +export function bindPrimaryBuHooks(engine: MinimalEngine, logger?: OptionalSharingLogger): void { if (typeof engine.registerHook !== 'function') return; if (typeof engine.unregisterHooksByPackage === 'function') { engine.unregisterHooksByPackage(PRIMARY_BU_HOOK_PACKAGE); @@ -131,7 +128,7 @@ export function bindPrimaryBuHooks(engine: MinimalEngine, logger?: MinimalLogger * `is_primary` member row, so pre-existing memberships (seeds, prior data) * project even though their inserts pre-dated the hooks. Idempotent. */ -export async function backfillPrimaryBu(engine: MinimalEngine, logger?: MinimalLogger): Promise<{ updated: number }> { +export async function backfillPrimaryBu(engine: MinimalEngine, logger?: OptionalSharingLogger): Promise<{ updated: number }> { let rows: any[] = []; try { rows = await engine.find('sys_business_unit_member', {