Skip to content

refactor(plugin-sharing): put the shared logger contract on the precise signature - #11236

Merged
os-sam merged 1 commit into
mainfrom
claude/issue-10692-sharing-logger-precise-signature
Aug 23, 2026
Merged

refactor(plugin-sharing): put the shared logger contract on the precise signature#11236
os-sam merged 1 commit into
mainfrom
claude/issue-10692-sharing-logger-precise-signature

Conversation

@claude

@claudeclaudeBot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Part of #10692

Implements option B, the remainder ruled in comment 5381088451 after PR #11068 landed the first half of this card.

⚠️Read every signature in this body as prose. The GitHub body sanitizer deletes short angle-bracket fragments — measured on this very card, where every Record parameterised with string keys and any values was truncated to a bare Record, in table cells, in inline backticks and inside a fenced code block. Because this change is a change of member signatures, each one below is stated in words as well as in code.

What changed

OptionalSharingLogger in logger-shapes.ts shipped with the loose spelling, inherited from the two byte-identical declarations PR #11068 collapsed into it. Its two members are now spelled precisely.

  • Before, in prose: each member took msg typed as any, followed by a rest parameter rest of any array, returning void.
  • After, in prose: each member takes msg typed as string, plus an optional second parameter meta typed as a Record of string keys to any values, returning void.

In code (expect the type arguments to be eaten on publish):

// before
info?: (msg: any, ...rest: any[])=>void;// afterinfo?: (msg: string,meta?: Record<string,any>)=>void;

sharing-rule-provenance.ts — which already declared exactly that stricter signature under its own local MinimalLogger — now imports the shared type instead of declaring a seventh copy. Its local interface is deleted.

Why the precise spelling, and not the loose one

Unifying on the loose spelling was the cheaper diff and was rejected. It would have deleted the checking sharing-rule-provenance.ts already had. A message typed any with an any rest parameter documents nothing and catches nothing — which is the identical complaint this card levels at bare Function. Adopting it to buy uniformity would be the card fixing itself by becoming the thing it filed against.

Taking the strict spelling instead tightens the two modules already on the shared type, and buys real arity and type checking at all 12 in-module call sites (7 warn, 5 info) — every one of which already passes exactly a string message plus an optional object literal.

Caller cost: zero, re-verified rather than inherited

Every caller of the affected binders passes ctx.logger as any or undefined, so no caller constrains the signature:

callerargument
sharing-plugin.ts:565bindPrimaryBuHooksctx.logger as any
sharing-plugin.ts:566backfillPrimaryBuctx.logger as any
sharing-plugin.ts:722bindBusinessUnitTreeRecomputectx.logger as any
sharing-plugin.ts:732bindRuleProvenanceStampctx.logger as any
bu-tree-recompute.test.ts (4 sites)undefined
sharing-rule-provenance.test.ts:172omitted

The object literals at bu-tree-recompute.test.ts:442 and :460 are the rules service (2nd parameter), not the logger.

⭐ The load-bearing check: gate membership is unchanged

check:optional-error-sink (#9754) came back with its population and verdict lines byte-identical, exit 0 both sides:

SINK CENSUS: 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).

The gate computes its own population and scores silent for every card, so dispatch-gates.mjs is structurally incapable of naming it. It was run by hand, before and after.

One informational line moved, diagnosed rather than papered over. The narrowings tally pure sink(s) declare no error at all went 59 → 58. Predicted in writing beforehand as exactly −1, and the cause verified by positive control:

  • the gate prefilters files by text before parsing, requiring error followed by an optional ? and then a colon or open-paren;
  • sharing-rule-provenance.ts passes that prefilter (its warn call site spells an error key), so its deleted local interface was one of the 59 → −1;
  • logger-shapes.tsfails the prefilter and so contributes 0 — verified by injecting a comment that trips the prefilter, which moved the tally to 59 while population and verdict stayed identical, then restoring byte-identically.

Proof the compiler was actually watching

A green typecheck that would have been green anyway is not a measurement. Ablation, with the diagnostic predicted in writing first: mutate the shared warn member's second parameter to a required number.

Predicted: 7 errors, all TS2345, exit 2 — 3 in bu-tree-recompute.ts, 3 in primary-bu-projection.ts, 1 in sharing-rule-provenance.ts.

Observed: exactly that.

src/bu-tree-recompute.ts(249,11): error TS2345: ... not assignable to parameter of type 'number'.
src/bu-tree-recompute.ts(279,13): error TS2345: ...
src/bu-tree-recompute.ts(287,71): error TS2345: ...
src/primary-bu-projection.ts(61,57): error TS2345: ...
src/primary-bu-projection.ts(67,59): error TS2345: ...
src/primary-bu-projection.ts(141,57): error TS2345: ...
src/sharing-rule-provenance.ts(68,92): error TS2345: ...

The load-bearing part is that sharing-rule-provenance.ts is among them: that is what proves it now consumes the shared declaration rather than a local copy, i.e. that option B actually landed.

Rebuild note, stated rather than boilerplated: this ablation needs no dist rebuild and none was done — the mutated file and all three consumers sit in one package compiled by a single tsc --noEmit program from source, so nothing resolves through dist/. The 7 diagnostics are themselves the proof the mutation reached the compiler. The dependency closure was built beforehand, so the TS2307 cascade trap does not apply. The mutation was confirmed on disk by anchored grep (an editor exit code proves nothing), and both mutation scripts carried a trap ... EXIT INT TERM restore.

Restore proved byte-identical by git hash-object (beb0ac13… both sides) and re-run to a real verdict: typecheck exit 0, 0 errors, tree clean.

⛔ Deliberately untouched

record-orphan-cleanup.ts's bare-Function members stay as they are. Tightening them requires first tightening two publicly exported option types, SharingServiceOptions and ShareLinkServiceOptions loggers — measured to move this gate from 37/2-red to 39/4-red and fail it. That is question 2 on #10692, sequenced behind #11069 and then a maintainer breaking-change call. #10692 stays open for it, which is why this PR says Part of.

Verification

All exit codes captured before any pipe; every verdict below is the gate's own printed line. Run on the final commit 8a680a46ae, clean tree.

  • pnpm --filter @objectstack/plugin-sharing typecheck — exit 0, 0 errors, before and after (one echoed script line, so not a zero-match filter)
  • pnpm --filter @objectstack/plugin-sharing testTest Files 25 passed (25), Tests 624 passed (624), exit 0
  • pnpm check:optional-error-sink — exit 0 both sides, population and verdict byte-identical
  • Gate union derived by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, no hand-supplied paths, on the final commit: 12 families + 1 convention-triggered, all green.
  • check:i18n first returned PREREQUISITE NOT MET … Nothing was checked at exit 1. Treated as NOT MEASURED, never a pass: built the CLI and re-ran to the real verdict — check-i18n-bundles: OK (9 package(s) …), plugins/plugin-sharing in sync (4 bundle(s)), exit 0.
  • Lint narrowed to the 2 changed files and declared as a narrowing: 2 files linted per --format json, 0 errors / 0 warnings, neither ignored; the repo runs one eslint.config.mjs which never enables type-aware linting for any file (its own comment records the positive control), so this diff cannot move the verdict of any untouched file. The repo-wide sweep remains CI's run.
  • pnpm check:nul-bytes green, plus a manual control-character grep over the changed files and this body.

Generated by Claude Code


Generated by Claude Code

…se signature
Spell OptionalSharingLogger's members as a string message plus an optional
Record of string keys to any values, and absorb sharing-rule-provenance.ts
onto the shared type instead of a seventh local MinimalLogger.
The shared shape shipped with the loose (msg: any, ...rest: any[]) spelling
inherited from the two byte-identical declarations it replaced. That spelling
documents nothing and catches nothing -- the same complaint this card levels
at bare Function -- so the third module was unified onto the strict spelling
it already declared, rather than the reverse.
Caller cost is zero: every caller passes ctx.logger as any, or undefined.
All 12 in-module call sites already pass exactly (string, object?).
check:optional-error-sink membership is unchanged: 37 / 12 / 23 / 2, still
2 baselined. record-orphan-cleanup.ts's bare-Function members stay untouched
-- tightening them moves that gate's population and remains open on #10692.
Part of #10692
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

What this run could not see
  • 2 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 38cf397ea9b1b5aea338805e7559f70576b88fb1packageMentionDocs.

Which tree this was computed on

This run read content/docs from af6c48ecfed30cf3aa1bbd714ffb1045a48e5344 — the merge of head 8a680a46ae4a037e40fc7b9e018f38aa77791a53 into base 38cf397ea9b1b5aea338805e7559f70576b88fb1, 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 af6c48ecfed30cf3aa1bbd714ffb1045a48e5344 && git checkout af6c48ecfed30cf3aa1bbd714ffb1045a48e5344
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 38cf397ea9b1b5aea338805e7559f70576b88fb1 8a680a46ae4a037e40fc7b9e018f38aa77791a53 && git checkout -B drift-repro 38cf397ea9b1b5aea338805e7559f70576b88fb1 && git merge --no-ff 8a680a46ae4a037e40fc7b9e018f38aa77791a53
node scripts/docs-audit/affected-docs.mjs --json 38cf397ea9b1b5aea338805e7559f70576b88fb1

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-sam@claude