Skip to content

fix(metadata-protocol): gate getMetaItem's overlay read on the metadata registry - #14908

Draft
os-musk wants to merge 1 commit into
mainfrom
claude/issue-14770-getmetaitem-overlay-precedence
Draft

fix(metadata-protocol): gate getMetaItem's overlay read on the metadata registry#14908
os-musk wants to merge 1 commit into
mainfrom
claude/issue-14770-getmetaitem-overlay-precedence

Conversation

@os-musk

@os-musk os-musk commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14770

getMetaItem — the singular /meta read verb — now resolves its own read scope through organizationIdForMetaRead, the registry-derived predicate the REST /meta doors have applied since #9454 and that #14683 (PR #14767) moved inside the plural verb.

The defect, measured

At the merge base 84b8190ae, packages/metadata-protocol/src/protocol.ts:7348:

const orgId = request.organizationId;

spent by both overlay reads in the method, the second of which is at :7452:

const record = (orgId ? await findOverlay(orgId) : undefined)
    ?? await findOverlay(null);

⚠️ The card and its triage comment recorded this site at :7382-7383 and then :7453; on the merge base it is :7348 / :7452. Immaterial to the finding, recorded because the file keeps moving.

?? is precedence, not the plural verb's union. On getMetaItems the two queryByOrg reads are UNIONed, so an ungated organization can only ADD rows — the resurrection #14683 is about. Here it can SUBSTITUTE: on a type the registry declares allowOrgOverride: false, a pre-#6190 phantom org-scoped row — the kind loadMetaFromDb walks past and reportUnhydratableOrgScopedRows warns about — was served instead of the live env-wide document, to a caller that asked for the live one. The served document then vanishes at the next restart, because boot hydration does not hydrate that row.

The change

One line, plus the comment that carries the reasoning:

const orgId = organizationIdForMetaRead(request.type, request.organizationId);

Three properties, each deliberate:

⛔ Why precedence stays — the question Zone 2 C asked

The card's title names the read as "?? PRECEDENCE, not a union". That is the defect statement, and it is not a licence to convert the combinator. The repo settles this directly, so no judgement call was needed:

⇒ The union question is answered against a union, with citations, rather than left open. §2 of the new test pins it so a future reading of the title fails a test instead of landing.

The idempotence proof, discharged before the gate was written

The triage made the callee-side direction conditional on showing that moving the predicate inside changes no already-gating caller's scope. Let f(t, o) = organizationIdForMetaRead(t, o).

  • f(t, undefined) === undefined — the predicate returns early on an undefined organization, before it consults the registry flag. Every caller that names no organization reads exactly what it read before: rest/src/import-mapping.ts:66, rest/src/import-prepare.ts:416, rest-server.ts:8634, plugin-email's template read, service-analytics' draft probe, plugin-auth's metaReader. Test §3 sweeps this over the complete accepted-spelling population.
  • f(t, f(t, o)) === f(t, o)f answers o for an overridable type and undefined otherwise, so a second application cannot move it. Test §4 asserts it over the same derived population rather than a hand-listed sample.
  • The REST by-name door gates on the string this method folds to. It computes organizationIdForMetaRead(canonicalMetaUrlType(req.params.type), readCtx?.tenantId) at rest-server.ts:5660 and then passes type: req.params.type, the raw segment; this method folds that segment with the same function, so request.type at the gate is the identical string the door gated on. Its cached arm reaches here through getMetaItemCached, which folds first and forwards the same hoisted readOrganizationId — the same no-op one hop later.
  • organizationIdForMetaWrite has a character-identical body, so the three internal write-side pre-reads — saveMetaItem's destructive-change probe, publishMetaItem's seed-loader adapter, publishPackageDrafts' build probes — now read the partition their write lands in. Read scope and write scope cannot disagree, which is the property Org-overridable metadata (view, dashboard) is accepted with a 200 state:'active' receipt but served by no read door #9454 chose this predicate for.
  • Not achieved by denying everyone: §4's last case pins that a door which already resolved an organization for an overridable type still reaches the org partition.

⇒ Nothing double-scopes and nothing is wrongly denied. What moves is the three ungated callers in packages/runtime/src/domains/meta.ts (:703, :745, :768), two of them hard-coded to type object, which is allowOrgOverride: false — callers that cannot be right about scope by construction. They are left as they are: the ruling was callee-side precisely so the next new caller needs no repair.

Tests

packages/metadata-protocol/src/get-meta-item-org-read-gate.test.ts, 14 cases, modelled on the plural door's pin.

  • §1 — the phantom-shadows-live-row case the card exists for. An env-wide object row and an org-scoped phantom of the same (type, name); a caller passing a raw active organization is served the env-wide document, and the phantom's partition is never read. Repeated across every non-overridable declared type, derived from the registry.
  • §2 — precedence preserved. For view, the org row still wins outright, and a key present only on the env-wide row does not appear in the served document — replacement, not a merge. Plus the ?? fallback chain when the org has no overlay of its own.
  • §3 / §4 — the two idempotence legs, over the complete META_URL_TO_SINGULAR ∪ registry-singular population.
  • §5 — the previewDrafts arm gets the same scope, both directions.
  • §0 pins the premises: the org-overridable set, and that object is not in it.

Ablation (fix committed first, then mutated, then restored):

leg evidence
mutation reached disk injected form 1→2 occurrences, gated form 2→1, both anchored by grep -c; blob 58769b0dd differs from HEAD blob 3111ed266
result with the gate reverted 4 failed / 10 passed — §1 all three (expected 'org_acme showcase_task' to be 'env showcase_task' — the phantom served, the defect reproduced) and §5's draft arm
§2/§3/§4 under mutation still green, as predicted — they do not depend on the gate
restore git diff HEAD empty, git status clean, working blob 3111ed266 byte-identical to the HEAD blob

No rebuild leg was needed: the test imports the subject relatively (./protocol.js) from within its own package, so vitest reads src/protocol.ts directly rather than a dist/ artifact.

Verification, at 23a2b51c2 (the final commit)

check result
pnpm --filter @objectstack/metadata-protocol test 158 files passed, 2 skipped; 2349 tests passed, 10 skipped, exit 0
pnpm --filter @objectstack/metadata-protocol typecheck exit 0 — and tsc --noEmit --listFiles shows the new test file is in the program (1 hit), so this covers it
pnpm --filter '@objectstack/metadata-protocol^...' build exit 0 (dependency closure built before any judgement)
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack 32 families derived from the merge base itself; 30 exit 0
check-test-completeness.mjs, pm/check-half-states.mjs exit 3 = PREREQUISITE NOT MET, recorded as NOT MEASURED on the gates' own instruction — the first needs a saved turbo run test log, the second needs repo-scoped REST egress this container does not have. Neither is a finding.
eslint . --no-inline-config (full repo, not narrowed) exit 0
node scripts/pm/check-governed-merges.mjs --test on the final file list 0 of 3 paths governed
check:nul-bytes plus a manual control-byte scan of all three files clean

Exit codes were captured by redirecting to a file before reading it, never through a pipe.

Scope

Three files. packages/spec is untouched — including the organizationId describe() wording tracked separately on #14772, which is not addressed here and stays open.

Filed while working this, out of scope for this PR: #14907getMetaItemLayered is the same series' third instalment, reached ungated from runtime/src/domains/meta.ts:345. It is deliberately not folded in: its caller population is different, so this PR's idempotence proof does not cover it, and its orgId binds before the canonical fold, so this one-liner does not port.

Changeset

patch, on @objectstack/metadata-protocol — a behaviour correction on an existing published read door. node scripts/check-changeset-no-major.mjs and node scripts/check-empty-changeset.mjs both exit 0. The body states what is unchanged for already-gating and organization-less callers alongside what moves, since that is the part an upgrading consumer needs.

Review

⚠️ needs:contract-review travels from the card to this PR and is cleared only at reviewer PASS — not by this seat. Left as a draft, auto-merge not armed, not marked ready.

🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68

Generated by Claude Code


Generated by Claude Code

…data registry

The singular `/meta` read verb applied no organization gate of its own: whatever
`organizationId` arrived was spent on whatever `type` arrived. Its two overlay
reads combine with `??` — precedence, not the plural verb's union — so an
ungated organization could SUBSTITUTE rather than merely add: on a type the
registry declares `allowOrgOverride: false`, a pre-#6190 phantom org-scoped row
was served instead of the live env-wide document.

Resolve the scope through `organizationIdForMetaRead` once, after the canonical
type fold, for both the active-overlay read and the ADR-0033 `previewDrafts`
read. ADR-0005's overlay-wins precedence is deliberately unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions github-actions Bot added the size/l label Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 3 documentable anchor(s).

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

  • content/docs/api/data-api.mdx (via /:object/export (route, bridged from symbol getMetaItem — its registrar handler names it))
  • content/docs/data-modeling/drivers.mdx (via getMetaItem (symbol, a method of class ObjectStackProtocolImplementation))
  • content/docs/data-modeling/schema-design.mdx (via /:object/export (route, bridged from symbol getMetaItem — its registrar handler names it))
  • content/docs/kernel/services-checklist.mdx (via getMetaItem (symbol, a method of class ObjectStackProtocolImplementation))
  • content/docs/permissions/permission-sets.mdx (via /:object/export (route, bridged from symbol getMetaItem — its registrar handler names it))
  • content/docs/protocol/objectui/actions.mdx (via /:object/export (route, bridged from symbol getMetaItem — its registrar handler names it))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 9 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 23619f579f37aa03fee76a8554ace54a42bd9748packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 23619f579f37aa03fee76a8554ace54a42bd9748

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

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

Projects

None yet

2 participants