Skip to content

fix(plugin-sharing): the business_unit recipient expands exactly one unit, not the whole subtree (#7807) - #8097

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-7807-business-unit-exact-width
Aug 12, 2026
Merged

fix(plugin-sharing): the business_unit recipient expands exactly one unit, not the whole subtree (#7807)#8097
os-zhuang merged 1 commit into
mainfrom
claude/issue-7807-business-unit-exact-width

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#7807

Implements the maintainer ruling of 2026-08-12 — direction 1: narrow the runtime to match the declaration.

The defect

The two business-unit recipient kinds were declared as two widths and enforced as one. SharingRuleService.expandRecipient routed both through the identical BusinessUnitGraphService.expandUsers call, whose first act is a BFS over parent_business_unit_id — so the two branches differed only in their comments.

A rule authored recipient_type: 'business_unit', which the spec (ShareRecipientType), the org-axis lint red-line table and ADR-0057 D5 all describe as "exactly one business unit's members (no subtree)", in fact reached that unit plus every descendant unit's members. On a three-level tree a division-anchored rule silently granted to every department and office beneath it — an over-grant, and one that left unit_and_subordinates, documented as the strictly wider grant of the pair, not wider at all.

The fix

business_unit now resolves through a new BusinessUnitGraphService.expandUnitMembers() — members whose business_unit_id equals the named unit, no descent. It keeps every other guarantee the subtree walk had: an inactive or out-of-tenant anchor contributes nobody, and an unreadable unit fails closed rather than granting.

unit_and_subordinates is unchanged and keeps the subtree walk. Both kinds survive; neither is merged into the other or retired, which the ruling names as the outcome to avoid. expandUsers() keeps its IBusinessUnitGraphService contract meaning (the contract declares it as the descendant expansion) for the bu: approver prefix and org rollups.

Two details worth review attention:

  • The widths cache separately. Both are keyed by business-unit id, so one shared map would let whichever width ran first answer for the other — the over-grant returning through the cache door. Pinned by a test.
  • packages/spec is untouched, per the ruling: the declaration was already right, the runtime was wrong.

Verification — predict-then-mutate ablation

The prediction was written down before either ablation ran.

Ablation A — revert only the narrowing (the defect shape). Predicted: exactly the narrow assertions go red, every wide one stays green. Measured: 8 failures, all narrow-width; the wide control and the whole business-unit-graph suite stayed green.

Ablation B — narrow BOTH branches (the failure mode this PR must not be). Predicted: the wide assertions go red, the narrow ones stay green. Measured: 8 failures, all wide-width, including four in the existing #7729 recompute suite, which turns out to be a second independent control for the wide width.

The symmetry is the point: it shows the wide pin is a real control rather than a tautology that passes because both branches happen to agree.

Tests

  • packages/plugins/plugin-sharing490 passed (19 files), typecheck clean.
  • packages/qa/dogfood — the BU-hierarchy e2e proof: 7 passed, including three new cases proving through the real REST stack that a business_unit rule reaches the anchor unit and a subordinate-unit member is denied. typecheck clean after building the closure.
  • Gates re-derived for the final surface with scripts/pm/dispatch-gates.mjs and run green: check:nul-bytes, check:engine-double-contract (the new fake is pinned for both verbs, not debt), check:test-source-alias, check:docs-audit-scope, check:changeset-gate-self-tests, check:objectui-changeset, check-changeset-no-major.

Fixture dispositions

Three existing fixtures spelled the limb this PR removes, and each was judged individually rather than batch re-spelled:

  • sharing-rule.test.ts — a test literally named "expands via the BU graph (BFS)" asserted the over-grant as if it were the contract. Replaced wholesale by the pair: the narrow kind expands one unit, and a new sibling case pins unit_and_subordinates expanding the subtree.
  • showcase-bu-hierarchy-sharing.dogfood.test.ts — authored business_unit to demonstrate hierarchy widening, which is unit_and_subordinates' own semantics. Re-spelled (its two authz-conformance rows are preserved), plus the narrow half added on the same booted stack.
  • bu-tree-recompute.test.ts — its business_unit case was already anchored at the unit the member belongs to directly, so it survives the narrowing unchanged; only its title, which asserted the now-false "they walk the same subtree resolver", was corrected.

Release note

A changeset is included and states plainly that this reduces visible rows for any out-of-tree deployment that authored business_unit rules relying on the subtree behaviour, with the migration path (switch the recipient to unit_and_subordinates if the subtree reach was what you wanted).

Re-measurement of the ruling's shelf-life claim

The ruling's "in-tree migration cost is zero" rested on a 2026-08-12 measurement that no in-tree rule authors business_unit. Re-verified on current origin/main and it still holds — the showcase app authors position and unit_and_subordinates, the CRM app authors position, and no seeded rule authors business_unit. The only in-tree business_unit authors were the two test fixtures dispositioned above.

Generated by Claude Code


Generated by Claude Code

…, not the subtree (#7807)
The two business-unit recipient kinds were declared as two widths and enforced
as one. `SharingRuleService.expandRecipient` routed both through the identical
`BusinessUnitGraphService.expandUsers` call, whose first act is a BFS over
`parent_business_unit_id`, so the branches differed only in their comments.
A rule authored `recipient_type: 'business_unit'` — declared by the spec, the
lint red-line table and ADR-0057 D5 as "exactly one business unit's members (no
subtree)" — reached that unit plus every descendant unit's members. On a
three-level tree a division-anchored rule silently granted to every department
and office beneath it, and `unit_and_subordinates`, documented as the strictly
wider grant of the pair, was not wider at all.
Maintainer ruling 2026-08-12, direction 1: narrow the runtime to the
declaration. `business_unit` now resolves through a new
`expandUnitMembers()` — no descent — while `unit_and_subordinates` keeps the
subtree walk unchanged. Both kinds survive; neither is merged or retired.
`expandUsers()` keeps its contract meaning for the `bu:` approver prefix and
org rollups. The narrow width caches separately, since one map keyed by unit id
would let whichever width ran first answer for the other.
Both widths are pinned as a pair on one three-level fixture, at the graph, the
service and the REST level, because a change narrowing BOTH kinds would satisfy
the headline while destroying the distinction. Verified by two ablations: with
the narrowing reverted exactly the 8 narrow assertions go red and every wide one
stays green; with both branches narrowed exactly the wide assertions go red.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 3:18pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-sharing, @objectstack/dogfood.

8 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, packages/qa/dogfood)
  • content/docs/permissions/delegated-administration.mdx(via packages/qa/dogfood)
  • 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.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sharing rules: the business_unit recipient expands the whole SUBTREE, but the spec declares it as exactly one unit (no subtree)

2 participants

@os-zhuang@claude