fix(plugin-sharing): recompute unit_and_subordinates grants on business-unit tree and membership changes

[os-help]

Fixes#7729

The defect

Business-unit subtree expansion for unit_and_subordinates sharing rules already resolved correctly through three levels and was symmetric on placement, so the tree was never the problem. The defect was timing: the placement or re-parent alone moved nothing. Grants only materialised, or de-materialised, when the shared record was next written.

Measured matrix from the QA run: inSubtree=1, reparentedOut(no touch)=1, reparentedOut(touched)=0, restored(no touch)=0, restored(touched)=1.

The security column is the second one. After a business unit is moved out of a shared subtree, its members keep read access — and keep it until somebody happens to write the shared record, which puts no bound at all on how long a revoked recipient keeps reading.

Root cause confirmed on origin/main by symbol: bindRuleHooks in rule-hooks.ts registers its recompute hooks scoped to each rule's own object_name, and a repo-wide search finds no hook registered on sys_business_unit anywhere. The only hooks on sys_business_unit_member are in primary-bu-projection.ts, which maintains the sys_user.primary_business_unit_id projection and never touches the rule recompute.

The fix

New bu-tree-recompute.ts binds afterInsert / afterUpdate / afterDelete on both business-unit tables, under its own package id so unbindAllRuleHooks (which every rule rebind calls) cannot tear it down. It carries no rule snapshot — the handler reads listRules() live — so a runtime-authored rule is picked up with no rebind wiring.

⚠️Scope, so nothing wider is inferred: this recompute fires only for rules whose recipients read the business-unit graph — unit_and_subordinates and business_unit, and only those two. A user, team, position or queue rule is not recomputed by a business-unit write at all. It is emphatically not the case that every rule now recomputes on every BU write; that would close the security hole and replace it with a load problem.

This applies the #4779 ruling on a new axis and reuses its seam rather than adding a second one:

• Revoke — synchronous and complete. A new SharingRuleService.revokeRuleGrantsForRetiredRecipients re-expands the rule's recipients and set-deletes the grants of everyone who dropped out. Scoped by recipient, which is what makes it affordable on a write path: one grant query, one subtree walk, one member query and a chunked delete per affected rule, with no record scan — so the cost does not grow with how many records the rule matches. A unit moved out of a shared subtree has lost its access before the write returns.
• Re-grant — asynchronous and coalesced. A unit moved into a shared subtree needs up to (matched records x new members) new grants, which is exactly the fan-out that must not sit on a write path. It is queued on the existing ruleRegrantQueue as a full evaluateRule pass, at most one outstanding per rule.

That split follows the maintainer's standing direction on this subsystem: over-granting is a security incident, under-granting is an availability wobble. The revocation window is therefore zero; only the grant direction is deferred, and kernel:bootstrapped's backfill repairs a re-grant lost to a crash.

Recipient kinds audited, not assumed

expandRecipient is the one switch that decides, and it reads the business-unit graph for two of the six kinds:

recipient_type	resolver	reads the BU graph
user	the literal id	no
team	TeamGraphService (sys_team_member, sys_member, sys_user)	no
business_unit	BusinessUnitGraphService.expandUsers	yes
position	PositionGraphService (sys_user_position, sys_member)	no
unit_and_subordinates	BusinessUnitGraphService.expandUsers	yes
queue	returns an empty list	no

business_unit is in scope on the strength of what the code does today: expandRecipient routes it through the sameexpandUsers call as unit_and_subordinates, so it walks the subtree and is equally exposed. Covering it is correct under either reading of its intended semantics — even a unit-only expansion still reads sys_business_unit for its own active flag and sys_business_unit_member for its members.

Everything else is deliberately not recomputed at all — not more cheaply, not at all. That exclusion is a requirement rather than an optimisation, and it is asserted directly.

Docs

content/docs/permissions/sharing-rules.mdx enumerated withdrawal as happening at exactly three moments, in a definitive table. This change adds a fourth, so that list would otherwise have been incomplete on precisely the topic this card is about. The count word is fixed and a row added, at the table's existing granularity, stating the asymmetry (revocation before the write returns; the grant direction queued and coalesced per rule) and scoping itself to the two BU-reading recipient kinds.

Two things checked in that file rather than assumed, both deliberately left unchanged:

• The "always recoverable from the API surface" paragraph below the table. Its subject is an over-granting rule recovered by switching it off or deleting it — the rule-write axis, which this change does not touch — so it stays exactly true. Its "not on the next time somebody happens to touch the record" phrasing is in fact the same house line the new row echoes.
• The recipient table's business_unit row ("exactly that business unit (no subtree)"). That states the declared contract and matches the spec enum and the ADR-0105 lint red-line. The runtime diverges by walking the subtree for both BU kinds — filed separately as sharing rules: the business_unit recipient expands the whole SUBTREE, but the spec declares it as exactly one unit (no subtree) #7807 — so rewriting the doc to match the runtime would document an over-grant as intended behaviour. The doc stays; sharing rules: the business_unit recipient expands the whole SUBTREE, but the spec declares it as exactly one unit (no subtree) #7807 owns the resolution.

sharing-service.mdx says nothing about recipient kinds or the BU graph. Absent is not wrong, so it is left alone.

Verification

• pnpm --filter @objectstack/plugin-sharing test — 446 passed / 17 files, on top of a merged origin/main. typecheck clean.
• pnpm --filter @objectstack/plugin-security test — 973 passed / 47 files (covers sys_record_share materialisation and the write-path composition).
• showcase-bu-hierarchy-sharing.dogfood.test.ts + sharing-rule-criteria-required.dogfood.test.ts — 9 passed. These boot the real showcase stack and insert BU and membership rows under a system context, which is precisely where the new hooks fire.
• pnpm check:engine-double-contract — OK, 150 pinned (the new fake is pinned to the producer's dispatch predicates for both write verbs). check:docs-audit-scope OK. check:nul-bytes OK.

Ablation. Predicted first, then run: neutering the hook registration should flip the new behaviour and binding cases red while every pre-existing rule-recompute pin stays green. Measured, twice, identically: 10 failed / 426 passed, all 10 in the new file, all 16 pre-existing files fully green. The cases that stayed green are the ones that should — inSubtree (materialised by the explicit boot-backfill analog, not by a hook), "member moved within the subtree keeps access", the four pure non-regression assertions, and the two pure-function cases.

The first ablation run also caught a flaw in the new test file itself: the security pin installs a gate on the module-scoped re-grant queue, and a gate left shut by a failing assertion blocked the chain so every later test died in whenIdle() — one clear failure arriving as 15 timeouts. The release moved to afterEach, so a future real regression stays legible as the one test it is.

Notes

• The seed-data-thin trap the card records is respected: a fresh boot seeds the BU tree and zerosys_business_unit_member rows, so the fixture places its member explicitly, three levels down.
• Bulk-write cost, stated rather than hidden. A predicate write over the BU tree dispatches these hooks once per matched row and the synchronous half runs on every dispatch. It is deliberately not memoised per write: memoising would judge the whole batch from the tree as it looked at the first row, re-opening the hole for rows 2..N. The repeats are wasted work rather than wrong work, each bounded by the per-rule cost above, and the granted.size === 0 short-circuit absorbs the common case. The asynchronous half is coalesced per rule, so a thousand-row import collapses to one re-grant pass per rule.
• record-share-grant-revoke: package-seeded sharing rules are unaddressable by name (org filter excludes org_id-null rows) #7676 landed mid-flight (as fix(plugin-sharing): make package-seeded (org-null) sharing rules visible and addressable by name for org-scoped admins #7760) in sharing-rule-service.ts, the file this change also touches. The merge is textually clean and semantically checked, not merely clean: fix(plugin-sharing): make package-seeded (org-null) sharing rules visible and addressable by name for org-scoped admins #7760's adminOrgScope widening applies only when the caller carries an org, and this change's handler reads listRules under a system context which carries none, so the predicate is unchanged on that path. This change to that file is purely additive (+80 lines, 0 deletions) and far from fix(plugin-sharing): make package-seeded (org-null) sharing rules visible and addressable by name for org-scoped admins #7760's region. fix(plugin-sharing): make package-seeded (org-null) sharing rules visible and addressable by name for org-scoped admins #7760 also removed a loose $or matcher from the sibling test fake; the same looseness was present in the new fake here and has been fixed the same way.

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 11, 2026 4:24pm

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-sharing.

7 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

• content/docs/kernel/runtime-services/examples.mdx(via @objectstack/plugin-sharing)
• content/docs/kernel/runtime-services/sharing-service.mdx(via @objectstack/plugin-sharing)
• content/docs/kernel/services-checklist.mdx(via @objectstack/plugin-sharing)
• content/docs/permissions/authorization.mdx(via packages/plugins/plugin-sharing)
• content/docs/permissions/permissions-matrix.mdx(via packages/plugins/plugin-sharing)
• content/docs/plugins/packages.mdx(via @objectstack/plugin-sharing)
• content/docs/protocol/objectql/security.mdx(via packages/plugins/plugin-sharing)

⛔ 1 release-owned page(s) also reference the affected code. These are read-only:

• content/docs/releases/implementation-status.mdx(via @objectstack/plugin-sharing)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

[os-help]

Patch round: TypeScript Type Check / pnpm check:type-check-debt red at 8f5b14bd9. Fixed in 189635e07, ledger not raised.

The two errors, verbatim:

src/bu-tree-recompute.test.ts(388,38): error TS7006: Parameter 'c' implicitly has an 'any' type.
src/bu-tree-recompute.test.ts(389,40): error TS7006: Parameter 'c' implicitly has an 'any' type.

Cause. The spies were declared ReturnType<typeof vi.spyOn> — the unparameterised spelling, which erases the signature, so mock.calls degrades to an implicit any per element.

Fix. Declared them from the service methods instead, MockInstance<SharingRuleService['revokeRuleGrantsForRetiredRecipients']> and MockInstance<SharingRuleService['evaluateRule']>. That resolves both errors and lets the as any cast on the first assertion go — c[0].id is now the checked argument type rather than an unchecked cast, so the assertion is strictly stronger than before.

The entry stays at 3. The measured count is back to exactly 3 and its composition again matches the note verbatim (TS6133 x2, TS18048 x1 — position-graph.test.ts and sharing-rule.test.ts, neither touched here).

Final gate output:

check-type-check-coverage: OK — 63/77 workspace packages type-checked (plus the root),
14 in the DEBT ledger (455 frozen raw errors), 1 exempt.
check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 295.0s,
1788 raw tsc error(s) total, none above its recorded number.

Behaviour unchanged: plugin-sharing still 446 passed / 17 files, typecheck clean. The change is confined to type annotations in the test file (+11/-4); no production code and no assertion semantics were touched, so the ACCEPT on #7729 does not need redoing.

One observation worth recording, since it will bite the next person measuring this gate locally. A second entry, @objectstack/spec-monorepo (+5), also drifted in my local runs and is not real: all five were TS2307: Cannot find module '@objectstack/connector-mcp' / '-openapi' / '-rest' / '-slack' / '@objectstack/cloud-connection', reached through scripts/analytics-reconcile/app-showcase.ts (the root tsconfig's exclude does not stop import-reached files). They are unbuilt packages in a partially-built worktree, AGENTS.md section 9's missing-artefact trap — building those five restored the count to 80 with no source change. CI never saw it because it builds the full workspace first, and it was present in my pre-fix run too, so it was never attributable to this branch.

Still draft, auto-merge not enabled.

---

Generated by Claude Code

---

Generated by Claude Code