Skip to content

Seed business-unit membership and let the platform derive the projection - #104

Merged
os-warren merged 3 commits into
mainfrom
claude/issue-74-business-unit-membership
Sep 1, 2026
Merged

Seed business-unit membership and let the platform derive the projection#104
os-warren merged 3 commits into
mainfrom
claude/issue-74-business-unit-membership

Conversation

@os-warren

@os-warren os-warren commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes #74

sys_user.primary_business_unit_id is a denormalised projection of sys_business_unit_member.is_primary, maintained by @objectstack/plugin-sharing (ADR-0057 addendum D12). The demo seed wrote the projection directly and never wrote a single junction row — so the value survived only because the recompute hook had never had an input to fire on.

What was measured

plugin-sharing/src/primary-bu-projection.ts binds afterInsert / afterUpdate / afterDelete on sys_business_unit_member and runs a backfillPrimaryBu sweep at every plugin start, "regardless of enforce — it is a data projection, not an access-control surface". Those hooks fire for system-context writes too, deliberately: "the projection must stay correct regardless of who mutates membership (seeds, HRIS sync, admin UI)". sharing is in PLATFORM_ALWAYS_ON_CAPABILITIES, so SharingServicePlugin is mounted on every objectstack dev boot of this app — the boot banner below lists it.

The defect was latent, not already firing: backfillPrimaryBu only sets from rows that exist, it never blanks, so an empty junction updated nobody. What ends that, neither loudly:

  • Someone writes one membership row — console, import, a later feature, a customer's own setup — and the hook recomputes that user's projection from the junction, replacing the seeded value or clearing it if the new row is not primary. src/flows/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.

Scope

  1. src/data/org.seed.ts writes sys_business_unit_member — one row per person, is_primary: true — and stops declaring sys_user.primary_business_unit_id at all. The dataset is keyed externalId: ['user_id', 'business_unit_id'], the composite spelling Seed.externalId documents for exactly this case ("a join / junction table keyed by both of its foreign keys … matched by their RESOLVED ids, so a composite of foreign keys dedupes correctly across restarts"). sys_business_unit_member has no single-column natural key, and without a composite one the dataset would duplicate the whole table on every boot — measured, see ablation C.
  2. test/business-unit-membership.test.ts asserts the derived value — see below.
  3. src/data/index.ts extends the existing ordering note rather than inventing a second convention: the junction goes third, after both endpoints, for the same reason sys_user goes first.

sys_user.manager_id is NOT a projection — checked, and left alone

Both columns are readonly: true and sit in the same Organization field group, and "both are readonly, so treat both the same" is the wrong inference. Measured on @objectstack/* 17.2.0:

  • The only writes to primary_business_unit_id anywhere in the platform are the two engine.update calls in primary-bu-projection.ts.
  • 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). It 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.

So manager_id stays a direct system-context write; there is no source table to write instead. The suite pins that asymmetry behaviourally: one membership move changes the projection and leaves manager_id untouched.

AGENTS.md gains that as the reusable lesson, next to the seed-writes-history section: isSystem exempts every readonly column, so it is a licence to write history, not a licence to write a column another component owns and recomputes — with the two-kinds table and the three checks (read the description, grep for a writer, find what that writer reads from).

The test asserts the derived value, not the written one

A test that reads the column back and compares it to what the seed said passes just as well against the broken code — which is why the defect survived. So follows the junction when a membership moves, and back again reads no seeded value at all: it moves a membership row to another unit, asserts the projection moves with it, moves it back, and asserts it comes back. Nothing a seed writes can satisfy that. It is the property packages/qa/dogfood/test/primary-bu-projection.dogfood.test.ts asserts upstream, restated against this app's data.

The suite mounts PlatformObjectsPlugin + SharingServicePlugin on purpose (added as devDependencies): the bare createStandaloneStack kernel every other suite boots mounts only datasource/metadata/objectql, where the platform tables are schemaless memory collections and nothing computes anything.

Reverse-verification — three ablations, each with the mutation confirmed on disk

Run from a committed state, restored by an EXIT/INT/TERM trap; git status clean after each. The confirmation is a grep -c on the exact text being changed, never an editor's exit code.

A. the pre-fix seed against the new suite (git checkout origin/main -- src/data/org.seed.ts src/data/index.ts; on disk: primary_business_unit_id: person.unit = 1 hit, businessUnitMemberSeed = 0 hits in both files) →

Error: the membership seed did not settle in 120000ms — 0 of 12 member rows
 Test Files  1 failed (1)      Tests  6 skipped (6)

B. the "belt and braces" regression — keep the junction, re-add the direct write (on disk: injected line = 1 hit, junction still wired = 3 hits) →

× declares no primary_business_unit_id anywhere in the user dataset
    AssertionError: expected [ 'Nadia Ilves', 'Tomas Bergh', …(10) ] to deeply equal []
× is still declared by the seed, and primary_business_unit_id is not
    AssertionError: the projection is not: expected true to be false
 Tests  2 failed | 4 passed (6)

B is worth stating plainly: with the junction populated, the derived-value assertions pass even with the direct write restored, because the hook wins. So the declaration guard is what pins "stopped writing it directly", and the move test is what pins "the hook is live". Both are load-bearing; neither is redundant.

C. drop the composite external id — delete the externalId line so the dataset falls back to the default 'name', which the junction records do not carry (on disk: composite line = 0 hits, dataset still wired = 3 hits) → test/seed.test.ts:

× re-running the whole seed over a populated database duplicates nothing
    AssertionError: a replay must insert nothing: expected 12 to be +0
 Tests  1 failed | 24 passed (25)

All twelve rows re-inserted on the second pass, with the load reporting success. That is what the third commit closes: sys_business_unit_member is now counted in the existing replay assertion, which was listing every other seeded object but not this one.

Verified with the real seed, not only in unit tests

rm -rf .objectstack/data && pnpm demo on its own port. Boot banner: Plugins: 41 loaded … PlatformObjects, Auth, … SharingServicePlugin, Seeds: ai.objectstack.duly 471 rows (was 459 — the twelve junction rows). Read back out of .objectstack/data/objectstack.db, joining the two sides against each other rather than against the fixture:

sys_business_unit_member rows       : 12   (all is_primary)
sys_user WITH primary_business_unit : 12
people with a membership row        : 12
of those, projection agrees with it : 12
projection set WITHOUT a member row : []
manager_id resolved to a real user  : 11   (Nadia is the top of the chain; Dev Admin has none)

Before this change the same database read member rows: 0 with the same twelve projections. The reachable state is unchanged; the mechanism is not — which is what makes this a fix and not a data edit.

Dev Admin gets no membership row, deliberately: one would make the projection hook UPDATE the live credential-bearing account, the one thing userSeed is shaped to avoid. They had no projection before and have none now.

Gates

All four green at 675579f — this PR's head, re-run after the third commit — under the shared verify lock:

pnpm validate   ✓ Validation passed (372ms)
pnpm typecheck  tsc --noEmit, no output
pnpm test       Test Files 28 passed (28)   Tests 680 passed (680)
pnpm build      ✓ Build complete (593ms)

They were green at c8b8544 too, which is the tree the pnpm demo verification above ran on; the third commit touches only test/seed.test.ts.

The hierarchy-security capability-provider warning on validate is the documented expected state for this repo, not a regression. No changeset — this repo has no changeset mechanism.


Generated by Claude Code

os-warren and others added 3 commits September 1, 2026 16:10
`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p
`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p
`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p
@os-warren
os-warren marked this pull request as ready for review September 1, 2026 16:27
@os-warren
os-warren merged commit 485c0c1 into main Sep 1, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The seed writes sys_user.primary_business_unit_id directly and leaves sys_business_unit_member empty — the platform owns that column and recomputes it

1 participant