Uh oh!
There was an error while loading. Please reload this page.
perf(core): batch resolveUserAuthzGrants' independent reads — 8 sequential legs become 4 (#10825) - #10981
perf(core): batch resolveUserAuthzGrants' independent reads — 8 sequential legs become 4 (#10825)#10981os-zhuang wants to merge 2 commits into
Conversation
📓 Docs Drift CheckThis PR changes 1 package(s): 9 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 6 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 23 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 fb669a832bff321fd15ab60064907cdb313004f0 && git checkout fb669a832bff321fd15ab60064907cdb313004f0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 28ad84af26ff3cac00cfd10d9868f8d3a45780f3 b3f2c26eb813aba71b71af4e60f803f5051ce3c8 && git checkout -B drift-repro 28ad84af26ff3cac00cfd10d9868f8d3a45780f3 && git merge --no-ff b3f2c26eb813aba71b71af4e60f803f5051ce3c8
node scripts/docs-audit/affected-docs.mjs --json 28ad84af26ff3cac00cfd10d9868f8d3a45780f3
|
Refs #10757
Fixes#10825
resolveUserAuthzGrantsis legs 6–13 of every authenticated request. It read eight tables one after another, eachawaitblocking the next, although only three of the seven edges between them are real data dependencies.cloud#1539 established causally (latency injection, R² = 0.9994) that server time follows
≈ 33 + L × 36.6ms whereLis the count of sequential legs, not of queries — "L, not N, is the multiplier. Batching is worth exactly as much as deleting." So this card's win is measured in legs, and the query count is deliberately left alone.What changed
The reads that depend on nothing are now issued in one wave. One genuine chain remains —
sys_positionneeds the position names wave 1 produces,sys_position_permission_setkeys on the position idssys_positionproduces, andsys_permission_setneeds the union of directly- and position-granted set ids:Fewer than four when the principal has less to resolve: 3 with no active
sys_positionrow backing any position name, 2 with no permission sets at all. Per-fixture leg counts are written out inBATCHED_LEGSin the test file rather than derived, so a regression that re-serialises one read fails as a number instead of as a slower suite.Four is the floor, not the card's hoped-for two or three. Collapsing waves 2→3 needs
sys_position_permission_setfiltered by position name, i.e. a join or a relationship traversal.resolveUserAuthzGrantstakesql: anyand is called with several different engines and test doubles; a traversal one of them silently mis-handles returns the wrong rows on the authorization path with nothing going red. That trade is not worth one leg, and it would need a contract this card is fenced out of.No caching
Nothing survives a request. Every read is live, so a grant revoked at T is not honoured at T+1; there is no TTL, no invalidation contract and no staleness window. That is why this is separable from #10757's caching tranche and why it can ship on its own review.
Legs 9 and 13 are batched, not deleted
Both duplicate a read made earlier in the request. PR #10824 measured that the removable half of each pair belongs to directions 1 and 2, so removing them here would be claiming another card's work. They are in wave 1, still issued, still live.
How the leg count was established
Three independent methods, because the query count is not the leg count and cannot be converted into one:
Dms, so wall clock /Dis the leg count regardless of how many queries a leg contains. AtD = 50: 412 ms → 206 ms (8 legs → 4). AtD = 25: 230 ms → 106 ms. Both runs resolve an identical grants envelope.Promise.allof any width counts one. Asserted per fixture across the 11-shape matrix.awaitpoints on the longest path.better-sqlite3is synchronous and in-process, so there is no round trip to save. That is exactly why cloud#1539 measured by injection rather than by reading a trace. What the live rig is used for below is equivalence, not speed.Equivalence — proven, not asserted
On an authorization path a batch that returns even one different row is a privilege bug that a green suite cannot see. So the deliverable here is the differential control, not the passing suite.
packages/core/src/security/resolve-authz-context.batch-equivalence.test.ts— every expectation was captured from the sequential implementation (git show 38bc74ed1:…/resolve-authz-context.ts) running against the same fixtures, then asserted against the batched one. Two independent goldens per fixture:positions,permissions,systemPermissions,org_user_ids,accessible_org_ids,tabPermissions,posture,email.{object, where, limit}triples issued, each withcontext.isSystem === true, plus an equality assertion on the query count. This is the "same filters, same tenancy scoping, same limits" half: widening an$in, dropping a tenancy filter or merging two reads changes this list even when the envelope happens to agree. Multiset rather than sequence, because parallelising is a change of issue order — what must not change is which reads happen and with what.The engine double enforces the
limitthe caller passes, like a real driver, so a changed limit is observable rather than theoretical.Principal shapes covered
empty-principal·multi-org-membership·lapsed-own-membership-among-active-peers·position-derived-grants·permission-set-derived-grants·tenant-admin-via-position·ai-seat-and-email-from-sys-user·ai-seat-denied·seeded-permissions-and-email·read-limits-truncate·no-active-org— spanning MEMBER / TENANT_ADMIN / PLATFORM_ADMIN, ADR-0091 validity windows (lapsed, not-yet-valid,untilexclusive), ADR-0049 deactivated positions and permission sets, org-scoped vs unscoped grants, and tab-permission merging.The divergence case
The card asked for a case where the two could plausibly diverge. It is
lapsed-own-membership-among-active-peers, and it is the reason the twosys_memberreads are not merged.sys_member {user_id}andsys_member {organization_id}now read the same table in the same wave, so the obvious next "improvement" is one$orread partitioned in memory. On this fixture that is a silent privilege escalation: the caller's own membership inorg_ahas lapsed while peers hold activeowner/adminrows in it. A merged read feeds those peer rows to theaccessible_org_idsloop (grantingorg_a— the entire read reach of thegroupposture) and to the org-role loop (grantingorg_owner, and with it TENANT_ADMIN). The golden pins the sequential answer:accessible_org_ids: [], noorg_owner, postureMEMBER, with the peers still present inorg_user_idsbecause that is what the fellow-org read is for.read-limits-truncatecovers the other named hazard — 205 own memberships against the 200 limit and 1005 peers against the 1000 limit, with truncation observable.Ablation — the controls can fail
Each leg mutated the batch, proved the mutation on disk by counting both the injected marker and the deleted text (never an editor's exit code), ran, restored, and proved restoration by
git hash-object==git rev-parse HEAD:PATH,git diff --exit-code= 0 and empty porcelain. The script carriestrap … EXIT INT TERMso a mid-mutation kill cannot leave a mutated tree behind. No rebuild is involved: the test imports./resolve-authz-context.jsrelative, inside its own package, so vitest resolves it to source — which the red results themselves demonstrate, since a dist-resolved test would have stayed green.sys_permission_set$inwidenedsys_member {user_id}limit 200 → 1000Leg D is the pair that makes both controls meaningful: re-serialising leaves both equivalence goldens green on all 11 fixtures and reddens only the leg count. The equivalence controls are therefore measuring rows rather than scheduling, and the leg control is measuring scheduling rather than rows.
] = await Promise.all([, which also occurs inresolveLocalizationContextUncachedin the same file, so the count read 2→1 instead of →0. The guard treated that as "did not land", restored, and reported it; the leg was re-run with a unique anchor (orgMembers, upsRowsAll] = await Promise.all([) and the numbers above are from that run. Reporting it rather than quietly re-running to a clean number is the point of counting both strings.Live-rig equivalence, against the real SQL driver
pnpm dev:crm --freshwithDEBUG=knex:query, twice — once withpackages/corebuilt from38bc74ed1(sequential), once from this branch — withpackages/core/dist/index.jsverified on disk each time to carry the intended shape:GET /api/v1/auth/me/permissions, 12,169 bytes, ids normalised)Server-Timing: db;desc="N queries")X-OS-Debug-Timing: json)That exercises the real ObjectQL engine and the real SQL driver, so the
$intranslations and tenancy predicates are checked as compiled SQL rather than only against a double.Expected non-effects, named before the runs
sys_userjoins wave 1 only when it will actually be consulted, so a caller that supplied both an email and theai_seatscope still causes no read at all (seeded-permissions-and-emailpins this).Verification
Gate union run after the final commit, on
b3f2c26eb. Gate list derived bynode scripts/pm/dispatch-gates.mjswith no path arguments; exits captured before any pipe.check:authz-resolvercheck:changeset-gate-self-testscheck:cross-package-test-inputs(+scripts/check-cross-package-test-inputs.mjs)check:kernel-hook-pairscheck:slot-lookupcheck:test-source-aliascheck:type-source-resolutioncheck:query-options-erasurecheck:type-check-coverage·check:type-check-debtcheck:engine-double-contractcheck:where-matchercheck:nul-bytesscripts/check-adr-0087-registration.mjs·check-changeset-no-major.mjs·check-ci-filter-parity.mjs·check-empty-changeset.mjs·check-plugin-teardown-shape.mjs·docs-audit/check-affected-docs.mjspnpm lint(full repo scan,eslint . --no-inline-config)check:objectui-changesetcheck:objectui-changesetcannot run on this macOS host. All 7 of its self-test failures arescripts/bump-objectui.sh: line 324: mapfile: command not found/status=127;mapfileis a bash ≥ 4 builtin and this host's shell isGNU bash 3.2.57.scripts/bump-objectui.shis byte-identical toorigin/main(git diff origin/main...HEAD -- scripts/bump-objectui.shis empty) and this diff touches no objectui pin. CI runs it on ubuntu/bash 5.Tests:
@objectstack/core@objectstack/plugin-hono-server@objectstack/plugin-security@objectstack/plugin-sharing@objectstack/service-automation@objectstack/runtime@objectstack/restThose seven are the direct consumers of
resolveAuthzContext/resolveUserAuthzGrants, found by grepping the call sites; the rest of the farm is CI's run. Consumer direction is downstream (...@objectstack/core).@objectstack/restfirst showed one failure —import-integration.test.ts > parses a native xlsx workbook server-side,Test timed out in 5000mson a coldexceljsdynamic import while six suites ran concurrently on one machine. Re-run alone: 31/31 green. It touches no authorization code.@objectstack/coredeclares notypecheckscript (it is ledger-covered), so type resolution is verified through itstsupDTS build, which succeeded, and throughcheck:type-check-debt --re-measureon the built closure.Out-of-scope finding
Filed as #10982, unassigned, and not addressed here:
sys_member's org-role projection intopositionsskips the ADR-0091 validity window that theaccessible_org_idsderivation from the same rows applies, so a lapsed membership would keep conferring theTENANT_ADMINrung while granting no org access. Latent today —sys_memberdeclares no validity columns — but the comment beside the honouring half promises it will "correct the moment they do", which holds for that half only. It changes authorization semantics, so it wants its own review rather than a rider on a batching PR. Thelapsed-own-membership-among-active-peersgolden added here pins the current behaviour either way.Generated by Claude Code