Skip to content

fix(objectql): converge the multi-tenant tenant-scope index at every /meta read exit (#8375) - #8458

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-8375-converge-multitenant-indexes-stamp
Aug 13, 2026
Merged

fix(objectql): converge the multi-tenant tenant-scope index at every /meta read exit (#8375)#8458
os-zhuang merged 2 commits into
mainfrom
claude/issue-8375-converge-multitenant-indexes-stamp

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8375

On a multi-tenant deployment GET /meta/object/:name served the object with no indexes key at all whenever the answer came from the metadata service or a sys_metadata overlay row, while the registry's own resolved schema — and the list read — carried indexes: [{ fields: ['organization_id'] }]. Same object, same moment, same host.

The root cause was a second implementation, not a missing line

The read exits converge the #6562 system-column stamp with applyInjectedSystemColumns (@objectstack/metadata-core). That function cannot import its producer: applySystemFields lives in @objectstack/objectql, and objectqlmetadata-protocolmetadata-core, so the import runs UP the dependency graph. So it re-implemented the half it could reach — the fields map — and silently omitted the half it could not, the index.

⚠️Two things the card states are wrong, and they change the fix.applySystemFields is not in packages/spec/src/data; it is packages/objectql/src/registry.ts. And it is therefore not callable from metadata-core — routing the converger through it inverts the layering. Adding an indexes line to the copy was ruled out for the right reason; calling the producer from the copy is impossible for a second one.

So the decision moves into one function, provisionTenantScopeIndex, called by the producer and by materializeBaseLayer — the #8268 seam every read exit already replays. That seam's doc promises "a fourth stamp added to this method is converged at the read exits the day it is added, with no fourth method and no fourth card". This is that fourth stamp, and the promise held: no edit to packages/metadata-protocol/src/protocol.ts was needed (it is at its 3-editor cap), and no new method anywhere.

The decision also had to move out of the injection branch. It was nested under !schema.fields?.organization_id, and at a read exit the column is already presentgovernServedItem runs applyInjectedSystemColumns before the seam — so that condition is false exactly where the convergence is needed. The predicate is now "the object carries a tenant column the PLATFORM provisioned", via isInjectedColumnDefinition, which answers the same question at either site in either order.

The write path owes a counterpart, and indexes concatenates

stripProvisionedTenantIndexFrom removes the last entry identical to the platform's own (the seam appends) and keeps the removal only when re-stamping the remainder reproduces the arriving list byte-for-byte — recomputed through the stamping function itself, so strip and stamp cannot drift. An emptied list drops the key rather than storing indexes: [].

That one comparison separates every case: an author's named entry is never a candidate; an author's own tenant index ordered before their others fails the comparison on order; the identical entry on a single-tenant deployment fails it because the seam would add nothing there. The residual it cannot separate — an author whose last index is byte-identical to the platform's on a multi-tenant host — is the trade stripProvisionedPrimaryFrom documents, bounded the same way: the entry is re-derived at every load, so no resolved answer changes.

Measured

#measurementpredictedactual
1premise on merged mainone diverging key indexes✅ as predicted
2after the fixby-name serves the index; pin flips✅ red at the exact line, then green
3GET → PUT → GET → PUTlist does not grow; row never carries it
4reverse verification, 2 armsread-side red / write-side red separately✅ both directions
5single-tenant controlno index on either route

One prediction was wrong, and it is recorded rather than retro-fitted. The brief expected an unbounded strip to grow the array every cycle. Measured with the strip fully ablated across three cycles, the stored row goes undefined → 1 → 1 → 1: it gains one phantom entry and then stabilizes, because declaresTenantIndex already guards the append. The real defect of a missing strip is a phantom customisation baked into sys_metadata.metadata, its checksum and every history diff — not growth. This matters for the gate: a test asserting only "length does not grow" would pass with the strip entirely absent. The pin asserts the stored row, which is what ablation B reddens, at cycle 1 stored.

Two expected-divergence pins flip, as each was written to

Both were deliberate tripwires, so failing them is the design working:

Verification

⚠️ Every measurement rebuilt dist first — these suites resolve metadata-protocol/objectql through it.


Generated by Claude Code

…/meta read exit (#8375)
`GET /meta/object/:name` served a multi-tenant object with no `indexes` key at
all when the answer came from the `metadata` service or a `sys_metadata` overlay
row, while the registry's own resolved schema — and the list read — carried
`indexes: [{ fields: ['organization_id'] }]`.
The cause was a second implementation, not a missing line.
`applyInjectedSystemColumns` (`@objectstack/metadata-core`) converges the #6562
system-column stamp at the read exits and cannot import its producer
(`applySystemFields`, `@objectstack/objectql`) without running UP the dependency
graph, so it re-implemented the half it could reach — the fields map — and
omitted the index.
So the decision moves into ONE function, `provisionTenantScopeIndex`, called by
the producer and by `materializeBaseLayer` — the #8268 seam every read exit
already replays. Adding a stamp there IS the convergence, which is the property
that seam exists to provide.
The write path owes the counterpart, and `indexes` concatenates under
`mergeObjectDefinitions`: `stripProvisionedTenantIndexFrom` removes the LAST
entry identical to the platform's own and keeps the removal only when re-stamping
the remainder reproduces the arriving list byte-for-byte. A named entry, an
author's tenant index ordered before their others, and the same entry on a
single-tenant deployment are all kept.
Two deliberate expected-divergence pins flip to assert convergence, as each was
written to: the `indexes` case in the rest materialization-agreement suite, and
the #6562/#6810 residual in `protocol-meta-effective-schema`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
@vercel

vercelBot commented Aug 13, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 13, 2026 2:06pm

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 13, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM note before review — three things I got wrong, and one of them was a vacuous gate

CI is running; this is not the review. But three assertions in my dispatch brief were falsified by measurement, and the third is serious enough that it needs recording before anyone reads the diff.

1. My ruled fix shape was impossible as stated

I ruled: "the fix must delete the second implementation and call the canonical one (d5031f6a1 shape), ⛔ not add an indexes line to the copy." The second half stands. The first half cannot be done: applySystemFields lives in packages/objectql/src/registry.ts, not packages/spec/src/data as the card says and as I repeated, and objectqlmetadata-protocolmetadata-core, so calling the producer from the converger runs up the dependency graph.

So the copy could not be deleted in favour of a call. The resolution here honours the intent — one implementation rather than two — by a mechanism I did not name: the decision moves into a single provisionTenantScopeIndex invoked by the producer and by materializeBaseLayer. That is the better answer, and it came from measuring the layering rather than accepting my premise.

2. #8268's seam promise paid off immediately, which is the real headline

materializeBaseLayer's doc promised "a fourth stamp added to this method is converged at the read exits the day it is added, with no fourth method and no fourth card." This is the fourth stamp, and no packages/metadata-protocol/src/protocol.ts edit was needed — which also means it did not consume the file's editor cap. A class fix that shipped ninety minutes ago has now absorbed its first new member at zero marginal cost. That is the argument for promoting #8268 from a third stamp-specific patch, and it is now measured rather than asserted.

Also worth recording: the decision had to move out of the injection branch. It was nested under !schema.fields?.organization_id, and at a read exit the column is already present — governServedItem runs applyInjectedSystemColumns before the seam — so that guard is false exactly where convergence is needed. That is a genuinely subtle failure mode and nothing in the card pointed at it.

3. ⚠️ My required measurement would have passed with the fix entirely absent

This is the one that matters. I wrote into the brief:

⚠️indexes is a concatenated array — an unbounded strip DUPLICATES on every GET→PUT; a two-cycle round trip is required.

Measured, with the strip fully ablated across three cycles: undefined → 1 → 1 → 1. The array gains one phantom entry and then stabilizes, because declaresTenantIndex already guards the append. So the real defect of a missing strip is not growth at all — it is a phantom customisation baked into sys_metadata.metadata, its checksum, and every history diff.

Which means a test asserting only "the length does not grow" — the gate I specified — would pass with the strip entirely absent. I prescribed a measurement that could not detect the defect it was meant to catch. The pin here asserts the stored row instead, which is what the write-side ablation actually reddens, at cycle 1 stored.

I am stating this at length because it is the most dangerous kind of PM error: not a wrong instruction, which a dev pushes back on, but a plausible-looking gate that a dev could have satisfied honestly while shipping nothing. The dev caught it by ablating and reading the numbers rather than by assuming my framing was sound. Predicting the direction of a measurement is not enough — the prediction has to be checked against what the ablation actually does, and that is what happened here.

Also noted

The second flipped pin — the #6562/#6810 residual in protocol-meta-effective-schema.test.ts, whose own comment read "If a served-document consumer of indexes[] ever appears, this is the line that says so"is not named in the issue. It surfaced from the full suite run. Both flipped pins were written as deliberate tripwires, so failing them is the design working, and I will check both were updated to assert the new truth rather than merely deleted.

Full review to follow once CI is green.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 13, 2026 15:31
@os-zhuang
os-zhuang added this pull request to the merge queueAug 13, 2026
Merged via the queue into main with commit fc3a819Aug 13, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8375-converge-multitenant-indexes-stamp branch August 13, 2026 15:51
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

2 participants

@os-zhuang@claude