Skip to content

Commit 275ad42

Browse files
fix(organizations): claimOrgSeedOwnership claims by paged predicate writes until no owner-less row is left (#23071)
Fixes #23054 Clause-②: no — a runtime fix inside one function: no published accept set and no public signature changes (`OrgScopingEngine` is package-private). ## What changed `claimOrgSeedOwnership` (`packages/plugins/organizations/src/claim-org-seed-ownership.ts`) read each object's owner-less ids once, at `limit: 10_000`, and updated them one at a time. Nothing read again, and the ADR-0093 D7 once-gate never re-runs the claim, so rows past 10,000 stayed owner-less for good. It now claims by paged predicate writes, the shape the twin `claimSeedOwnership` took in #14530 and #14719: - **One predicate write per object:** `update(schema.name, { owner_id: owner }, { where: { organization_id: org, owner_id: null }, multi: true, context: CLAIM_WRITE_CTX })`. Below the engine's per-row hook ceiling, that is the whole claim, and it issues no read. - **A page when the engine refuses it.** Above `MAX_BULK_PER_ROW_HOOK_ROWS` (10,000) matched rows the engine refuses the write whole with `ERR_BULK_PER_ROW_HOOK_LIMIT` and writes nothing (ADR-0058 D6). The claim then reads one page of ids (`CLAIM_PAGE_ROWS` = ceiling / 2 = 5,000, under `{ isSystem: true }`), re-owns that page, and retries the whole remainder until one write carries it. - **The page write keeps the full predicate** and ANDs `id: { $in: page }` onto it. The twin writes the page by id alone. Here, a row claimed by someone else between the page read and its write keeps its owner, and the write never leaves the organization. - **`count` is the sum of the affected-row counts** of every write the claim made for the object, never a length the function counted itself. - **Termination guards, each with a warning:** a page that re-owns 0 rows stops the claim (otherwise it would read the same page forever); so does a refused write whose page read finds nothing, and a result that is not an affected-row count. A 1,000-page belt covers a driver that over-reports. Each object is best-effort: a failure is logged with the count already claimed, that count stays in the result, and later objects are still claimed. - `CLAIM_WRITE_CTX = { isSystem: true, skipAutomations: true }` (#23008) is unchanged and is on every write. Changeset: `.changeset/23054-org-seed-claim-paged.md` (patch, `@objectstack/organizations`). One test-infrastructure line beyond the claim file and its tests: `packages/plugins/organizations/vitest.config.ts` gains one anchored alias, `@objectstack/spec/data` to `packages/spec/src/data/index.ts`. The claim and its tests now import `MAX_BULK_PER_ROW_HOOK_ROWS` and `BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE` as values, and `check:test-source-alias` refused the suite reading them from `dist/`. This alias is the gate's prescribed fix (precedent: `packages/triggers/trigger-record-change/vitest.config.ts`). ## Measurements against the dispatch's hypotheses - **H1 holds** on `84c4fffbc`. Line 120 is `{ where: { organization_id: organizationId, owner_id: null }, limit: 10_000, fields: ['id'] }`, and line 134 is the per-row `ql.update(schema.name, { id: row.id, owner_id: ownerUserId }, { context: CLAIM_WRITE_CTX })`. - **H2 holds: the shape fits.** - The two doors it uses, `find` and `update(object, data, options?: any)`, are already named on `OrgScopingEngine`, so `org-scoping-engine.ts` is untouched. - On a real `ObjectQL` over `SqliteWasmDriver`, with one code-registered hook on the object, a `multi: true` write over 10,500 rows under `{ isSystem: true, skipAutomations: true }` is refused with `code: ERR_BULK_PER_ROW_HOOK_LIMIT, limit: 10000`, nothing written. The claim then pages to every row. Pinned in the new real-engine case. - The `tenant-audit-census` row for this file is unchanged (`update`, `schema.name`, `undecidable`, `elevated`, 1): the write still names `schema.name` at its call site. - **H3 holds on the predicate path.** - `engine.ts` `buildSession` stamps `skipAutomations` and `skipTriggers` on the batch context. `dispatchPerRowBeforeHooks` and `buildPerRowAfterContexts` spread that context into every per-row context, and `triggerHooks` skips `entry.meta` hooks. - Measured on the real engine: 0 metadata-hook write dispatches over 12 rows (whole-set write) and over 10,500 rows (paged). 2N code-hook dispatches, one per row and phase, and every one carries `skipAutomations: true` and `skipTriggers: true`. - Positive controls on the same engine: the same predicate shape under plain `{ isSystem: true }` fires the metadata hook 2N times (12 rows) and 6 times (3 rows). On the paged run, the page read is seen by the metadata hook as `beforeFind`/`afterFind` with `isSystem: true` and no `skipAutomations`. ## Pin: 10,500 owner-less rows are all owned after one pass - **Real engine** (`claim-org-seed-ownership-dispatch.pin.test.ts`, new case): `SqliteWasmDriver :memory:`, 10,500 owner-less rows in one organization, plus one owned row and one row of another organization. The result is `[{ object: probe_deal, count: 10500 }]`. The read-back sees 10,502 rows and none of the 10,500 is owner-less. The owned row and the foreign row are untouched. There are exactly 10,500 per-row dispatches of each phase. - **Unit** (`claim-org-seed-ownership.test.ts`): the same pin over a fake that imports `assertEngineUpdateDispatch` (and refuses any non-`multi` write) and models D6 with the contract's own code. Also: a single predicate write and no read below the ceiling; the page/remainder split (5,000 then 5,500) with the predicate on the page write; a row claimed between page read and write is left alone; a page that re-owns 0 rows stops after 2 writes and warns; a later failure keeps the count already landed, and the next object is still claimed. - **Cost:** the real-engine 10,500 case takes **1,516 ms** on the fix (verbose run, claim blob `d60e2ef023e0`, the same in both commits). On the old single-pass code it took 7,877 ms and 8,396 ms (the two ablation runs below). The unit 10,500 case takes 47 ms. These are shared-box seconds; the ratio is the portable reading. The dispatch-pin file stays under 10 s. ## Negative-pin ablation (committed first, trap-restored) Run twice: on `1faee8fa7` (the fix commit) and again on the final head `ddefed998` (after the alias commit). Both runs gave the same reading. The mutation went through `node scripts/ablation-replace.mjs` (anchor hit x1, blob `d60e2ef023e0` to `7b1b63404b95`, `grep -c 'claimUnowned(io'` 1 to 0, `grep -c 'limit: 10_000'` 1 to 2). It put the old single-pass block back verbatim: the read at `limit: 10_000` and the per-row `update` by id. The run was wrapped in a `trap restore EXIT INT TERM` that restores with `git checkout HEAD --` an absolute path and compares blob hashes. No build leg: the subject is imported by relative path (`./claim-org-seed-ownership.js`) from both test files, so vitest reads `src/`, not a `dist/`. Red, real engine: ```text × ... [#23054] 10 500 owner-less rows are all owned after one pass: refused whole under the flag, then paged to every row 8396ms AssertionError: expected [ { object: 'probe_deal', ...(1) } ] to deeply equal [ { object: 'probe_deal', ...(1) } ] - "count": 10500, + "count": 10000, Test Files 2 failed (2) Tests 8 failed | 5 passed (13) ``` The 7 other reds: the unit fakes refuse a by-id write as not `multi` (the old shape), so every claiming unit case answers `[]`, and the "stops after 2 writes" case saw 10,000 writes. The 12-row real-engine case saw the old per-object read (`beforeFind`/`afterFind`) where the fix issues none. Restore, proven on disk: `blob after restore d60e2ef == blob at HEAD d60e2ef`, `git diff HEAD` empty, `git status --porcelain` empty. Green re-run from the committed state: `Tests 13 passed (13)`. The second run's restore proof is the same: `trap-restore: blob d60e2ef... == HEAD blob`, `git diff HEAD` empty. ## Local verification (HEAD `ddefed998`) - `pnpm --filter @objectstack/organizations test`: `Test Files 13 passed (13) · Tests 160 passed (160)`. - `pnpm --filter @objectstack/organizations typecheck` (`tsc --noEmit` + `check:test-typecheck`, test layer included): green, 0 debt. - Gates: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived the same 64 commands as the dispatch's list. See the report for the per-command readings and the two that need a prerequisite this checkout lacks. ## Acceptance notes - **Below the ceiling the claim no longer reads.** A metadata-bound `beforeFind` hook on the object therefore no longer sees the claim, and no longer narrows which rows it selects. The selection is the write's own predicate, composed by the same write-scoping middleware chain as any predicate write. The twin has the same shape. The page read on the paged path is still a plain system read. - **The unit of failure moved from the row to the write.** A hook or validation rule that refuses a predicate write refuses all of it, so that object (or page) stays owner-less, and the warning names the object and what was already claimed. The twin made the same trade. - **The unreleased sibling changeset will read stale next to this one.** `.changeset/23008-org-seed-claim-skip-automations.md` says "The read is unchanged. The claim's per-object read is still a plain system read." When both compile into the same release, this PR's changeset states the new behaviour. The 23008 file is not edited here; whoever compiles the release may want to drop that bullet. - **The read no longer accepts a `{ records }` envelope,** matching the twin, which measured that `ObjectQL.find` resolves a bare array. An engine that answered an envelope would page nothing and get a warning, not a silent pass. - **Not filed (read-only inference, not reproduced):** the twin's page write is by id alone (`{ id: { $in: ids } }`), so a row given a human owner between its page read and its write would be re-owned to the platform admin. It is in `plugin-security`, epic #15194 territory, which this card does not write. --- _Generated by [Claude Code](https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f93bf12 commit 275ad42

5 files changed

Lines changed: 601 additions & 106 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@objectstack/organizations': patch
3+
---
4+
5+
The organization seed ownership claim now owns every owner-less row, not only the first 10,000 of each object
6+
7+
Clause-②: no
8+
9+
`claimOrgSeedOwnership` hands an organization's owner-less seeded rows to the organization's owner. It read at most 10,000 owner-less ids per object and updated them one at a time, and nothing read again. The default-organization bootstrap runs the claim once (the ADR-0093 D7 once-gate), so on an object with more than 10,000 owner-less rows in one organization the rest stayed owner-less for good. Measured on a real engine over the SQLite WASM driver: 10,500 owner-less rows, one pass, 10,000 owned and 500 left.
10+
11+
The claim now re-owns each object's owner-less rows with one predicate write (`owner_id = owner` where `organization_id = org` and `owner_id` is null), the shape the single-tenant claim `claimSeedOwnership` in `@objectstack/plugin-security` already uses.
12+
13+
- **Every owner-less row is owned after one pass.** When an object has more rows than one write may fire per-row hooks over (the engine's per-row hook ceiling, 10,000), the engine refuses the write and nothing is written. The claim then re-owns one page of 5,000 rows and tries the whole remainder again, until no owner-less row is left.
14+
- **The returned count is every row claimed.** `count` in each `{ object, count }` entry is the sum of the affected-row counts of every write the claim made for that object.
15+
- **Same scope and the same write context.** Only this organization's rows with no owner are written, a page write included. Every write still runs with `{ isSystem: true, skipAutomations: true }`: no hook bound from app metadata and no record-change flow runs, and hooks that plugins register in code (audit, capability gates, the sharing projection) still run once per claimed row.
16+
- **The claim reads only when it pages.** Below the ceiling it issues no read: the write's own predicate selects the rows. The page read, when there is one, is a plain system read.
17+
- **A failure now costs the write, not one row.** A hook or validation rule that refuses a predicate write refuses all of it, so that object's rows (or that page) stay owner-less and the claim logs a warning naming the object and how many rows it had already claimed. Other objects are still claimed.

‎packages/plugins/organizations/src/claim-org-seed-ownership-dispatch.pin.test.ts‎

Lines changed: 91 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@
88
*
99
* `claimOrgSeedOwnership` re-owns an organization's seeded rows to its owner.
1010
* That is attribution, the step that completes the seed, not a user event, so
11-
* its write runs under `{ isSystem: true, skipAutomations: true }`. This file
12-
* pins what that context reaches on the engine, because a flag that is
11+
* its write runs under `{ isSystem: true, skipAutomations: true }`. Since
12+
* [#23054] that write is a PREDICATE write (`multi: true`), paged only when the
13+
* engine refuses it for its per-row hook budget. This file pins what that
14+
* context reaches on the engine along that path, because a flag that is
1315
* silently dropped on the way would read as green in every double:
1416
*
1517
* 1. **the metadata-hook filter** — a hook bound through the real binder
@@ -24,20 +26,24 @@
2426
* `RecordChangeTrigger`'s handler reads before it dispatches a flow
2527
* (`skipAutomations` implies it, `engine.ts` `buildSession`).
2628
*
27-
* And it measures the read the flag deliberately stays off: the claim's
28-
* per-object read is a plain system read, so the same metadata hook, bound on
29-
* `beforeFind`/`afterFind` too, sees it. That is also the in-run positive
30-
* control: the metadata hook is live on this engine DURING the claim, so its
31-
* silence on the write events is the filter, not a hook that was never bound.
29+
* Each case carries a positive control on the SAME engine: the claim's own
30+
* write shape with a plain `{ isSystem: true }` context fires the metadata
31+
* hook per row and phase. Without it, "the hook fired 0 times" could be a hook
32+
* that was never bound.
3233
*
33-
* Each case carries a second positive control on the SAME engine: the claim's
34-
* own write shape with a plain `{ isSystem: true }` context fires the metadata
35-
* hook per row and phase.
34+
* [#23054] And it pins the claim's reach past the per-row hook ceiling: 10 500
35+
* owner-less rows of one object in one organization are ALL owned after one
36+
* pass. The flag does not exempt the write from that ceiling (code-registered
37+
* hooks still cover the object), so the whole-set write is refused and the
38+
* claim pages; the page read is a plain system read, so the metadata hook,
39+
* bound on `beforeFind`/`afterFind` too, sees it — the in-run positive control
40+
* of that case.
3641
*/
3742

3843
import { describe, it, expect, afterEach } from 'vitest';
3944
import { ObjectQL, bindHooksToEngine } from '@objectstack/objectql';
4045
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
46+
import { BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE, MAX_BULK_PER_ROW_HOOK_ROWS } from '@objectstack/spec/data';
4147

4248
import { claimOrgSeedOwnership } from './claim-org-seed-ownership.js';
4349

@@ -174,15 +180,10 @@ describe('[#23008] the organization seed-ownership claim dispatches no metadata-
174180
expect(owners.get('owned')).toBe(SOMEONE);
175181
expect(owners.get('foreign')).toBeNull();
176182

177-
// ⭐ 1 — the metadata-hook filter: not one write dispatch, before or after.
178-
expect(writeEvents(metadataSeen), 'a metadata-bound hook fired for the claim write').toEqual([]);
179-
// In-run positive control, and the read's measurement: the claim's one read
180-
// of this object is a plain system read, so the same metadata hook saw it.
181-
expect(metadataSeen.map((s) => s.event)).toEqual(['beforeFind', 'afterFind']);
182-
for (const s of metadataSeen) {
183-
expect(s.isSystem).toBe(true);
184-
expect(s.skipAutomations).toBeUndefined();
185-
}
183+
// ⭐ 1 — the metadata-hook filter: not one dispatch, before or after. Under
184+
// the ceiling the claim is one predicate write and reads nothing, so the
185+
// hook (bound on the read events too) sees nothing at all.
186+
expect(metadataSeen, 'a metadata-bound hook fired for the claim').toEqual([]);
186187

187188
// ⭐ 2 — the per-row path still runs code-registered hooks: one dispatch per
188189
// claimed row and phase, so audit and sharing keep their per-row input.
@@ -198,13 +199,15 @@ describe('[#23008] the organization seed-ownership claim dispatches no metadata-
198199
}
199200

200201
// Positive control, same engine, same rows: the claim's own write shape
201-
// (a by-id `update`) under a plain system context fires the metadata hook
202-
// per row and phase, and carries no skip flag.
202+
// (a predicate write over this organization) under a plain system context
203+
// fires the metadata hook per row and phase, and carries no skip flag.
203204
metadataHook.length = 0;
204205
codeHook.length = 0;
205-
for (const id of claimed) {
206-
await (engine as any).update(OBJECT, { id, stage: 'touched' }, SYS);
207-
}
206+
await (engine as any).update(OBJECT, { stage: 'touched' }, {
207+
where: { organization_id: ORG, owner_id: OWNER },
208+
multi: true,
209+
...SYS,
210+
});
208211
expect(writeEvents(metadataHook)).toHaveLength(2 * N);
209212
expect(codeHook).toHaveLength(2 * N);
210213
for (const s of codeHook) {
@@ -225,4 +228,68 @@ describe('[#23008] the organization seed-ownership claim dispatches no metadata-
225228
expect(writeEvents(metadataHook)).toEqual([]);
226229
expect(codeHook).toEqual([]);
227230
}, 120_000);
231+
232+
it('[#23054] 10 500 owner-less rows are all owned after one pass: refused whole under the flag, then paged to every row', async () => {
233+
const { engine, metadataHook, codeHook } = await boot();
234+
const N = 10_500;
235+
expect(N, 'the fixture must be past the per-row hook ceiling').toBeGreaterThan(MAX_BULK_PER_ROW_HOOK_ROWS);
236+
const claimed = Array.from({ length: N }, (_, i) => `d${String(i).padStart(5, '0')}`);
237+
await seed(engine, [
238+
...claimed.map((id, i) => ({ id, name: `Deal ${i}`, stage: 'open', organization_id: ORG, owner_id: null })),
239+
{ id: 'owned', name: 'Owned', stage: 'open', organization_id: ORG, owner_id: SOMEONE },
240+
{ id: 'foreign', name: 'Foreign', stage: 'open', organization_id: OTHER_ORG, owner_id: null },
241+
]);
242+
243+
// The measurement the paging rests on: with metadata hooks switched off,
244+
// code-registered hooks still cover the object, so the ceiling still
245+
// applies and refuses the whole-set write — nothing written, no hook run.
246+
await expect(
247+
(engine as any).update(OBJECT, { owner_id: OWNER }, {
248+
where: { organization_id: ORG, owner_id: null },
249+
multi: true,
250+
context: { isSystem: true, skipAutomations: true },
251+
}),
252+
).rejects.toMatchObject({ code: BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE, limit: MAX_BULK_PER_ROW_HOOK_ROWS });
253+
expect([...(await ownersOf(engine)).values()].filter((o) => o === OWNER)).toEqual([]);
254+
expect(codeHook).toEqual([]);
255+
metadataHook.length = 0;
256+
257+
const result = await claimOrgSeedOwnership(engine, ORG, OWNER);
258+
const metadataSeen = [...metadataHook];
259+
const codeSeen = [...codeHook];
260+
261+
expect(result).toEqual([{ object: OBJECT, count: N }]);
262+
const owners = await ownersOf(engine);
263+
expect(owners.size, 'the read-back saw every row').toBe(N + 2);
264+
expect(claimed.filter((id) => owners.get(id) !== OWNER), 'rows the pass left owner-less').toEqual([]);
265+
expect(owners.get('owned')).toBe(SOMEONE);
266+
expect(owners.get('foreign')).toBeNull();
267+
268+
// ⭐ 1 on every landed write, page or whole set; the page read is the
269+
// in-run positive control: a plain system read the metadata hook sees.
270+
expect(writeEvents(metadataSeen), 'a metadata-bound hook fired on a claim write').toEqual([]);
271+
expect(metadataSeen.map((s) => s.event)).toEqual(['beforeFind', 'afterFind']);
272+
for (const s of metadataSeen) {
273+
expect(s.isSystem).toBe(true);
274+
expect(s.skipAutomations).toBeUndefined();
275+
}
276+
// ⭐ 2 and 3: one dispatch per claimed row and phase, every one flagged.
277+
for (const event of ['beforeUpdate', 'afterUpdate']) {
278+
expect(codeSeen.filter((s) => s.event === event), `${event} per claimed row`).toHaveLength(N);
279+
}
280+
expect(new Set(codeSeen.filter((s) => s.event === 'afterUpdate').map((s) => s.id)).size).toBe(N);
281+
for (const s of codeSeen) {
282+
expect(s.skipAutomations).toBe(true);
283+
expect(s.skipTriggers).toBe(true);
284+
}
285+
286+
// Positive control on this engine too, kept under the ceiling.
287+
metadataHook.length = 0;
288+
await (engine as any).update(OBJECT, { stage: 'touched' }, {
289+
where: { id: { $in: claimed.slice(0, 3) } },
290+
multi: true,
291+
...SYS,
292+
});
293+
expect(writeEvents(metadataHook)).toHaveLength(6);
294+
}, 120_000);
228295
});

0 commit comments

Comments
 (0)