Skip to content

refactor(plugin-sharing): collapse the two byte-identical MinimalLogger shapes - #11068

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-10692-sharing-minimal-logger-divergence
Aug 22, 2026
Merged

refactor(plugin-sharing): collapse the two byte-identical MinimalLogger shapes#11068
os-warren merged 1 commit into
mainfrom
claude/issue-10692-sharing-minimal-logger-divergence

Conversation

@os-warren

@os-warrenos-warren commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Part of #10692

Collapses the two genuinely byte-identical MinimalLogger declarations in
plugin-sharing onto one shared internal type. Internal types only — none of the
seven local MinimalLogger interfaces was exported, so no published surface and no
runtime behaviour changes.

This is Part of, not a closing keyword, and deliberately so: the divergence the card
is about is not fully closed by this subset. Five declarations remain, and two of
them turn out to sit behind a contract decision that is the PM's to make, not a
refactor. The measurement is below.

The count: it is SEVEN, not six

Derived by enclosing declaration, not by grep hit: every interface MinimalLogger
/ type MinimalLogger = in the package, then each one's members read off the
declaration itself.

packages/plugins/plugin-sharing/src declares seven interfaces named
MinimalLogger, one per file — all seven spell the same name. The card's title says
six and its body says "Seven rows, six of which spell the same name"; both are off by
one against the tree at 072d072d2. Nineteen textual MinimalLogger hits resolve to
7 declarations + 12 type annotations.

Normalising away doc comments, there are 5 distinct structural shapes:

shapefilesin check:optional-error-sink population?
{ info?, warn, error? } — all (msg: any, ...rest: any[]) => voidbulk-recompute.tsyes — it is the guaranteed sink
{ info?, warn } — same signaturesrule-hooks.ts, record-share-cascade.tsno (error absent)
{ info?, warn? } — same signaturesbu-tree-recompute.ts, primary-bu-projection.tsno
{ info?, warn? } — but a string message + Record meta signature (see below)sharing-rule-provenance.tsno
{ info?: Function, warn?: Function }record-orphan-cleanup.tsno (and invisible — see below)

⚠️ The card's "three already-identical shapes" is really two

The card groups three files as { info?, warn? }. That grouping normalises away the
member signatures, and they differ: sharing-rule-provenance.ts spells its members
a string message plus a Record meta object, while bu-tree-recompute.ts and
primary-bu-projection.ts spell theirs with an any message and a rest parameter:

// sharing-rule-provenance.ts — precise: catches a non-string message and a bad meta
info?: (msg: string,meta?: Record<string,any>)=>void;// bu-tree-recompute.ts / primary-bu-projection.ts — loose: catches nothing
info?: (msg: any, ...rest: any[])=>void;

Only the latter two are byte-identical, so only those two are unified here. Folding
sharing-rule-provenance.ts in would go one of two ways, and both change meaning,
so neither is a de-duplication:

  • onto the any spelling → deletes real checking at its call sites;
  • the others onto its spelling → tightens two modules.

What is unified

bu-tree-recompute.ts and primary-bu-projection.ts now share
OptionalSharingLogger in the new src/logger-shapes.ts.

The shared type is given a different name on purpose. The card's complaint is the
diagnostic Two different types with this name exist, but they are unrelated. A future
forwarding edge between this shape and a module still declaring its own MinimalLogger
now names two different types instead of the same name twice. (OptionalLogger was
not reused — plugin-webhooks/src/auto-enqueuer.ts already declares a different type
under that name, which is the same hazard.)

record-orphan-cleanup.ts was left alone — it is blocked, not skipped

The card asks for its bare Function members to be replaced with real signatures.
That is not possible without a contract decision, and the reason is measured, not
argued:

Type 'Function' is not assignable to type '(msg: any, ...rest: any[]) => void'.
Type 'Function' provides no match for the signature '(msg: any, ...rest: any[]): void'.

Function is assignable to no concrete signature. The two loggers actually handed to
sweepOrphanedRowsByRecordExistence are SharingServiceOptions['logger']
(sharing-service.ts:1381) and ShareLinkServiceOptions['logger']
(share-link-service.ts:704), and both are themselves spelled with bare
Function
— uncast, unlike every other logger entry point in this package, which
arrives as ctx.logger as any. So tightening the consumer requires tightening those
two producers first.

I measured what that costs, then reverted it byte-identically:

population declaring errorpermit silence (RED)gate
today372 (2 baselined)✓ passes
producers tightened394fails

Tightening them costs 0 compile errors — and enrols two new RED sinks:

✗ 2 sink type(s) declare an optional `error` with no guaranteed fallback channel
packages/plugins/plugin-sharing/src/share-link-service.ts:345
sink : inline type logger@ShareLinkServiceOptions { info? warn? error? debug? }
packages/plugins/plugin-sharing/src/sharing-service.ts:280
sink : inline type logger@SharingServiceOptions { info? warn? error? debug? }

Both shapes are latently red today: they declare error? beside an optional warn,
which is exactly what #9754 forbids. They escape the gate only because bare Function
is not a FunctionTypeNode, so the gate's structural matcher never sees them. And the
gate's own prescribed repair — dropping the ? from warn — lands on
SharingServiceOptions and ShareLinkServiceOptions, both publicly exported from
src/index.ts, so it is a breaking change for any host passing { info, error }.

That is a contract decision for the #10556 family. Recorded on #10692 for the PM rather
than done quietly here.

check:optional-error-sink — the load-bearing check, before and after

Population unchanged, quoting the gate's own verdict lines:

Before

SINK CENSUS [optional-error-sink-contract] (#9754): 37 sink type(s) declaring `error` in
packages/** — 12 declare it REQUIRED, 23 declare it optional beside a REQUIRED `warn`,
2 permit silence (2 optional-fallback, 0 no-fallback).
✓ optional-error sink contract: every sink declaring an optional `error` guarantees a
`warn` channel (2 baselined, shrink-only).

After — the two enforced numbers and the verdict line are identical:

SINK CENSUS [optional-error-sink-contract] (#9754): 37 sink type(s) declaring `error` in
packages/** — 12 declare it REQUIRED, 23 declare it optional beside a REQUIRED `warn`,
2 permit silence (2 optional-fallback, 0 no-fallback).
✓ optional-error sink contract: every sink declaring an optional `error` guarantees a
`warn` channel (2 baselined, shrink-only).

One informational number did move, and it is not a membership change: the
narrowings: line's tally of pure sinks declaring no error went 56 → 54. I
predicted 55 and was wrong by one; the miss is explained rather than waved at. Two
declarations were removed and one added, so -2 +1 = -1 was the prediction. The new
declaration is not counted at all, because the gate prefilters files by text before
parsing:

if(!/\berror\s*\??\s*[:(]/.test(text))continue;

logger-shapes.ts never spells error followed by : or (, so it is skipped before
the parser sees it. The two files it replaced matched that regex only because their call
sites pass { error: err?.message } — a literal error:. Verified directly: deleting
logger-shapes.ts leaves the tally at 54, unchanged.

This is sound for what the gate enforces (a sink declaring error necessarily
matches the regex), but it means the advertised "cost of the narrowing" tally
undercounts. Filed separately as #11069; no behaviour here depends on it.

Proof that tsc would have failed

A type change has no runtime behaviour to assert, so the compiler is the instrument —
and an instrument not shown to be live is not a measurement. Signature predicted
before running:

Predicted — mutate the shared type's warn to (msg: string, code: number) => void;
expect 6 × TS2345 "not assignable to parameter of type 'number'", 3 in each file,
exit 2; errors in both files being the load-bearing part, since that is what proves
both modules really consume the single shared declaration.

Observed — exactly that:

src/bu-tree-recompute.ts(249,11): error TS2345: Argument of type '{ rule: string; error: any; }' is not assignable to parameter of type 'number'.
src/bu-tree-recompute.ts(279,13): error TS2345: Argument of type '{ rule: string; object: string; error: any; }' is not assignable to parameter of type 'number'.
src/bu-tree-recompute.ts(287,71): error TS2345: Argument of type '{ object: string; event: string; error: any; }' is not assignable to parameter of type 'number'.
src/primary-bu-projection.ts(61,57): error TS2345: Argument of type '{ userId: string; error: any; }' is not assignable to parameter of type 'number'.
src/primary-bu-projection.ts(67,59): error TS2345: Argument of type '{ userId: string; error: any; }' is not assignable to parameter of type 'number'.
src/primary-bu-projection.ts(141,57): error TS2345: Argument of type '{ error: any; }' is not assignable to parameter of type 'number'.

6 errors, 3 per file, exit 2 — prediction and observation agree on count, code, shape
and distribution.

Restore proved byte-identical by git hash-object
(2533d6dad5348f3728a6db38edb0eafad3b56a61 before the mutation and after the restore),
and the restore leg re-run to a real verdict rather than trusted on the hash: tsc
exit 0, 0 errors, git status --porcelain empty.

The two call sites the card names still compile and their warn requirement is
unchanged — rule-hooks.ts and record-share-cascade.ts are untouched by this PR.

Gates

Union derived on the final commit5eb6636f2, clean tree,
node scripts/pm/dispatch-gates.mjs with no path arguments (it takes the change set
from the merge base itself: 4 paths vs 072d072d2). It named 11 families plus one
convention-triggered. It did not name check:optional-error-sink — that gate
computes its own population and scores silent for every card — which is exactly why
the card mandated it by hand.

All verdicts below are the gates' own printed lines; exit codes were captured before any
pipe.

gateverdict
check:optional-error-sink✓ optional-error sink contract … (2 baselined, shrink-only)
plugin-sharing typecheckexit 0, 0 errors
plugin-sharing testTest Files 25 passed (25) · Tests 624 passed (624)
check:changeset-gate-self-tests✓ check-empty-changeset --self-test: 118 assertions over real temp git repos
check:objectui-changeset✓ objectui-changeset-digest: 70058c167c14..a90fbe836c37 walks completely
check:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new
check:test-source-aliascheck-test-source-alias OK — 72 packages with tests scanned
check:type-source-resolutioncheck-type-source-resolution OK — 77 packages with a tsconfig.json scanned
check-adr-0087-registration✓ this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen)
check-changeset-no-major✓ This diff introduces no \major` bump.`
check-ci-filter-parityOK: all 83 declared cross-package glob(s) (72 unique) are covered
check-empty-changeset✓ No empty-frontmatter changeset introduced by this diff
check-plugin-teardown-shape✓ 63 Plugin implementation(s) across 4446 source(s); baseline fully burned down
check-affected-docs✓ affected-docs self-test: 339 cases pass.
check:nul-bytes✓ check-nul-bytes --self-test: 75 assertions over a temp git repo
check:i18ncheck-i18n-bundles: OK (9 package(s) — all bundles in sync) · plugins/plugin-sharing in sync (4 bundle(s))

check:i18n first returned PREREQUISITE NOT MET — the workspace CLI is not built … Nothing was checked at exit 1. That is not measured, never a pass, so the CLI was
built (turbo run build --filter=@objectstack/cli) and the gate re-run to the real
verdict quoted above.

Open question for the PM — not decided here

What should this package's logger contract be? Three candidates, with cost:

  1. Leave the five as they are (this PR). Cost: the seam stays open; the next
    forwarding edge re-opens the same diagnostic.
  2. One { info?, warn? } contract with a precise signature
    (a string message plus a Record meta object), absorbing sharing-rule-provenance.ts,
    bu-tree-recompute.ts and primary-bu-projection.ts. Cost: 0 at every caller
    all three receive ctx.logger as any or undefined, so no caller constrains them —
    and it gains arity/type checking at ~12 in-module call sites, which is precisely
    what the card says bare Function fails to provide. This is my recommendation for
    the { info?, warn? } family, but it tightens two modules, so it is a contract call,
    not a refactor.
  3. One contract for the whole package, including error. ⛔ Not recommended. It
    enrols four modules into check:optional-error-sink's population — a ledger being
    paid down deliberately (Pay down the optional-error sink ledger — 13 paid, 2 remain and both are DESIGN CALLS (was: "15 sink types") #10556: 15 → 3 → 2, shrink-only) — and the repair it then
    demands is a breaking change to two exported option types.

The record-orphan-cleanup.ts producer question above is the sharper one, and it is
live whether or not this card proceeds: two shapes are red under #9754's rule today and
are hidden only by a spelling.


Generated by Claude Code

…er shapes
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

5 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 3e26359a7d87fbe0e03c7236346166c71a821ec0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 837b34dbcc48334e1a78448f7328c96ff0b1f2b8 — the merge of head 5eb6636f2b7780b9eb9e390be4e634ebdbf74255 into base 3e26359a7d87fbe0e03c7236346166c71a821ec0, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 837b34dbcc48334e1a78448f7328c96ff0b1f2b8 && git checkout 837b34dbcc48334e1a78448f7328c96ff0b1f2b8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3e26359a7d87fbe0e03c7236346166c71a821ec0 5eb6636f2b7780b9eb9e390be4e634ebdbf74255 && git checkout -B drift-repro 3e26359a7d87fbe0e03c7236346166c71a821ec0 && git merge --no-ff 5eb6636f2b7780b9eb9e390be4e634ebdbf74255
node scripts/docs-audit/affected-docs.mjs --json 3e26359a7d87fbe0e03c7236346166c71a821ec0

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32581078922 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 4.21s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 82 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Merged via the queue into main with commit 93304c2Aug 22, 2026
35 checks passed
@os-warren
os-warren deleted the claude/issue-10692-sharing-minimal-logger-divergence branch August 22, 2026 15:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mtooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-warren@claude