Uh oh!
There was an error while loading. Please reload this page.
perf(plugin-security): batch the identity boot seeds' existence read and skip no-op writes - #11116
Conversation
…and skip no-op writes Every declared permission set and every declared position cost 4 sequential DB round trips on every kernel boot, 2 of them an UPDATE that fired when nothing had changed. Hoist ONE $in existence read out of each loop and write only when the stored row actually differs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
…ispatch predicate check:engine-double-contract and check:where-matcher both reddened on the new counting double: its update() did not route through assertEngineUpdateDispatch, and its WHERE matcher read a combinator as a field name. Fixed in the double — the shrink-only baseline is untouched; only the pinned (tightening) ledger grew. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
… stays at 11 check:type-check-debt --re-measure went 11 -> 12: the upgrade fixture widens a grant literal, which the inferred type rejects (TS2353). Fixed by declaring the fixture's type — the ledger entry is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
📓 Docs Drift CheckThis PR changes 1 package(s): 6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 5 release-owned page(s) also name something this change touched. These are read-only:
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): 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 340de11ee4d6bc5633e6e42d862eaee08bbbe723 && git checkout 340de11ee4d6bc5633e6e42d862eaee08bbbe723
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 13c0b166b8ed498ce7d6db7fd5ae70f1761e848c 4568d6fff2c3cf50d627dc94fa14fc4afe8b1822 && git checkout -B drift-repro 13c0b166b8ed498ce7d6db7fd5ae70f1761e848c && git merge --no-ff 4568d6fff2c3cf50d627dc94fa14fc4afe8b1822
node scripts/docs-audit/affected-docs.mjs --json 13c0b166b8ed498ce7d6db7fd5ae70f1761e848c
|
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32590087822 红了。队列跑的是全量套件(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.
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32590211119 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Fixes#10946
Both halves of the card are discharged: the per-item existence read is hoisted out of both loops, and the unconditional
UPDATEnow fires only when the stored row actually differs.What was costing what
Every declared permission set and every declared position cost 4 sequential database round trips on every kernel boot — 2 existence
SELECTs, 1UPDATE, 1SELECT— and theUPDATEfired whether or not anything had changed.bootstrapDeclaredPermissionsawaitedupsertPackagePermissionSetonce per set inside aforloop; the position binder had the same shape. Invisible on a local file database; one sequential HTTP request per leg on a remote libsql/Turso database, i.e. on every hosted environment.Round trips: measured here, and measured where
Reproduced in this PR (a COUNT, which needs no hosted rig) — at the OBJECTQL FACADE layer.
packages/plugins/plugin-security/src/bootstrap-seed-round-trips.test.tscounts everyfind/insert/updatethe seeders issue against a call-counting ObjectQL double. Both columns below were produced by running the same counter twice: once with the four touched source files reverted toorigin/main, once at this branch's head. Steady-state rebuild — rows already seeded, nothing to change:Identical figures for both loops. Measured slope: 2.0 facade calls per declared item, going to 0 — the rebuild cost stops growing with N. The "after" column is what the committed suite asserts. The count is asserted, never the wall time.
findplus oneupdateper item); the card's counter sits at@libsql/client, where each facade call expands into more than one statement — its own histogram shows the per-itemUPDATEarriving with aSELECTbeside it. The two layers agree at a 2:1 expansion. The driver-level figure is the card's measurement, not mine, and I could not re-take it: the rig isscripts/dev-local/bootstrap-curve.mjsinobjectstack-ai/cloud, outside this session's access.Inherited from the card, NOT reproduced. The latency figure — the whole
bootstrapstep growing 171.7 ms per ms of injected RTT, R² = 0.998 — and the driver-level curve fit (slope exactly 4.0000, R² = 1.000000) both come from that hosted rig. Nothing in this PR re-ran it, and nothing here measures wall time.The card's negative control still holds, by reading rather than by re-measurement. Objects/views/artifact seeds add 0.00 round trips each because schema sync is already batched behind
TursoDriver.supports.batchSchemaSync. I confirmed the code path is unchanged by this PR — nothing here touches schema sync — but I did not re-run the 0-to-400-objects leg, so "still 0.00" is inherited too. Identity content was the one content axis not batched; after this PR the capability seeder is the one that remains (filed as #11096).Half 2 changes WHEN writes happen — what I established first
The card flagged this as the risky half. What I found:
security-plugin.ts:2840, the ADR-0086 P2 publish materializer, computedapplied = r.seeded + r.updatedand used it for two things: the publish success/failure report, and whetherreconcileAudienceBindingSuggestionsruns. It asks "did the record end up matching the published body", which was accidentally the same number as "was a write issued" only because the seeder always wrote. Left alone, a re-publish of an identical body would have reported success: false. Fixed by teaching the consumer the new counter:applied = r.seeded + r.updated + r.unchanged. The three refusal branches (foreign package, env-authored name, no owning package) all leaveunchangedat 0, so every case that reported 0 before still reports 0 — behaviour preserved case by case, not approximately.plugin-audit'swriteAuditdiffs before/after and returns before writing any row when the diff is empty (audit-writers.ts:1374-1375), andupdated_at/updated_by/created_at/created_byareNOISE_FIELDSdropped from that diff. So a no-op bootUPDATEwrites nosys_audit_logorsys_activityrow today — nothing is lost by not issuing it. Both objects declareenable.trackHistory: true, which is what made this worth checking.updated_atbumping is not a dependency either. It is displayed in threesys_positionlist views and is the optimistic-concurrency token (expectedVersionmatches the currentupdated_at). Boot no longer bumping it makes the displayed "Updated" reflect the last real change instead of the last restart, and makes client version tokens survive a restart. Both are improvements, not losses.data.record.updatedrealtime event reachesservice-knowledgeonly for explicitly configured knowledge sources (sourcesForObject), and nothing in this repo declares one oversys_permission_setorsys_position. An environment that configured one would today re-upsert an identical document on every boot.One observation I am NOT deciding, recorded for the maintainer rather than acted on: the seeders write with
context: { isSystem: true }but do not setskipAutomations, so an environment-authored automation bound tosys_position/sys_permission_setupdate fires today once per item per boot with an empty diff. Nothing in the tree declares one, and nothing can depend on it, so this PR does not change that flag.The four pinned directions
UPDATE. For permission sets the comparison isrecordDiffersFromBody, the same predicate the ADR-0094 boot reconciler already trusts, over exactly the columnspermissionSetRowFieldswrites.nullis not "not mine" (the approvals: adepartmentapprover never resolves when the business unit hasorganization_id = null(every seeded BU) #3807 class) — the newseed-name-lookup.tsjudges the seam on whether the driver returned a result set, never on whether the array came back empty. A thrown read, or a response that is neither an array nor a records-wrapper, is "could not answer";[]is the answer "none exist", and the first-boot path depends on that answer being trusted.Point 4 is stricter than the code it replaces, deliberately. The old per-item shape turned a failed read into an insert attempt and leaned on the
nameunique index to refuse it — a database constraint standing in for a decision the seeder should have been making. Writing the test for this is what exposed it: my first implementation degraded to per-item reads and, against a double with no unique index, happily re-created all four rows. The lookup now reports three outcomes rather than two, and a name whose record cannot be read is declined and counted (unreadable) rather than inserted.Proof
Ablation A — signature predicted in writing before mutating (remove the equality guard, restore the unconditional
UPDATE, keep the batched read):Direction fully correct; breadth under-called — I predicted one drift failure and got three more counter assertions red ("hand-edit healed back", "absent into present"), because the ablation moves
unchangedeverywhere it is asserted. Reported as observed, not as predicted.The informative asymmetry: under ablation A the pre-existing suite (
bootstrap-declared-permissions.test.ts,bootstrap-declared-positions.test.ts) stayed 100% green. The old suite could not see this defect at all.Ablation B — the load-bearing leg (skip every write: the "beautiful curve, reconciles nothing" implementation): round-trip tests GREEN, drift tests RED, and two pre-existing tests red as well. A vacuous pass is impossible from two independent directions.
Restore, proved and re-run. Every restore was byte-identical by
git hash-object—bootstrap-declared-permissions.tsat3ad017323f162701ed9d4a56baf0b20330db5980before and after each leg, and the measurement revert restored all four touched files to their committed hashes (3ad01732…,51ec4c4b…,5ea4e4e9…,60f48959…). Each restore leg was re-run to a real verdict: 71 files / 1371 tests passed, three times.src vs dist, in the falsifiable form.
dist/was built from the implementation commit, then asrc-only mutation carrying the markerOS_ABLATION_MARKER_10946flipped the verdict (4 tests red) whiledist/bytes stayed identical (md5 of all 6 files, diffed clean across both ablations) and the marker had 0 occurrences in everydistfile. So the suite resolves throughsrc/. Separately,ablation-dist-preflight.mjsconfirms the shipped predicate did reachdist/(2 built files) for the consumers that read the artifact.Reverse verification of the cross-package type change.
PermissionSeedOutcomegained two required fields; a probe constructing the pre-change literal was rejected withTS2739 ... is missing the following properties from type 'PermissionSeedOutcome': unchanged, unreadable, proving the rebuilt declarations were being read rather than a cache. Probe removed, tree clean.Downstream sweep, direction stated.
pnpm --filter '...@objectstack/plugin-security'— the prefix form, i.e. dependents — is 26 packages. 23 of them ship atypecheckscript; all 23 ran (script name echoed 23 times, "Done" 23 times) and all 23 passed with 0 errors. The other 3 (@objectstack/hono,@objectstack/cloud-connection,@objectstack/service-automation) ship notypecheckscript at all — verified by reading eachpackage.json, not assumed. Their dependency closure had to be built first with the both-directions filter; the dependents-only filter produced cascadingTS2307s from an unbuilt@objectstack/service-datasource, which are not findings.Zero-hit counter-check, positive control run first. The control-character scan was first run against a file that does contain one (a bell byte, reported at line 2), proving the pattern fires, and only then against all 10 changed files: no hits.
check:nul-bytesagrees over 6417 files.Gates
Union derived on the final commit
4568d6fff, clean tree, withnode scripts/pm/dispatch-gates.mjsand no path arguments (the script reads the change set from the merge base itself). Exit codes captured before any pipe. Re-deriving after the second commit added two families the first derivation could not name —check:entry-guardandcheck:parse-guard, pulled in by the pinned-ledger path — which is exactly why the derivation is re-run on the final committed diff.Each gate below is quoted by its own verdict line, never a bare shell status:
check:changeset-gate-self-tests— 118 + 212 + 116 assertions over real temp git reposcheck:cross-package-test-inputs/check-cross-package-test-inputs.mjs— "OK: 13 package(s) read outside themselves, all declared"check:entry-guard— "136 scripts/ file(s) ... every entry guard goes through invoked-as.mjs"check:objectui-changeset— passcheck:parse-guard— "135 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs"check:slot-lookup— "ratchet holds: 107 unswept site(s) in 25 file(s), none new"check:test-source-alias— "OK — 72 packages with tests scanned"check: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 nomajorbump"check-ci-filter-parity— "OK: all 83 declared cross-package glob(s)"check-empty-changeset— "No empty-frontmatter changeset introduced by this diff"check-plugin-teardown-shape— "63 Plugin implementation(s) ... baseline fully burned down"check-affected-docs— passcheck:query-options-erasure— "ratchet holds: 67 unswept non-test site(s), none new"check:i18n— "OK (9 package(s) — all bundles in sync)"check:type-check-coverage— "OK — 65/78 workspace packages type-checked"check:nul-bytes— "OK (scanned 6417 text file(s) ... no raw ASCII control bytes)"check:optional-error-sink— "every sink declaring an optionalerrorguarantees awarnchannel" (this one can never be named by path derivation; run because the card said so)check:engine-double-contract— reddened first, see below — now "OK — 378 pinned, 133 in the DEBT ledger, 2 exempt"check:where-matcher— reddened first, see below — now "279 matcher(s) discovered, 279 answer the combinator battery correctly or refuse it loudly (168 refuse)"check:type-check-debt --re-measure— reddened first, see below — now "OK — 33 ledger entr(ies) re-measured in 338.2s, 1908 raw tsc error(s) total, none above its recorded number". Run against a fully built workspace closure (70/70 tasks), so this is a measurement, not a refusal.The three that reddened, and how each was repaired
All three were caused by the new counting double, and all three were repaired in the code:
check:engine-double-contract— the double'supdate()did not route throughassertEngineUpdateDispatch. Repaired by routing it through the real dispatch predicate from@objectstack/metadata-core. The follow-up "RETAINED" verdict then asked for the pinned ledger to learn about the new coverage;--writeadded exactly one row there. ⛔ The shrink-onlyengine-double-contract.baseline.jsonis untouched — verified bygit statuson that file specifically.check:where-matcher— the double's WHERE matcher read a combinator as a field name. Repaired by making the double refuse the combinators it does not implement, which is the convention most discovered matchers already follow. The baseline was not grown.check:type-check-debt --re-measure—@objectstack/plugin-securityTEST_DEBT drifted 11 to 12. The +1 was my own fixture: an inferred grant literal that the upgrade fixture widens (TS2353,allowEditdoes not exist in the inferred type). Repaired by declaring the fixture's type; re-measured back to exactly 11. ⛔ The ledger entry was not raised.Declared narrowing
The dogfood real-engine seeding tests were not run locally.
showcase-permission-seeding.dogfood.test.tsandshowcase-declarative-rbac-seeding.dogfood.test.tswere attempted and killed mid-boot by this session's 10-minute foreground ceiling, behind a contended shared verify lock — twice the budget went to waiting rather than running. CI runs the dogfood farm exactly once regardless. What stands in for it locally: the membership query shape this PR now issues is already in production on both tables —explain-engine.ts:389readssys_positionwith a name membership predicate, andsecurity-plugin.ts:997readssys_permission_setwith one — so the engine answering this shape on these two tables is an existing shipped fact, not a new assumption. Stated as a narrowing rather than left as a gap.Out of scope, filed not repaired here
bootstrapDeclaredCapabilitiesseedssys_capabilityone at a time too — the same per-item read + unconditional UPDATE #10946 removed from the two sibling loops #11096 —bootstrapDeclaredCapabilitiesseedssys_capabilitywith the identical per-item read plus unconditionalUPDATE. Same defect class, third table, and the capability count is the union of every package'ssystemPermissions, so it is typically larger than either axis measured here.customized, which is outside whatrecordDiffersFromBodycompares, so a naive skip would stop maintaining that flag.Neither is touched by this PR. Sibling cards in the same boot-cost campaign: #10945 (engine), #10979 (cli).
Generated by Claude Code