diff --git a/.changeset/25668-claim-by-row-stored-expression.md b/.changeset/25668-claim-by-row-stored-expression.md new file mode 100644 index 00000000000..c2e400329a4 --- /dev/null +++ b/.changeset/25668-claim-by-row-stored-expression.md @@ -0,0 +1,16 @@ +--- +'@objectstack/plugin-security': patch +'@objectstack/organizations': patch +--- + +fix(plugin-security,organizations): the seed-ownership claims re-own row by row when a stored expression reads `owner_id` + +Clause-②: no +Metadata-protocol: none + +- **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. +- **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. +- **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. +- `@objectstack/organizations` now depends on `@objectstack/formula`. + +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. diff --git a/packages/plugins/organizations/package.json b/packages/plugins/organizations/package.json index 81222d65f64..e6d23441d71 100644 --- a/packages/plugins/organizations/package.json +++ b/packages/plugins/organizations/package.json @@ -21,6 +21,7 @@ }, "dependencies": { "@objectstack/core": "workspace:*", + "@objectstack/formula": "workspace:*", "@objectstack/plugin-auth": "workspace:*", "@objectstack/spec": "workspace:*", "@objectstack/types": "workspace:*" diff --git a/packages/plugins/organizations/src/claim-org-seed-ownership-stored-expression.pin.test.ts b/packages/plugins/organizations/src/claim-org-seed-ownership-stored-expression.pin.test.ts new file mode 100644 index 00000000000..4b7a2653ef7 --- /dev/null +++ b/packages/plugins/organizations/src/claim-org-seed-ownership-stored-expression.pin.test.ts @@ -0,0 +1,247 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25668] The organization seed-ownership claim on an object whose STORED + * computed field reads `owner_id` — measured on a REAL `ObjectQL` over a real + * `SqliteWasmDriver` (the driver `objectstack dev` uses). The multi-tenant twin + * of plugin-security's `claim-seed-ownership-stored-expression.pin.test.ts`. + * + * A predicate update stores ONE value per stored field for every matched row, + * and the engine refuses one that moves the field to different values on + * different rows, by contract ("update those rows by id"). The claim's patch + * moves only `owner_id`, so the refusal reaches it exactly when a stored + * expression reads `owner_id` — directly, or one hop through the lookup — and + * the rows compute different values; before this, every seeded row of such an + * object stayed owner-less. + * + * What is pinned: + * + * 1. both shapes (direct, and one hop through the lookup): after the claim, + * every seeded owner-less row of the organization is owned by its owner, + * and each row's stored value is its own expression over its own row; + * 2. the rows the claim must leave alone stay as they were: another + * organization's row, a row a member owns, and a guest's row the seed + * replay never reported (ADR-0138 D1); + * 3. an object whose stored expression does not read `owner_id` keeps the + * chunked predicate write — one write per chunk, not one per row; + * 4. a row whose write is refused stays owner-less and is reported, while the + * other rows of that object are claimed. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; + +import { claimOrgSeedOwnership } from './claim-org-seed-ownership.js'; + +const SYS = { context: { isSystem: true } } as const; +const ORG = 'org_alpha'; +const OTHER_ORG = 'org_north'; +const OWNER = 'usr_org_owner'; +const OWNER_NAME = 'Nina Reyes'; +const SOMEONE = 'usr_someone'; +const PERSON = 'probe_person'; + +/** The HotCRM `display_title` shape: the owner's name read one hop through the lookup. */ +const HOP_SOURCE = + 'record.?owner_id.?name.orValue("") == "" ? record.name : record.name + " · " + record.?owner_id.?name.orValue("")'; + +interface Shape { + what: string; + object: string; + source: string; + /** The value the stored field must hold for a row named `name` once the owner owns it. */ + expected: (name: string) => string; +} + +const SHAPES: Shape[] = [ + { + what: 'direct — the expression reads `owner_id` itself', + object: 'probe_forecast_direct', + source: 'record.name + " · " + record.?owner_id.orValue("")', + expected: (name) => `${name} · ${OWNER}`, + }, + { + what: 'one hop — the expression reads the owner\'s name through the lookup', + object: 'probe_forecast_hop', + source: HOP_SOURCE, + expected: (name) => `${name} · ${OWNER_NAME}`, + }, +]; + +/** A stored expression that reads no owner: the claim's patch never moves it. */ +const CONTROL_OBJECT = 'probe_forecast_plain'; + +const objectWith = (name: string, source: string): any => ({ + name, + label: name, + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + owner_id: { name: 'owner_id', type: 'lookup', reference: PERSON }, + organization_id: { name: 'organization_id', type: 'text' }, + display_title: { name: 'display_title', type: 'text', expression: { dialect: 'cel', source } }, + }, +}); + +const PERSON_OBJECT: any = { + name: PERSON, + label: 'Probe Person', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + organization_id: { name: 'organization_id', type: 'text' }, + }, +}; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(): Promise { + const engine = new ObjectQL(); + engine.registerDriver(new SqliteWasmDriver({ filename: ':memory:' } as never) as never, true); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.org-claim-stored-expression-25668', + name: 'Organization claim stored expression', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + PERSON_OBJECT, + ...SHAPES.map((s) => objectWith(s.object, s.source)), + objectWith(CONTROL_OBJECT, 'record.name + " (seeded)"'), + ], + } as never); + await engine.syncSchemas(); + engines.push(engine); + + const people = (engine as any).getDriverForObject(PERSON); + await people.bulkCreate(PERSON, [ + { id: OWNER, name: OWNER_NAME, organization_id: ORG }, + { id: SOMEONE, name: 'Omar Diaz', organization_id: ORG }, + ]); + return engine; +} + +const SEEDED = ['f1', 'f2', 'f3', 'f4']; +const nameOf = (id: string) => `Q4 2026 ${id}`; + +async function seed(engine: ObjectQL, object: string): Promise { + // Straight through the driver, the way the replay's rows are already stored + // when the claim runs; the stored column holds what an owner-less write computed. + const driver = (engine as any).getDriverForObject(object); + const row = (id: string, organization_id: string, owner_id: string | null) => + ({ id, name: nameOf(id), organization_id, owner_id, display_title: nameOf(id) }); + await driver.bulkCreate(object, [ + ...SEEDED.map((id) => row(id, ORG, null)), + row('foreign', OTHER_ORG, null), + row('owned', ORG, SOMEONE), + row('guest', ORG, null), + ]); +} + +async function rowsOf(engine: ObjectQL, object: string): Promise> { + const rows = await (engine as any).find(object, { fields: ['id', 'owner_id', 'display_title'] }, SYS); + return new Map(rows.map((r: any) => [r.id, { owner: r.owner_id ?? null, title: r.display_title ?? null }])); +} + +function recordingLogger() { + const warns: { message: string; meta?: Record }[] = []; + return { + warns, + logger: { + info: () => {}, + warn: (message: string, meta?: Record) => { warns.push({ message, meta }); }, + }, + }; +} + +/** Count the engine writes per object, without changing what they do. */ +function countUpdates(engine: ObjectQL): Map { + const calls = new Map(); + const original = engine.update.bind(engine); + (engine as any).update = (object: string, data: any, options?: any) => { + calls.set(object, (calls.get(object) ?? 0) + 1); + return original(object, data, options); + }; + return calls; +} + +/** The replay reported every seeded row, plus rows the claim's predicate must still refuse. */ +const seededOf = (object: string) => (name: string): readonly string[] => + (name === object ? [...SEEDED, 'foreign', 'owned'] : []); + +describe('[#25668] the organization seed-ownership claim on an object whose stored expression reads owner_id (real engine)', () => { + for (const shape of SHAPES) { + it(`${shape.what}: every seeded row is owned and its stored value matches its own row`, async () => { + const engine = await boot(); + await seed(engine, shape.object); + const { warns, logger } = recordingLogger(); + + const result = await claimOrgSeedOwnership(engine as never, ORG, OWNER, { + logger, + seededRecordIds: seededOf(shape.object), + }); + + expect(warns.filter((w) => w.message.includes('claimOrgSeedOwnership failed')), 'the claim was refused').toEqual([]); + expect(result).toEqual([{ object: shape.object, count: SEEDED.length }]); + + const rows = await rowsOf(engine, shape.object); + for (const id of SEEDED) { + expect(rows.get(id), id).toEqual({ owner: OWNER, title: shape.expected(nameOf(id)) }); + } + expect(rows.get('foreign')).toEqual({ owner: null, title: nameOf('foreign') }); + expect(rows.get('owned')).toEqual({ owner: SOMEONE, title: nameOf('owned') }); + expect(rows.get('guest')).toEqual({ owner: null, title: nameOf('guest') }); + }); + } + + it('an object whose stored expression reads no owner keeps the chunked predicate write', async () => { + const engine = await boot(); + await seed(engine, CONTROL_OBJECT); + const calls = countUpdates(engine); + + const result = await claimOrgSeedOwnership(engine as never, ORG, OWNER, { + seededRecordIds: seededOf(CONTROL_OBJECT), + }); + + expect(result).toEqual([{ object: CONTROL_OBJECT, count: SEEDED.length }]); + // Six seeded ids are one chunk: one predicate write. + expect(calls.get(CONTROL_OBJECT)).toBe(1); + const rows = await rowsOf(engine, CONTROL_OBJECT); + for (const id of SEEDED) expect(rows.get(id)?.owner, id).toBe(OWNER); + }); + + it('a row whose write is refused stays owner-less and reported; the rest of the object is claimed', async () => { + const engine = await boot(); + const shape = SHAPES[1]!; + await seed(engine, shape.object); + // A code-registered hook refusing one row: the shape of a capability gate. + engine.registerHook('beforeUpdate', async (ctx: any) => { + const ids = [ctx.input?.id, ctx.previous?.id, ctx.input?.options?.where?.id].filter(Boolean); + if (ids.includes('f3')) throw new Error('probe refusal for f3'); + }, { object: shape.object, packageId: 'probe.refusing-hook' }); + const { warns, logger } = recordingLogger(); + + const result = await claimOrgSeedOwnership(engine as never, ORG, OWNER, { + logger, + seededRecordIds: seededOf(shape.object), + }); + + const failed = warns.filter((w) => w.message.includes(`claimOrgSeedOwnership failed for ${shape.object}`)); + expect(failed, 'the refused row was not reported').toHaveLength(1); + expect(failed[0]!.meta?.error).toContain('probe refusal for f3'); + expect(result).toEqual([{ object: shape.object, count: SEEDED.length - 1 }]); + + const rows = await rowsOf(engine, shape.object); + expect(rows.get('f3')).toEqual({ owner: null, title: nameOf('f3') }); + for (const id of ['f1', 'f2', 'f4']) { + expect(rows.get(id), id).toEqual({ owner: OWNER, title: shape.expected(nameOf(id)) }); + } + }); +}); diff --git a/packages/plugins/organizations/src/claim-org-seed-ownership.ts b/packages/plugins/organizations/src/claim-org-seed-ownership.ts index 5d36e564ca9..a5185bdb6a5 100644 --- a/packages/plugins/organizations/src/claim-org-seed-ownership.ts +++ b/packages/plugins/organizations/src/claim-org-seed-ownership.ts @@ -88,8 +88,29 @@ * writes nothing, so that chunk stays owner-less and the warning says so. The * next chunk and the next object are still claimed. The twin made the same * trade. + * + * ## [#25668] …except where a stored expression reads `owner_id`: by row + * + * A stored computed field (a non-formula field declaring an `expression`) is + * computed by the engine on every write, and a predicate write stores ONE + * value per such field for every matched row: one that moves the field to + * different values on different rows is refused whole, by contract ("update + * those rows by id"). The claim's patch moves only `owner_id`, so the refusal + * reaches it exactly when a stored expression reads `owner_id` (directly, or + * one hop through it) and the rows compute different values — and then every + * chunk of that object stayed owner-less. + * + * So such an object ({@link storedExpressionReadsOwner}) is claimed ROW BY + * ROW: one compare-and-set write per seeded id, + * `{ organization_id, owner_id: null, id }` with `multi: true`, which the + * engine evaluates as a predicate over that one row and computes from it. The + * scope and the owner-less term stay in every write. Every other object keeps + * the chunked predicate write. A row whose own write is refused stays + * owner-less and is reported; the object's other rows are still claimed. The + * twin made the same change. */ +import { analyzeRelationshipTraversals } from '@objectstack/formula'; import type { ServiceObject } from '@objectstack/spec/data'; import { MAX_BULK_PER_ROW_HOOK_ROWS } from '@objectstack/spec/data'; @@ -194,6 +215,52 @@ async function claimChunk( } } +/** + * [#25668] Re-own one chunk's seeded rows one write each: the chunk's scope and + * owner-less terms plus the row's id. Returns the sum of the affected-row + * counts. Never throws: a refused row stays owner-less and the chunk's + * refusals are reported once, with the first refusal's message. + */ +async function claimChunkByRow( + reown: (where: Record) => Promise, + objectName: string, + organizationId: string, + where: Record, + logger: ClaimOwnershipOptions['logger'], +): Promise { + const ids = (where.id as { $in: readonly string[] }).$in; + let claimed = 0; + let countUnknown = false; + const refused: string[] = []; + let firstError: string | undefined; + for (const id of ids) { + try { + const count = affectedRowCount(await reown({ ...where, id })); + if (count === undefined) countUnknown = true; + else claimed += count; + } catch (e) { + refused.push(id); + firstError ??= (e as Error).message; + } + } + const at = { object: objectName, organizationId, ids: ids.length }; + if (refused.length > 0) { + logger?.warn?.( + `[org-scoping] claimOrgSeedOwnership failed for ${objectName} in org ${organizationId}: ` + + `${refused.length} of the ${ids.length} seeded row(s) of this chunk stay owner-less`, + { ...at, refused: refused.length, refusedIds: refused.slice(0, 20), error: firstError }, + ); + } + if (countUnknown) { + logger?.warn?.( + `[org-scoping] claimOrgSeedOwnership could not read an affected-row count for ${objectName} ` + + `in org ${organizationId}: seeded rows were re-owned but this run cannot say how many`, + at, + ); + } + return claimed; +} + function hasField(schema: ServiceObject, field: string): boolean { const fields: any = (schema as any)?.fields; if (!fields) return false; @@ -201,6 +268,47 @@ function hasField(schema: ServiceObject, field: string): boolean { return Object.prototype.hasOwnProperty.call(fields, field); } +/** + * The `record.` names one stored expression reads — a hop counts as a + * read of the field it goes through — or `null` when they cannot be named + * (another dialect, or a source that does not parse). Read with the platform's + * one member reader, the one the engine's stored-expression planning uses. + */ +function expressionReads(expression: unknown): ReadonlySet | null { + const expr = typeof expression === 'string' ? { source: expression } : expression as { dialect?: unknown; source?: unknown }; + if (!expr || (expr.dialect !== undefined && expr.dialect !== 'cel') || typeof expr.source !== 'string') return null; + const analysis = analyzeRelationshipTraversals(expr.source); + return analysis ? new Set([...analysis.bareFields, ...analysis.traversals.keys()]) : null; +} + +/** + * [#25668] Does a stored computed field of this object read `owner_id` — + * directly, one hop through it, or through another stored field that does? + * The twin of plugin-security's check of the same name: a stored field is a + * non-`formula` field declaring an `expression`, planned off a field MAP as + * the engine plans it, and an expression whose reads cannot be named counts + * as a reader. + */ +function storedExpressionReadsOwner(schema: ServiceObject): boolean { + const fields: any = (schema as any)?.fields; + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return false; + const stored = Object.entries(fields as Record).filter( + ([, def]) => def && typeof def === 'object' && def.type !== 'formula' && def.expression, + ); + const moved = new Set(['owner_id']); + for (let grew = true; grew;) { + grew = false; + for (const [name, def] of stored) { + if (moved.has(name)) continue; + const reads = expressionReads(def.expression); + if (reads !== null && ![...reads].some((read) => moved.has(read))) continue; + moved.add(name); + grew = true; + } + } + return moved.size > 1; +} + /** * Assign `owner_id = ownerUserId` to every NULL-owned seed row of `organizationId`. * @@ -211,7 +319,8 @@ function hasField(schema: ServiceObject, field: string): boolean { * (d) have rows the org's seed replay landed on ({@link ClaimOwnershipOptions.seededRecordIds}), * and re-owns the org's unowned seeded rows as `isSystem` with * `skipAutomations`, in one predicate write per chunk of - * {@link CLAIM_CHUNK_IDS} seeded ids. Returns a per-object summary whose + * {@link CLAIM_CHUNK_IDS} seeded ids — [#25668] one write per row instead on an + * object whose stored expressions read `owner_id`. Returns a per-object summary whose * `count` is the sum of every write's affected-row count. */ export async function claimOrgSeedOwnership( @@ -263,6 +372,10 @@ export async function claimOrgSeedOwnership( { where, multi: true, context: CLAIM_WRITE_CTX }, ); + // [#25668] One predicate write cannot store a per-row value of a stored + // field the claim's `owner_id` moves; such an object is claimed by row. + const claim = storedExpressionReadsOwner(schema) ? claimChunkByRow : claimChunk; + let updated = 0; for (let at = 0; at < seededIds.length; at += CLAIM_CHUNK_IDS) { const where = { @@ -270,7 +383,7 @@ export async function claimOrgSeedOwnership( owner_id: null, id: { $in: seededIds.slice(at, at + CLAIM_CHUNK_IDS) }, }; - updated += await claimChunk(reown, schema.name, organizationId, where, logger); + updated += await claim(reown, schema.name, organizationId, where, logger); } if (updated > 0) results.push({ object: schema.name, count: updated }); } diff --git a/packages/plugins/organizations/vitest.config.ts b/packages/plugins/organizations/vitest.config.ts index 0f4c6ed08a9..a6a619cf20e 100644 --- a/packages/plugins/organizations/vitest.config.ts +++ b/packages/plugins/organizations/vitest.config.ts @@ -112,6 +112,15 @@ export default defineConfig({ find: /^@objectstack\/spec\/data$/, replacement: path.resolve(__dirname, '../../spec/src/data/index.ts'), }, + { + // [#25668] A VALUE reach from source: `claim-org-seed-ownership.ts` + // asks `analyzeRelationshipTraversals` which fields a stored + // expression reads, to route an object whose stored values follow + // `owner_id` to the row-by-row claim. Resolved from `dist/`, that + // routing would be the last build's member reader. + find: /^@objectstack\/formula$/, + replacement: path.resolve(__dirname, '../../formula/src/index.ts'), + }, { find: /^@objectstack\/spec\/contracts$/, replacement: path.resolve(__dirname, '../../spec/src/contracts/index.ts'), diff --git a/packages/plugins/plugin-security/src/claim-seed-ownership-stored-expression.pin.test.ts b/packages/plugins/plugin-security/src/claim-seed-ownership-stored-expression.pin.test.ts new file mode 100644 index 00000000000..9628fdb41d7 --- /dev/null +++ b/packages/plugins/plugin-security/src/claim-seed-ownership-stored-expression.pin.test.ts @@ -0,0 +1,249 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25668] The seed-ownership claim on an object whose STORED computed field + * reads `owner_id` — measured on a REAL `ObjectQL` over a real `SqlDriver`. + * + * A stored field (a non-formula field declaring an `expression`) is computed by + * the engine on every write. A predicate update stores ONE value per stored + * field for every matched row, so the engine refuses one that moves the field + * to different values on different rows ("update those rows by id"), and that + * refusal is deliberate. The claim's patch moves only `owner_id`, so the + * refusal reaches it exactly when a stored expression reads `owner_id` — + * directly, or one hop through the lookup — and the rows compute different + * values. HotCRM's `crm_forecast.display_title` is that shape; before this, + * every seeded row of such an object stayed ownerless after the claim. + * + * What is pinned: + * + * 1. both shapes (direct, and one hop through the lookup): after the claim, + * every seeded and seed-identity row is owned by the admin, and each row's + * stored value is its own expression over its own row; + * 2. the rows the claim must leave alone stay as they were: a guest's unowned + * row the seed never reported (ADR-0138 D1) and a row a human owns; + * 3. an object whose stored expression does not read `owner_id` keeps the + * bulk path — one predicate write per unowned shape, not one per row; + * 4. a row whose write is refused stays unowned and is reported with the + * claim's existing `claimSeedOwnership failed for …` warning, while the + * other rows of that object are claimed. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; + +import { claimSeedOwnership } from './claim-seed-ownership.js'; +import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; + +const SYS = { context: { isSystem: true } } as const; +const ADMIN = 'usr_admin_human'; +const ADMIN_NAME = 'Nina Reyes'; +const SEED_IDENTITY = 'usr_system'; +const SOMEONE = 'usr_someone'; +const PERSON = 'probe_person'; + +/** The HotCRM `display_title` shape: the owner's name read one hop through the lookup. */ +const HOP_SOURCE = + 'record.?owner_id.?name.orValue("") == "" ? record.name : record.name + " · " + record.?owner_id.?name.orValue("")'; + +interface Shape { + what: string; + object: string; + source: string; + /** The value the stored field must hold for a row named `name` once the admin owns it. */ + expected: (name: string) => string; +} + +const SHAPES: Shape[] = [ + { + what: 'direct — the expression reads `owner_id` itself', + object: 'probe_forecast_direct', + source: 'record.name + " · " + record.?owner_id.orValue("")', + expected: (name) => `${name} · ${ADMIN}`, + }, + { + what: 'one hop — the expression reads the owner\'s name through the lookup', + object: 'probe_forecast_hop', + source: HOP_SOURCE, + expected: (name) => `${name} · ${ADMIN_NAME}`, + }, +]; + +/** A stored expression that reads no owner: the claim's patch never moves it. */ +const CONTROL_OBJECT = 'probe_forecast_plain'; + +const objectWith = (name: string, source: string): any => ({ + name, + label: name, + fields: { + id: { type: 'text', label: 'Id', primary: true }, + name: { type: 'text', label: 'Name' }, + owner_id: { type: 'lookup', label: 'Owner', reference: PERSON }, + display_title: { type: 'text', label: 'Display title', expression: { dialect: 'cel', source } }, + }, +}); + +const PERSON_OBJECT: any = { + name: PERSON, + label: 'Probe Person', + fields: { + id: { type: 'text', label: 'Id', primary: true }, + name: { type: 'text', label: 'Name' }, + }, +}; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(): Promise { + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), + true, + ); + await engine.init(); + const objects = [ + PERSON_OBJECT, + ...SHAPES.map((s) => objectWith(s.object, s.source)), + objectWith(CONTROL_OBJECT, 'record.name + " (seeded)"'), + ]; + engine.registerApp({ + id: 'com.objectstack.claim-stored-expression-25668', + name: 'Claim stored expression', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects, + } as any); + await engine.syncSchemas(); + // The rows below carry no organization: a store built before 18.x + // (ADR-0131 D1 makes a fresh tenant column NOT NULL). + await relaxTenantColumns(engine, objects.map((o) => o.name)); + engines.push(engine); + + const people = (engine as any).getDriverForObject(PERSON); + await people.bulkCreate(PERSON, [ + { id: ADMIN, name: ADMIN_NAME }, + { id: SOMEONE, name: 'Omar Diaz' }, + ]); + return engine; +} + +/** The rows one object starts with, and which of them the seed reported. */ +const SEEDED_NULL = ['f1', 'f2', 'f3', 'f4']; +const SEEDED_SYSTEM = ['s1', 's2']; +const nameOf = (id: string) => `Q4 2026 ${id}`; + +async function seed(engine: ObjectQL, object: string): Promise { + // Straight through the driver, the way a seed's rows are already stored when + // the claim runs; the stored column holds what an ownerless write computed. + const driver = (engine as any).getDriverForObject(object); + await driver.bulkCreate(object, [ + ...SEEDED_NULL.map((id) => ({ id, name: nameOf(id), owner_id: null, display_title: nameOf(id) })), + ...SEEDED_SYSTEM.map((id) => ({ id, name: nameOf(id), owner_id: SEED_IDENTITY, display_title: nameOf(id) })), + // Out of the claim's reach: a guest's row the seed never reported, and a row a human owns. + { id: 'guest', name: nameOf('guest'), owner_id: null, display_title: nameOf('guest') }, + { id: 'owned', name: nameOf('owned'), owner_id: SOMEONE, display_title: nameOf('owned') }, + ]); +} + +async function rowsOf(engine: ObjectQL, object: string): Promise> { + const rows = await (engine as any).find(object, { fields: ['id', 'owner_id', 'display_title'] }, SYS); + return new Map(rows.map((r: any) => [r.id, { owner: r.owner_id ?? null, title: r.display_title ?? null }])); +} + +function recordingLogger() { + const warns: { message: string; meta?: Record }[] = []; + return { + warns, + logger: { + info: () => {}, + warn: (message: string, meta?: Record) => { warns.push({ message, meta }); }, + }, + }; +} + +/** Count the engine writes per object, without changing what they do. */ +function countUpdates(engine: ObjectQL): Map { + const calls = new Map(); + const original = engine.update.bind(engine); + (engine as any).update = (object: string, data: any, options?: any) => { + calls.set(object, (calls.get(object) ?? 0) + 1); + return original(object, data, options); + }; + return calls; +} + +describe('[#25668] the seed ownership claim on an object whose stored expression reads owner_id (real engine)', () => { + for (const shape of SHAPES) { + it(`${shape.what}: every seeded row is owned and its stored value matches its own row`, async () => { + const engine = await boot(); + await seed(engine, shape.object); + const { warns, logger } = recordingLogger(); + + const result = await claimSeedOwnership(engine, ADMIN, { + logger, + seededRecordIds: (object) => (object === shape.object ? [...SEEDED_NULL, 'owned'] : []), + }); + + expect(warns.filter((w) => w.message.includes('claimSeedOwnership failed')), 'the claim was refused').toEqual([]); + expect(result).toEqual([{ object: shape.object, count: SEEDED_NULL.length + SEEDED_SYSTEM.length }]); + + const rows = await rowsOf(engine, shape.object); + for (const id of [...SEEDED_NULL, ...SEEDED_SYSTEM]) { + expect(rows.get(id), id).toEqual({ owner: ADMIN, title: shape.expected(nameOf(id)) }); + } + // ADR-0138 D1: the guest's row keeps no owner; the human's row keeps its owner. + expect(rows.get('guest')).toEqual({ owner: null, title: nameOf('guest') }); + expect(rows.get('owned')).toEqual({ owner: SOMEONE, title: nameOf('owned') }); + }); + } + + it('an object whose stored expression reads no owner keeps the bulk path: one write per unowned shape', async () => { + const engine = await boot(); + await seed(engine, CONTROL_OBJECT); + const calls = countUpdates(engine); + + const result = await claimSeedOwnership(engine, ADMIN, { + seededRecordIds: (object) => (object === CONTROL_OBJECT ? SEEDED_NULL : []), + }); + + expect(result).toEqual([{ object: CONTROL_OBJECT, count: SEEDED_NULL.length + SEEDED_SYSTEM.length }]); + // The NULL arm's one chunk, then the seed identity: two predicate writes. + expect(calls.get(CONTROL_OBJECT)).toBe(2); + const rows = await rowsOf(engine, CONTROL_OBJECT); + for (const id of [...SEEDED_NULL, ...SEEDED_SYSTEM]) expect(rows.get(id)?.owner, id).toBe(ADMIN); + }); + + it('a row whose write is refused stays unowned and reported; the rest of the object is claimed', async () => { + const engine = await boot(); + const shape = SHAPES[1]!; + await seed(engine, shape.object); + // A code-registered hook refusing one row: the shape of a capability gate. + engine.registerHook('beforeUpdate', async (ctx: any) => { + const ids = [ctx.input?.id, ctx.previous?.id, ctx.input?.options?.where?.id].filter(Boolean); + if (ids.includes('f3')) throw new Error('probe refusal for f3'); + }, { object: shape.object, packageId: 'probe.refusing-hook' }); + const { warns, logger } = recordingLogger(); + + const result = await claimSeedOwnership(engine, ADMIN, { + logger, + seededRecordIds: (object) => (object === shape.object ? SEEDED_NULL : []), + }); + + const failed = warns.filter((w) => w.message.includes(`claimSeedOwnership failed for ${shape.object}`)); + expect(failed, 'the refused row was not reported').toHaveLength(1); + expect(failed[0]!.meta?.error).toContain('probe refusal for f3'); + expect(result).toEqual([{ object: shape.object, count: SEEDED_NULL.length - 1 + SEEDED_SYSTEM.length }]); + + const rows = await rowsOf(engine, shape.object); + expect(rows.get('f3')).toEqual({ owner: null, title: nameOf('f3') }); + for (const id of ['f1', 'f2', 'f4', ...SEEDED_SYSTEM]) { + expect(rows.get(id), id).toEqual({ owner: ADMIN, title: shape.expected(nameOf(id)) }); + } + }); +}); diff --git a/packages/plugins/plugin-security/src/claim-seed-ownership.ts b/packages/plugins/plugin-security/src/claim-seed-ownership.ts index 6773c073552..f18af1a534c 100644 --- a/packages/plugins/plugin-security/src/claim-seed-ownership.ts +++ b/packages/plugins/plugin-security/src/claim-seed-ownership.ts @@ -175,8 +175,36 @@ * being the engine's answer to a different question, while staying well above * plugin-sharing's 1 000-row recompute cap — so a page is still large enough to * take the trailing-batch branch rather than N per-row recomputes. + * + * ## [#25668] …except where a stored expression reads `owner_id`: by row + * + * A stored computed field — a non-formula field declaring an `expression` — + * is computed by the engine on every write, and a predicate write stores ONE + * value per such field for every matched row. A predicate write that moves the + * field to different values on different rows is refused whole, nothing + * written, and the refusal is the engine's contract, not a defect: its message + * says "update those rows by id". The claim's patch moves only `owner_id`, so + * that refusal reaches it exactly when a stored expression reads `owner_id` + * (`record.owner_id`, or one hop through it, `record.owner_id.name`) and the + * rows compute different values — every row, every boot, because the next + * pass sent the same write. HotCRM's `crm_forecast.display_title` is the shape. + * + * So an object whose stored expressions read `owner_id` + * ({@link storedExpressionReadsOwner}) is claimed ROW BY ROW: one + * compare-and-set write per row, `{ , id }` with + * `multi: true`, which the engine evaluates as a predicate over that one row + * and computes from that row. The owner term stays in the write, so a row + * someone owned in the meantime is not taken from them. The NULL arm's rows + * are the seeded ids it already names; the `usr_system` arm reads its ids a + * page at a time ({@link CLAIM_PAGE_ROWS}), the bound the paged fallback reads + * by. Every other object keeps the predicate write above: this is the cost of + * the row-by-row write — a full engine write per row — paid only where one + * write cannot store the result. A row whose own write is refused stays + * unowned and is reported with the per-predicate warning, and the object's + * other rows are still claimed. */ +import { analyzeRelationshipTraversals } from '@objectstack/formula'; import type { ServiceObject } from '@objectstack/spec/data'; import { BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE, MAX_BULK_PER_ROW_HOOK_ROWS } from '@objectstack/spec/data'; import { SystemUserId } from '@objectstack/spec/system'; @@ -311,6 +339,52 @@ function hasOwnerField(schema: ServiceObject): boolean { return Object.prototype.hasOwnProperty.call(fields, 'owner_id'); } +/** + * The `record.` names one stored expression reads — a hop counts as a + * read of the field it goes through — or `null` when they cannot be named + * (another dialect, or a source that does not parse). + * + * Read with the platform's one member reader (`analyzeRelationshipTraversals`), + * the reader the engine's own stored-expression planning uses, so the two + * cannot disagree about which spellings name a field. + */ +function expressionReads(expression: unknown): ReadonlySet | null { + const expr = typeof expression === 'string' ? { source: expression } : expression as { dialect?: unknown; source?: unknown }; + if (!expr || (expr.dialect !== undefined && expr.dialect !== 'cel') || typeof expr.source !== 'string') return null; + const analysis = analyzeRelationshipTraversals(expr.source); + return analysis ? new Set([...analysis.bareFields, ...analysis.traversals.keys()]) : null; +} + +/** + * [#25668] Does a stored computed field of this object read `owner_id` — + * directly, one hop through it, or through another stored field that does? + * + * A stored field is the spec's: a non-`formula` field declaring an + * `expression`, computed and stored on every write. Planned off a field MAP, + * as the engine plans it; an expression whose reads cannot be named counts as + * a reader, because claiming such an object row by row costs time, while + * sending it the predicate write leaves every row unowned. + */ +function storedExpressionReadsOwner(schema: ServiceObject): boolean { + const fields: any = (schema as any)?.fields; + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return false; + const stored = Object.entries(fields as Record).filter( + ([, def]) => def && typeof def === 'object' && def.type !== 'formula' && def.expression, + ); + const moved = new Set(['owner_id']); + for (let grew = true; grew;) { + grew = false; + for (const [name, def] of stored) { + if (moved.has(name)) continue; + const reads = expressionReads(def.expression); + if (reads !== null && ![...reads].some((read) => moved.has(read))) continue; + moved.add(name); + grew = true; + } + } + return moved.size > 1; +} + /** * The affected-row count a predicate write resolved, or `undefined` when the * result is not one. @@ -463,6 +537,92 @@ async function claimPredicate( return total; } +/** + * [#25668] Re-own the given rows of one unowned predicate one write each: the + * predicate's own terms plus the row's id, a compare-and-set the engine + * evaluates over that one row. Never throws for a row: a refused row stays + * unowned and is reported once for the batch, with the first refusal's message. + */ +async function claimRows( + io: ObjectWriter, + objectName: string, + where: Record, + ids: readonly string[], + logger: ClaimOwnershipOptions['logger'], +): Promise<{ claimed: number; countUnknown: boolean }> { + let claimed = 0; + let countUnknown = false; + const refused: string[] = []; + let firstError: string | undefined; + for (const id of ids) { + try { + const count = affectedRowCount(await io.reown({ ...where, id })); + if (count === undefined) countUnknown = true; + else claimed += count; + } catch (e) { + refused.push(id); + firstError ??= (e as Error).message; + } + } + if (refused.length > 0) { + logger?.warn?.( + `[security] claimSeedOwnership failed for ${objectName}; ${refused.length} row(s) stay unowned until the ` + + 'claim next runs — the next seed settle (`app:seeded`, on this boot or a later one) or the next ' + + 'platform-admin promotion', + { object: objectName, where: whereForLog(where), refused: refused.length, ids: refused.slice(0, 20), error: firstError }, + ); + } + if (countUnknown) { + logger?.warn?.( + `[security] claimSeedOwnership could not read an affected-row count for ${objectName} ` + + '— rows were re-owned but this run cannot say how many', + { object: objectName, where: whereForLog(where) }, + ); + } + return { claimed, countUnknown }; +} + +/** + * [#25668] Re-own every row matching one unowned predicate ROW BY ROW — the + * path for an object whose stored expressions read `owner_id` + * ({@link storedExpressionReadsOwner}), where one predicate write cannot store + * a value per row. + * + * The NULL arm names its rows (`id IN (seeded ids)`, one chunk), so they are + * written without a read. The `usr_system` arm names none, so its ids are read + * a page at a time and written, until a read finds none; a page that re-owns + * nothing ends the pass, because re-reading would return the same rows. Returns + * the sum of the affected-row counts the writes resolved. + */ +async function claimPredicateByRow( + io: ObjectWriter, + objectName: string, + where: Record, + logger: ClaimOwnershipOptions['logger'], +): Promise { + const named = (where.id as { $in?: unknown } | undefined)?.$in; + if (Array.isArray(named)) { + return (await claimRows(io, objectName, where, named.map(String), logger)).claimed; + } + let total = 0; + for (let page = 0; page < MAX_CLAIM_PAGES; page += 1) { + const ids = idsFrom(await io.readPage(where)); + if (ids.length === 0) return total; + const { claimed, countUnknown } = await claimRows(io, objectName, where, ids, logger); + total += claimed; + // Unknown counts cannot tell a page that moved from one that did not, and + // a page that moved nothing would be read again: stop either way. + if (countUnknown || claimed === 0) return total; + } + logger?.warn?.( + `[security] claimSeedOwnership stopped after ${MAX_CLAIM_PAGES} page(s) on ${objectName}; ` + + 'unowned rows may remain until the claim next runs — the next seed settle (`app:seeded`, on this ' + + 'boot or a later one) or the next platform-admin promotion', + { object: objectName, where: whereForLog(where), pages: MAX_CLAIM_PAGES }, + ); + return total; +} + /** * Say what this pass did AND whether its reading was FINAL — once, always. * @@ -586,8 +746,9 @@ function reportClaimPass( * off ({@link CLAIM_WRITE_CTX}), in one predicate write per * {@link unownedPredicates} entry — the seeded ids' chunks under the NULL arm, * then the seed identity — paging that write only when the engine refuses it - * for its per-row hook budget. Returns a per-object summary whose `count` is - * the sum of every write's affected-row count. + * for its per-row hook budget, and [#25668] one write per row instead on an + * object whose stored expressions read `owner_id`. Returns a per-object summary + * whose `count` is the sum of every write's affected-row count. */ export async function claimSeedOwnership( ql: any, @@ -646,11 +807,14 @@ export async function claimSeedOwnership( const seededIds = seededRecordIds?.(schema.name) ?? []; seededRows += seededIds.length; + // [#25668] One predicate write cannot store a per-row value of a stored + // field the claim's `owner_id` moves; such an object is claimed by row. + const claim = storedExpressionReadsOwner(schema) ? claimPredicateByRow : claimPredicate; let updated = 0; for (const where of unownedPredicates(seededIds)) { try { - updated += await claimPredicate(io, schema.name, where, logger); + updated += await claim(io, schema.name, where, logger); } catch (e) { // Best-effort per predicate, exactly as the per-id loop was: one // predicate that cannot land must not cost the object its other one, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 05c6e984678..375eb1a7aac 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1558,6 +1558,9 @@ importers: '@objectstack/core': specifier: workspace:* version: link:../../core + '@objectstack/formula': + specifier: workspace:* + version: link:../../formula '@objectstack/plugin-auth': specifier: workspace:* version: link:../plugin-auth