Seed business-unit membership and let the platform derive the projection - #104
Merged
Merged
Conversation
`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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #74
sys_user.primary_business_unit_idis a denormalised projection ofsys_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.tsbindsafterInsert/afterUpdate/afterDeleteonsys_business_unit_memberand runs abackfillPrimaryBusweep at every plugin start, "regardless ofenforce— 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)".sharingis inPLATFORM_ALWAYS_ON_CAPABILITIES, soSharingServicePluginis mounted on everyobjectstack devboot of this app — the boot banner below lists it.The defect was latent, not already firing:
backfillPrimaryBuonly sets from rows that exist, it never blanks, so an empty junction updated nobody. What ends that, neither loudly:src/flows/assignment.flow.tsreadsprimary_business_unit_idto stampduly_task.business_uniton fan-out, so a cleared projection silently stamps nothing.Scope
src/data/org.seed.tswritessys_business_unit_member— one row per person,is_primary: true— and stops declaringsys_user.primary_business_unit_idat all. The dataset is keyedexternalId: ['user_id', 'business_unit_id'], the composite spellingSeed.externalIddocuments 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_memberhas no single-column natural key, and without a composite one the dataset would duplicate the whole table on every boot — measured, see ablation C.test/business-unit-membership.test.tsasserts the derived value — see below.src/data/index.tsextends the existing ordering note rather than inventing a second convention: the junction goes third, after both endpoints, for the same reasonsys_usergoes first.sys_user.manager_idis NOT a projection — checked, and left aloneBoth columns are
readonly: trueand sit in the sameOrganizationfield group, and "both are readonly, so treat both the same" is the wrong inference. Measured on@objectstack/*17.2.0:primary_business_unit_idanywhere in the platform are the twoengine.updatecalls inprimary-bu-projection.ts.manager_idat all. Every occurrence in the plugins is a read (fields: ['id', 'manager_id']inbusiness-unit-graph.ts,team-graph.ts,approval-service.ts). It isreadonlybecause org-structure maintenance is its own admin surface — ADR-0092,SYS_USER_PROFILE_EDIT_FIELDSexcludes it — not because anything recomputes it.So
manager_idstays 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 leavesmanager_iduntouched.AGENTS.mdgains that as the reusable lesson, next to the seed-writes-history section:isSystemexempts 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 againreads 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 propertypackages/qa/dogfood/test/primary-bu-projection.dogfood.test.tsasserts upstream, restated against this app's data.The suite mounts
PlatformObjectsPlugin+SharingServicePluginon purpose (added as devDependencies): the barecreateStandaloneStackkernel 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/TERMtrap;git statusclean after each. The confirmation is agrep -con 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) →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) →
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
externalIdline 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:All twelve rows re-inserted on the second pass, with the load reporting success. That is what the third commit closes:
sys_business_unit_memberis 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 demoon 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:Before this change the same database read
member rows: 0with 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 Admingets no membership row, deliberately: one would make the projection hook UPDATE the live credential-bearing account, the one thinguserSeedis 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:They were green at
c8b8544too, which is the tree thepnpm demoverification above ran on; the third commit touches onlytest/seed.test.ts.The
hierarchy-securitycapability-provider warning onvalidateis the documented expected state for this repo, not a regression. No changeset — this repo has no changeset mechanism.Generated by Claude Code