Skip to content

Commit 43edf90

Browse files
fix(plugin-security,organizations): seed-ownership claims re-own by row when a stored expression reads owner_id (#25724)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X5kePHewF7u1mfD8BHmUxYd)_ Fixes #25668 Clause-②: no Metadata-protocol: none ## What changed Both seed-ownership claims re-owned seeded rows with one `multi: true` predicate update. A stored computed field (a non-formula field with an `expression`) that reads `owner_id`, directly or one hop through the lookup, computes a different value on each row once the owner is set. The engine refuses such a predicate update ("update those rows by id"), so every seeded row of that object stayed ownerless. Triage `6385814124` keeps that refusal and routes the fix to the two claim sites. This PR does that. - `packages/plugins/plugin-security/src/claim-seed-ownership.ts`: a new `storedExpressionReadsOwner(schema)` decides, per object, whether a stored expression reads `owner_id`. It counts a direct read, a hop through `owner_id`, and a read through another stored field that does. An expression whose reads cannot be named counts as a reader. Reads come from `analyzeRelationshipTraversals` (`@objectstack/formula`), the member reader the engine's stored-expression planning uses. Such an object goes to `claimPredicateByRow`: - The NULL arm writes each seeded id it already names, with no read. - The `usr_system` arm reads ids a page at a time (`CLAIM_PAGE_ROWS`, the bound the paged fallback already reads by) and writes them. It stops when a read is empty, or when a page re-owns nothing. - Each write is a compare-and-set: the arm's owner term plus the row's `id`, `multi: true`. The engine evaluates that as a predicate over one row and computes the stored value from that row. A row someone owned in the meantime is not taken. - Every other object keeps the predicate write and its paged fallback, unchanged. - `packages/plugins/organizations/src/claim-org-seed-ownership.ts`: the same check (a twin, as the two files already twin their helpers) and `claimChunkByRow`. It writes one compare-and-set per seeded id of each chunk: `{ organization_id, owner_id: null, id }`. - A row whose own write is refused stays unowned. It is reported once per batch with the claim's existing warning (`claimSeedOwnership failed for …` / `claimOrgSeedOwnership failed for …`), with the refused count, up to 20 ids and the first refusal's message. The object's other rows are still claimed. - `@objectstack/organizations` gains the `@objectstack/formula` dependency (`package.json`, `pnpm-lock.yaml`). Its `vitest.config.ts` gains the matching source alias, which `pnpm check:test-source-alias` asked for. File surface: the claim's two files, their tests, one changeset, plus the three organizations manifest/config lines above. Nothing under `packages/objectql/**`. ## Evidence Head `1e608716`. Every number below was read on this branch. - New pins, real engine: - `plugin-security/src/claim-seed-ownership-stored-expression.pin.test.ts`, on `SqlDriver` (better-sqlite3). - `organizations/src/claim-org-seed-ownership-stored-expression.pin.test.ts`, on `SqliteWasmDriver`. - Each pins: the direct and the one-hop shape (the hop is HotCRM's `display_title` expression), each with four seeded rows that compute different values. The plugin-security pin adds two seed-identity rows. After the claim every such row is owned and its stored `display_title` is its own expression over its own row. The guest row the seed never reported, another organization's row and a human-owned row are untouched. A control object whose stored expression reads no owner keeps the bulk path: plugin-security makes 2 predicate writes and organizations 1. A row refused by a code-registered hook stays unowned and reported while the rest are claimed. - Result: `Tests 4 passed (4)` in each file. - Ablation (restore the bulk write via `scripts/ablation-replace.mjs`, run, restore; the tool verified the blob back to HEAD and `git diff HEAD` empty): - plugin-security: `const claim = claimPredicate;` gives `Tests 3 failed | 1 passed (4)`. The failures carry the engine's own refusal: `'display_title' on 'probe_forecast_direct' is computed from its expression on every write, and this update changes its inputs on rows where it then computes different values, … update those rows by id`. That is the card's measured symptom. - organizations: `const claim = claimChunk;` gives `Tests 3 failed | 1 passed (4)`, with the same refusal. This measures the organizations claim, which the card had only read. - Package suites: `@objectstack/plugin-security` `Test Files 223 passed`, `Tests 4632 passed | 45 skipped`. `@objectstack/organizations` `Test Files 17 passed`, `Tests 243 passed` (re-run at `1e608716`). `typecheck` exit 0 on both. `eslint --no-inline-config` over both packages exit 0. - Derived gates: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 84 families on this diff, and all 84 exit 0 at `1e608716`. Reconciled: `dispatch-gates --ran` reads "84 derived famil(ies) accounted for — 84 run, 0 NOT-MEASURED (a DERIVED zero — all 84 recorded an exit code and none of them is 3)". The first round, at `5aa93aa9`, found one real red, `check:test-source-alias` (the new `@objectstack/formula` import had no source alias in organizations), fixed in `1e608716`. It also had three prerequisite exits (3): `check:dual-build-cjs-loads` and `check:i18n` (some packages had no `dist/`) and `check-plugin-teardown-shape --self-test` (shallow clone; its pinned fixture commit fetched). All three exit 0 on the final run. Repo-wide families (`pnpm lint` and the CI roster) are CI's. ## Acceptance notes - **Row-by-row cost.** It is a full engine write per row, paid only on objects whose stored expressions read `owner_id`. The NULL arm is bounded by the seed's own report (chunks of `CLAIM_PAGE_ROWS`). The seed-identity arm is bounded by pages of `CLAIM_PAGE_ROWS` and `MAX_CLAIM_PAGES`. Not measured at seed sizes beyond the pins. - **Spellings.** The routing reads `owner_id` with the platform's one member reader. A stored expression that reaches `owner_id` through a spelling that reader does not name would still get the predicate write and the existing warning. No such spelling is known. Noted, not filed. - **Twin helpers.** `storedExpressionReadsOwner` / `expressionReads` exist in both claim files, as `affectedRowCount` and `hasField` already do. The two packages share no runtime dependency that could host them. Noted, carrier: none. - **Not measured here.** The card's second "Done when" line, HotCRM's patched `display_title` booting with 0 ownerless forecasts, needs a release that carries this. It is the `repo:hotcrm` seat's reading on hotcrm#2242. - The card's "Context, not asked here" item (a stored title reading a lookup goes stale when the lookup target is renamed) is untouched. --- _Generated by [Claude Code](https://claude.ai/code/session_01T8djanm8XBb36LFwcQEXJP)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 888d686 commit 43edf90

8 files changed

Lines changed: 807 additions & 5 deletions
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
'@objectstack/organizations': patch
4+
---
5+
6+
fix(plugin-security,organizations): the seed-ownership claims re-own row by row when a stored expression reads `owner_id`
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
- **Before.** Both seed-ownership claims (`claimSeedOwnership` in `@objectstack/plugin-security`, `claimOrgSeedOwnership` in `@objectstack/organizations`) re-owned seeded rows with one `multi: true` predicate update. A stored computed field (a non-formula field with an `expression`) that reads `owner_id`, directly (`record.owner_id`) or one hop through it (`record.owner_id.name`), computes a different value on each row once the owner is set. One predicate update can store only one value, so the engine refused the write, and every seeded row of that object stayed ownerless. The boot logged only `claimSeedOwnership failed for …`, and the next pass sent the same write.
12+
- **Now.** An object whose stored expressions read `owner_id`, directly, one hop through it, or through another stored field that does, is claimed with one write per row. Each write keeps the claim's own terms (`owner_id IS NULL` or the seed identity, and the organization in the multi-tenant claim) plus the row's id, so each row's stored value is computed from that row, and a row someone owned in the meantime is not taken from them. Reads come from the same member reader the engine uses (`analyzeRelationshipTraversals` in `@objectstack/formula`). An expression whose reads cannot be named also takes the row-by-row path.
13+
- **Unchanged.** Every other object keeps the predicate write, and its paging. The engine's refusal of a predicate update that would store different values stays as it is. A row whose own write is refused stays ownerless and is reported with the claim's existing warning, and the object's other rows are still claimed.
14+
- `@objectstack/organizations` now depends on `@objectstack/formula`.
15+
16+
What to do: nothing. For an app whose stored expression reads `owner_id`, such as a title that includes the owner's name, the single-tenant claim hands the seeded rows to the platform admin at the next seed settle (`app:seeded`), on the next boot. The organization claim covers organizations whose owner is bound from this release on. It runs in the call that replays their seed.

‎packages/plugins/organizations/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
},
2222
"dependencies": {
2323
"@objectstack/core": "workspace:*",
24+
"@objectstack/formula": "workspace:*",
2425
"@objectstack/plugin-auth": "workspace:*",
2526
"@objectstack/spec": "workspace:*",
2627
"@objectstack/types": "workspace:*"
Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#25668] The organization seed-ownership claim on an object whose STORED
5+
* computed field reads `owner_id` — measured on a REAL `ObjectQL` over a real
6+
* `SqliteWasmDriver` (the driver `objectstack dev` uses). The multi-tenant twin
7+
* of plugin-security's `claim-seed-ownership-stored-expression.pin.test.ts`.
8+
*
9+
* A predicate update stores ONE value per stored field for every matched row,
10+
* and the engine refuses one that moves the field to different values on
11+
* different rows, by contract ("update those rows by id"). The claim's patch
12+
* moves only `owner_id`, so the refusal reaches it exactly when a stored
13+
* expression reads `owner_id` — directly, or one hop through the lookup — and
14+
* the rows compute different values; before this, every seeded row of such an
15+
* object stayed owner-less.
16+
*
17+
* What is pinned:
18+
*
19+
* 1. both shapes (direct, and one hop through the lookup): after the claim,
20+
* every seeded owner-less row of the organization is owned by its owner,
21+
* and each row's stored value is its own expression over its own row;
22+
* 2. the rows the claim must leave alone stay as they were: another
23+
* organization's row, a row a member owns, and a guest's row the seed
24+
* replay never reported (ADR-0138 D1);
25+
* 3. an object whose stored expression does not read `owner_id` keeps the
26+
* chunked predicate write — one write per chunk, not one per row;
27+
* 4. a row whose write is refused stays owner-less and is reported, while the
28+
* other rows of that object are claimed.
29+
*/
30+
31+
import { describe, it, expect, afterEach } from 'vitest';
32+
import { ObjectQL } from '@objectstack/objectql';
33+
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
34+
35+
import { claimOrgSeedOwnership } from './claim-org-seed-ownership.js';
36+
37+
const SYS = { context: { isSystem: true } } as const;
38+
const ORG = 'org_alpha';
39+
const OTHER_ORG = 'org_north';
40+
const OWNER = 'usr_org_owner';
41+
const OWNER_NAME = 'Nina Reyes';
42+
const SOMEONE = 'usr_someone';
43+
const PERSON = 'probe_person';
44+
45+
/** The HotCRM `display_title` shape: the owner's name read one hop through the lookup. */
46+
const HOP_SOURCE =
47+
'record.?owner_id.?name.orValue("") == "" ? record.name : record.name + " · " + record.?owner_id.?name.orValue("")';
48+
49+
interface Shape {
50+
what: string;
51+
object: string;
52+
source: string;
53+
/** The value the stored field must hold for a row named `name` once the owner owns it. */
54+
expected: (name: string) => string;
55+
}
56+
57+
const SHAPES: Shape[] = [
58+
{
59+
what: 'direct — the expression reads `owner_id` itself',
60+
object: 'probe_forecast_direct',
61+
source: 'record.name + " · " + record.?owner_id.orValue("")',
62+
expected: (name) => `${name} · ${OWNER}`,
63+
},
64+
{
65+
what: 'one hop — the expression reads the owner\'s name through the lookup',
66+
object: 'probe_forecast_hop',
67+
source: HOP_SOURCE,
68+
expected: (name) => `${name} · ${OWNER_NAME}`,
69+
},
70+
];
71+
72+
/** A stored expression that reads no owner: the claim's patch never moves it. */
73+
const CONTROL_OBJECT = 'probe_forecast_plain';
74+
75+
const objectWith = (name: string, source: string): any => ({
76+
name,
77+
label: name,
78+
fields: {
79+
id: { name: 'id', type: 'text', primaryKey: true },
80+
name: { name: 'name', type: 'text' },
81+
owner_id: { name: 'owner_id', type: 'lookup', reference: PERSON },
82+
organization_id: { name: 'organization_id', type: 'text' },
83+
display_title: { name: 'display_title', type: 'text', expression: { dialect: 'cel', source } },
84+
},
85+
});
86+
87+
const PERSON_OBJECT: any = {
88+
name: PERSON,
89+
label: 'Probe Person',
90+
fields: {
91+
id: { name: 'id', type: 'text', primaryKey: true },
92+
name: { name: 'name', type: 'text' },
93+
organization_id: { name: 'organization_id', type: 'text' },
94+
},
95+
};
96+
97+
const engines: ObjectQL[] = [];
98+
afterEach(async () => {
99+
while (engines.length) {
100+
try { await engines.pop()?.destroy(); } catch { /* noop */ }
101+
}
102+
});
103+
104+
async function boot(): Promise<ObjectQL> {
105+
const engine = new ObjectQL();
106+
engine.registerDriver(new SqliteWasmDriver({ filename: ':memory:' } as never) as never, true);
107+
await engine.init();
108+
engine.registerApp({
109+
id: 'com.objectstack.org-claim-stored-expression-25668',
110+
name: 'Organization claim stored expression',
111+
version: '1.0.0',
112+
type: 'plugin',
113+
scope: 'system',
114+
objects: [
115+
PERSON_OBJECT,
116+
...SHAPES.map((s) => objectWith(s.object, s.source)),
117+
objectWith(CONTROL_OBJECT, 'record.name + " (seeded)"'),
118+
],
119+
} as never);
120+
await engine.syncSchemas();
121+
engines.push(engine);
122+
123+
const people = (engine as any).getDriverForObject(PERSON);
124+
await people.bulkCreate(PERSON, [
125+
{ id: OWNER, name: OWNER_NAME, organization_id: ORG },
126+
{ id: SOMEONE, name: 'Omar Diaz', organization_id: ORG },
127+
]);
128+
return engine;
129+
}
130+
131+
const SEEDED = ['f1', 'f2', 'f3', 'f4'];
132+
const nameOf = (id: string) => `Q4 2026 ${id}`;
133+
134+
async function seed(engine: ObjectQL, object: string): Promise<void> {
135+
// Straight through the driver, the way the replay's rows are already stored
136+
// when the claim runs; the stored column holds what an owner-less write computed.
137+
const driver = (engine as any).getDriverForObject(object);
138+
const row = (id: string, organization_id: string, owner_id: string | null) =>
139+
({ id, name: nameOf(id), organization_id, owner_id, display_title: nameOf(id) });
140+
await driver.bulkCreate(object, [
141+
...SEEDED.map((id) => row(id, ORG, null)),
142+
row('foreign', OTHER_ORG, null),
143+
row('owned', ORG, SOMEONE),
144+
row('guest', ORG, null),
145+
]);
146+
}
147+
148+
async function rowsOf(engine: ObjectQL, object: string): Promise<Map<string, { owner: unknown; title: unknown }>> {
149+
const rows = await (engine as any).find(object, { fields: ['id', 'owner_id', 'display_title'] }, SYS);
150+
return new Map(rows.map((r: any) => [r.id, { owner: r.owner_id ?? null, title: r.display_title ?? null }]));
151+
}
152+
153+
function recordingLogger() {
154+
const warns: { message: string; meta?: Record<string, any> }[] = [];
155+
return {
156+
warns,
157+
logger: {
158+
info: () => {},
159+
warn: (message: string, meta?: Record<string, any>) => { warns.push({ message, meta }); },
160+
},
161+
};
162+
}
163+
164+
/** Count the engine writes per object, without changing what they do. */
165+
function countUpdates(engine: ObjectQL): Map<string, number> {
166+
const calls = new Map<string, number>();
167+
const original = engine.update.bind(engine);
168+
(engine as any).update = (object: string, data: any, options?: any) => {
169+
calls.set(object, (calls.get(object) ?? 0) + 1);
170+
return original(object, data, options);
171+
};
172+
return calls;
173+
}
174+
175+
/** The replay reported every seeded row, plus rows the claim's predicate must still refuse. */
176+
const seededOf = (object: string) => (name: string): readonly string[] =>
177+
(name === object ? [...SEEDED, 'foreign', 'owned'] : []);
178+
179+
describe('[#25668] the organization seed-ownership claim on an object whose stored expression reads owner_id (real engine)', () => {
180+
for (const shape of SHAPES) {
181+
it(`${shape.what}: every seeded row is owned and its stored value matches its own row`, async () => {
182+
const engine = await boot();
183+
await seed(engine, shape.object);
184+
const { warns, logger } = recordingLogger();
185+
186+
const result = await claimOrgSeedOwnership(engine as never, ORG, OWNER, {
187+
logger,
188+
seededRecordIds: seededOf(shape.object),
189+
});
190+
191+
expect(warns.filter((w) => w.message.includes('claimOrgSeedOwnership failed')), 'the claim was refused').toEqual([]);
192+
expect(result).toEqual([{ object: shape.object, count: SEEDED.length }]);
193+
194+
const rows = await rowsOf(engine, shape.object);
195+
for (const id of SEEDED) {
196+
expect(rows.get(id), id).toEqual({ owner: OWNER, title: shape.expected(nameOf(id)) });
197+
}
198+
expect(rows.get('foreign')).toEqual({ owner: null, title: nameOf('foreign') });
199+
expect(rows.get('owned')).toEqual({ owner: SOMEONE, title: nameOf('owned') });
200+
expect(rows.get('guest')).toEqual({ owner: null, title: nameOf('guest') });
201+
});
202+
}
203+
204+
it('an object whose stored expression reads no owner keeps the chunked predicate write', async () => {
205+
const engine = await boot();
206+
await seed(engine, CONTROL_OBJECT);
207+
const calls = countUpdates(engine);
208+
209+
const result = await claimOrgSeedOwnership(engine as never, ORG, OWNER, {
210+
seededRecordIds: seededOf(CONTROL_OBJECT),
211+
});
212+
213+
expect(result).toEqual([{ object: CONTROL_OBJECT, count: SEEDED.length }]);
214+
// Six seeded ids are one chunk: one predicate write.
215+
expect(calls.get(CONTROL_OBJECT)).toBe(1);
216+
const rows = await rowsOf(engine, CONTROL_OBJECT);
217+
for (const id of SEEDED) expect(rows.get(id)?.owner, id).toBe(OWNER);
218+
});
219+
220+
it('a row whose write is refused stays owner-less and reported; the rest of the object is claimed', async () => {
221+
const engine = await boot();
222+
const shape = SHAPES[1]!;
223+
await seed(engine, shape.object);
224+
// A code-registered hook refusing one row: the shape of a capability gate.
225+
engine.registerHook('beforeUpdate', async (ctx: any) => {
226+
const ids = [ctx.input?.id, ctx.previous?.id, ctx.input?.options?.where?.id].filter(Boolean);
227+
if (ids.includes('f3')) throw new Error('probe refusal for f3');
228+
}, { object: shape.object, packageId: 'probe.refusing-hook' });
229+
const { warns, logger } = recordingLogger();
230+
231+
const result = await claimOrgSeedOwnership(engine as never, ORG, OWNER, {
232+
logger,
233+
seededRecordIds: seededOf(shape.object),
234+
});
235+
236+
const failed = warns.filter((w) => w.message.includes(`claimOrgSeedOwnership failed for ${shape.object}`));
237+
expect(failed, 'the refused row was not reported').toHaveLength(1);
238+
expect(failed[0]!.meta?.error).toContain('probe refusal for f3');
239+
expect(result).toEqual([{ object: shape.object, count: SEEDED.length - 1 }]);
240+
241+
const rows = await rowsOf(engine, shape.object);
242+
expect(rows.get('f3')).toEqual({ owner: null, title: nameOf('f3') });
243+
for (const id of ['f1', 'f2', 'f4']) {
244+
expect(rows.get(id), id).toEqual({ owner: OWNER, title: shape.expected(nameOf(id)) });
245+
}
246+
});
247+
});

0 commit comments

Comments
 (0)