Skip to content

fix(plugin-security,plugin-sharing): land #10556 ruled sink-ledger shape (5 rows to 1) - #11856

Merged
os-sam merged 2 commits into
mainfrom
claude/issue-10556-sink-ledger-ruled-shape
Aug 24, 2026
Merged

os-sam merged 2 commits into
mainfrom
claude/issue-10556-sink-ledger-ruled-shape

Conversation

@claude

@claude claude Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Part of #10556

Lands the three design answers the maintainer ruled on 2026-08-24 (comment 5394009460), taking scripts/optional-error-sink-contract.baseline.json from 5 rows to 1.

⚠️ Read this first: two premise facts I measured, and both differ from the dispatch

1. The ruling's closing line about "the 13 mechanical entries" is stale — re-confirmed, not inherited. On origin/main at 78f65ef518 the 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 on 5389623194. Measured on origin/main:

938:    ctx.logger.info('Starting Security Plugin...');
942:    // [#10706] Bind the report sink FIRST — above the two bail-outs below.
948:    this.logger = ctx.logger;          ← the assignment
958:      ctx.logger.warn('ObjectQL or metadata service not available…');  return;   ← bail 1
963:      ctx.logger.warn('ObjectQL engine does not support middleware…');  return;   ← bail 2

this.logger has 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 holds ctx.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 before start() runs at all — construction and init(). That window is narrow (the plugin's report-bearing methods are published through the security service, which start() registers after line 948), but it is not empty: SecurityPlugin is exported from this package's index.ts, so a host or a test holding the instance directly reaches getReadFilter/canExport/checkAuthoredRowWrite with the sink still unbound. The larger thing (a) buys is that the declaration stops lying: warn can 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 deleted

security-plugin.ts: the field moves from an inline { info?; warn?; error? } = {} to a named module-local SecurityReportSink with warn non-optional, initialised to a frozen CONSOLE_SECURITY_SINK carrying the two REPORT channels (warn, error). error deliberately stays optional — #9754 option C is falsified, hosts do inject reduced sinks. info is 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.ts was 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.

⚠️ No @ts-expect-error compile-time pin, on purpose. packages/plugins/plugin-security/tsconfig.json excludes every *.test.ts under src (TEST_DEBT ledger), so a @ts-expect-error there evaluates never — that is not a weak pin, it is no pin, and it has bitten five seats. The compile-time half is carried by pnpm check:optional-error-sink, which runs on every PR from lint.yml with no paths: filter and turns red the moment warn goes 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 1

Ledger row kept; note rewritten to read as design-of-record, and the same reasoning added at the declaration in settings-service.types.ts so the next author finds it at the code rather than only in a JSON file.

⚠️ Measured deviation from the ruling's wording, flagged rather than smoothed over. The ruling says this sink "keeps its single-member {error?} surface". It is no longer single-member: #10250 added a warn?. The checker reads it today as optional-fallback over { error? warn? }, and the census prints 0 no-fallback sinks — so the row's own verdict: "no-fallback", its members: "{ error? }" and its claim to be "the ONLY no-fallback sink left in the tree" were all three stale.

The ruled answer is unaffected: the assignability reason of record is about warn being OPTIONAL and is indifferent to whether warn is 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.

⚠️ The dispatch named verdict as one of the row's identity keys. It is not — check-optional-error-sink-contract.mjs keys rows as baselineKey = (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-sharing options types — tightened · 3 rows deleted

SharingServiceOptions, ShareLinkServiceOptions, SharingRuleServiceOptions — all three PUBLICLY EXPORTED from index.ts. Not a ? deletion: the members were bare Function, which is not assignable to a concrete signature, so each needed a real one. Now (msg: any, ...rest: any[]) => void with warn required, matching the house shape bulk-recompute.ts's MinimalLogger already carries.

Shipped minor with the published-contract break named in the changeset: a host passing { info, error } compiles today and does not after. Zero call sites inside plugin-sharing moved — 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's MinimalLogger ({ 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 commit bf036b4b09.

The gate's own verdict, before and after:

before  ✓ optional-error sink contract: … guarantees a `warn` channel (5 baselined, shrink-only).
        SINK CENSUS … 24 declare it optional beside a REQUIRED `warn`, 5 permit silence (5 optional-fallback, 0 no-fallback).
after   ✓ optional-error sink contract: … guarantees a `warn` channel (1 baselined, shrink-only).
        SINK CENSUS … 28 declare it optional beside a REQUIRED `warn`, 1 permit silence (1 optional-fallback, 0 no-fallback).

Population unchanged at 41 both sides — this PR repaired sinks, it did not move what the gate can see.

Ablation on (c) — sharing-service.ts reverted to its origin/main spelling, everything else untouched. Build-free by construction: the checker is a syntactic source scanner (collectSourceFiles skips dist), so the "a failed build leaves a mutated dist/" trap cannot reach this measurement.

positive control (unmutated, same channel)  EXIT=0  ✓ … (1 baselined, shrink-only).
mutation on disk, anchored BOTH directions: new signature present [expect 0] → 0
                                            old bare-Function present [expect 1] → 1
                                            disk hash a92632034… ≠ HEAD blob b97e31c29…
ablated run                                 EXIT=1
  ✗ 1 sink type(s) declare an optional `error` with no guaranteed fallback channel …
    packages/plugins/plugin-sharing/src/sharing-service.ts:280
  census: 2 permit silence  (the reverted row + the ruled (b) row)

⚠️ The first restore leg FAILED and the git hash-object check is what caught it. git checkout origin/main -- <path> stages as well as writes, so the trap's git checkout -- <path> restored from the index — i.e. restored the ablated blob — and reported success. Corrected to git checkout HEAD -- <path>; verified byte-identical (b97e31c29a463c0b82abfa65d394bc5157492d3c on disk, in the index and at HEAD) with an empty git 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:

green leg    (repaired tree)     EXIT=0   Test Files 1 passed (1) · Tests 3 passed (3)
ablated leg  (security-plugin.ts at origin/main)   EXIT=1   Tests 2 failed | 1 passed (3)
  × reports a fail-closed refusal to the console when no host sink was injected
  × guarantees a `warn` channel on the default sink, and routes it to the console
  ✓ is REPLACED by the host sink `start()` binds, not kept beside it
mutation on disk: repaired field spelling [expect 0] → 0 · console const [expect 0] → 0 · old `= {}` [expect 1] → 1
restored: disk hash == HEAD blob 706edf699e3a86d4f5ba976b0054a839a64b6ea2, `git status` empty

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):

pnpm --filter @objectstack/plugin-security  typecheck   EXIT=0
pnpm --filter @objectstack/plugin-sharing   typecheck   EXIT=0
pnpm --filter @objectstack/service-settings typecheck   EXIT=0
pnpm --filter @objectstack/runtime          typecheck   EXIT=0
plugin-security  vitest run   EXIT=0   Test Files 80 passed (80) · Tests 1515 passed (1515)
plugin-sharing   vitest run   EXIT=0   Test Files 27 passed (27) · Tests  652 passed (652)
service-settings vitest run   EXIT=0   Test Files 29 passed (29) · Tests  514 passed (514)
runtime/share-links-enforcement-context  EXIT=0   Test Files 1 passed (1) · Tests 13 passed (13)
turbo run build --filter=./packages/* --filter=./packages/*/*   EXIT=0   70 successful, 70 total
pnpm check:type-check-coverage                                  EXIT=0   65/78 packages type-checked, ledgers intact

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:

$ grep -rn --include=*.ts -E "SharingServiceOptions|ShareLinkServiceOptions|SharingRuleServiceOptions|new (SharingService|ShareLinkService|SharingRuleService)\b" \
    packages apps examples | grep -v "^packages/plugins/plugin-sharing/"
→ 9 files, in exactly 2 packages, and every one of them is a TEST file:
    packages/plugins/plugin-security/src/*.test.ts            (8 files)
    packages/runtime/src/domains/share-links-enforcement-context.test.ts  (1 file)

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 bf036b4b09 with a clean tree — derived with node 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:

optional-error-sink 0 · nul-bytes 0 · adr-0087-registration 0 · changeset-gate-self-tests 0
empty-changeset 0 · changeset-no-major 0 · objectui-changeset 0 · release-rehearsal --self-test 0
engine-double-contract 0 · where-matcher 0 · cross-package-test-inputs 0 · query-options-erasure 0
slot-lookup 0 · i18n 0 · agent-test-spelling 0 · entry-guard 0 · parse-guard 0
pnpm-filter-targets 0 · published-files 0 · test-source-alias 0 · type-source-resolution 0
plugin-teardown-shape 0 · docs-audit/check-affected-docs 0

check:query-options-erasure and check:slot-lookup caught a real defect on their first run: **/*.test.ts written 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:

  1. Population from ESLint's own config, not my guess: fed all 9 changed paths; ESLint's config itself excluded the two .md and the .json as ignored, leaving the 6 .ts files it accepts.
  2. Count from --format json: 9 results returned, 6 linted, 0 errors, 3 warnings — all three the "file ignored" notices for the paths in (1). Exit 0.
  3. Invariance for untouched files: this repo runs one eslint.config.mjs which never enables type-aware linting for any file (no parserOptions.project, no typed @typescript-eslint rules) — stated and measured with a positive control at eslint.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.

⚠️ Not run locally, stated rather than implied — one item. pnpm check:type-check-debt (the --re-measure ratchet) is NOT MEASURED. Its first attempt refused correctly, for a reason worth recording: the (a) ablation had rewritten security-plugin.ts, so that package's dist/*.d.ts was older than its sources and the gate refused rather than measure a ledger number through a dist describing 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. Four os-verify-lock acquisitions across this task returned VERDICT queue-timeout (exit 99) · never acquired; ~20 minutes queued with no turn. exit 99 and an unexplained 143 are 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.

⚠️ One declared narrowing. The @objectstack/runtime typecheck + 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 lint repo 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's packages/** scan. packages/spec has zero ownership in this lane and is not edited. No call-site spelling was changed — the logger?.warn?.(…) backstop stays, as bulk-recompute.ts documents for hosts the type cannot reach.

⛔ Draft on purpose. needs:contract-review is not cleared here — that is the contract-review tier's to clear. Part of rather than Fixes: 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 wrong Fixes would silently close a card still under contract review.


Generated by Claude Code

…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
@github-actions

github-actions Bot commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-security, @objectstack/plugin-sharing, @objectstack/service-settings, touching 6 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-settings/src/settings-service.types.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/field-level-security.mdx (via SecurityPlugin (symbol))
  • content/docs/permissions/index.mdx (via SecurityPlugin (symbol))
  • content/docs/plugins/packages.mdx (via SecurityPlugin (symbol))
  • content/docs/ui/forms.mdx (via SecurityPlugin (symbol))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via SecurityPlugin (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-settings/src/settings-service.types.ts) — pages documenting those are invisible to this run
  • 3 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 21 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 589758d22ccacf9cc56b5bc8a9f9766cb7e2a93a → packageMentionDocs.

Which tree this was computed on

This run read content/docs from cf7406b0a3903719d2148898d51bee51a94692e2 — the merge of head 040932a13130bfbbf9b4d708dc2d7f5d401c3758 into base 589758d22ccacf9cc56b5bc8a9f9766cb7e2a93a, 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 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

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 589758d22ccacf9cc56b5bc8a9f9766cb7e2a93a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… `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
Merged via the queue into main with commit 18b53ac Aug 24, 2026
32 checks passed
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 1, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants