From 4a957797a6bc325f919618b80d6c5eda9da27f6c Mon Sep 17 00:00:00 2001 From: os-warren Date: Tue, 1 Sep 2026 16:05:55 +0000 Subject: [PATCH 1/3] Seed business-unit membership and let the platform derive the projection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sys_user.primary_business_unit_id` is a denormalised projection of `sys_business_unit_member.is_primary`, maintained by plugin-sharing (ADR-0057 addendum D12). The demo seed wrote the projection directly and left the junction empty, so the value survived only because no membership row had ever existed to fire the recompute hook. The seed now writes the source rows — one primary membership per person, keyed on the composite `['user_id', 'business_unit_id']` so a replay dedupes — and stops declaring the column at all. `sys_user.manager_id` is left as a direct write: it is `readonly` for a different reason (its own admin surface, ADR-0092), and no code in the platform recomputes it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- package.json | 2 + pnpm-lock.yaml | 6 + src/data/index.ts | 15 +- src/data/org.seed.ts | 127 ++++++++++-- test/business-unit-membership.test.ts | 265 ++++++++++++++++++++++++++ 5 files changed, 395 insertions(+), 20 deletions(-) create mode 100644 test/business-unit-membership.test.ts diff --git a/package.json b/package.json index acb939a..4a8fde1 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,9 @@ }, "devDependencies": { "@objectstack/cli": "^17.2.0", + "@objectstack/platform-objects": "^17.2.0", "@objectstack/plugin-email": "^17.2.0", + "@objectstack/plugin-sharing": "^17.2.0", "@objectstack/service-automation": "^17.2.0", "@objectstack/service-job": "^17.2.0", "@objectstack/service-messaging": "^17.2.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 11aa839..ac05eb1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -30,9 +30,15 @@ importers: '@objectstack/cli': specifier: ^17.2.0 version: 17.2.0(@better-auth/utils@0.4.2)(@better-fetch/fetch@1.3.1)(better-call@1.4.0(zod@4.5.4))(kysely@0.29.5)(mongodb@7.6.0)(nanostores@1.5.2)(vitest@4.1.11(vite@8.2.2(esbuild@0.28.2)(tsx@4.23.13)(yaml@2.9.0))) + '@objectstack/platform-objects': + specifier: ^17.2.0 + version: 17.2.0(vitest@4.1.11(vite@8.2.2(esbuild@0.28.2)(tsx@4.23.13)(yaml@2.9.0))) '@objectstack/plugin-email': specifier: ^17.2.0 version: 17.2.0(vitest@4.1.11(vite@8.2.2(esbuild@0.28.2)(tsx@4.23.13)(yaml@2.9.0))) + '@objectstack/plugin-sharing': + specifier: ^17.2.0 + version: 17.2.0(vitest@4.1.11(vite@8.2.2(esbuild@0.28.2)(tsx@4.23.13)(yaml@2.9.0))) '@objectstack/service-automation': specifier: ^17.2.0 version: 17.2.0 diff --git a/src/data/index.ts b/src/data/index.ts index b0fd53b..8feb606 100644 --- a/src/data/index.ts +++ b/src/data/index.ts @@ -19,7 +19,7 @@ import { assignmentSeed } from './assignment.seed.js'; import { catalogSeed } from './catalog.seed.js'; import { dutySeed } from './duty.seed.js'; import { logEntrySeed } from './log-entry.seed.js'; -import { businessUnitSeed, userSeed } from './org.seed.js'; +import { businessUnitMemberSeed, businessUnitSeed, userSeed } from './org.seed.js'; import { taskAdHocSeed, taskAdHocTouchSeed, @@ -29,6 +29,7 @@ import { export { assignmentSeed, + businessUnitMemberSeed, businessUnitSeed, catalogSeed, dutySeed, @@ -58,6 +59,16 @@ export { * know that to see why the seed works. (#32: without the user rows, every * task row is refused with `Owner is required` — measured, 0 inserted, 4 * errored.) + * - **`sys_business_unit_member` comes THIRD — after both of them.** It is + * the junction between the two, so both endpoints must exist before its + * `user_id` / `business_unit_id` natural keys can resolve; a reference that + * resolves to nothing on a `required: true` column takes the whole row with + * it, exactly as `owner` does above. Same ordering rule as the bullet + * above, one level further in — which is why it is stated here rather than + * invented as a second convention. (#74: it is also the dataset that makes + * `sys_user.primary_business_unit_id` exist at all — plugin-sharing derives + * the projection from these rows; nothing writes that column directly any + * more. See `org.seed.ts`.) * - **The two `mode: 'update'` task passes come LAST, after both inserts.** * Datasets targeting the same object keep their relative order through the * sort (it is stable), and these two only work if the rows they backdate @@ -74,8 +85,10 @@ export { */ export const demoSeeds: Seed[] = [ // 1. The org, first — everything below resolves its people and units here. + // The junction is third because it references the two above it. businessUnitSeed, userSeed, + businessUnitMemberSeed, // 2. What roles owe, and who owes it. catalogSeed, diff --git a/src/data/org.seed.ts b/src/data/org.seed.ts index 402b3ff..29412f7 100644 --- a/src/data/org.seed.ts +++ b/src/data/org.seed.ts @@ -5,19 +5,20 @@ import type { Seed } from '@objectstack/spec/data'; import { ADMIN, PEOPLE, UNITS, emailOf } from './demo-org.js'; /** - * The org: `sys_business_unit` and `sys_user`. + * The org: `sys_business_unit`, `sys_user`, and the membership junction + * between them. * - * ── Why these two are plain `Seed` literals and not `defineSeed(...)` ───── - * `defineSeed` infers its record keys from an `ObjectSchema`, and both objects - * here are the PLATFORM's, declared in `@objectstack/platform-objects`, not in - * `src/objects/`. There is no schema in this repo to hand it. Inventing a local - * stand-in to satisfy the signature would be worse than typing the literal: it - * would read as this app's description of a table it does not own, and would - * silently stop matching the day the platform adds a column. Every `duly_*` - * dataset in this directory does use `defineSeed`. + * ── Why these are plain `Seed` literals and not `defineSeed(...)` ───────── + * `defineSeed` infers its record keys from an `ObjectSchema`, and all three + * objects here are the PLATFORM's, declared in `@objectstack/platform-objects`, + * not in `src/objects/`. There is no schema in this repo to hand it. Inventing + * a local stand-in to satisfy the signature would be worse than typing the + * literal: it would read as this app's description of a table it does not own, + * and would silently stop matching the day the platform adds a column. Every + * `duly_*` dataset in this directory does use `defineSeed`. * - * Field names are the platform's actual ones (`manager_id`, - * `primary_business_unit_id`, `manager_user_id`, `parent_business_unit_id`) — + * Field names are the platform's actual ones (`manager_id`, `manager_user_id`, + * `parent_business_unit_id`, `user_id`, `business_unit_id`, `is_primary`) — * not guesses. `sys_user` has no `username` column, so there is none here. * * ── These must be seeded FIRST, and it is not a style preference ────────── @@ -30,21 +31,26 @@ import { ADMIN, PEOPLE, UNITS, emailOf } from './demo-org.js'; * barrel lists them first so the ordering is legible without knowing that. * * ── One asymmetry worth knowing before you read a test ──────────────────── - * References FROM a `duly_*` object INTO these two always resolve: the + * References FROM a `duly_*` object INTO these objects always resolve: the * reference is declared on the `duly_*` schema, which this app owns, so the * loader looks the target up in the database by name and finds it. * - * References BETWEEN these two — `sys_user.manager_id`, - * `sys_user.primary_business_unit_id`, `sys_business_unit.manager_user_id` and - * `parent_business_unit_id` — only resolve where the platform objects are - * actually registered. Under `objectstack dev` they are (`serve` mounts - * `PlatformObjectsPlugin`), so the org chart and the manager chain link up. - * Under the bare `createStandaloneStack` kernel the vitest suites boot, they + * References BETWEEN the platform objects — `sys_user.manager_id`, + * `sys_business_unit.manager_user_id`, `parent_business_unit_id`, and the + * junction's `user_id` / `business_unit_id` — only resolve where the platform + * objects are actually registered. Under `objectstack dev` they are (`serve` + * mounts `PlatformObjectsPlugin`, and `sharing` is in + * `PLATFORM_ALWAYS_ON_CAPABILITIES` so `SharingServicePlugin` is mounted too), + * so the org chart, the manager chain and the membership rows all link up. + * Under the bare `createStandaloneStack` kernel most vitest suites boot, they * are not registered at all: the loader finds no field definitions for * `sys_user`, builds no reference list for it, and writes the natural key * through verbatim. That is why `test/seed.test.ts` asserts the manager chain * from THIS module rather than from the seeded rows — the fixture is the * contract; what a reference column resolves to is the runtime's business. + * `test/business-unit-membership.test.ts` is the suite that boots the two + * platform plugins on purpose, because the value it reads back is one only + * they compute. */ /** Three levels: one company, three sites, two teams under one of them. */ @@ -77,6 +83,33 @@ export const businessUnitSeed: Seed = { * one field the seed declares unchanged, and SKIPS without writing. Adding an * `email` or a `manager_id` here would turn that skip into an UPDATE against a * live credential-bearing account. See `demo-org.ts` for the full reasoning. + * + * ── `manager_id` is written directly; `primary_business_unit_id` is NOT ──── + * Both columns are `readonly: true` on `sys_user`, and reading that as one + * fact is the mistake #74 was filed about. `readonly` marks two different + * things and only one of them fights back: + * + * - **`primary_business_unit_id` is a projection another component owns.** + * `@objectstack/plugin-sharing` recomputes it from + * `sys_business_unit_member.is_primary` — `primary-bu-projection.ts` binds + * afterInsert/afterUpdate/afterDelete hooks on the junction and runs a + * `backfillPrimaryBu` sweep at every plugin start (ADR-0057 addendum D12). + * Those hooks fire for system-context writes too, deliberately: "the + * projection must stay correct regardless of who mutates membership + * (seeds, HRIS sync, admin UI)". So this seed writes the SOURCE — see + * {@link businessUnitMemberSeed} — and lets the platform derive the + * column. It is not declared here at all. + * - **`manager_id` is a projection of nothing.** It is `readonly` because + * org-structure maintenance is its own admin surface (ADR-0092 — + * `SYS_USER_PROFILE_EDIT_FIELDS` deliberately excludes it), not because + * something recomputes it. Measured on `@objectstack/*` 17.2.0: the only + * writes to `sys_user.primary_business_unit_id` anywhere in the platform + * are the two `engine.update` calls in `primary-bu-projection.ts`, and + * there is NO writer of `manager_id` at all — every occurrence in the + * plugins is a read (`fields: ['id', 'manager_id']` in + * `business-unit-graph.ts`, `team-graph.ts`, `approval-service.ts`). So + * the direct system-context write below is the sanctioned way to seed it, + * and there is no junction to write instead. */ export const userSeed: Seed = { object: 'sys_user', @@ -88,7 +121,63 @@ export const userSeed: Seed = { name: person.name, email: emailOf(person.name), manager_id: person.manager, - primary_business_unit_id: person.unit, })), ], }; + +/** + * Which unit each person belongs to — the SOURCE `sys_user. + * primary_business_unit_id` is derived from. + * + * ── Why this dataset exists at all ──────────────────────────────────────── + * Before #74 the seed set `sys_user.primary_business_unit_id` directly and + * left this junction empty. That worked, and the reason it worked is the + * reason it had to change: plugin-sharing recomputes the projection from + * `sys_business_unit_member`, our table had zero rows, so no hook ever fired + * and the hand-written value simply survived. The app looked correct because + * the mechanism that would correct it had never been triggered. Two ways that + * ends, neither of them loud: + * + * - Someone writes a membership row — console, import, a later feature, a + * customer's own setup — and the hook recomputes THAT user's projection + * from the junction. The seeded value is replaced by whatever the junction + * says, or cleared when the new row is not primary. `assignment.flow.ts` + * reads `primary_business_unit_id` to stamp `duly_task.business_unit` on + * fan-out, so a cleared projection silently stamps nothing. + * - Anything resolving people THROUGH membership rather than through the + * projection sees nobody in any unit — sharing rules and hierarchy scopes + * being the obvious candidates. Our permission sets happen to read the + * projection today, which is luck, not design. + * + * ── Twelve rows, not thirteen ───────────────────────────────────────────── + * `Dev Admin` gets no membership row, for the same reason their `sys_user` + * row carries nothing but a name: a membership row would make the projection + * hook UPDATE the live credential-bearing account. The reachable state is + * unchanged from before this dataset existed — twelve users with a primary + * unit, the admin without one — which is what makes this a change of + * MECHANISM and not of data. + * + * ── The composite external id is what makes a replay idempotent ─────────── + * `sys_business_unit_member` has no single-column natural key (no `name`, and + * its unique index is `(business_unit_id, user_id)`), so the dataset is keyed + * on both foreign keys — the spelling `Seed.externalId` documents for exactly + * this case: "a join / junction table keyed by both of its foreign keys … + * The reference fields are matched by their RESOLVED ids, so a composite of + * foreign keys dedupes correctly across restarts." With a single-field key + * (or none) the dataset would fall back to `mode: 'insert'` semantics and + * duplicate the whole table on every boot. + * + * `is_primary` is stated on every row even though the platform defaults it to + * `true`: it is the exact flag the projection reads, and a seed that leaned + * on the default would leave the one load-bearing column implicit. + */ +export const businessUnitMemberSeed: Seed = { + object: 'sys_business_unit_member', + externalId: ['user_id', 'business_unit_id'], + mode: 'upsert', + records: PEOPLE.map((person) => ({ + user_id: person.name, + business_unit_id: person.unit, + is_primary: true, + })), +}; diff --git a/test/business-unit-membership.test.ts b/test/business-unit-membership.test.ts new file mode 100644 index 0000000..c87f99e --- /dev/null +++ b/test/business-unit-membership.test.ts @@ -0,0 +1,265 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { AppPlugin, ObjectKernel, createStandaloneStack } from '@objectstack/runtime'; +import { PlatformObjectsPlugin } from '@objectstack/platform-objects/plugin'; +import { SharingServicePlugin } from '@objectstack/plugin-sharing'; + +import { PEOPLE } from '../src/data/demo-org.js'; +import { businessUnitMemberSeed, userSeed } from '../src/data/org.seed.js'; + +/** + * `sys_user.primary_business_unit_id` is DERIVED, and this suite is the one + * that proves it. + * + * ── Why this file exists apart from `test/seed.test.ts` ─────────────────── + * Every other suite boots the bare `createStandaloneStack` kernel, which + * mounts exactly three plugins — datasource, metadata, objectql. Neither + * `@objectstack/platform-objects` (which declares `sys_user` and + * `sys_business_unit_member`) nor `@objectstack/plugin-sharing` (which owns + * the projection) is in it, so in those suites the platform tables are + * schemaless memory collections and nothing computes anything: the seed's + * natural keys are written through verbatim and read back unchanged. + * + * That is fine for what those suites assert, and useless for this one. A real + * `objectstack dev` boot mounts BOTH — `serve` auto-registers + * `PlatformObjectsPlugin`, and `sharing` is in + * `PLATFORM_ALWAYS_ON_CAPABILITIES` so `SharingServicePlugin` is mounted + * whether or not the app names it in `requires`. This suite mounts the same + * two, because the value under test is one only they produce. + * + * ── The assertion that decides whether the fix is real ──────────────────── + * The defect (#74) was that the seed wrote `sys_user.primary_business_unit_id` + * directly and left `sys_business_unit_member` empty. That seed passes any + * test that reads the column back and compares it to what the seed said — the + * value is there, it is correct, and it is correct for the wrong reason: no + * membership row has ever existed, so the recompute hook has never fired. + * + * So `follows the junction` below does not read the seeded value at all. It + * MOVES a membership row to another business unit and asserts the projection + * moves with it, then moves it back and asserts it comes back. Nothing a seed + * writes can satisfy that; only a live hook can. It is the same property + * `packages/qa/dogfood/test/primary-bu-projection.dogfood.test.ts` asserts + * upstream, restated against this app's own data. + */ + +const DEMO_SEED_ENV_VAR = 'DULY_DEMO_SEED'; +const SYSTEM = { isSystem: true } as const; + +let kernel: any; +let data: any; + +const all = async (object: string): Promise => + (await data.find(object, {}, { context: SYSTEM })) ?? []; + +const one = async (object: string, where: Record): Promise => { + const rows = (await data.find(object, { where }, { context: SYSTEM })) ?? []; + expect(rows.length, `exactly one ${object} matching ${JSON.stringify(where)}`).toBe(1); + return rows[0]; +}; + +/** Poll until `check` is true, or fail naming what was actually observed. */ +const settle = async (label: string, check: () => Promise, ms = 120_000): Promise => { + const deadline = Date.now() + ms; + let last: string | null = 'never evaluated'; + for (;;) { + last = await check(); + if (last === null) return; + if (Date.now() > deadline) throw new Error(`${label} did not settle in ${ms}ms — ${last}`); + await new Promise((resolve) => setTimeout(resolve, 250)); + } +}; + +beforeAll(async () => { + // The demo is opt-in and the gate is read at module-evaluation time, so the + // variable must be set before anything imports the config. Same reasoning, + // and the same literal, as `test/seed.test.ts`. + vi.stubEnv(DEMO_SEED_ENV_VAR, '1'); + const stack = (await import('../objectstack.config.js')).default as unknown as Record; + + const { plugins } = await createStandaloneStack({ + databaseDriver: 'memory', + skipSeedData: true, + // Point the artifact lookup at a path that cannot exist, or a local + // `pnpm build` leaves `dist/objectstack.json` where the kernel loads + // metadata from the last BUILD rather than from the config imported above. + artifactPath: 'dist/objectstack.this-suite-must-not-load-an-artifact.json', + }); + kernel = new ObjectKernel(); + for (const plugin of plugins) await kernel.use(plugin); + // The two the bare standalone stack leaves out, in the order `serve` mounts + // them: the object declarations first, then the plugin whose hooks act on + // them. + await kernel.use(new PlatformObjectsPlugin()); + await kernel.use(new SharingServicePlugin()); + await kernel.use(new AppPlugin(stack as any, undefined, { skipSeedData: false })); + await kernel.bootstrap(); + data = kernel.getService('data'); + + // The inline seed is raced against a budget rather than awaited by + // bootstrap, so wait for the junction to be fully populated rather than for + // a fixed delay. + await settle('the membership seed', async () => { + const rows = await all('sys_business_unit_member'); + return rows.length >= PEOPLE.length ? null : `${rows.length} of ${PEOPLE.length} member rows`; + }); +}, 240_000); + +afterAll(async () => { + await kernel?.shutdown?.(); + vi.unstubAllEnvs(); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('the seed writes membership, not the projection', () => { + it('declares no primary_business_unit_id anywhere in the user dataset', () => { + // The direct write is the defect. A seed record carrying this column would + // land — the loader writes under `{ isSystem: true }`, which exempts it + // from the readonly strip — and would then be silently replaced the first + // time anything touched that user's membership. Absence is the fix, so + // absence is what is pinned. + const offenders = (userSeed.records as Record[]).filter( + (record) => 'primary_business_unit_id' in record, + ); + expect(offenders.map((record) => record.name)).toEqual([]); + }); + + it('writes one primary membership row per person, and none for the admin', async () => { + const members = await all('sys_business_unit_member'); + expect(members.length, 'one row per person in PEOPLE').toBe(PEOPLE.length); + expect(members.every((row) => row.is_primary === true), 'every seeded row is the primary one').toBe(true); + // One row per user, or "the primary one" is not a single answer. + expect(new Set(members.map((row) => String(row.user_id))).size).toBe(PEOPLE.length); + // `Dev Admin` is deliberately absent: a membership row for them would make + // the projection hook UPDATE the live credential-bearing account, which is + // the one thing `userSeed` is shaped to avoid. See `org.seed.ts`. + expect(businessUnitMemberSeed.records.length).toBe(PEOPLE.length); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('the projection is derived from it', () => { + it('every person reads back the unit their membership row names', async () => { + // Read BOTH sides out of the database and compare them to each other — + // never to the fixture. The fixture is what the seed said; these two rows + // are what the platform did with it. + for (const person of PEOPLE) { + const user = await one('sys_user', { name: person.name }); + const member = await one('sys_business_unit_member', { user_id: String(user.id) }); + expect( + String(user.primary_business_unit_id ?? ''), + `${person.name}: projection vs membership`, + ).toBe(String(member.business_unit_id)); + // And it is the unit the fixture meant — otherwise both sides could + // agree on a wrong value. + const unit = await one('sys_business_unit', { id: String(member.business_unit_id) }); + expect(unit.name, `${person.name}'s unit`).toBe(person.unit); + } + }); + + it('follows the junction when a membership moves, and back again', async () => { + // The assertion the old seed could not have passed. Nothing here reads a + // seeded value: the projection is compared against a membership row this + // test moves at runtime, so it can only match if plugin-sharing's + // afterUpdate hook recomputed it. + const person = PEOPLE.find((candidate) => candidate.manager)!; + const user = await one('sys_user', { name: person.name }); + // Captured before the move; asserted unchanged after it. See the + // `manager_id` describe below for why that half is here at all. + const managerBefore = user.manager_id ?? null; + const member = await one('sys_business_unit_member', { user_id: String(user.id) }); + const home = String(member.business_unit_id); + + const elsewhere = (await all('sys_business_unit')).find((unit) => String(unit.id) !== home); + expect(elsewhere, 'the fixture must contain a second unit to move to').toBeTruthy(); + const away = String(elsewhere.id); + expect(away).not.toBe(home); + + const readProjection = async (): Promise => + String((await one('sys_user', { name: person.name })).primary_business_unit_id ?? ''); + + // Baseline, so a projection that was ALREADY `away` cannot read as a pass. + expect(await readProjection(), 'baseline before the move').toBe(home); + + try { + await data.update( + 'sys_business_unit_member', + { id: String(member.id), business_unit_id: away }, + { context: SYSTEM }, + ); + await settle('the projection after the move', async () => { + const now = await readProjection(); + return now === away ? null : `still ${now || '(empty)'}, expected ${away}`; + }, 30_000); + } finally { + // Restore, and assert the restore — the projection following BOTH ways + // is the difference between a live hook and a one-off coincidence, and + // leaving the row moved would hand every later assertion a mutated tree. + await data.update( + 'sys_business_unit_member', + { id: String(member.id), business_unit_id: home }, + { context: SYSTEM }, + ); + } + await settle('the projection after the restore', async () => { + const now = await readProjection(); + return now === home ? null : `still ${now || '(empty)'}, expected ${home}`; + }, 30_000); + + // The other half of the same observation: `manager_id` sat through both + // moves untouched. One membership write moved one column and not the + // other, in one kernel, at one moment — which is the difference between + // the two `readonly` columns stated as a measurement rather than as a + // claim about the platform's source. + const after = await one('sys_user', { name: person.name }); + expect(after.manager_id ?? null, 'manager_id is not a projection of membership').toEqual(managerBefore); + }, 120_000); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('manager_id is NOT the same kind of column', () => { + it('is still declared by the seed, and primary_business_unit_id is not', () => { + // Both columns are `readonly: true` on `sys_user`, and reading that as one + // fact is exactly the inference #74 was filed about. `manager_id` is + // readonly because org-structure maintenance is its own admin surface + // (ADR-0092 — `SYS_USER_PROFILE_EDIT_FIELDS` excludes it), not because + // anything recomputes it: measured on `@objectstack/*` 17.2.0, the only + // writes to `primary_business_unit_id` in the whole platform are the two + // `engine.update` calls in plugin-sharing's `primary-bu-projection.ts`, + // and there is NO writer of `manager_id` anywhere — every occurrence in + // the plugins is a read. So one of these columns is seeded through its + // source table and the other is seeded directly, and that asymmetry is + // deliberate. + const declared = (userSeed.records as Record[]) + .filter((record) => record.name !== 'Dev Admin'); + expect(declared.length).toBe(PEOPLE.length); + expect(declared.every((record) => 'manager_id' in record), 'manager_id is written directly').toBe(true); + expect(declared.some((record) => 'primary_business_unit_id' in record), 'the projection is not').toBe(false); + }); + + it('lands on the row, with the top of the chain terminating', async () => { + // ⚠️ What `manager_id` HOLDS here is not asserted, and the omission is the + // point. This kernel mounts `PlatformObjectsPlugin` and + // `SharingServicePlugin`, which between them declare + // `sys_business_unit_member` — so the junction's `user_id` / + // `business_unit_id` resolve to real ids above. Neither declares + // `sys_user` (that is `plugin-auth`, on a real `objectstack dev` boot), + // so the loader builds no reference list for it and writes `manager_id`'s + // natural key through VERBATIM here. `test/seed.test.ts` states the same + // boundary: the fixture is the contract, and what a reference column + // resolves to is the runtime's business. `pnpm demo` is where the + // resolved chain is measured. + // + // What IS asserted is the part that holds in every kernel: the column is + // populated for everyone who has a manager, and empty for the one person + // who does not. + const managed = PEOPLE.filter((person) => person.manager); + expect(managed.length).toBeGreaterThanOrEqual(10); + for (const person of managed) { + const user = await one('sys_user', { name: person.name }); + expect(Boolean(user.manager_id), `${person.name} carries a manager`).toBe(true); + } + const top = await one('sys_user', { name: PEOPLE.find((person) => !person.manager)!.name }); + expect(top.manager_id ?? null, 'the chain terminates rather than pointing at itself').toBeNull(); + }); +}); From c8b854475ec2246a015ac2e201cbc146a5321f74 Mon Sep 17 00:00:00 2001 From: os-warren Date: Tue, 1 Sep 2026 16:09:51 +0000 Subject: [PATCH 2/3] Record which readonly columns a seed may write, and which it may not `isSystem` exempts every readonly column, so the seed-writes-history section needed the other half: a column another component recomputes is written through its SOURCE table, not directly. Uses the two `sys_user` columns that sit in the same field group with the same flag and are opposite cases. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- AGENTS.md | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 6dd7368..8afceab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -340,6 +340,48 @@ column is protected — assert against `protocol.createData` when the refusal is thing you care about. Flows are unaffected today because `assignment.flow.ts` declares `runAs: 'system'`, which is elevated regardless. +### `isSystem` is a key to history, not a key to other people's columns + +`{ context: { isSystem: true } }` exempts a write from the readonly strip. That +is what makes the section above work, and it is the whole mechanism — which +means it exempts **every** `readonly` column, not just the ones this app owns. +So it is a licence to write *history*, and it is not a licence to write a column +another component maintains. + +**`readonly: true` marks two different things, and only one of them fights +back.** + +| What the flag means | Example | How you seed it | +|:--------------------|:--------|:----------------| +| **A component owns this column and recomputes it.** There is a source table, and a hook derives this value from it. | `sys_user.primary_business_unit_id` — plugin-sharing recomputes it from `sys_business_unit_member.is_primary` (ADR-0057 addendum D12) | **Write the source.** Seed the rows the projection is computed from and let the platform derive the column. | +| **Nothing recomputes it; its maintenance is just somebody else's surface.** No hook, no source table — the flag keeps it off the ordinary edit form. | `sys_user.manager_id` — `readonly` because org-structure maintenance is its own admin surface (ADR-0092); `completed_at`, for that matter | **Write it directly**, from a system context, exactly as above. There is nothing else to write. | + +The first kind fails in a way no gate catches, because it does not fail at +write time at all. A direct write lands, reads back correct, and survives every +boot **for as long as the source table stays empty** — the recompute has simply +never had an input. The day anything writes one source row for that record, the +hook fires and replaces your value with whatever the source says, or clears it. +Nothing errors. Measured on #74: twelve users carried a hand-written +`primary_business_unit_id` and `sys_business_unit_member` had 0 rows, for as +long as the seed had existed. + +**How to tell which kind you are looking at**, before you write it: + +1. Read the column's `description` in `@objectstack/platform-objects`. The first + kind says so — "a denormalised projection of …, maintained by …. Do not edit + directly; set it via …". Take that sentence literally; it is not style. +2. Grep the platform for a **writer**: `grep -rn "\s*:" packages/plugins` + in the monorepo. The first kind has one (an `engine.update` in the plugin + that owns it) and a hook that calls it. The second kind has only reads. +3. If it has a writer, find what that writer reads **from**. That table is what + your seed writes. + +⛔ Do not generalise from one column to its neighbours. `manager_id` and +`primary_business_unit_id` sit next to each other in the same `Organization` +field group with the same `readonly: true`, and they are opposite cases. "Both +are readonly, so treat both the same" is precisely the inference that produced +the defect. + ## Product invariants — do not "improve" these away These are the product, not preferences. If a task seems to require breaking one, From 675579ff821a51e79013fcf48cd68b1637e89f10 Mon Sep 17 00:00:00 2001 From: os-warren Date: Tue, 1 Sep 2026 16:23:28 +0000 Subject: [PATCH 3/3] Count the junction in the seed replay assertion `sys_business_unit_member` is the one seeded object with no single-column natural key, so its replay safety rests entirely on the composite externalId. Nothing was counting it, so a dataset that could not match its own rows would have re-inserted all twelve on every boot and reported success. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- test/seed.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/seed.test.ts b/test/seed.test.ts index be027a9..2be6994 100644 --- a/test/seed.test.ts +++ b/test/seed.test.ts @@ -438,6 +438,13 @@ describe('idempotence', () => { const objects = [ 'sys_business_unit', 'sys_user', + // The junction (#74). It is the one dataset here with no single-column + // natural key, so its replay safety rests entirely on the composite + // `externalId: ['user_id', 'business_unit_id']` — and a dataset that + // cannot match its own rows re-inserts every one of them on every boot, + // reporting success each time. Counted here so that is a red test rather + // than a table that grows twelve rows per restart. + 'sys_business_unit_member', 'duly_catalog_item', 'duly_duty', 'duly_assignment',