Repository navigation
fix(plugin-security,plugin-sharing): land #10556 ruled sink-ledger shape (5 rows to 1) - #11856
Conversation
…shape Part of #10556 — the three design answers the maintainer ruled on 2026-08-24, which take `scripts/optional-error-sink-contract.baseline.json` from 5 rows to 1. (a) `SecurityPlugin`'s own report sink defaults to a console-backed sink instead of `= {}`, so its fail-closed refusals reach an operator before a host injects one. `warn` becomes non-optional (#9754); `error` stays optional. Silent-by-declaration was rejected by the ruling. (c) The three PUBLICLY EXPORTED `plugin-sharing` options types tighten per #9754: bare `Function` members become real signatures and `warn` becomes required. Shipped `minor` with the published-contract break named in the changeset. This also clears #10692's producer-side blocker. (b) `SettingsDiagnosticsLogger` KEEPS its optional `warn` as a recorded deliberate exception. Its ledger row stays; its `note` is rewritten to read as design-of-record, and the same reasoning is added at the declaration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
📓 Docs Drift CheckThis PR changes 3 package(s): 4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 21 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin cf7406b0a3903719d2148898d51bee51a94692e2 && git checkout cf7406b0a3903719d2148898d51bee51a94692e2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 589758d22ccacf9cc56b5bc8a9f9766cb7e2a93a 040932a13130bfbbf9b4d708dc2d7f5d401c3758 && git checkout -B drift-repro 589758d22ccacf9cc56b5bc8a9f9766cb7e2a93a && git merge --no-ff 040932a13130bfbbf9b4d708dc2d7f5d401c3758
node scripts/docs-audit/affected-docs.mjs --json 589758d22ccacf9cc56b5bc8a9f9766cb7e2a93a
|
… `warn` Part of #10556 — repairs the `Type Check · debt ledger` failure on this PR. `check:type-check-debt` re-measured `@objectstack/plugin-sharing` at 4 raw tsc errors against a TEST_DEBT entry recording 3. The +1 is this PR's own: sharing-service.test.ts(1585,7): error TS2741: Property 'warn' is missing in type '{ error: (...args: any[]) => void; }' but required in type '{ info?…; warn: (msg: any, ...rest: any[]) => void; error?…; debug?… }' — the "required in type" is verbatim the signature limb (c) introduced. Fixed at the test, never at the ledger: the entry stays at 3, the tightened contract stays as ruled. Both channels land in the same `logged` array on purpose — #9754's rule is that a fail-closed report DEGRADES to `warn` when `error` is absent, so a double capturing only `error` is one that would not notice the degradation this test exists to assert on. The package's own `typecheck` and its vitest suite were both green while this was red: the tsconfig hides tests from tsc (which is what TEST_DEBT measures) and vitest does not typecheck. Reproduced with a probe tsconfig written INSIDE the repo, mirroring `remeasureProject`, removed under a trap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
…n every required warn (objectstack-ai#10692) (objectstack-ai#12275) Part of objectstack-ai#10692 — the ruled remainder (option A, maintainer 2026-08-25) after PR objectstack-ai#11856 landed the three publicly exported producers under objectstack-ai#10556 limb (c). - record-orphan-cleanup.ts's module-local MinimalLogger drops its bare Function members for the producers' real signatures and makes warn REQUIRED: every report the sweep emits lands on warn, so a logger without a guaranteed warn is one the sweep loses its only output into (objectstack-ai#9754's permit-silence shape, one module downstream of the tightened producers). Zero in-repo call-site cost: both callers forward the owning services' logger options, whose warn is already required. - logger-required-warn.pin.ts pins the contract at compile time for all four sinks (three public options types + the sweep parameter): a { info, error }-only literal is refused, a bare { warn } stub stays legal, and a value of type Function no longer satisfies the members. Pins live in a src-level .pin.ts because this package's tsconfig excludes tests from tsc; the file deliberately declares no type literals with channel members so check:optional-error-sink's census never reads it. - logger-shapes.ts's shape map is updated: the record-orphan-cleanup row no longer describes bare Function as blocked, and names the pin file. - Changeset: minor with the published-contract break declared (BREAKING) and the one-line host fix named, per the launch-window convention. Claude-Session: https://claude.ai/code/session_01UQgPSniH1GFM9ZDeGyuGUa Co-authored-by: Claude <noreply@anthropic.com>
Part of #10556
Lands the three design answers the maintainer ruled on 2026-08-24 (comment
5394009460), takingscripts/optional-error-sink-contract.baseline.jsonfrom 5 rows to 1.1. The ruling's closing line about "the 13 mechanical entries" is stale — re-confirmed, not inherited. On
origin/mainat78f65ef518the ledger holds exactly 5 entries and none is a?deletion. The twelve went in #10691; the thirteenth (logger@ApprovalServiceOptions) in #10937. Nothing in this PR looks for them.2. Option D is already landed, so limb (a) does NOT close the hole the dispatch asked me to measure. The dispatch asked whether a console-backed default closes the "bailed
start()reports go nowhere" hole recorded on5389623194. Measured onorigin/main:this.loggerhas exactly one assignment in the whole 6606-line file (grep -n 'this\.logger\s*='→ one hit, line 948) and it now sits above both bails. #10706 already hoisted it. So on a bailed boot the field already holdsctx.logger, and limb (a) changes nothing there — D is moot for that path, exactly as the dispatch said it would be if the default survived a bailed start.What the
= {}default still covered, and what (a) actually buys, is the window beforestart()runs at all — construction andinit(). That window is narrow (the plugin's report-bearing methods are published through thesecurityservice, whichstart()registers after line 948), but it is not empty:SecurityPluginis exported from this package'sindex.ts, so a host or a test holding the instance directly reachesgetReadFilter/canExport/checkAuthoredRowWritewith the sink still unbound. The larger thing (a) buys is that the declaration stops lying:warncan only be made non-optional once the initialiser guarantees it, which is what lets the ledger row leave without minting a required member on an empty object.⛔ Per the ruling, D is not this PR's reason and no logger-binding order was touched. Case 3 of the new suite pins #10706's hoist so a future edit cannot undo it silently.
The three limbs
(a)
logger@SecurityPlugin— console-backed default · row deletedsecurity-plugin.ts: the field moves from an inline{ info?; warn?; error? } = {}to a named module-localSecurityReportSinkwithwarnnon-optional, initialised to a frozenCONSOLE_SECURITY_SINKcarrying the two REPORT channels (warn,error).errordeliberately stays optional — #9754 option C is falsified, hosts do inject reduced sinks.infois deliberately absent from the default: it carries no durability report, so a host-less boot spraying routine chatter at stdout would be noise bought with nothing.Test-first, and provable.
packages/plugins/plugin-security/src/default-report-sink.test.tswas authored before any source edit, against the unmodified tree, with the signature predicted in writing first. Its RED observation was taken by ablation (see below) because the shared verify lock was held at the moment it was written — the authoring order is real, the observation was deferred.@ts-expect-errorcompile-time pin, on purpose.packages/plugins/plugin-security/tsconfig.jsonexcludes every*.test.tsundersrc(TEST_DEBT ledger), so a@ts-expect-errorthere evaluates never — that is not a weak pin, it is no pin, and it has bitten five seats. The compile-time half is carried bypnpm check:optional-error-sink, which runs on every PR fromlint.ymlwith nopaths:filter and turns red the momentwarngoes back to optional. That is a stronger pin than the one the trap forbids, and it lives where it actually evaluates.(b)
SettingsDiagnosticsLogger— row STAYS, note rewritten · 1 of 1Ledger row kept;
noterewritten to read as design-of-record, and the same reasoning added at the declaration insettings-service.types.tsso the next author finds it at the code rather than only in a JSON file.{error?}surface". It is no longer single-member: #10250 added awarn?. The checker reads it today asoptional-fallbackover{ error? warn? }, and the census prints 0 no-fallback sinks — so the row's ownverdict: "no-fallback", itsmembers: "{ error? }"and its claim to be "the ONLYno-fallbacksink left in the tree" were all three stale.The ruled answer is unaffected: the assignability reason of record is about
warnbeing OPTIONAL and is indifferent to whetherwarnis declared at all. I corrected the two descriptive fields rather than leave them, because a ledger row asserting a verdict the gate no longer renders is precisely the un-re-measured claim this file's header exists to refuse.verdictas one of the row's identity keys. It is not —check-optional-error-sink-contract.mjskeys rows asbaselineKey = (x) => \${x.file}::${x.sink}``, file and sink only. Both are byte-unchanged; the gate passes with the row, and the ablation below shows it still fails without a row it needs. If the reviewer would rather the two stale fields stayed as written, that is a one-line revert.(c) the three
plugin-sharingoptions types — tightened · 3 rows deletedSharingServiceOptions,ShareLinkServiceOptions,SharingRuleServiceOptions— all three PUBLICLY EXPORTED fromindex.ts. Not a?deletion: the members were bareFunction, which is not assignable to a concrete signature, so each needed a real one. Now(msg: any, ...rest: any[]) => voidwithwarnrequired, matching the house shapebulk-recompute.ts'sMinimalLoggeralready carries.Shipped
minorwith the published-contract break named in the changeset: a host passing{ info, error }compiles today and does not after. Zero call sites insideplugin-sharingmoved — the cost falls entirely on external hosts, which is why it is declared rather than shipped as a patch.#10692's producer-side blocker clears.
record-orphan-cleanup.ts'sMinimalLogger({ info?: Function; warn?: Function }) could not tighten while these producers stayed loose. It can now — the two signatures are identical. ⛔ That work is out of scope here and #10692 stays open for its own dispatch; this PR only removes what was blocking it.Verification
Every exit code below was captured before any pipe (
cmd > log 2>&1; EXIT=$?), and each verdict quoted is the gate's own line, never a bare$?. All runs on final commitbf036b4b09.The gate's own verdict, before and after:
Population unchanged at 41 both sides — this PR repaired sinks, it did not move what the gate can see.
Ablation on (c) —
sharing-service.tsreverted to itsorigin/mainspelling, everything else untouched. Build-free by construction: the checker is a syntactic source scanner (collectSourceFilesskipsdist), so the "a failed build leaves a mutateddist/" trap cannot reach this measurement.git hash-objectcheck is what caught it.git checkout origin/main -- <path>stages as well as writes, so the trap'sgit checkout -- <path>restored from the index — i.e. restored the ablated blob — and reported success. Corrected togit checkout HEAD -- <path>; verified byte-identical (b97e31c29a463c0b82abfa65d394bc5157492d3con disk, in the index and at HEAD) with an emptygit status, and the gate re-run green. Recording it because the failure mode is silent and exit-0.Ablation on (a), same discipline, direction predicted in writing first:
The prediction was 2 red, 1 green — case 3 pins #10706's pre-existing hoist, which this PR must not break, so it must survive the ablation. It did. A three-red result would have meant the suite was measuring the wrong thing.
Suites and typechecks (each
pnpm --filter <pkg> …echoed its script name, so none is a silent zero-match exit-0):Limb (c)'s in-repo consumer surface, measured rather than assumed. A contract tightening is only interesting where something consumes it, so instead of a blind 28-package sweep I asked who can actually be hit — every file naming one of the three options types or constructing one of the three services:
All nine are covered by the green suites above — which is also the direct confirmation of #11069's measurement that this tightening costs zero compile errors inside the repo, with the whole cost falling on external hosts. Note the direction:
...@objectstack/plugin-sharing(PREFIX) is the downstream-consumer form; the suffix form would have swept upstream dependencies, which a contract narrowing cannot reach.Gate families, re-run as a union on the final commit
bf036b4b09with a clean tree — derived withnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(no hand-built path list; the script read the change set from the merge base itself), which named several families the dispatch did not. All green:check:query-options-erasureandcheck:slot-lookupcaught a real defect on their first run:**/*.test.tswritten inside a block comment in the new test file contains*/, which terminated the comment and made the file unparseable. Fixed and both re-run green.Lint — a DECLARED narrowing, with its three evidences:
.mdand the.jsonas ignored, leaving the 6.tsfiles it accepts.--format json: 9 results returned, 6 linted, 0 errors, 3 warnings — all three the "file ignored" notices for the paths in (1). Exit 0.eslint.config.mjswhich never enables type-aware linting for any file (no parserOptions.project, no typed @typescript-eslint rules) — stated and measured with a positive control ateslint.config.mjs:327. With no cross-file type information, no verdict on a file this diff does not touch can move as a function of this diff.Additionally the two ESLint-population ratchets above walked all of
packages/**for parse failures and each reported "every file measured parsed" and "baseline key set verified against 78f65ef: no files added" — a wider ESLint pass than the six files.pnpm check:type-check-debt(the--re-measureratchet) is NOT MEASURED. Its first attempt refused correctly, for a reason worth recording: the (a) ablation had rewrittensecurity-plugin.ts, so that package'sdist/*.d.tswas older than its sources and the gate refused rather than measure a ledger number through adistdescribing a package that no longer exists (#6376). I rebuilt the package — and the re-run was killed at the container's 10-minute foreground cap (exit 143), mid---re-measure, with no verdict written. Fouros-verify-lockacquisitions across this task returnedVERDICT queue-timeout (exit 99) · never acquired; ~20 minutes queued with no turn.exit 99and an unexplained143are not measured, never a pass, and neither is written up as one.check:type-check-coverage— the STRUCTURAL half of the same pair — did run and is green; the RATCHET half goes to CI.@objectstack/runtimetypecheck + its one targeted test (27s combined) ran outside the shared verify lock after the fourth queue-timeout, rather than spending a fifth 9-minute budget on 27 seconds of work. Declared here because an undeclared narrowing is just a gate that did not run.The full
pnpm lintrepo scan is CI's by standing rule; the declared-narrowing evidence for it is directly above.Scope
Untouched, as the card and the ruling both fence: the 16th red sink in
examples/app-showcase/src/system/server/recalc-endpoint.ts, and any widening of the checker'spackages/**scan.packages/spechas zero ownership in this lane and is not edited. No call-site spelling was changed — thelogger?.warn?.(…)backstop stays, asbulk-recompute.tsdocuments for hosts the type cannot reach.⛔ Draft on purpose.
needs:contract-reviewis not cleared here — that is the contract-review tier's to clear.Part ofrather thanFixes: the ruled shape lands complete (5 → 1) and the one remaining row is the ruled permanent exception, but whether that closes the card is the PM's disposition and a wrongFixeswould silently close a card still under contract review.Generated by Claude Code