Skip to content

fix(objectql): declare the tenant-scope index on an author-declared organization_id (#8459) - #8609

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-8459-tenant-index-declared-column
Aug 14, 2026
Merged

fix(objectql): declare the tenant-scope index on an author-declared organization_id (#8459)#8609
os-zhuang merged 3 commits into
mainfrom
claude/issue-8459-tenant-index-declared-column

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8459

Implements the maintainer ruling of 2026-08-13 (option A): on a multi-tenant deployment the platform declares the tenant-scope index whenever the object carries organization_id — whether the platform provisioned that column or the author declared it.

What was wrong

provisionTenantScopeIndex gated the index on the column being the platform's own definition, byte-for-byte (isInjectedColumnDefinition). An author who declared their own organization_id — adding a label, making it required, pointing it at their own org table — kept their column and silently lost the index on it. computeTenantLayer0Filter (plugin-security) goes on AND-composing the tenant predicate onto essentially every read of that object, so the deployment's hottest predicate ran unindexed. Isolation still held; it was slow, not wrong, which is exactly why it went unreported.

The condition was never argued for. Before #8375 the index push sat physically nested inside the field-injection branch, so "the author declared the column" and "the platform declares no index" were the same condition by nesting. #8375 lifted the decision into one named predicate and preserved the behaviour exactly rather than widening it inside a convergence fix — which is what made it a decision that could then be taken on purpose.

The change

One line of behaviour, inside the one named predicate #8375's convergence created. platformOwnsTenantColumn (eligibility AND platform-provenance) becomes carriesTenantScopeColumn (eligibility alone). ⛔ Not re-nested in the field-injection branch; ⛔ no type inspection — a text org code is indexed too, which was the rejected option C.

Unchanged and pinned, all three asserted on the stored/answered value:

  1. the platform still never overwrites an author-declared organization_id (the field half stays exactly as it was);
  2. an object that already declares its own single-column tenant index still gets none from the platform — the opt-out for a different index shape, unbypassed;
  3. a single-tenant deployment declares no tenant index at all, and an object that opts out of the tenant column gets neither column nor index.

The write-side strip needed no edit: stripProvisionedTenantIndexFrom re-stamps the remainder through provisionTenantScopeIndexitself, so widening the stamp widened the strip in the same edit. That is measured rather than assumed — see the round-trip case below.

⚠️ Operational note

DDL-bearing on the next syncSchema for deployments carrying author-declared organization_id columns: the driver will create an index it did not create before. Index creation is additive and idempotent — no data migration, no column change, re-running it is a no-op — but large existing tables will pay an index build once, at the first sync after upgrading.

Prediction vs. measurement

Predictions were written to file before each run.

Ablation 0 — the change absent, new pins present. Predicted 9 red / 10 green, with the identities named per case. Measured exactly that: 9 red / 10 green, every identity as predicted. The reds are the producer, the registry answer, the read exit (each for both column shapes), the composite-index case, and idempotence.

Four targeted ablations then ran from the committed state, because a pin that is green both before and after the change proves nothing on its own:

ablationpredictedmeasuredagreed
remove the declaresTenantIndex opt-out guardthe 4 opt-out cases red, plus pre-existing pins — more than the minimum13 red: 4 opt-out + idempotence + 4 read-exit/registry (double-stamp) + 2 round-trip + 2 from the #6810 pinyes
drop the field-injection branch guardGREEN — a no-op, because additions LOSE to schema.fields in the mergegreen, 140/140yes
make the platform definition WIN (overrides)field-half cases red5 red, including the ruling's named pin "does NOT overwrite an author-declared organization_id"yes, on the second attempt
drop the multiTenant gatesingle-tenant pins red across all four files5 red across all four filesyes
swap the eligibility gate for bare column-presenceexactly the 2 systemFields.tenant: false cases red5 red — the 2 predicted plus 3 round-trip pins I did not predictno

Where they differed, and what it taught:

  • The overrides ablation was wrong on the first attempt — I restored registry.ts from the commit and then applied only half of a two-part ablation, so the overwrite line sat behind the guard that stops it running on the author-declared path. It measured green, contradicting my prediction. The mechanism was my ablation, not the code; applying both halves reddened the field-half pins as predicted. Worth recording because a half-applied ablation reads exactly like a vacuous pin.
  • The eligibility-gate ablation reddened three round-trip pins I had not predicted, and the reason matters enough that it is now a comment in the source. The save path strips the injected columns before it strips the materialized stamps (stripMaterializedFromRegistry(type, stripServedSystemColumns(type, item)), metadata-protocol). So by the time the strip re-stamps through this predicate, the body no longer has an organization_id at all: a field-map predicate answers "not tenant-scoped", the re-stamp adds nothing, the lists differ, the strip refuses, and the platform's own entry is baked into sys_metadata.metadata, its checksum and every history diff — the [P3] Read decorations (_diagnostics, _draft) round-trip into persisted sys_metadata bodies #4326 regression. Reading the object's declarations reaches the same verdict on a stripped body as on a whole one, which is what makes the stamp and its inverse agree. That makes the eligibility gate load-bearing for the write path, not merely the minimal diff.
  • The naive field-half ablation being a no-op is itself the finding that the field half is protected by merge order, not by the branch condition that appears to guard it.

Why the harness is not vacuous

Not "the tests pass". Every new assertion reads the stored or answered value — the whole indexes array by value, and the whole field definition by value. No assertion anywhere counts entries or compares a delta, deliberately: with the strip fully ablated the served list goes undefined, 1, 1, 1 across round trips — one phantom entry, then it stabilizes, because declaresTenantIndex guards the append — so "the length did not grow" is green with the change entirely absent.

Each pin's colour was measured in both directions rather than assumed: the ones that go red when the change is removed (ablation 0), and the ones that are green either way, each of which has a named ablation above that reddens it. The two controls exist so that a change indexing unconditionally would fail: the single-tenant deployment, and the object that opts out of the tenant column.

Verification

  • pnpm --filter @objectstack/objectql test203 files, 3579 tests passed; typecheck clean.
  • Downstream consumer sweep (prefix filter direction, 43 packages downstream of @objectstack/objectql; the ones reading this surface run here): rest 1881, metadata-protocol 1261, plugin-security 1079, metadata 603, metadata-core 162, lint 2016, driver-sql 1480, driver-mongodb 384 — all passing.
  • Gates re-derived from the actual changed paths with scripts/pm/dispatch-gates.mjs and re-run after the final commit and after merging origin/main: check:nul-bytes, check:changeset-gate-self-tests, check:durability-log-level, check:objectui-changeset, check:query-options-erasure, check:type-check-coverage, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-engine-split-ratio — all pass. pnpm check:type-check-debt (self-test + re-measure, on a fully built workspace): OK, surplus: none. No ledger grew.

Adjacent finding, filed separately

#8608 — two derivations of "is this object tenant-scoped" disagree on systemFields: false. plugin-security's tenancyDisabled reads only tenancy.enabled === false / systemFields.tenant === false, so an object using the hard opt-out while declaring its own organization_id still gets the wall predicate composed onto every read, while the platform withholds the index. Same shape as this card, reached by a different route, and not introduced by this change. Filed unassigned; it needs its own decision because one of the candidate fixes narrows a wall.


Generated by Claude Code

os-zhuangand others added 3 commits August 13, 2026 23:54
…rganization_id (#8459)
On a multi-tenant deployment the platform declares
`indexes: [{ fields: ['organization_id'] }]` so the tenant wall's predicate --
AND-composed by `computeTenantLayer0Filter` onto essentially every read -- is
served by an index. That declaration was gated on the column being the
platform's OWN injected definition, byte-for-byte.
So an author who declared their own `organization_id` kept the column and
silently lost the index on it. Adding a label, making it required, pointing it
at their own org table: an additive-looking authoring move that removed a
performance guarantee the author never knew they held. Isolation still held --
slow, not wrong, which is why it went unreported.
Per the 2026-08-13 maintainer ruling (option A), the injected-column condition
comes off the INDEX half only, inside the one named predicate #8375's
convergence created. One rule, stated once: on a walled deployment the wall's
predicate is indexed, whoever typed the column. The declared type is NOT
inspected -- a `text` org code is indexed too; that judgement was considered and
rejected as a third predicate at the site where #8375 reduced two to one.
Unchanged, and pinned: the platform never overwrites an author-declared
`organization_id`; an object declaring its own single-column tenant index still
gets none from the platform; a single-tenant deployment declares none at all;
and an object that opts out of the tenant column gets neither column nor index.
The write-side strip needed no edit -- it re-stamps through
`provisionTenantScopeIndex` itself, so widening the stamp widened the strip.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
…clarations, not the field map
Measured while reverse-verifying #8459: swapping the gate for a field-map check
breaks the WRITE path for ordinary platform-provisioned objects, because the
save path strips the injected columns before it strips the materialized stamps,
so the strip's re-stamp sees a body with no organization_id and refuses --
baking the platform's index entry into sys_metadata (#4326).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
@vercel

vercelBot commented Aug 14, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 14, 2026 1:29am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql.

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

  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx(via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx(via @objectstack/objectql)
  • content/docs/deployment/vercel.mdx(via @objectstack/objectql)
  • content/docs/kernel/contracts/data-engine.mdx(via @objectstack/objectql)
  • content/docs/kernel/runtime-services/examples.mdx(via packages/objectql)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/objectql)
  • content/docs/kernel/services.mdx(via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx(via @objectstack/objectql)
  • content/docs/permissions/system-context.mdx(via packages/objectql)
  • content/docs/plugins/index.mdx(via @objectstack/objectql)
  • content/docs/plugins/packages.mdx(via @objectstack/objectql)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx(via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx(via @objectstack/objectql)

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

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

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.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 14, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 14, 2026 01:44
@os-zhuang
os-zhuang enabled auto-merge August 14, 2026 01:45
@os-zhuang
os-zhuang added this pull request to the merge queueAug 14, 2026
Merged via the queue into main with commit 0a5dc29Aug 14, 2026
28 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8459-tenant-index-declared-column branch August 14, 2026 01:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An author-declared organization_id withholds the platform's tenant index — the wall's hottest predicate runs unindexed on a multi-tenant deployment

2 participants

@os-zhuang@claude