Skip to content

revert(plugin-sharing): drop the NULL-inclusive business-unit screen added by #14949 before 17.3 is cut; keep the strict member screen (ADR-0131 D8) #15030

Description

@hotlong

Filed on the maintainer's ruling (2026-09-04, live chat: 「接受你的建议」on the post-17.2 audit under #13564 / ADR-0131). ⚠️ Must land before the 17.3 tag.

业务一句话

#14949(修 #14547)把 driver 那条「本组织的行,或者没标签的行」放行规则在共享规则服务里又抄了一份(business-unit-graph.ts 的 orgScope 改成 $or: [{organization_id: X}, {organization_id: null}])。这正是 ADR-0131 要退役的形状(同一谓词两处实现,#10103 的病根),而且它还没发布。发版前把这一半退回去;同一 PR 里成员表用严格等式那一半是对的,保留。

Scope

  1. In packages/plugins/plugin-sharing/src/business-unit-graph.ts, restore orgScope to the strict equality it had before e560b4d ({ ...filter, organization_id: this.organizationId }); delete the docblock that argues for the NULL-inclusive unit screen. Keep memberScope (strict, fix(plugin-sharing): a seeded business unit is a usable sharing-rule recipient, and its members are tenant-screened #14949's second half) and every test that pins it.
  2. Rewrite the business-unit-graph.test.ts / recipient-width.test.ts cases that pin the NULL-inclusive unit screen: a seeded unit with organization_id = NULL is not a usable recipient for an org-stamped rule in 17.x — the same behaviour 17.2.0 ships — and the test names Sharing rules with a business-unit recipient silently grant nothing when the unit row has organization_id = NULL #14547 as the open defect it reproduces.
  3. Changeset patch: "17.3 does not ship the NULL-inclusive business-unit screen added after 17.2.0; Sharing rules with a business-unit recipient silently grant nothing when the unit row has organization_id = NULL #14547 remains as in 17.2.0 and is fixed structurally in v18 (ADR-0131 C1: the Default Organization exists before application seed datasets load, and the seed loader stamps sys_business_unit seeds)".
  4. Comment on Sharing rules with a business-unit recipient silently grant nothing when the unit row has organization_id = NULL #14547 (do not reopen anything it already closed): the 17.x symptom stands; the root cause is the seed loader's sys_ exemption + first-boot ordering, owned by ADR-0131 C1 on the v18 line.

⛔ Do not add any other NULL arm, and do not touch SqlDriver.applyTenantScope.

Acceptance

git grep -n 'organization_id: null' -- packages/plugins/plugin-sharing/src returns no non-test hit; memberScope still strict and pinned; plugin-sharing suite green; the revert commit is shown NOT to be an ancestor of @objectstack/account@17.2.0 (positive control: one 17.2.0 commit IS).

Refs: #14949 · #14547 · #10103 (cause 1) · ADR-0131 (PR #14976) D3 / D8 / D14 · #13564.

Activity

  1. self-assigned this
    on Sep 3, 2026
  2. claude commented on Sep 3, 2026

    @claude
    Contributor

    Claim: domain:services execution seat, session session_01AUF1NoViznQK32gqpK8wS8, branch claude/issue-15030-revert-null-inclusive-unit-screen.

    Clause-②: no

    Round R24. pm:queue → pm:dispatched in one label write (read-modify-write, read immediately before the write, comparative read-back — matched). Taken ahead of everything else queued in this lane because it is priority:p1 + target:v17 and the card says it must land before the 17.3 tag.

    Premise re-verified on origin/main before dispatch

    check reading
    the NULL arm is still there business-unit-graph.ts:328 — $or: [{ organization_id: this.organizationId }, { organization_id: null }], inside orgScope at :324
    the PR it came from is on main git merge-base --is-ancestor e560b4d51 origin/main → yes (PR #14949)
    the half to KEEP exists memberScope present in the same file (7 references)

    ⇒ The revert has something to revert, and the strict half it must preserve is there. Not stale.

    Disclosure — this seat landed the thing being reverted, today

    PR #14949 was accepted and landed by this seat at 12:39:16Z. The review verified the asymmetric pair as implemented (unit screen null-inclusive, member screen strict, both halves ablated independently) and did not question whether the null-inclusive shape was the right one. The maintainer's ruling is that it is not: it re-implements the driver's own "this org's rows, or untagged rows" predicate a second time, which is the duplication ADR-0131 exists to retire (#10103's root cause), and it is unreleased, so reverting now costs nothing while shipping it would create a v18 breaking change with a migration obligation.

    ⛔ That history does not make this card negotiable, and the dispatched scope is the ruling as written — not a re-litigation of it. Recording it so the dev knows the code it is reverting was reviewed and accepted here, and that the acceptance was about implementation, not shape.

    What is dispatched

    The card's Scope 1–4 verbatim: restore orgScope to strict equality, delete the docblock arguing for the NULL-inclusive screen, keep memberScope and every test pinning it, rewrite the tests that pin the NULL-inclusive unit screen so a NULL-org seeded unit is not a usable recipient for an org-stamped rule in 17.x (naming #14547 as the open defect they reproduce), patch changeset with the card's wording, and a comment on #14547.

    ⛔ Do not add any other NULL arm. ⛔ Do not touch SqlDriver.applyTenantScope. ⛔ Do not reopen anything #14547 already closed.

    Clause-② is no — this narrows an accept set back to what 17.2.0 already ships; no exported symbol, no payload key. The dev re-derives it from its own diff.

    Size S, opus.


    Generated by Claude Code

  3. claude commented on Sep 3, 2026

    @claude
    Contributor

    ⚠️ The card's prescribed wording cannot pass CI — deviation recorded, not silently taken

    domain:services execution seat, session session_01AUF1NoViznQK32gqpK8wS8. Raising this on the card rather than only in the PR, because it is a defect in this card's instructions, not in the dev's execution.

    PR #15078 is red on Lint & Repo Gates — scripts/check-adr-anchors.mjs, 4 unresolvable ADR-0131 citations:

    A citation is a promise that the decision is readable at the other end … It is also a squat: whoever later writes a real ADR-0131 retroactively falsifies all 4 of those citation(s) at once (#6634, where one number had accumulated 77 of them).

    Measured on origin/main:

    reading result
    docs/adr/ contains 0131 no — records stop at 0130-release-artifact-as-co-ownership-boundary.md (control: 134 files present, so the zero is a reading)
    PR #14976 (ADR-0131) state: open, draft: true — unmerged
    Lint & Repo Gates on main success — so the red is this PR's

    ⇒ This card's Scope 3 mandates the citation that fails. Its changeset wording is given verbatim as "…fixed structurally in v18 (ADR-0131 C1: the Default Organization exists before application seed datasets load…)", and Scope 1's docblock work carries ADR-0131 D8. Following the card exactly produces this red.

    Why there is no forward-reference escape

    I read the gate rather than guessing. Only CROSS_REPO_QUALIFIERS = {objectui, object-ui, cloud} exempt a citation, so ObjectUI ADR-0131 is skipped but ADR-0131 (PR #14976) is not — the qualifier must be a sibling repo, not a PR. The allowlist is shrink-only and its own doc-block says to keep it empty.

    ⛔ Writing docs/adr/0131-*.md here is not available either: that file belongs to PR #14976, and docs/adr/** is a governed surface.

    What this seat decided, and why not the alternative

    Decision: keep the substance, drop the ADR token. The dev is instructed to carry the reference as PR #14976 in prose (e.g. "the v18 organization-ownership decision (PR #14976), D8") across the docblocks, test comments and the changeset, leaving every issue/PR reference — #13564, #14547, #10103, #14949 — untouched. The disclosure goes in the PR body as an explicit deviation, and is recorded here so it is visible from the card too. The ADR id can be restored once #14976 lands.

    The alternative was to block this card on PR #14976, exactly as #15024 is blocked on #15023 (recorded in that card's comment 5530972375). ⛔ I did not take it: this card is priority:p1 target:v17 and states it must land before the 17.3 tag, while the blocking dependency is purely a citation spelling — nothing about the revert's correctness waits on the ADR text existing. Blocking a release-critical revert on a docs-only draft PR would trade the ruling's actual purpose for its footnote.

    ⚠️ If the maintainer would rather the ADR id be preserved verbatim, then this card is blocked on #14976 and should be relabelled pm:blocked with Blocked-by: #14976 — say so and I will flip it. The deviation is disclosed precisely so that choice stays open rather than being made silently by whoever writes the changeset.

    Nothing else in the dispatched scope changes: memberScope stays strict and its pins untouched, sharing-rule-service.ts / sharing-rule.test.ts are not reverted (in #14949 but outside this card), and the ancestry check is run with its positive control.


    Generated by Claude Code

  4. claude commented on Sep 3, 2026

    @claude
    Contributor

    Landed — PR #15078 merged at 2026-09-03T21:43:39Z; this card auto-closed at 21:43:41Z via its Fixes reference. main is now ab47816914.

    pm:dispatched stripped in a read-modify-write, read back and diffed against the expected union — domain:services, priority:p1, target:v17 remain (routing and severity are not state). No other label moved.

    Disclosed deviation, restated at landing so it is not lost in the thread. The card mandated an ADR-0131 citation. That citation cannot pass check-adr-anchors, so the substance was kept and the token dropped; the deviation was disclosed on both the PR and this card when the ruling was taken. The alternative the seat left explicitly open — blocking this revert on #14976 instead — was not exercised, and remains available to the maintainer as a follow-up if the citation is judged load-bearing.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions