Uh oh!
There was an error while loading. Please reload this page.
perf(plugin-security): claim seed ownership with one predicate write per unowned shape (#14530) - #14718
Conversation
…per unowned shape Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…aim-seed-ownership-predicate-write
`check:where-matcher` flagged the new fixture matcher as silently wrong on a combinator query. `claimSeedOwnership` issues none, so the double refuses rather than implementing them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
📓 Docs Drift Check10 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. 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 0bd67657268b25d3274027be2c77dbfcea268a84 && git checkout 0bd67657268b25d3274027be2c77dbfcea268a84
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dbf115284295b1989d4648dbfbd7e5f3f96357dc 5808133df23e49fbc6e4b4c9369537fbc0af00c5 && git checkout -B drift-repro dbf115284295b1989d4648dbfbd7e5f3f96357dc && git merge --no-ff 5808133df23e49fbc6e4b4c9369537fbc0af00c5
node scripts/docs-audit/affected-docs.mjs --json dbf115284295b1989d4648dbfbd7e5f3f96357dc |
…aim-seed-ownership-predicate-write
os-sales
commented
Sep 2, 2026
CI red on |
…cts over the per-row hook ceiling are still claimed An unpaged predicate write is refused whole above MAX_BULK_PER_ROW_HOOK_ROWS (ADR-0058 D6), so a 21k-row object claimed nothing where the pre-#14530 loop claimed 10k. `owner_id` is a record-access field, so that is a permission outcome, not an observability one. The unit of work is now a page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…ema.name call site Paging behind an `objectName` parameter made `check:tenant-audit-census` read the write as `undecidable` and took its self-test to 5-of-19 red. Measured: the same file from 39126dc reproduces that on origin/main, and origin/main itself is green. The two engine calls are now bound where `schema.name` is a literal argument, so the census's answer about this file is byte-identical to its pre-change one -- no ledger row degraded to buy a green gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
os-sales
commented
Sep 2, 2026
Blocker record — this PR's own gates, and what is queued behind themPutting this on the PR because the seat's instructions to the dev agent went out over a channel that did not survive: the container running this session restarted at ~21:45Z and killed the agent working on this branch. Nothing was lost from the branch (its head Gate 1 — |
…aim-seed-ownership-predicate-write
…ership doubles `check:engine-double-contract` reported RETAINED [update] on `claim-seed-ownership.test.ts`: the paging pins grew its engine doubles from 1 to 5, which is the direction this ledger wants, so the gate's own prescribed fix is to regenerate. Regenerated with `--write` on the merged tree (the ledger is contended by sibling PRs), never hand-edited; the regeneration reports "1 added or grown, 0 lost". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… write first, page only on refusal
The changeset still described the always-paged shape ("read at most 5 000 ids,
re-own them, repeat"), which was measured 13x slower on the sizes every real
install has and is not what landed. Restated: one predicate write per unowned
shape, a page off the top only when the engine refuses that write for its
per-row hook budget, plus the re-measured over-ceiling number (21 000 of 21 000
claimed, 8 engine writes) that the paging exists to produce.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8…aim-seed-ownership-predicate-write
Uh oh!
There was an error while loading. Please reload this page.
Fixes#14530
Triage ruled disposition 2 (14530#issuecomment-5508899386): turn
claimSeedOwnership's single-id loop into a predicate write per object, take the missing latency measurement first, and confirm at step 3 that the cap / trailing-batch branch inrule-hooks.tsactually engages on the new write shape. All three steps are below, with numbers.Seat review 14530#issuecomment-5516113880 then ruled patch round 1 into this PR rather than deferring it: an over-ceiling predicate write is refused whole, which took an object above the ceiling from partially claimed to not claimed at all. That is fixed here, and measured.
plugin-sharingis untouched throughout — that the fix needs no change there is the whole argument for disposition 2.The change
claimSeedOwnershipscanned everyowner_id-declaring object twice atlimit: 10_000and then issued one single-idupdateper matched id — up to 20 000 full engine writes for one object. It now issues one predicate write per unowned shape, and pages that write only when the engine refuses it:Two writes per object on every install small enough for them, which in practice is all of them. The scans are gone with the loop: the predicates they resolved are the predicates the writes now carry, so the matched set is unchanged row for row, and the per-object count is the sum of the affected-row counts the writes themselves resolve (#4639) rather than a length this function counted.
The two predicates stay two narrow writes rather than one
OR/INfor the reason the scans were two — driver portability — and they stay disjoint in this order, because the NULL write landsadminUserId, which can never beusr_system(that target is refused at the top of the function).Step 1 — the latency the card said was missing
The card is explicit that there is no number. Here are two, before and after, on a real ObjectQL engine.
Method.
new ObjectQL()onInMemoryDriver(persistence: false), one objectcrm_leaddeclaringid/name/owner_id, N rows seeded straight through the driver (so setup pays no hook cost) alternatingowner_id: nullandowner_id: 'usr_system'. plugin-sharing's realbindRuleHooksis bound over a countingSharingRuleServicedouble with one active rule on that object, so every branch ofrule-hooks.tsthe write shape can reach is counted. The window isclaimSeedOwnershipitself,process.hrtime.bigint()either side, plus a drain ofruleRegrantQueue.engine.updateis wrapped to count the caller's writes. The "before" shape is the pre-change body inlined verbatim fromorigin/mainc616c2cc2; the "after" shape is imported from the built package.Baseline A —
origin/main(c616c2cc2), i.e. before#13533updatecallsevaluateAllForRecord6.7x / 12.4x / 23.8x, and the write count stops scaling with N.
evaluateAllForRecord: 0and one skip notice per object are the signature of the pre-#13533 world:bindRuleHooksstill opensafterInsert/afterUpdatewithif (ctx?.session?.isSystem) { noteSystemWriteSkipped(); return; }, so no sharing materialisation runs at all on this path yet.Baseline B — PR #14528 head (
e9b612a7a), i.e. after#13533Same harness, same object, run in a separate throwaway worktree at that PR's head with this branch's
claim-seed-ownership.tscopied in and rebuilt.updatecallsevaluateAllForRecordrevokeRuleGrantsForObjectevaluateAllRulesForObject8.4x and 24.3x.
skipNotices: 0is the discriminator that says these rows were measured in the post-#13533 world.Step 2 to Step 3 — the cap / trailing-batch branch DOES engage
This is the stop-and-report condition, and it passes — conditionally on #13533, which is the honest reading:
RULE_RECOMPUTE_ROW_CAP= 1 000): the bounded branch runs —evaluateAllForRecord = 800,revokeRuleGrantsForObject = 0. Same per-record grant work as before, from 2 engine writes instead of 800.readAffectedRowsreturnsover-cap,afterUpdatetakesrevokeThenQueueRegrant, and the counters readevaluateAllForRecord = 0,revokeRuleGrantsForObject = 2,evaluateAllRulesForObject = 2— one set-based revoke plus one queued full reconcile per predicate write. That is exactly the branch triage asked to see engaged, reached with no change toplugin-sharing.affected.kind === 'rows'with a single id, and the cap can never fire.revokeRuleGrantsForObject = 0at 5 000 rows confirms it.On today's
origin/mainthe branch is NOT reached — #13533's PR #14528 is still open, so theisSystemskip inafterUpdatereturns beforeaffectedFrom(ctx)is ever consulted. The write-count and latency win is real and complete today; the cap-branch half arrives with #13533 and is measured above at that PR's head.Patch round 1 — paging past the engine's per-row hook ceiling
A predicate write carries no
limit, so the bound is the engine's ownMAX_BULK_PER_ROW_HOOK_ROWS(10 000):beforeUpdate/afterUpdatehooks are contracted to fire per matched row on a predicate write (ADR-0058 D6), and every object carries such hooks in practice, so an over-sized write is refused whole — nothing written.owner_idis a record-access field, so "the object was not claimed" is a permission outcome, not an observability detail.The refusal is a declared, total verdict whose own message names pagination as the remedy, so it is answered: take one page of ids off the top (
CLAIM_PAGE_ROWS, half the ceiling) and re-attempt the whole set. Each page shrinks what is left until one write can carry it, and the pass ends on a whole-set write rather than on a count of pages.Measured on the same 21 000-row object, one shape per row:
limit: 10_000)Order is load-bearing, not cosmetic. Paging unconditionally measured 13x slower on the sizes every real install has: an
id IN (...)page is evaluated byInMemoryDriveras a linear scan of the id list PER ROW (memory-matcher.ts,target.includes(value)), so an always-paged claim is quadratic there where the natural predicate is linear — 5 000 rows: 528 ms whole-set versus 5 865 ms always-paged, same engine, same driver, same row set. The page is what the engine's refusal buys, not the default.CLAIM_PAGE_ROWSis derived from the ceiling rather than chosen (half of it), so it moves with the contract: the margin covers a driver's own bound-parameter limit on theid IN (...)list, and half the ceiling is still far above plugin-sharing's 1 000-row recompute cap, so a full page is still seen as a batch by the trailing-batch branch rather than recomputed row by row.Termination does not rest on the page counter: a page that re-owns rows makes them stop matching the predicate, so the remainder strictly shrinks.
MAX_CLAIM_PAGESis the belt for the one shape that reasoning does not cover — a driver reporting an affected count for rows it did not write — and hitting it is warned loudly, never silent. Three separate stop-and-warn paths cover an unreadable affected count, a page that matches rows but re-owns none, and a write that refuses for any code other than the declared per-row-hook budget.#14719 was filed as the home for this work if paging turned out not to reach it. It did reach it — the number above is 21 000 of 21 000 — so that card is now a PM-seat judgement on this measurement, and this PR does not touch it.
⛔ Raising or exempting
MAX_BULK_PER_ROW_HOOK_ROWSwas refused: it is imported from@objectstack/spec/dataand merely re-exported atengine.ts:7984, so moving it is apackages/speccontract change and thedomain:specseat's alone.Premise re-verification (the card and triage read
4a37870;mainhas moved)origin/mainc616c2cc2claim-seed-ownership.ts:121-128, the shape quoted verbatim in triageisSystemremoval landed?state: open,merged: false;rule-hooks.tsstill carries the skip instashAffectedRows,afterInsert,afterUpdate. Both worlds measured aboveRULE_RECOMPUTE_ROW_CAP = 1_000;revokeThenQueueRegrantreachesservice.evaluateAllRulesForObject(objectName)viaruleRegrantQueueP4, precisely.
bootstrap-platform-admin.ts:623(not:533) calls it, viasecurity-plugin.ts:3330.meta resyncreaches it indirectly, throughbootstrapPlatformAdmin(ql, sets, { resync: true })atresync.ts:192— that file names noclaimSeedOwnershipof its own. Andensure-default-organization.ts:406is not a call site of this function: it invokes an injectedoptions.claimSeedOwnershipwith a different signature(ql, organizationId, userId, options), supplied by the enterprise organizations package;auth-plugin.ts:1100passes{ logger }only, so on this repo that hook is never wired.Both remaining entries also short-circuit at
already_have_admin(bootstrap-platform-admin.ts:412) before the claim, so the loop is reachable only while the install has no platform admin. That narrows the exposure the card described — it does not change the fix.Tests
All figures below are from the final commit
5808133dfunless stated.pnpm --filter @objectstack/plugin-security test— Test Files 95 passed (95) · Tests 1795 passed (1795), exit 0.pnpm --filter @objectstack/plugin-security typecheck— exit 0, verdict linecheck:test-typecheck: OK — @objectstack/plugin-security's test layer compiles under packages/plugins/plugin-security/tsconfig.test.json;tsc -p tsconfig.test.json --listFilesconfirms both edited files are in the program (1 hit each), so that verdict really covers them.npx eslint . --no-inline-config(the repo-wide scan, not a narrowing) — exit 0, no output, 100 s.pnpm check:nul-bytes—check-nul-bytes: OK (scanned 8047 text file(s) -- 8047 tracked, 0 untracked-not-ignored; skipped 7 binary; no raw ASCII control bytes).Gate family re-derived on the final commit with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(44 commands; the derivation's stderr names this repo and commit5808133df, with no stale-tree warning) and run: 39 green, 5 NOT MEASURED — each reported as such by its own verdict line, never read as green or red:check-test-completeness— exit 3,PREREQUISITE NOT MET — this gate grades a saved 'turbo run test' log, and no log was named.check:dual-build-cjs-loads— exit 3,Run 'pnpm build' first. ⛔ This is NOT a pass: nothing was measured.check:type-check-debt— exit 3,check-type-check-coverage: PREREQUISITE NOT METcheck:i18n— exit 1,check-i18n-bundles: PREREQUISITE NOT MET — the workspace CLI is not builtscripts/pm/check-half-states.mjs— hit its 300 s bound having printed onlyre-exec with --use-env-proxy: HTTPS_PROXY is set and node's fetch does not read it; it is a PM tool that reaches GitHub, not a tree gate.CI runs all five after a full build.
The two gates that were red, and their final verdict lines
check:engine-double-contractwasx RETAINED [update]: claim-seed-ownership.test.ts now pins 5 engine double(s), ledger records 1.The gate's own prescribed fix is to ratchet, since coverage grew in the direction the ledger wants.scripts/engine-double-contract.pinned.jsonis contended by sibling PRs, soorigin/mainwas merged and committed first and the regeneration ran on the merged tree — never hand-edited. Regeneration reported1 added or grown, 0 lost; the diff is one field,pinned: 1topinned: 5. Final verdict line:check-tenant-audit-censusis green on both legs, and theself-test: 5 of 19half was diagnosed rather than assumed:The five failures reproduce locally at
39126dc02and at no other commit measured. They are one root cause, not five: all five are the live-tree family, which asserts the committed census artefacts still match the tree, and at39126dc02the tree had drifted (objectName | undecidablewhere the committed row saysschema.name, and the corpus-scale count 15 to 16). They were caused by this PR's diff at that commit and were already answered at0418ccf8fby binding the engine calls at theschema.namecall site, so the census's answer about this file is what it was before the paging fallback existed, with no ledger row degraded to buy a green gate.Measured, never inferred — the self-test at each commit:
39126dc02(this branch, before the call-site binding)0418ccf8f(this branch, after it)75adf11da(origin/mainat this branch's base)af44044a8(origin/main, mid-range)4d0d9445a(origin/main, current then)So it was never red on
origin/main, and it is not red now.node scripts/tenant-audit-census.mjs --writeon the merged tree rewrites only the explicitly non-enforced corpus-scale block and its measurement date (tracked non-test sources scanned 534 to 539), which moved becausemaingrew files — not because of this diff. That rewrite was reverted rather than committed: the gate is green with the committed values, and the block's own prose says those figures are required to be present and dated, and their values are not compared.Ablation — three legs, all red, all provably restored
The subject is imported relatively (
import { claimSeedOwnership } from './claim-seed-ownership.js'), so vitest compiles the source directly; nodist/sits in the resolution path, which is the conditionscripts/ablation-dist-preflight.mjsstates for its own hazard ("any test whose subject resolves through the dependency'sexports"). Each leg proves the mutation landed on disk before its colour is read — aglobalThismarker (never a comment, which esbuild strips), the removed anchor text counted to 0, and the blob hash moved off HEAD's — and proves the restore landed after — blob hash back to HEAD'sd5512db31c4b66c1b0080ec0227c6141ada7d486andgit diff HEAD -- PATHempty. The restore isgit checkout HEAD -- ABSOLUTE_PATH, never the bare form, and the driver carriestrap restore EXIT INT TERM.4319c4ab...off HEADreownreads ids and issues one by-idupdateeach2851cc3e...{ owner_id: SystemUserId.SYSTEM }dropped fromUNOWNED_PREDICATES08a8256b...Leg C is the one that judges patch round 1, and its red set is exactly the paging pins — nothing else moves:
Legs A and B are the earlier rounds' legs, re-run on the paged implementation to confirm they were not carried off by it. Both still redden the equivalence and shape pins:
The equivalence pin re-states the OLD rule (two scans at
limit: 10_000, deduped) rather than calling into the implementation under test, on a fixture that includes an absent column, a present-but-undefined, an empty string and ausr_system_adminprefix collision, and asserts the new write claims that id set exactly. The over-ceiling pin compares against the uncapped unowned set instead, because past 10 000 the old rule was itself lossy and the new one must beat it, not match it.Clause ②:
noRe-derived from the current diff on the final commit:
No new exported symbol.
grep exportcannot see a changed signature on an already-exported declaration, so that was checked separately:claimSeedOwnershipis the file's only export, and its signature is byte-identical toorigin/main's —(ql: any, adminUserId: string, options: ClaimOwnershipOptions = {}): Promise SUMMARY_ARRAY, where the summary element type{ object: string; count: number }is also unchanged.CLAIM_PAGE_ROWS,MAX_CLAIM_PAGES,UNOWNED_PREDICATES,ObjectWriter,claimPredicate,isPerRowHookBudgetRefusal,affectedRowCountandidsFromare all module-private. No carrier is owed.One accept-set note that is not an export: the
typeof ql.find !== 'function'precondition is retained, because the paged fallback reads.Not done, deliberately
Disposition 3 (a boot-phase predicate) is out of scope and not this seat's to authorise; the hooks are not made to coalesce across writes;
packages/specis not touched.packages/cli'srun-dev-unbuilt-workspace.e2e.test.tsis a known repo-wide flake (#14648 / #14727,domain:cliis on it). It is unrelated to this branch and is neither fixed nor skipped here.🤖 Generated with Claude Code
https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8