fix(plugin-sharing): a seeded business unit is a usable sharing-rule recipient, and its members are tenant-screened

[os-sales]

Fixes #14547

Implements recommendation A as ruled: the null-inclusive orgScope and a tenant screen on the member queries, in one PR, preceded by the write-path measurement the triage names as its prerequisite. Draft, as the ruling directs.

Re-lands the work of the closed PR #14572, which was closed unmerged for a jurisdiction reason and not a technical one. Every measurement below was re-run against today's origin/main rather than carried forward.

1. The prerequisite measurement, re-run

Question: are sys_business_unit_member rows organization-stamped on every write path? Answer: no. Re-measured at origin/main 431979e67:

write path	stamped?	evidence
REST / session write	yes	the engine threads the caller's tenantId into DriverOptions; SqlDriver.injectTenantOnInsert fills the injected column
seed replay	no	packages/metadata-protocol/src/seed-loader.ts:926 withholds the single-org fallbackOrgId from every object matching `/^(sys_
elevated (system-context) write	no	sys_business_unit_member is absent from PLATFORM_OBJECT_TENANCY, so it classifies unclassified and Engine.resolveSystemInsertOrganization (packages/objectql/src/engine.ts:3944) returns early
driver-memory / driver-mongodb	no	neither implements tenant stamping; both refuse to boot multi-tenant, which is what makes that safe

main has moved since the first round took this reading — notably #14484 landed and added sys_record_share to PLATFORM_OBJECT_TENANCY as tenant-scoped. That entry covers sys_record_share and no other table, so it does not move the member-row answer; the residual classification gap for sys_business_unit_member is tracked at #14570 and is not closed here.

Because the answer is still "no", the two screens are asymmetric. A NULL organization on a unit row is the documented platform/seeded class; a NULL on a membership row means unknown tenancy.

2. The premise still holds

Verified before the first edit, at 431979e67:

• business-unit-graph.ts — orgScope is still the strict equality, verbatim as the card quotes it;
• seedIsUsable still runs it as the first screen, so an org-NULL unit reads as "does not exist";
• both member reads — expandUnitMembers and expandUsers — still query sys_business_unit_member with no organization predicate at all, under a SYSTEM_CTX that carries no tenant. The member queries are unscoped by organization.

3. What changed

The UNIT screen is now null-inclusive. orgScope emits (organization_id = rule org OR organization_id IS NULL) — the predicate SqlDriver.applyTenantScope writes, whose own comment names business units among the populations its NULL arm exists for (#2734). The same predicate is already written twice in this codebase for these rows: SharingRuleService.adminOrgScope (#7676) and ApprovalService.businessUnitOrgScope (#3807). This file was the outlier, and sharing-rule-service.ts names the mistake in prose while this file made it.

The MEMBER screen is new, and strict. A new memberScope applies organization_id = rule org to both member reads. This is the half that matters most: the strict unit screen was the only thing keeping an org-stamped rule away from those unscoped queries, so widening the unit screen alone would have converted a silent under-grant into a silent cross-tenant over-grant — a seeded unit id exists identically in every tenant. Strict rather than null-inclusive because of the measurement in section 1: a grant fails closed on unknown tenancy. The sibling recipient widths already read their membership rows this way (TeamGraphService screens sys_team_member, PositionGraphService screens sys_user_position, both strict).

Every recipient width was checked, not only the narrow one the card names. business_unit and unit_and_subordinates both route through this graph and both carried the hole; user, team, position and queue do not reach it.

An active business-unit rule that expands to nobody now warns, once per rule per process, naming the rule, the object, the recipient kind, the unit and the organization. That is the half independent of any screen — the reported failure was not merely that the expansion was empty, it was that nothing recorded it — and it also covers the case the screens deliberately leave empty: a rule whose unit and membership rows were both seeded still grants nobody, and now says so.

4. Tests — the security half pinned apart from the functional half

A fix that expands the right members while also expanding another organization's members satisfies the functional pin completely, so the two are pinned in separate blocks and demonstrated separately by ablation.

Two stale pins were replaced, not flipped. business-unit-graph.test.ts carried a [divergence] test asserting that an org-scoped rule does not see a null-org unit, on the stated grounds that the case was unreachable. It was reachable — that is this card. Both it and the narrow width is org-predicated exactly like the wide one would have kept passing after the fix for a different reason (the member screen answering where the unit screen used to), which is the class of assertion that stops guarding anything; they are rewritten to state which screen answers.

Membership fixtures in sharing-rule.test.ts and business-unit-graph.test.ts gained organization_id. They were org-less while their units and rules were org-stamped, and expanded anyway — precisely because the member read had no predicate. The fixture became faithful; no assertion was relaxed.

Ablations, direction predicted before running

ablation	predicted	observed
A — orgScope back to the strict equality	RED on the functional and seeded-unit pins; sharing-rule.test.ts green (its units are org-stamped, strict still matches)	13 failed / 147 passed, exactly that set
B — memberScope removed from both member reads	RED on the security pins only; the functional WIDE/NARROW grant pins stay GREEN	9 failed / 151 passed, exactly that set

B's asymmetry is the evidence the security half is independently pinned: removing the member screen leaves every functional assertion green and reds only the cross-tenant ones.

Both ablations proved the mutation reached disk before reading any result (removed-text occurrence count driven to 0, injected marker counted, mutated blob hash differing from the HEAD blob — a // comment was never used, esbuild strips them) and proved each restore (blob hash equal to the HEAD blob and an empty git diff HEAD). The mutation was applied to a committed implementation, so the restore leg had something true to restore to. Ablation validity: these suites import the subject through same-package relative specifiers, so vitest resolves them from src and no dist sits between the mutation and the run.

Runs

At final head 7e27ab512:

• pnpm --filter @objectstack/plugin-sharing build — check-dts-emitted: 1/1 declared declaration file(s) present
• pnpm --filter @objectstack/plugin-sharing test — Test Files 32 passed (32) · Tests 790 passed (790)
• pnpm --filter @objectstack/plugin-sharing typecheck — green, and it covers the test layer: the package excludes *.test.ts from tsconfig.json, but its typecheck script chains check:test-typecheck, which reported OK — the test layer compiles under tsconfig.test.json. So the edited test files are measured, not merely adjacent to a green check.

5. Gate family

Derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, re-derived after the commit that added the docs file: 66 commands. All 66 run; every exit code captured by redirect-then-read, never across a pipe.

61 green. 0 findings. Five are NOT MEASURED — each refuses to reach a verdict without a whole-tree build, in its own words, and CI supplies that build:

gate	exit	its own verdict
check-test-completeness.mjs	3	"Nothing was measured... It is NOT a finding"
check:dual-build-cjs-loads	3	"PREREQUISITE NOT MET... This is NOT a pass: nothing was measured"
check:type-check-debt	3	"NOT a pass and NOT a finding: nothing was measured"
check:i18n	1	"Nothing was checked: no bundle was compared and no config was parsed"
check:skill-examples	1	"packages/client-react/dist holds no .d.ts declarations... a verdict now would be a FALSE GREEN"

The last two exit 1 rather than 3, so they read like findings and are not; their own text is quoted above so a reviewer need not re-derive that.

check:system-context-census did red — pure line rot, my insertion in sharing-rule-service.ts shifted two isSystem read sites. Repaired only with the gate's own pnpm check:system-context-census --fix, never by hand; it re-anchored :157 to :165 and :382 to :390 and now reports OK — 109 elevation read sites... all anchored.

No new rows were added to scripts/engine-double-contract.pinned.json: the end-to-end tests were written into recipient-width.test.ts, whose engine double is already pinned for both write verbs, rather than into a new file that would have needed fresh ledger entries.

6. Clause-② (contract review): no

Measured, not asserted — built dist/index.d.ts on both sides of the diff (BASE source built, restored, verified by src-tree hash equality and an empty git diff HEAD):

• exported symbol set: 84 before, 84 after — zero added, zero removed;
• top-level export/declare lines: 62 before, 62 after;
• the only three non-comment .d.ts additions are private readonly emptyUnitExpansionSeen, private warnOnEmptyUnitExpansion and private memberScope. TypeScript emits private members as name-only placeholders; they are not callable, nameable or assignable from outside the class, so they are neither an exported symbol nor a payload key. SharingRuleEvaluationResult is untouched, and the warn's structured fields go to a logger sink, not a result type.

This disagrees with the prior round's shape, deliberately. PR #14572 exposed a public getter emptyUnitExpansionRuleKeys on SharingRuleService. That is a new exported symbol and would have flipped clause-② to yes. It is dropped here — the once-per-rule dedup is asserted through the logger spy the tests already use — so the whole diff stays internal to plugin-sharing.

7. Out of scope, filed not fixed

#14946 — plugin-approvals has the identical defect one plugin over: ApprovalService.expandBusinessUnitUsers screens the unit rows null-inclusively (since #3807) but reads sys_business_unit_member at approval-service.ts:1771 with no organization predicate, under a SYSTEM_CTX carrying no tenant. On a seeded org chart a department approver on one tenant's request resolves to another tenant's users. Not fixed here: different package, different tests, different gate surface. That issue carries the evidence and the suggested asymmetric shape.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-sharing, touching 9 documentable anchor(s).

⛔ 1 release-owned page(s) name something this change touched. These are read-only:

• content/docs/releases/implementation-status.mdx (via expandRecipient (symbol, a method of class SharingRuleService))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json aae0cb395f50c5491442540bbb453265519a1e57 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from d82c7543db408fcc8e54656d30e611eace07d52d — the merge of head 7e27ab5122974b2970ce69d7c295223425a987d8 into base aae0cb395f50c5491442540bbb453265519a1e57, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d82c7543db408fcc8e54656d30e611eace07d52d && git checkout d82c7543db408fcc8e54656d30e611eace07d52d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin aae0cb395f50c5491442540bbb453265519a1e57 7e27ab5122974b2970ce69d7c295223425a987d8 && git checkout -B drift-repro aae0cb395f50c5491442540bbb453265519a1e57 && git merge --no-ff 7e27ab5122974b2970ce69d7c295223425a987d8

node scripts/docs-audit/affected-docs.mjs --json aae0cb395f50c5491442540bbb453265519a1e57

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs aae0cb395f50c5491442540bbb453265519a1e57 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-sales]

Docs Drift Check disposed — ⛔ do not edit the release page, and no issue is owed either

domain:services execution seat (reviewer of record). Measured on origin/main so this does not cost the dev a round, and posted now because the drift row names a release-owned page and I want the ⛔ unambiguous before anyone is tempted.

⛔ The page is read-only, full stop

content/docs/releases/implementation-status.mdx is under content/docs/releases/, which is written centrally at release time. A code PR editing it is precisely what that guardrail exists to stop, and it is also the repo's hottest merge-conflict magnet. Do not touch it in this PR — not to correct it, not to soften it, not as a rider.

And the escape hatch does not apply, because the line is not wrong

The guardrail's remedy when a release page is actually wrong is to file an issue. I checked whether one is owed. It is not. The anchoring line (:438) reads:

every authorable recipient maps 1:1 onto an enforced expandRecipient branch (plugin-sharing/sharing-rule-service.ts) — user, team, position, business_unit, and unit_and_subordinates …

That is a claim about authoring-surface completeness: every recipient a user can author has a branch that implements it. This card's defect is that two of those branches — business_unit and unit_and_subordinates — grant nobody when the unit row carries organization_id = NULL. A defect inside a branch does not make "there is a branch for every authorable recipient" false; the mapping the sentence asserts holds both before and after this PR.

⇒ The honest reading is the same shape as PR #14930's drift row, and in the same direction: this PR moves reality toward what the page already claims, rather than away from it. After it lands, "enforced" is more true than it is today, not less. Filing an issue to report that a sentence slightly overclaims in exactly the edge case an open priority:p1 card is fixing would be noise, and would age badly the moment this merges.

Control

expandRecipient → 1 occurrence in that page, at :438; the row is a correct precision-first match on a real anchor. Nothing else in the file names it.

⚠️ One thing the drift bot itself flags and I am passing on rather than swallowing: it can only list pages sharing an identifier with the diff. A page that states this rule by its inputs — "a business-unit recipient grants its members" — shares no token with expandRecipient and cannot appear on any run. So the bot's list is not a clearance. If your change alters what a BU recipient resolves to in a way a prose page restates without naming the symbol, that page is still yours to find by hand. Worth one targeted look given this card's fix is precisely about which members a unit resolves to.

---

Generated by Claude Code

[os-sales]

ACCEPT — the asymmetric pair is verified at the tree, and the security half is pinned independently of the functional half

domain:services execution seat, reviewer of record, session session_01AUF1NoViznQK32gqpK8wS8. Head 7e27ab512.

⛔ Verified against the repository, not against the report. The load-bearing claim on a priority:p1 security card is that the two screens are asymmetric in the right direction, so I read the shipped file rather than the prose:

screen	population	predicate	sites
orgScope	sys_business_unit — the anchor the rule names	null-inclusive: $or: [{ organization_id: <rule org> }, { organization_id: null }] (:95)	:104, :137
memberScope	sys_business_unit_member — the set being granted	strict equality, with a docblock ⛔ stating it is not orgScope	:185, :216

Both member reads carry it — expandUnitMembers (:185) and expandUsers (:216). That was the specific trap in the dispatch order: screening the unit alone would have converted a silent under-grant into a silent cross-tenant over-grant, because both member reads previously queried with no organization predicate at all under a tenant-less system context. It is closed at both.

What earns the ACCEPT beyond the code being right

• The prerequisite was re-measured, not inherited. The order required this explicitly because main had moved since the closed PR fix(plugin-sharing): a seeded business unit is a usable rule recipient, and its members are tenant-screened (#14547) #14572. The answer came back unchanged — member rows are still not organization-stamped on the seed and elevated-system paths — with the mechanism named (seed-loader.ts withholding fallbackOrgId from the sys_/cloud_/ai_ namespaces; the object absent from PLATFORM_OBJECT_TENANCY, so resolveSystemInsertOrganization returns early). And it correctly scoped what finding: every sys_record_share grant row lands organization_id NULL — SharingService writes under a bare system context and the row literal never carries the column #14484 changed: sys_record_share only, which is why the asymmetry survives.
• The ablation legs are asymmetric, and that asymmetry is the evidence. Leg A (unit screen reverted) → 13 failed / 147 passed, reddening the functional and seeded-unit pins while sharing-rule.test.ts stayed green. Leg B (member screens removed) → 9 failed / 151 passed, reddening the security pins while the functional WIDE/NARROW grant pins stayed green. Neither red set is a subset of the other, which is precisely what shows the two screens are two independent facts rather than one restated.
• Two stale pins were REPLACED, not flipped, with the right reason: both "would have kept passing for a different reason after the fix." A pin that passes for the wrong reason is worse than one that fails, because it silently stops testing what it was written to test. Catching that without being asked is the difference between running the tests and reading them.
• Clause-② no, measured and now legible. 84 exported symbols before and after, zero added and zero removed, the only three non-comment .d.ts additions being private members — and the dev deliberately dropped closed PR fix(plugin-sharing): a seeded business unit is a usable rule recipient, and its members are tenant-screened (#14547) #14572's public emptyUnitExpansionRuleKeys getter to keep the whole diff internal. check-clause2-carriers --pair 14949 exits 0. ⛔ No needs:contract-review carrier is owed and none is hung.

The out-of-scope finding is the most valuable thing in this round

#14946 — plugin-approvals carries the identical defect one package over: ApprovalService.expandBusinessUnitUsers screens the unit rows null-inclusively (since #3807) but reads sys_business_unit_member at approval-service.ts:1771 with no organization predicate under a tenant-less system context. On a seeded org chart, a department approver on one tenant's request resolves to another tenant's users.

Filed rather than fixed in place, and the judgement is right: different package, so it adds a verification surface and may be held by another lane. ⛔ Not folded in here.

Landing

CI green on 7e27ab512 read job-level, not aggregate: 34 runs, 32 success + 2 skipped, zero failures, zero pending. git merge-tree --write-tree against current origin/main exits 0 with zero CONFLICT lines. Undrafting and arming auto-merge.

---

Generated by Claude Code