diff --git a/.changeset/systemfields-owner-guidance-org-skips-owner-id.md b/.changeset/systemfields-owner-guidance-org-skips-owner-id.md new file mode 100644 index 0000000000..c910bfbb91 --- /dev/null +++ b/.changeset/systemfields-owner-guidance-org-skips-owner-id.md @@ -0,0 +1,55 @@ +--- +"@objectstack/spec": patch +--- + +fix(spec): the `systemFields.owner` rescue no longer tells authors that `ownership: 'org'` picks a different principal (#6365) + +`systemFields` has never declared an `owner` key, and the field doc above the +block names one — so an author (or an AI writing metadata) who follows that +prose lands on the block's `guidance.owner` prescription. That prescription +said: + +> `owner_id` injection is governed by the object-level `ownership` property +> (`ownership: 'none'` skips it; `'user'`/`'org'` choose the principal). + +The second half was false. `'org'` does not choose a different principal — it +injects **no** `owner_id` at all. The authority `applySystemFields` consumes, +`resolveInjectedSystemColumns` (`packages/spec/src/data/injected-system-columns.ts`), +admits exactly two spellings: + +```ts +const owner = ownershipEligible && (ownership === undefined || ownership === 'user'); +``` + +and the `ownership` property's own JSDoc, ~90 lines above the guidance, already +said so correctly (`org` / `none` — no per-record owner; `owner_id` is NOT +injected). The guidance was the wrong side of that contradiction. + +Why it was worth fixing rather than leaving as prose drift: this is the text an +author is handed at the exact moment they are already confused about where owner +injection is configured, and it sent them to `ownership: 'org'` expecting an +org-keyed owner column. Nothing rejects `ownership: 'org'`, so the mistake +ships silently and every owner-keyed feature quietly does nothing — +owner-scoped RLS, "My" views, owner reports, the first-admin bootstrap handoff. +That is the failure mode the `guidance` machinery exists to prevent, inverted: +a wrong-key rescue handing out a second wrong answer. + +The rescue now states the injection rule as the authority implements it — +`'user'` (or omitted) injects `owner_id`; `'org'` and `'none'` **both** skip it +and no `owner_id` is injected at all — while keeping the two skipping values +visibly distinct in intent (`'org'` for an org-wide catalog, `'none'` for a +junction/link table), since that distinction is the reason the enum carries +both. + +The sibling `guidance.ownership` message is widened in the same pass. It was not +wrong, only out of date: since #5677 / ADR-0117 D1 the `ownership` property +governs **both** record-ownership anchors, so the message now says it decides +whether `owner_id` **and** `owning_business_unit_id` are injected, rather than +naming only the first. + +Text only — no acceptance change. Every value `ObjectSchema` accepted before is +accepted now, every value it rejected is still rejected, and the injection +behaviour is untouched. The new pin tests assert the message's substance against +`resolveInjectedSystemColumns` rather than echoing the sentence, so the +prescription can only stay green while it still describes what the injection +pass really does. diff --git a/packages/spec/src/data/object-strictness-batch20.test.ts b/packages/spec/src/data/object-strictness-batch20.test.ts index 17191b3350..846258e99d 100644 --- a/packages/spec/src/data/object-strictness-batch20.test.ts +++ b/packages/spec/src/data/object-strictness-batch20.test.ts @@ -47,6 +47,7 @@ import { IndexSchema, defineObjectExtension, } from './object.zod'; +import { resolveInjectedSystemColumns } from './injected-system-columns'; import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; /** Reject `value` through `schema` and return its issues as a searchable string. */ @@ -270,6 +271,54 @@ describe('#4001 批 20 — curation is anchored to the sibling contract that mak accept(ObjectSchema, { ...OBJ, ownership: 'none' }); }); + // #6365 — the wrong-key rescue used to hand out a SECOND wrong answer. It + // said `'user'`/`'org'` "choose the principal", so an author who wanted an + // org-owned record was sent to `ownership: 'org'` — which injects no + // `owner_id` at all, and nothing rejects it, so every owner-keyed feature + // (owner-scoped RLS, "My" views, owner reports, the first-admin bootstrap + // handoff) quietly does nothing. The assertions below are anchored to the + // injection authority rather than to the sentence, so the prescription can + // only stay green while it still describes what really happens. + it("`systemFields.owner`'s prescription matches the injection authority — `'org'` SKIPS `owner_id`, it does not pick a different principal (#6365)", () => { + // The authority `applySystemFields` consumes (`resolveInjectedSystemColumns`). + // `'org'` sits with `'none'` on the withheld side, not opposite it. + expect(resolveInjectedSystemColumns({ ...OBJ, ownership: 'org' }).owner).toBe(false); + expect(resolveInjectedSystemColumns({ ...OBJ, ownership: 'none' }).owner).toBe(false); + expect(resolveInjectedSystemColumns({ ...OBJ, ownership: 'user' }).owner).toBe(true); + expect(resolveInjectedSystemColumns(OBJ).owner, 'omitted behaves as `user`').toBe(true); + + const msg = rejectOnObject({ systemFields: { tenant: true, owner: false } }); + // …and the message says exactly that: the two skipping values are named + // TOGETHER, and the absence is stated as an absence. + expect(msg).toContain("`'org'` and `'none'` BOTH skip it"); + expect(msg).toContain('no `owner_id` is injected at all'); + // The retired claim, pinned by name so it cannot come back by paraphrase + // of the same idea — `'org'` as a second *principal*. + expect(msg).not.toContain('choose the principal'); + expect(msg).not.toContain('principal'); + // …while `'org'` and `'none'` stay visibly DISTINCT in intent, which is + // the reason the enum carries both (the `ownership` JSDoc's own split: + // Dataverse-style catalog vs junction table). + expect(msg).toContain('org-wide catalog'); + expect(msg).toContain('junction/link'); + }); + + it('`systemFields.ownership` names BOTH ownership anchors — since #5677 the property governs `owning_business_unit_id` too (#6365)', () => { + const msg = rejectOnObject({ systemFields: { tenant: true, ownership: 'none' } }); + expect(msg).toContain('TOP-LEVEL'); + expect(msg).toContain('owner_id'); + expect(msg).toContain('owning_business_unit_id'); + + // The claim, against the authority: across the whole AUTHORABLE enum the + // two anchors move together — injected under `'user'`/omitted, withheld + // under `'org'`/`'none'`. (ADR-0117 D1's fourth tier is the one case that + // splits them, and it is deliberately unauthorable today — #5678.) + for (const ownership of [undefined, 'user', 'org', 'none'] as const) { + const plan = resolveInjectedSystemColumns({ ...OBJ, ownership }); + expect(plan.owningBusinessUnit, `ownership: ${String(ownership)}`).toBe(plan.owner); + } + }); + it('`external.allowWrites` names the DOUBLE opt-in — the datasource half and the object half', () => { const msg = rejectOnObject({ external: { remoteName: 'r', allowWrites: true } }); expect(msg).toContain('writable'); diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 09d044a62f..b00fbca326 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -1391,12 +1391,15 @@ const ObjectSchemaBase = z.object({ // here. `ownership` is the real, enforced lever. owner: '`owner` is not a `systemFields` key — `owner_id` injection is governed by the ' + - "object-level `ownership` property (`ownership: 'none'` skips it; " + - "`'user'`/`'org'` choose the principal). `systemFields` controls only `tenant` " + - '(organization_id) and `audit` (created_at/created_by/updated_at/updated_by).', + "object-level `ownership` property: `'user'` (or omitted) injects it, while " + + "`'org'` and `'none'` BOTH skip it and no `owner_id` is injected at all — " + + "`'org'` for an org-wide catalog (Dataverse-style), `'none'` for a junction/link " + + 'table. `systemFields` controls only `tenant` (organization_id) and `audit` ' + + '(created_at/created_by/updated_at/updated_by).', ownership: '`ownership` is a TOP-LEVEL object key, not a `systemFields` key — write it ' + - 'beside `systemFields`. It, not this block, decides whether `owner_id` is injected.', + 'beside `systemFields`. It, not this block, decides whether the ownership ' + + 'anchors (`owner_id` and `owning_business_unit_id`) are injected.', }, }, { tenant: z.boolean().optional().describe('Inject the organization_id column. Default true (the column is always provisioned; the multi-tenant flag governs only its index).'),