Skip to content

feat(plugin-sharing,plugin-approvals,metadata-protocol)!: 18.x step 7 part 7a-3 — the organization-less readers of sharing rules, approval units and the seed loader retire - #24828

Merged
objectstack-fleet[bot] merged 57 commits into
mainfrom
claude/issue-24583-7a3-sharing-null-readers
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 57 commits into
mainfrom
claude/issue-24583-7a3-sharing-null-readers

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Part of #24583 — step 7, part 7a-3 (claim 6285134534; governing text: execution record 6285122667, ADR-0131 D2 and D8). The other step-7 parts (7a-1 driver, 7a-2 authz, 7b-*, 7c webhooks) are not in this PR, and #24583 remains open.

Clause-②: yes (narrowing): a declared sharing rule served under single gains the deploymentWide: true key on the wire in place of organization_id: null (widening); the sharing-rule admin reads, the approval unit screen and the scoped seed pre-load stop reading rows with no organization (narrowing). A declared rule keeps applying deployment-wide; nothing an author declares is refused.

What changes

plugin-sharing sharing-rule-service.ts

  • A declared rule's single projection carries a typed marker, deploymentWide: true (DeploymentWideSharingRule, isDeploymentWideSharingRule), and no organization_id key. This is the shape of the declared webhooks' deploymentWide (ruling A, 6278609968). The scope list holds a DEPLOYMENT_WIDE_SCOPE symbol in place of null.
  • criteriaContext loses its NULL branch. It answers one of three named cases:
    • the rule's organization, which is the tenant predicate;
    • a deployment-wide projection, which carries no tenant (D8 computes no predicate under single);
    • a composition with no organization object (the lean embedding, no-organization-object), which also carries no tenant.
    • Otherwise it answers null: a stored row with no organization sweeps nothing and grants nothing, and the service logs it once at error with the remedy.
  • How the sweep runs (decided and pinned):
    • Under single: one deployment-wide pass, and each grant is stamped with its record's organization.
    • Under a wall: one projection per organization, each inside that organization's one predicate, with grants stamped with it. No projection carries the marker under a wall, so Layer 0's predicate is never widened.
  • adminOrgScope and findRuleRowByName read the caller's own organization only.
  • assertCanDeletePlatformGlobalRule is deleted, together with its call.
  • sweepOrphanedRuleGrants and ruleGrantScope follow the marker.

plugin-approvals ApprovalService.businessUnitOrgScope: a strict equality. The ceremony attributes sys_business_unit, and a fresh install stamps it.

metadata-protocol seed-loader.ts: the unownedHolders read is deleted, together with its refuse decision and the refusal text #24681 had rewritten. A scoped load issues no organization_id IS NULL read on any path.

Pins rewritten to the new rule (no assertion weakened):

Premise measured first (step 7's "after certification")

Every reader retired here was dead on all three trees:

tree tenant-column tables rows with NULL organization_id
certified database (ceremoniedFixture over the 18.0 fixture), after the ceremony and again after a HEAD boot 66 0
fresh birth-marked single install (showcase --fresh) all 0 (sys_business_unit 5 rows, all stamped)
fresh isolated install, real organizations runtime all 0

One live path remains. It is kept, not deleted: the lean embedding with no organization object, which still writes NULL rows by design. It keeps the deployment-wide sweep, keyed on the composition fact (sys_organization not registered), never on a row's NULL. The production os serve boot without OS_AUTH_SECRET is that same composition today; 7b-3 makes it refuse.

Proof

Final head 357025dc97. The boots, harness and suites ran on 62227f5632, which differs from it only by main's README commit.

  • R1: a fresh showcase boot on a random port, run on the pre-merge head and again on the final head:
    • health 200, seeded-admin sign-in 200, 0 error lines;
    • 0 NULL organization_id rows in any tenant table;
    • the declared rule share_key_account_qualified_contacts_with_managers materialised a stamped grant.
  • R2 + R3: upgrade-18x-harness.dogfood.test.ts + organization-ownership-boot-refusal-order.dogfood.test.ts passed, together with the two sharing dogfood files: 33/33. This run includes the ceremonied fixture, and the v17 refusal fired before the ceremony.
  • R4:
    • An isolated os serve with @objectstack/organizations (invite-only membership, owner email set), on both heads: health 200, sign-in 200, 0 error lines, 0 NULL rows. GET /sharing/rules serves both declared rules as per-organization projections, with no marker. The grant is stamped.
    • Step 4's 7 walled e2e cases (serve-organizations-host-resolution + serve-organizations-mount-failure, OS_TEST_TIERS=nightly): 7/7.
  • Suites:
    • plugin-sharing 51 files / 1181 tests;
    • plugin-approvals 86 / 1147;
    • metadata-protocol 251 / 20815;
    • runtime 431 / 6781;
    • typecheck of plugin-sharing, plugin-approvals, metadata-protocol, runtime and dogfood: exit 0.
  • Gates: dispatch-gates --commands on the 12 code paths derived 75 commands. All 75 ran, and every exit code was captured before any pipe.
    • 73 exited 0 on the first run.
    • check-system-context-census was red: content/docs/permissions/system-context.mdx still cited the deleted assertCanDeletePlatformGlobalRule. The citation was dropped, and the re-run exited 0.
    • check-plugin-teardown-shape --self-test was a shallow-clone artefact: its pinned fixture commit was absent. It exited 0 once that commit was fetched.
    • Re-derived on the final 15 paths (1,641 changed lines, under the 3,000 line): the 26 newly derived docs families plus the 15 ratchet and test-reading families, 41 commands, all exit 0.
  • More pins the suites found, rewritten:
    • packages/runtime/src/standalone-stack-seeder-declaration-copy.test.ts: the served declared shape now carries the marker.
    • sharing-rule-positions-name-authority.test.ts: retitled. Its delete case is refused by the scope gate, not by the retired guard.
  • A sweep of 11 more candidate files, all green:
    • 7 dogfood files (scope-depth ×3, BU projection, BU/user delete, single platform insert, single-tenant identity create);
    • the cli declared-rule-mirrors integration test;
    • the rest canonical-query-AST test;
    • the cloud-connection posture-gate test.
  • Ablation of the main pin: restoring the NULL branch (return SYSTEM_CTX for every org-less rule) turned exactly the two predicted cases red ("a STORED rule with NO organization …", whole-rule and per-record), and the other 6 stayed green. Restore was proven by blob == HEAD and an empty git diff HEAD.

Acceptance notes

  • main's recipient-membership-recompute.ts (ruleReadsRowsOf, landed in 0b7286ff) still reads rule.organization_id == null as "reads every row". With the marker the deployment-wide projection has no organization_id, so it is still counted as affected, which is the right answer. The spelling is a NULL-shaped reader outside this part's file surface.
  • sharing-plugin.ts's resolveDeclaredRuleScopes still spells single as [null]. setDeclaredRuleScopes translates it at that one door, so no NULL travels past it. Retyping the plugin side is outside the claimed surface.
  • approval-service.ts teamIsProvablyOutsideOrg keeps its "no organization ⇒ leave routing" limb (a team screen). It is not in this part's surface, and its doc cross-reference was corrected.
  • The ORG-STAMPED pin in rule-criteria-org-scope.test.ts still expects deal_p1 through the driver's NULL record arm. That is 7a-1's arm and 7a-1's pin. Separately, an org-less RECORD matched by two walled projections aborts the second pass with RECORD_NOT_FOUND. That case cannot occur on a certified database, and it goes away with 7a-1.
  • File surface: beyond the claim's list, this PR edits one hand-written docs page (content/docs/permissions/system-context.mdx, row 40: the deleted guard's citation, which its census gate requires) and pins in packages/runtime and packages/qa/dogfood. It touches no docs/adr/** and no content/docs/releases/.
  • The seed pin files keep their null-organization-holder names: scripts/engine-double-contract.pinned.json registers them by path.

Generated by Claude Code

claude added 30 commits October 19, 2026 13:06
…r retired; webhook activation door

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…he 18.0 dual read; door pins for webhook

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…asserts no row and a dispatch

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…the checklist, the webhooks guide and the showcase; changeset

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…tcher's switch read

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…posture builds no wall, as its 18.0 seeded copy stamped with the Default Organization was

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…red writes; the declared-webhook double is drivable by the limit gate

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…does not implement

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…authors; checklist and admitsOrganization docblock say a declared webhook is deployment-wide under single

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…the fixture's customized declared-webhook mirror

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…n fail on a double delivery

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…0g-webhooks-registry

# Conflicts:
#	content/docs/permissions/tenant-audit-census.mdx
#	docs/audits/2026-08-tenant-audit-write-call-sites.counts.md
…0g-webhooks-registry

# Conflicts:
#	content/docs/permissions/tenant-audit-census.mdx
#	docs/audits/2026-08-tenant-audit-write-call-sites.counts.md
…-registry' into claude/issue-24581-ceremony-open
…he ceremony, then boot; boot-refusal-order step 2 back on the binary
…el — the cipher names no organization

Claude-Session: https://claude.ai/code/session_014UATXWqAXqkaZkbG92ox6V
Co-authored-by: Claude <noreply@anthropic.com>
…sys_flow_credential leave reach)

Claude-Session: https://claude.ai/code/session_014UATXWqAXqkaZkbG92ox6V
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/metadata-protocol, @objectstack/plugin-approvals, @objectstack/plugin-sharing, @objectstack/spec, touching 44 documentable anchor(s).

12 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class), sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/concepts/metadata-lifecycle.mdx (via /api/v1/sharing/rules (route, a path literal in a comment in SharingRuleService))
  • content/docs/data-modeling/validation.mdx (via SeedLoaderService (symbol, a top-level class))
  • content/docs/deployment/cli.mdx (via sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/deployment/seed-tenancy-repair.mdx (via sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/kernel/runtime-services/sharing-service.mdx (via defineRule (symbol, a method of class SharingRuleService), deleteRule (symbol, a method of class SharingRuleService))
  • content/docs/permissions/administrator-guide.mdx (via sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/permissions/sharing-rules.mdx (via /sharing/rules/:idOrName (route, bridged from symbol deleteRule — its route source's handler names it))
  • content/docs/permissions/system-context.mdx (via defineRule (symbol, a method of class SharingRuleService))
  • content/docs/protocol/kernel/config-resolution.mdx (via sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/protocol/objectql/schema.mdx (via sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/protocol/objectql/state-machine.mdx (via SeedLoaderService (symbol, a top-level class))

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

  • content/docs/releases/implementation-status.mdx (via sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/releases/v17/17-0.mdx (via defineRule (symbol, a method of class SharingRuleService), /api/v1/sharing/rules (route, a path literal in a comment in SharingRuleService))
  • content/docs/releases/v17/17-1.mdx (via SeedLoaderService (symbol, a top-level class), sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/releases/v17/17-4.mdx (via sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/releases/v17/17-5.mdx (via sys_organization (literal, a string literal in ORGANIZATION_OBJECT))
  • content/docs/releases/v17/17-6.mdx (via ApprovalService (symbol, a top-level class))

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
  • 2 anchor(s) matched too much of the corpus to be a work list: PERMISSION_DENIED (literal, 41 pages), organization_id (literal, 35 pages)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 144 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 7c2eab929c6b724672849c5b295fa5e2f35d04c0 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 78ccb8e203143571a69c68b0988aecd3e33bd9a6 — the merge of head 236d39daf47ce4b06c9e8757ccbe695af1ca1e19 into base 7c2eab929c6b724672849c5b295fa5e2f35d04c0, 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 78ccb8e203143571a69c68b0988aecd3e33bd9a6 && git checkout 78ccb8e203143571a69c68b0988aecd3e33bd9a6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7c2eab929c6b724672849c5b295fa5e2f35d04c0 236d39daf47ce4b06c9e8757ccbe695af1ca1e19 && git checkout -B drift-repro 7c2eab929c6b724672849c5b295fa5e2f35d04c0 && git merge --no-ff 236d39daf47ce4b06c9e8757ccbe695af1ca1e19

node scripts/docs-audit/affected-docs.mjs --json 7c2eab929c6b724672849c5b295fa5e2f35d04c0

⚠️ 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 7c2eab929c6b724672849c5b295fa5e2f35d04c0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 357025dc9728e877d0c1c8acc3ef2c49058c9c73
Local-runs: none

Read-only review of PR #24828 (18.x step 7, part 7a-3 of #24583): diff ddb3360a4..357025dc9, 15 files, +671/-970, read in a detached worktree; the card's report 6286869893, claim 6285134534, execution record 6285122667 and ruling A 6278609968; objectui at the pinned .objectui-sha (12dc7b4) and at its origin/main. Head check-runs at review time: every completed run is success or skipped (Build Core, Build Docs, Type Check x4, Lint x8, Check Changeset, PR Size, Governed Surface Queue Guard, the claim/single-writer guards, Dogfood Regression Gate); Test Core (1/6) through (6/6) were still in_progress. This PASS does not stand in for their green: a red shard is blocking under AGENTS.md's queue rule, and the merge-base is unchanged (origin/main = ddb3360a4).

① Derived judgments

1. The shape (D2, ruling A) — holds.

  • projectDeclaredRule (sharing-rule-service.ts:204-208) spreads { deploymentWide: true } for DEPLOYMENT_WIDE_SCOPE and { organization_id: scope } otherwise, so a single projection carries the marker and no organization_id key, and a walled projection carries its organization and no marker (pinned at sharing-rule.test.ts:1881-1885 and :1991-1994). The scope list is a unique symbol (:154), translated from the plugin's [null] at exactly one door, setDeclaredRuleScopes (:537-539); addDeclaredRuleScope (:558-563) refuses to add an organization beside the deployment scope. The switch stays the deployment-level activation-ledger row (readSwitchedOffDeclaredRules, applied before projection at :943-945). That is ruling A's shape for declared webhooks (plugin-webhooks/src/auto-enqueuer.ts:170,583): one declaration, one deployment-level switch, each grant stamped with its record's organization.
  • Forgery through a stored row: not possible. isDeploymentWideSharingRule (:176-178) requires deploymentWide === true AND isDeclaredSharingRule (managed_by === 'package' && id === name, :228-230). Every stored row enters the service through rowFromRule (:125-146), which copies a fixed field list and never the marker — all seven stored-row materialisations use it (:804, :808, :827, :853, :908, :1119, :1121); the two remaining sys_sharing_rule reads (:948 holders, :1271 orphan sweep) project name/organization_id/id only. defineRule writes a fixed column list (:836-852), spreads no input, and sys-sharing-rule.object.ts declares no deploymentWide column. The only public method taking a rule object, reconcileRuleForUser, is fed from service.listRules (recipient-membership-recompute.ts:270-281), not from a caller-supplied row. No REST door accepts a rule body for evaluation.

2. criteriaContext, no NULL branch — holds.

  • criteriaContext (:1778-1783): a non-empty string organization → { ...SYSTEM_CTX, tenantId }; else the marker or a lean composition → SYSTEM_CTX; else null. sweepContextOrReport (:1802-1818) logs once per rule at error (fallback warn) with the ceremony remedy and is applied on all three doors: the whole-rule sweep (findMatchingRows :1836-1837 → []), the per-record hook (matchRecord :1864-1865 → null) and the grant write itself (grantOne :2289-2290 → false), so no caller can write a grant around the sweep. Pinned with a real SqlDriver at rule-criteria-org-scope.test.ts:376-394 (matched 0, granted 0, exactly one error line across two evaluations) and :459-471 (per-record). Note the consequence the log line does not state: an empty desired set makes reconcile revoke what that row had materialised earlier — the fail-closed direction, and the stated remedy still applies.
  • Under single a deployment-wide projection runs under SYSTEM_CTX with no tenantId, which is the predicate D8 computes under single (none); grants are stamped by SharingService.resolveSystemGrantOrganization from the record (sharing-service.ts:1604-1609, threaded organization first, record second). Pinned: rule-criteria-org-scope.test.ts:340-355 (deal_a1→ORG_A, deal_b1/b2→ORG_B).
  • Under a wall each projection carries its organization as a string, so the sweep threads tenantId = organization through the driver's chokepoint and matches no other organization's rows; recipients expand through TeamGraphService/BusinessUnitGraphService/PositionGraphService built with rule.organization_id (:2037-2081); the grant context carries the same organization (activeOrganizationId(context) wins at sharing-service.ts:1604). Pinned at rule-criteria-org-scope.test.ts:357-374 (isolated, two organizations, three grants, each in its own organization) and sharing-rule.test.ts:1988-1998. A literal user recipient (:2041) is returned unscreened — unchanged by this PR and pre-existing for stored rules; the grant row is stamped with the record's organization, so it is readable only inside that wall. One caveat, named in the report and not this PR's: until 7a-1 lands, SqlDriver.applyTenantScope still carries OR organization_id IS NULL, so a walled projection can match an organization-less RECORD (never another organization's); the test removes deal_p1 for that reason (:362-363).

3. The lean-embedding key — robust, not a leak.

  • organizationsComposed (:1792-1795) = typeof engine.getSchema === 'function' ? !!engine.getSchema('sys_organization') : true. The engine handed in is the real IObjectQLEngine (sharing-plugin.ts:661,772-773), whose getSchema is this._registry.getObject(objectName) (engine.ts:11458-11460) — the same fact the engine's own D9 stamper reads as organizationObjectRegistered (engine.ts:7420-7424). sys_organization is declared by platform-objects/src/identity/sys-organization.object.ts and registered in every standard boot; a walled boot cannot run without it (resolveDeclaredRuleScopes enumerates it, sharing-plugin.ts:105; R4 measured). An engine that cannot answer reads as composed, i.e. refuses. The lean branch is reachable only by a STORED row with no string organization — a walled declared projection never reaches it. Pinned both ways: lean CONTROL (rule-criteria-org-scope.test.ts:396-418, no sys_organization, sweeps 4, no error) against the organizations case (:376-394). No path by which a walled deployment evaluates it as true.

4. Readers — hold.

  • adminOrgScope (:888-892) and findRuleRowByName (:1145-1153) are strict own-organization; system context / platform operator keep the unfiltered read asserted upstream by assertCanManageRules. deleteRule (:1156-1200) resolves through the same findRuleRow: an organization-bound admin can no longer reach an organization-less row at all (silent no-op, as for any row the caller cannot see; a declared name → 403 at :1163-1170), and system/platform callers are exactly the callers the deleted guard returned early for (row.organization_id != null → return; isSystem → return; hasPlatformAuthority → return). No admin, Setup or API path needs assertCanDeletePlatformGlobalRule: the row class it gated is unreachable by the only caller it refused. hasPlatformAuthority keeps its one live caller (:718). Setup's delete of a declared rule stays 403 (sharing-rule.test.ts:1967).
  • ApprovalService.businessUnitOrgScope (approval-service.ts:2755-2761) is a strict equality; businessUnitMemberScope unchanged; pinned at approval-service.test.ts:968-980 (an organization-less unit resolves to the dead literal) and the re-stamped business-unit-member-org-screen.test.ts fixtures. teamIsProvablyOutsideOrg keeps its no-organization limb (:2584-2590, cross-reference corrected) — outside the 7a-3 surface, as the report says.
  • Seed loader: loadExistingRecords (seed-loader.ts:3361-3384) issues one scoped read; decideWriteAction loses refuse. After certification a NULL holder goes unseen, it is not yet impossible: the 18.x boot guard is marker-based (organization-ownership-boot-guard.ts:37, "boots, whatever the columns hold"), and the execution record itself cites 7b-3's measurement of a fresh birth-marked install writing NULL rows, with 7b-0 (NOT NULL at createTable) as the closure. Until then a scoped upsert over such a row inserts an organization-stamped twin beside it — the benign direction (no adoption, no cross-organization write, D9 stamps the twin). Residual R3.

5. Wire change — the console needs no change.

  • objectui at the pinned sha 12dc7b4 (the objectui#12982 landing, regime C) and at origin/main (cd4623d): Setup lists declared rules from GET /meta/sharing_rule, not from /sharing/rules; it reads GET /sharing/rules/:name for one rule only, as SharingRuleProjection { name, object_name, criteria, recipient_type, recipient_id, access_level } (packages/app-shell/src/views/metadata-admin/sharing-rule-catalog.ts:73-80, 88-105). git grep over apps/* and packages/* at origin/main finds zero organization_id === null / == null / deploymentWide readers. Nothing labels a declared rule by its NULL organization.
  • In-repo consumers of the served row: ruleReadsRowsOf (recipient-membership-recompute.ts:203) reads rule.organization_id == null as "every row" — with the key absent on the deployment-wide projection the answer stays correct (the report's dedupe note). The four pins that read organization_id: null on the served shape (two dogfood files, standalone-stack-seeder-declaration-copy.test.ts) are rewritten to the marker and assert the key's absence.
  • No published schema describes the served row beyond SharingRuleRow in packages/spec/src/contracts/sharing-service.ts:481-483 (organization_id?: string | null, no deploymentWide); no OpenAPI/zod response schema for /sharing/rules exists in packages/spec or packages/server. Residual R1.

6. Tests — pins rewritten, none weakened; the ablation is well-formed.

  • Every replaced assertion is stronger or equal: grantsAtRest pins record AND organization of each grant where sharedRecordIds pinned ids; getRule → null by name and by id; evaluateRule → rejects RULE_NOT_FOUND with an empty grant table; the seed-loader pins add a nullReads spy on both the engine double and the real driver (seed-loader-null-organization-holder.test.ts:122-130, real-driver :151-160) and the update mode. The Decision: may an org-level sharing admin DELETE a platform-global (organization_id = null) sharing rule? #7795 block is removed with its guard (the feature no longer exists; see R2 for the successor pin). One coverage note: the real-driver case "an unscoped load matches NULL rows and inserts no twin" was deleted rather than re-pinned; the behaviour is unchanged by this diff and the metadata-protocol twin keeps the unscoped case with an own row.
  • Ablation: :1780 (string organization) precedes :1781, so an unconditional return SYSTEM_CTX at :1781 leaves org-stamped and declared cases untouched and reds exactly the two stored-organization-less cases (:376, :459) — the report's 2 red / 6 green. Restore verified by blob equality.

7. Overlap with PR #24825 (7a-1) on rule-criteria-org-scope.test.ts — #24825 is open, draft, head 2ee8e8d87, base main, merge-base 307ce4c4b. It rewrites the header (lines 29-80: "applyTenantScope turns tenantId into organization_id = ?"; deal_p1 paragraphs) and the ORG-STAMPED case (['deal_a1'], matched 1). This PR rewrites the adjacent header paragraph (lines 44-62), adds BootOptions/DECLARED/grantsAtRest and four cases, and leaves the "field = ? OR field IS NULL" sentence (lines 51-55) in place. The second lander owes: (a) a textual merge of the two header rewrites into one that names neither a "platform-global sweep" (7a-1's wording, a class 7a-3 retires) nor an OR IS NULL arm (7a-3's remaining sentence, an arm 7a-1 retires); (b) 7a-1's ORG-STAMPED expectation kept, 7a-3's four cases kept; (c) the driver.delete(OBJECT, 'deal_p1') pre-step at :362-363 and its comment become unnecessary once the arm is gone — drop or re-justify; (d) the report's "second interaction" note (RECORD_NOT_FOUND on the second walled projection over an organization-less record) is closed by 7a-1 and needs no carrier.

8. Semver and Clause-② — correct. AGENTS.md §3: yes takes at least minor; (narrowing) is BREAKING and must carry its FROM → TO and an ADR-0087 marker. The changeset has all three: minor x3, a bang in the subject, FROM rule.organization_id === null TO rule.deploymentWide === true with the two endpoints named, and adr-0087: not-required (no-migration-prescription). No major changeset exists anywhere in .changeset/; the step-7 siblings (15212-8b-*, 15212-webhook-declared-name-guard) are feat(...)!: at minor, so this is the line's convention. The arm reading is right: yes because nothing an author declares or authors is refused (a declared rule keeps granting, the console's authoring door is untouched); (narrowing) because readers narrow — the new key on the wire is additive, but the removal of organization_id from the served declared row is the BREAKING half, and the changeset states it. The face is declaration (Clause-②: yes), hence TIER_DEFAULT.

② Semver level

minor on @objectstack/plugin-sharing, @objectstack/plugin-approvals, @objectstack/metadata-protocol, each with the BREAKING bang, the migration (the ceremony command, stated) and the ADR-0087 disposition — matches the repo rule and the line's precedent. Public surface added: DeploymentWideSharingRule, isDeploymentWideSharingRule (plugin-sharing exports, additive). Public surface removed: none exported (assertCanDeletePlatformGlobalRule was private); the seed-loader refuse decision was internal. The one wire shape change is the declared rule's served row.

③ Boundary flags

  • R1 (residual, spec lane, CONTRACT_REVIEW_TIER when carried): SharingRuleRow in packages/spec/src/contracts/sharing-service.ts:481 still declares organization_id?: string | null and no deploymentWide; the marker is a plugin-local type. Until the contract carries deploymentWide?: true (and says the key is absent under a wall), the served key is undocumented. Correctly outside this claim's file surface (⛔ spec is a published schema path); carrier: a step-7 follow-up on the spec lane.
  • R2 (residual, test pin): the deleted Decision: may an org-level sharing admin DELETE a platform-global (organization_id = null) sharing rule? #7795 block pinned "the refusal leaves the rule row AND every tenant's grants intact". Its successor on the new rule is unpinned: deleteRule(orgLessRowId, ORG1_ADMIN) and deleteRule(orgLessName, ORG1_ADMIN) should be pinned as a silent no-op that leaves the row and its grants in place (and a system context still deleting it). Today only getRule/evaluateRule on that row are pinned (sharing-rule.test.ts:1101-1116, 1288-1292); a regression to an unfiltered by-id lookup inside deleteRule would purge grants silently. Fix: about 12 lines in the [#7761] block.
  • R3 (residual, carried by 7b-0/7b-3): the seed loader's NULL holder is unseen, not impossible, until the tenant column is NOT NULL at createTable (7b-0) and the auth-secret-less os serve composition refuses (7b-3). Consequence meanwhile: an organization-stamped twin beside a stray NULL row; no adoption, no cross-organization write.
  • R4 (residual, overlap): the second of feat(driver-sql,objectql)!: a tenant-scoped read no longer answers a row with no organization (18.x step 7, part 7a-1) #24825 / feat(plugin-sharing,plugin-approvals,metadata-protocol)!: 18.x step 7 part 7a-3 — the organization-less readers of sharing rules, approval units and the seed loader retire #24828 to land owes the rule-criteria-org-scope.test.ts reconciliation in ① item 7.
  • Open question 1 of the report (A accepted): setDeclaredRuleScopes(ReadonlyArray of string or null) and resolveDeclaredRuleScopes still spell single as [null] at the plugin door; translated at one line (:538), no NULL travels past it. Retyping both to the marker is about 10 lines and can ride the next step-7 part that touches sharing-plugin.ts. Related pre-existing observation: the service's constructor default is the single shape (:512, was [null]); the plugin sets scopes before any read (sharing-plugin.ts:772-810, only registerService between), so nothing is served before the posture is known.
  • Open question 2 (A accepted): an engine that cannot answer reads as composing organizations — the refusing reading, and the same one resolveSystemWriteOrganization takes.
  • Open question 3 (A accepted): content/docs/permissions/system-context.mdx row 40 is a one-row citation drop forced by check-system-context-census after the guard's deletion; content/docs/** is not on the Prime Directive feat: Comprehensive CRM example demonstrating all ObjectStack protocol features #14 register, and the Governed Surface Queue Guard is green. The packages/runtime and packages/qa/dogfood pin edits are consequences of the wire change and are inside the claim's "pins for the above".
  • Out-of-scope notes in the report (ruleReadsRowsOf spelling, teamIsProvablyOutsideOrg NULL limb, stale resolveSystemGrantOrganization doc, [tenant-audit] warnings on the walled showcase boot): read and agreed as not this PR's; none is a leak on a certified database.
  • CI: Test Core 1/6–6/6 were in progress at review time; the lane seat reads them green before queueing (AGENTS.md: in_progress is not a pass).

Implemented-by: claude/issue-24583-7a3-sharing-null-readers
Reviewed-by: session_01Y5583bs2RDd53MH6kM1ca3

VERDICT: PASS

… marker; an org-less row's delete by an org admin is pinned a no-op

Claude-Session: https://claude.ai/code/session_01XFEb4fSDbe4fK94S3x5FKX
Co-authored-by: Claude <noreply@anthropic.com>
…o driver NULL arm, declared rules deployment-wide by marker

Claude-Session: https://claude.ai/code/session_01XFEb4fSDbe4fK94S3x5FKX
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review (delta after PASS at 357025d)

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 236d39d
Local-runs: none
Scope: packages/spec/src/contracts/sharing-service.ts + .changeset/24583-7a3-sharing-null-readers.md, packages/plugins/plugin-sharing/src/sharing-rule-service.ts, packages/plugins/plugin-sharing/src/sharing-rule.test.ts, packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts, packages/plugins/plugin-sharing/src/sharing-plugin.ts, packages/drivers/driver-sql/src/sql-driver.ts (at 7c2eab9), packages/plugins/plugin-approvals/src/approval-service.ts (main's hunk only), packages/client/src/index.ts, packages/spec/scripts/build-api-surface.ts, packages/spec/scripts/check-generated.ts, packages/spec/api-surface/contracts.json, packages/spec/api-surface-signatures.json, packages/spec/export-origins/contracts.json, packages/spec/scripts/build-spec-changes.ts, scripts/check-adr-0087-registration.mjs, docs/adr/0131-total-organization-ownership-no-null-organization-id.md, AGENTS.md

Read-only delta review of PR #24828 at its new head, in a detached worktree, against the PASS record at 357025dc9. Commits since the PASS: 228e74a00 (R1 + R2), 7d8257d60 (changeset), 32b3d1c54 (merge of origin/main through 7c2eab929), 236d39daf (R4). Merge-base moved ddb3360a4 to 7c2eab929 = the 7a-1 landing (#24825). The PR's own diff is 16 files, +698/-985; packages/spec/src/contracts/sharing-service.ts is the one new file, which makes this a SCHEMA face.

① Derived judgments

1. R1 — SharingRuleRow.deploymentWide?: true matches the runtime exactly.

  • Contract: packages/spec/src/contracts/sharing-service.ts:484-492, an optional literal-true member beside the unchanged organization_id?: string | null (:483). Runtime: the only writer of the key is projectDeclaredRule (sharing-rule-service.ts:206-220), whose spread at :209 picks { deploymentWide: true } for DEPLOYMENT_WIDE_SCOPE and { organization_id: scope } otherwise, so a single projection carries the marker and no organization_id key, and a walled projection carries its organization and no marker. The scope list is a unique symbol (:154), the plugin's [null] is translated at one door (setDeclaredRuleScopes :539-540), and resolveDeclaredRuleScopes returns [null] exactly when the posture enforces no wall (sharing-plugin.ts:94-99), so "the one projection a declared rule is served as under the single posture" is the literal behaviour; a lean embedding is that posture. Stored rows never carry it: rowFromRule (:125-146) copies a fixed field list, defineRule writes fixed columns, no sys_sharing_rule column exists. The narrowing type DeploymentWideSharingRule (:168-170) and the guard isDeploymentWideSharingRule (:178-180, marker AND isDeclaredSharingRule :230-232) are unchanged; the marker still cannot be forged through a stored row.
  • TSDoc (:485-491): every clause verified — set on a DECLARED rule's deployment-sweeping projection, the one served under single; carries no organization_id; absent on every stored row and on per-organization projections under a wall. ADR-0131 D2 and D8 exist (docs/adr/0131-...md:242, :578). One nit, non-blocking: the interface-level comment above it (:477-480, "Stored shape ... Maps 1-to-1 to sys_sharing_rule") is now slightly stale, since the same type serves a declared projection with a non-column key; the member's own TSDoc says so, and no reader is misled.
  • Grading: additive optional member; @objectstack/spec: minor added to the shared changeset frontmatter (.changeset/24583-7a3-sharing-null-readers.md:2) with a bullet stating the contract change (:17). AGENTS.md §3 yes takes at least minor; sharing the feat(...)!: changeset without naming spec in the subject scope is the line's convention (precedents feat(automation)! and feat(plugin-webhooks,runtime)!, both bumping spec minor). No major changeset anywhere.
  • Spec gates: none needs an artifact or an entry. api-surface/contracts.json:297 records SharingRuleRow (interface) as a name-and-kind row only; api-surface-signatures.json hashes defineX factories only (build-api-surface.ts:166-171, every key is define*); export-origins/contracts.json:297 is name to file; the declaration map does not list it. The liveness gate covers authorable schema properties (packages/spec/scripts/liveness/ has no contracts reference) and the ADR-0087 registration gate's own text says a type under packages/spec/src/contracts/ is not an object definition (scripts/check-adr-0087-registration.mjs:230-258), so the existing adr-0087: not-required (no-migration-prescription) arm stands; check:spec-changes is a pure projection of the D2/D3 registries (build-spec-changes.ts:5-7). The dev's check:generated exit 0 with nothing regenerated is what these inputs predict; Spec property liveness, Lint & Repo Gates, Lint · repo & tooling gates, Build Core and Type Check · source gates are green on this head. @objectstack/client re-exports the type from @objectstack/spec/contracts (packages/client/src/index.ts:12-13, 176-177), so the member flows through with no client source change.

2. sharing-rule-service.ts since the PASS — TSDoc only. The whole inter-head diff is three comment lines on DeploymentWideSharingRule (:165-167, "the marker itself is declared on the spec contract ... this type narrows it to present"); main did not touch the file. No executable line moved, so every PASS judgment stands: the marker-cannot-be-forged chain (isDeploymentWideSharingRule requires marker AND managed_by === 'package' && id === name; rowFromRule never copies the marker), the walled per-organization scoping (criteriaContext :1783 — string organization first, marker or lean composition to SYSTEM_CTX second, else refused), and organizationsComposed (:1794-1797) reading the real engine's getSchema.

3. R2 — pinned, both the row and its grants, with something to lose. sharing-rule.test.ts:1299-1309 in the [#7761] block: the organization-less row (defineRule under BOOT, :1236-1240; BOOT carries no organization, :1216) is first evaluated under BOOT and grantsOf(seededId) (:1224-1225, source === 'rule' && source_id === ruleId off the unscoped table) is asserted toHaveLength(1); the engine double's getSchema returns undefined (:75), so organizationsComposed() is false and the lean sweep really grants p_red. Then deleteRule(seededId, ORG1_ADMIN) resolves (no throw), the row is still in sys_sharing_rule and the grant count is still 1. Runtime path: deleteRule :1158-1175 resolves through findRuleRow :1104-1126 (by-id under adminOrgScope, then by-name under the same scope) — both miss, declaredRuleNamed returns null with no declared source, silent return. A regression to an unfiltered by-id lookup inside deleteRule would now purge the grant and red this pin, which is the exposure the PASS named. Residual (small): the by-name address deleteRule(SEEDED-name, ORG1_ADMIN) and a system-context delete of the same row are not pinned; the by-id pin exercises the scoped by-name fallthrough only with a string that matches no row's name.

4. R4 — reconciled, holds 7a-1, keeps 7a-3, weakens nothing.

  • 7a-1's ORG-STAMPED expectation is kept verbatim in both passes: expect(granted).not.toContain('deal_p1'), toEqual(['deal_a1']), matchedRecords 1 (rule-criteria-org-scope.test.ts:307-309); the per-record ORG-STAMPED case grants 0 (:393-417). Every line 7a-1 added that the PR's final diff removes is header prose (the "platform-global sweep", "OR field IS NULL" and "The Layer-0 tenant wall's strict equality annihilates the driver's platform bucket: #2734's fix is defeated on every walled read, and the org-less RBAC catalog reads ZERO for every principal #10103 compatibility note" sentences) — none is an assertion.
  • The roster is identical to the PASS head: 8 cases, the two ORG-STAMPED ones plus the 7a-3 set (DECLARED single x2, DECLARED under a wall, STORED no-organization x2, lean CONTROL; :276, :312, :329, :346, :367, :393, :419, :429).
  • Header: the file now has zero hits for platform-global, IS NULL, OR field and compatibility arm; :32 states the driver turns tenantId into organization_id = ?, and the deal_p1 section (:61-68) states that only the deployment-wide sweep reaches it — pinned at :325 (['deal_p1', null]). The title (:4-5) names the declared rule's deployment-wide projection in place of 7a-1's "platform-global rule".
  • The removed deal_p1 pre-delete is safe and the case is now STRONGER: with deal_p1 present, the DECLARED-under-a-wall case asserts grantsAtRest() equals exactly [deal_a1 ORG_A, deal_b1 ORG_B, deal_b2 ORG_B] (:340-344), so a walled projection reaching the organization-less record would red it. At the merge-base the driver has no NULL arm: SqlDriver.applyTenantScope (sql-driver.ts:15587-15621 at 7c2eab929) emits whereIn(field, tenantIds) or where(field, String(tenantId)) and nothing else, with the v17 OR field IS NULL arm named as retired in its own comment. The PASS's "second interaction" note (RECORD_NOT_FOUND on the second walled projection) is closed the same way: no walled projection matches deal_p1, so none re-reads it.

5. Merge hygiene — clean. 32b3d1c54 merges origin/main at exactly 7c2eab929 (its second parent); every file it changed against the PR side is in main's ddb3360a4..7c2eab929 range (set difference empty). Comparing each PR file's own diff against its own merge-base, 11 of 16 files are byte-identical hunk sets to the PASS head; the 5 that differ are precisely R1 (sharing-service.ts new, sharing-rule-service.ts TSDoc, changeset), R2 (sharing-rule.test.ts) and R4 plus the 7a-1 merge resolution (rule-criteria-org-scope.test.ts). Main's only change to a PR file besides that test is approval-service.ts (#24753, decideAndResume, far from the PR's five hunks at :2582-2851, which hash identical). origin/main has since moved one commit past the merge-base (41646a57), touching none of the PR's files.

6. CI on 236d39daf (40 check-runs): every completed run is success (27, including Build Core, Build Docs, Check Changeset, Governed Surface Queue Guard, Spec property liveness, Dogfood Regression Gate, the three completed Type Check gates, all eight Lint gates and the claim/single-writer guards) or skipped (6 opt-in/matrix placeholders). Still in_progress: Test Core (1/6) through (6/6) and Type Check · workspace. No failed check. This PASS does not stand in for their green: under AGENTS.md's queue rule in_progress is not a pass and a red shard is blocking.

② Semver level

minor on @objectstack/spec (new optional contract member, additive), unchanged minor with the BREAKING bang, the FROM/TO migration and the ADR-0087 disposition on @objectstack/plugin-sharing, @objectstack/plugin-approvals, @objectstack/metadata-protocol. Public surface added since the PASS: SharingRuleRow.deploymentWide?: true. Removed: none. No generated artifact, liveness row or ADR-0087 registry entry is owed for the member.

③ Boundary flags

  • R1: closed. The marker is on the contract, documented, graded minor, with no gate artifact owed.
  • R2: closed for the by-id address, row and grants both asserted. Residual, non-blocking: the by-name address and a system-context delete of the organization-less row are unpinned (about 6 lines in the same block).
  • R4: closed. 7a-1's expectation kept, 7a-3's cases kept, header names neither retired arm, the pre-delete's removal strengthens the walled case.
  • R3 (carried by 7b-0/7b-3): unchanged — the seed loader's NULL holder is unseen, not impossible, until NOT NULL at createTable.
  • Open question 1 of the report: unchanged — the plugin still spells single as [null] at setDeclaredRuleScopes's door, translated at one line.
  • Nit (spec, non-blocking): the interface-level TSDoc of SharingRuleRow (sharing-service.ts:477-480) still reads "Stored shape ... maps 1-to-1 to sys_sharing_rule"; a one-line amendment can ride the next step-7 part that touches the spec contract.
  • CI: Test Core 1/6–6/6 and Type Check · workspace in progress at review time; the lane seat reads them green before queueing.

Implemented-by: claude/issue-24583-7a3-sharing-null-readers
Reviewed-by: session_01Y5583bs2RDd53MH6kM1ca3

VERDICT: PASS

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 10:42
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 10:42
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 7cacad5 Oct 20, 2026
43 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24583-7a3-sharing-null-readers branch October 20, 2026 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants