Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-security): bind the report sink above start()'s two bail-outs (#10706) - #11055
Conversation
…uts (#10706) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 14 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 32578127586 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
跨 PR 相同签名（24h，按失败测试文件聚合）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#10706
SecurityPlugin.start()bound its report sink below the two bail-outs, so a degraded boot leftthis.loggerat its= {}construction default for the lifetime of the instance. This moves the binding above both.The mechanic
private logger … = {}(:776) is an empty object from construction, andthis.logger = ctx.loggerwas its only assignment — sitting in the "capture handles" block at:890, below the tworeturns at:878(ObjectQL/metadata unresolvable) and:883(engine withoutregisterMiddleware). Every report site is writtenthis.logger.warn?.(…), so an unbound sink is not a state any caller can notice: the reports simply do not happen.The assignment now runs immediately after the
Starting Security Plugin...line. Independent of #10556's open design call on what the default sink should be — only the placement changes, the= {}default is untouched, and the fix is correct under every option there.Step 1 — the card's one undemonstrated row: REFUTED, on two independent grounds
The card flagged
checkAuthoredRowWriteas possibly live and asked the next seat to settle it. Verdict: latent, not live.(a) No caller reaches it on the instance. Positive control first — the instrument names the known path before its silence is read as evidence: the sweep finds the service literal delegation (
:1149), the registration (:1235), and every service-mediated consumer. Against that working instrument, the instance-level reaches are:plugin-sharing/sharing-service.ts:841probe = this.securityService?.()→ctx.getService('security')service-analytics/plugin.ts:386(getReadFilter)ctx.getService('security')security-plugin.ts:5564this.checkAuthoredRowWriteinsideprivate assertControlledByParentWriteplugin-dev/dev-plugin.ts:759childPlugins: Plugin[], driven only viainit/start/destroycontrolled-by-parent-master-widener.test.ts:503,515vi.spyOn(h.plugin as any, …)registerService('security', …)is at:1235, insidestart()and below the bail-outs — so on a bailed-out boot the service is never registered and every one of those consumers getsundefinedand fails closed toabstain.(b) Even a hypothetical direct instance caller cannot reach the report site.
checkAuthoredRowWriteguardsif (!this.ql) return 'abstain'before its onlythis.logger.warn?.()at:3647.this.qlwas assigned in the same straight-line block as the logger, with no branch between — sothis.qlbound ⟹ logger bound. On exactly the boots where the sink is empty, the method short-circuits before reporting.The defect is still real on its own terms: a sink that can never be bound after an early return is unrepresentable as a state the code can notice.
The card corrected itself from 6 to 11
this.logger.*sites. Re-derived by enclosing scope with the TypeScript AST — at24ba050f3, the card's own revision, where:722/:836/:812–3084/:3501all match it exactly — the true count is 18. All 11 the card listed are real; it omitted 7, including three ingetReadFilter(:3701,:3734,:3765) — a public method in the same structural category ascheckAuthoredRowWrite.That omission matters, because
getReadFilterhas nothis.qlguard. Its on-behalf-of refusal (:3735) depends on none of the handles captured below the bail-outs, so it is reachable on a bailed-out instance — it is caller-gated only, not doubly unreachable.getReadFilter, notcheckAuthoredRowWrite, was the structurally-live path. Still latent (all callers are service-mediated), but one ground instead of two.A plain
grep -n 'this\.logger\.'finds 17 of the 18 onmain::5673is writtenthis.logger?.warn?.(— optional chain on the field itself — and nothis\.logger\.pattern can see it. Reported as a derived count, not a grep hit count.What is pinned —
start-logger-binding.test.ts, 7 teststhis.loggerisctx.logger(identity), and the sink RECEIVES a report — driven throughgetReadFilter's on-behalf-of refusal, the one site reachable on a bailed-out instance. Asserting "non-empty" would assert nothing:{}and a real logger both satisfytypeof x === 'object'.registerMiddlewarenot called,'security'not among the registered names. A "fix" that deleted a bail-out would pass a logger-only suite while changing boot behaviour; these assertions refuse it.security middleware not registeredthroughctx.logger.'security'registered, neither bail-out taken.Ablation — signature predicted before mutating
Prediction written to file before the mutation, then the fix hunk reverted in place:
2 failed | 5 passed (7)2 failed | 5 passed (7)expect(sinkOf(plugin)).toBe(ctx.logger), both postures:117, both postures{}received where ctx.logger expectedAssertionError: expected {} to be { info: [Function Mock], …(2) }probeSinkReceivesnever executes in the mutated leg — the identity assertion throws first. That is why both halves are asserted.src/vsdist/, argued from the files and then falsified. The subject is imported by the relative specifier./security-plugin.js;vitest.config.tscarries only two anchored aliases (/^@objectstack\/driver-sql$/,/^@objectstack\/objectql$/), neither of which can match a relative path — so the subject is transformed fromsrc/. The falsifiable consequence: the ablation mutatedsrc/and ran no build, and it reddened. Had the suite been readingdist/, an unbuiltsrcmutation would have stayed green.Restore.
git hash-objectbefore ablation and after restore both3a2ed20f24a1da854708d6d9c3e0b1adea44698a— byte-identical. The restore leg was then re-run to a real verdict rather than trusted on the hash:Test Files 1 passed (1) / Tests 7 passed (7).Verification — all on the final commit
518ef0855, clean treeDependency closure built first; every exit code captured before any pipe (
cmd > file 2>&1; EXIT=$?), and each gate quoted by its own verdict line.pnpm --filter @objectstack/plugin-security test—Test Files 70 passed (70) / Tests 1355 passed (1355)pnpm --filter @objectstack/plugin-security typecheck— exit 0,> tsc --noEmitechoed (the package does ship the script; a filter matching none would exit 0 having run nothing)Gate union from
node scripts/pm/dispatch-gates.mjswith no path arguments — 3 paths vs merge base2866d5f97, 136 families discovered:check:changeset-gate-self-testscheck:cross-package-test-inputsOK: 13 package(s) read outside themselves, all declaredcheck:objectui-changesetcheck:slot-lookupcheck:test-source-aliasOK — 72 packages with tests scannedcheck:type-source-resolutionOK — 77 packages with a tsconfig.json scannedcheck-adr-0087-registration.mjscheck-changeset-no-major.mjsmajorbumpcheck-ci-filter-parity.mjsOK: all 83 declared cross-package glob(s)coveredcheck-cross-package-test-inputs.mjsOK: 13 package(s), turbo.json hashes every globcheck-empty-changeset.mjscheck-plugin-teardown-shape.mjscheck-affected-docs.mjsConvention-triggered (new test file · i18n-owning package):
check:query-options-erasurecheck:engine-double-contractOK — 377 pinned, 133 in the DEBT ledger, 2 exemptcheck:where-matchercheck:type-check-coverageOK — 65/78 workspace packages type-checkedcheck:type-check-debt--re-measure: OK — 33 ledger entr(ies) re-measured in 266.9s … none above its recorded numbercheck:i18nOK (9 package(s) — all bundles in sync)check:nul-bytesOK (scanned 6391 text file(s) … no raw ASCII control bytes)check:i18nfirst returnedPREREQUISITE NOT MET — the workspace CLI is not built … Nothing was checked. That is not measured, never a pass — the row above is the re-run afterturbo run buildover the workspace closure, which is also what letcheck:type-check-debt --re-measurerun its ratchet instead of refusing.Class #10309 (dispatch lists short by the five
.changeset/**-triggered families, because the changeset does not exist when the PM derives): my re-derivation ran after the changeset was committed and did name all five —check:changeset-gate-self-tests,check:objectui-changeset,check-adr-0087-registration,check-changeset-no-major,check-empty-changeset. All green.Not in scope
= {}default itself — untouched.this.rlsCompiler.setLogger?.(ctx.logger)sits on the line the binding vacated and is still below both bail-outs, so the RLS compiler's ADR-0056 D4 dropped-policy warnings have no sink on a degraded boot. Same class, adjacent line, deliberately not moved: it is a different object's sink and its correct shape is not pinned by existing evidence. Filed separately rather than ridden along.Generated by Claude Code