Skip to content

fix(metadata-protocol): let an org-scoped caller revert an env-wide commit (#7819 tier 1) - #7857

Merged
huangyiirene merged 2 commits into
mainfrom
claude/issue-7819-org-scope-tier1
Aug 11, 2026
Merged

fix(metadata-protocol): let an org-scoped caller revert an env-wide commit (#7819 tier 1)#7857
huangyiirene merged 2 commits into
mainfrom
claude/issue-7819-org-scope-tier1

Conversation

@huangyiirene

@huangyiirene huangyiirene commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Part of #7819

Part of, not Fixes — deliberately. This is tier 1 only (revertCommit, rollbackToPackageCommit's target lookup). Tier 2 (duplicatePackage, reassignOrphanedMetadata) is a different table whose step one is the unanswered "are these states even reachable", and #7819 stays open to carry it.

The defect

Both sites resolved their target commit with the strict equality the rest of this family carried:

const where = { id: request.commitId };
if (request.organizationId) where.organization_id = request.organizationId;

organization_id = 'org' matches no row whose column is NULL, so an org-scoped caller got COMMIT_NOT_FOUND (404) for any commit recorded env-wide — a row that demonstrably exists and that the same caller's listCommits hands back.

Env-wide commit rows are not hypothetical: recordPackageCommit stores request.organizationId ?? null, and the publish door forwards an org only when resolveActiveOrganizationId yields one — a resolver that answers undefined both for a session with no active organization and for any throw on the auth seam. A publish made before an org was selected lands its commit env-wide, permanently, since the timeline is append-only.

User-visible change: an org-scoped rollback past an env-wide publish now performs the rollback instead of refusing it. #7814 had already turned this from silent to loud (pre-#7814: {success: true, revertedCommits: []} with the changes still live; after: success: false naming the commit), so this closes a blocked-but-attributable operation, not a silent data defect.

The design decision — the $or was chosen, not copied

Unlike every earlier member of this family, where is keyed on id — a primary-key lookup — so the org predicate reads like an authorization filter on a unique key, and widening it would be widening an authorization boundary.

Measured against the only door, it is not one:

  1. Authorization on POST /packages/:id/commits/:commitId/revert and POST /packages/:id/rollback is requireManageMetadata, checked before the protocol call (packages/runtime/src/domains/packages.ts). The org never gates the call.
  2. The organizationId that arrives is the session's active org selection from resolveActiveOrganizationId (packages/runtime/src/http-dispatcher.ts), whose body is entirely catch-wrapped.
  3. On any auth-seam throw it answers undefined, which omits the predicate — the widest reading, every organization's commits.

A boundary that fails open is not a boundary. That rules out remedy 3 (keep the check, distinguish "not yours" from "no such commit"): there is no authorization here to make precise, and asserting one would be inventing a boundary rather than repairing one.

Remedy 2 (drop the predicate outright — defensible on an id lookup) was rejected because it would newly let an org caller revert another organization's commit by id, a widening this card never asked for. The $or admits the env-wide rows and refuses exactly that — pinned as its own case on both sites.

The decisive in-code evidence: #7559 already made revertCommit resolve each item's scope from the row rather than the request, with the rationale that "a batch legitimately mixes an env-wide artifact with an org overlay". Verified rather than taken on faith: resolveMetaItemOrgScope answers null — env scope — for an item whose history is env-wide even when the request carries an org. The body already processed env-wide rows for an org caller while the lookup above refused to hand them over. rollbackToPackageCommit closed the argument: since #7814 it plans from listCommits (org + env-wide) and fed each id straight back into a lookup that refused half of them — one function contradicting itself inside a single call.

The no-org branch is deliberately not narrowed to organization_id IS NULL, exactly as #7705 and #7779 left theirs: the direct-mount REST registrar passes no organizationId at all, and restricting that door to env-wide rows would make every org-scoped commit unrevertable — the same bug pointed the other way. Both no-org doors are pinned.

Verification record

Premise re-verified at the branch point: exactly four occurrences remain, at :11792, :11981, :12215, :12490. Positive control: 6 hits for the organization_id: null ($or) shape. After this PR only the two tier-2 sites (:11792, :11981) retain the strict equality.

The pinpackages/runtime/src/package-revert-commit-org-scope.integration.test.ts, a real ObjectQL over a real SqlDriver on better-sqlite3, seeded through the real publish path. Real engine and real driver because the question is whether organization_id = 'org' matches a NULL column — a property of the driver's SQL, not of a stub's filter(). It lives in packages/runtime because metadata-protocol cannot import objectql (dependency cycle). Eight cases:

case site
the premise, read straight out of SQLite (organization_id really is NULL)
positive: env-wide commit resolves for an org caller revertCommit
negative: another organization's commit still refused revertCommit
the no-org door: still reverts an org-scoped commit revertCommit
positive: env-wide target resolves for an org caller rollbackToPackageCommit
negative: another organization's target still refused rollbackToPackageCommit
negative: another package's commits not reached by the planner rollbackToPackageCommit
the no-org door: still rolls back to an org-scoped target rollbackToPackageCommit

Refusals are asserted on code and status (COMMIT_NOT_FOUND / 404) per ADR-0112, never on "it threw".

Pre-fix measurement: the two positive cases failed with COMMIT_NOT_FOUND thrown at protocol.ts:12218; all negative directions and both no-org doors already passed.

The handoff assertion changed, as the card required. package-list-commits-org-scope.integration.test.ts (#7814) pinned rollback.success === false / failed == [c2] as a known-incomplete state; it now asserts failed == [], success === true, revertedCommits == [c2] and survives as the family's end-to-end case.

Reverse verification, direction predicted first: restoring the strict equality was predicted to turn exactly the two positive cases red plus the updated handoff assertion, leaving both negative directions and both no-org doors green (strict equality is narrower than the $or). Measured: 3 failed | 11 passed — exactly those three. ⚠️ For the next author: these suites resolve @objectstack/metadata-protocol through its dist (stack traces are source-mapped back to src, which is misleading), so a source-only revert measures nothing — rebuild between measurements.

Patch round — a blind test double, taught rather than accommodated

CI came back red on packages/objectql/src/protocol-commit-history.test.ts: two org-scoped revert cases failed with COMMIT_NOT_FOUND. Measured, not assumed — its matchesWhere was pure flat equality, so it compared row['$or'] against the array and matched nothing.

The double was the blind party, not the fix. Both failing rows carry the caller's own org (organization_id: 'org_a', request org 'org_a'), so they match the first $or branch outright — the same row the strict equality already accepted. Nothing about their subject (#6602's registry org-asymmetry) involves the commit lookup; it is merely the door they enter through. The production fix was not weakened.

It now understands $or/$and, conjoined with the sibling keys in the entries loop — the corrected form #7846 landed across six doubles in this package (part of #7620) an hour before this round, deliberately matched rather than re-invented. Not the early-returning if ($or) return …some(…) shape those six carried before it: that discards sibling keys, so { id, $or: [...] } would stop constraining id and the lookup could return some other commit whose org matched. This file was not among #7846's six because it had no operator handling to correct, so it reads as a new member of the #7620 lane rather than a regression of it.

Judgment asked for, answered plainly: the two assertions remain meaningful. Their subject is the registry org-asymmetry, not the predicate — the lookup is only how they reach it, and the rows they seed match the $or's first branch, so nothing about the operator is doing the work. What would be a test of the double is any assertion whose subject is the org predicate; none exists in that file (which is exactly why it could never see this family), and a comment there now says so and asks that org-scoping cases not be added. The operator's real behaviour against a real driver stays pinned on the real engine in packages/runtime. Flagging the residual honestly: matchesWhere is now a reimplementation of $or semantics, and its fidelity to the real driver is itself unpinned.

Gates (re-run after the rebase onto 55635fc)

gate result
pnpm check:durability-log-level ✅ 24 durability seams, 63 read seams
pnpm check:changeset-gate-self-tests ✅ 118 + 153 + 117 assertions
pnpm check:nul-bytes ✅ 7201 files scanned
pnpm build (full) ✅ 71/71 tasks
@objectstack/objectql 185 files / 3274 tests (was 184/3272 with the two failures — exactly those two restored, nothing else moved)
@objectstack/metadata-protocol ✅ 72 files / 1066 tests
@objectstack/runtime ✅ 135 files / 2071 tests (baseline 134 / 2063; +1 file, +8 tests = this pin)
@objectstack/client ✅ 21 files / 282 tests
@objectstack/runtime typecheck ✅ clean
eslint on the 4 changed files ✅ clean

Coverage sweep after the miss: every suite in the repo that exercises revertCommit / rollbackToPackageCommit was enumerated and run — client, metadata-protocol, objectql, runtime. protocol-revert-org-scope.test.ts already carries the corrected $or form from #7619; the remaining doubles are green unchanged.

No new error code, so check:error-code-casing does not apply.

Scope discipline

packages/metadata-protocol/src/protocol.ts is serialized and was held for tier 1 alone: two hunks, at the two tier-1 sites, nothing else. Two adjacent observations were reported rather than acted on:

Repricing input for tier 2: this change leaves duplicatePackage / reassignOrphanedMetadata completely unaffected — different table (sys_metadata), no shared helper, no shared caller, no diff overlap. Tier 2's step one ("is the state reachable at all") is unchanged and its answer is not implied by anything measured here. One transferable asset: the boot/seed harness in the new suite is a working template for a real-engine sys_metadata pin, so tier 2's pinning cost is lower even though its measurement cost is not.

@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 11, 2026 10:23pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

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

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/metadata-protocol)

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

  • content/docs/releases/v9.mdx (via @objectstack/metadata-protocol)

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.

claude added 2 commits August 11, 2026 21:52
…ommit (#7819 tier 1)

`revertCommit` and `rollbackToPackageCommit`'s target lookup each resolved their
target commit with a strict `organization_id` equality, which matches no row
whose column is NULL. An org-scoped caller therefore got COMMIT_NOT_FOUND (404)
for any commit recorded env-wide — a row that demonstrably exists and that the
same caller's `listCommits` hands back. Both lookups now accept org-scoped or
env-wide rows, the same `$or` `deletePackage` (#7705) and `listCommits` (#7779)
already carry.

The `$or` was chosen over the two alternatives rather than copied. `where` is
keyed on `id`, so the predicate reads like an authorization filter on a unique
key; measured against the only door it is not one. Authorization is
`requireManageMetadata`, checked before the call, and the `organizationId` that
arrives is the session's active org selection from `resolveActiveOrganizationId`
— a resolver whose body is entirely catch-wrapped and whose `undefined` omits
the predicate, i.e. the widest reading. A boundary that fails open is not a
boundary, which rules out "keep the check but distinguish 'not yours' from 'no
such commit'". Dropping the predicate outright would newly let an org caller
revert another organization's commit by id, a widening this card never asked
for. The body already agreed with the `$or`: #7559 made each item resolve its
scope from the row, and since #7814 `rollbackToPackageCommit` plans from
`listCommits` (org + env-wide) and fed each id back into a lookup that refused
half of them.

The no-org branch is deliberately left un-narrowed, exactly as #7705 and #7779
left theirs.

Pinned by a new real-engine/real-driver suite in packages/runtime (eight cases:
the premise out of SQLite, the positive per site, both negative directions, and
the no-org door per site; refusals asserted on code AND status per ADR-0112).
The #7814 handoff assertion that pinned this as known-incomplete now asserts the
rollback succeeds. Reverse verification, direction predicted first: 3 failed |
11 passed, exactly the three positive cases.

Tier 1 only — `duplicatePackage` and `reassignOrphanedMetadata` are untouched
and #7819 stays open to carry them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019hxiiv8qFCUmDThHU1k7HV
… tier 1, part of #7620)

CI red on PR #7857: `packages/objectql/src/protocol-commit-history.test.ts` had
two org-scoped revert cases fail with COMMIT_NOT_FOUND. Measured rather than
assumed: its `matchesWhere` was pure flat equality, so the widened lookup
`{ id, $or: [{organization_id: <org>}, {organization_id: null}] }` compared
`row['$or']` against the array and matched nothing.

The double is the blind party, not the fix. Both failing rows carry the
CALLER'S OWN org (`organization_id: 'org_a'`, request org `'org_a'`), so they
match the FIRST `$or` branch outright — the same row the strict equality already
accepted. No real behaviour changed, and neither case's subject (#6602's
registry org-asymmetry) involves the commit lookup at all; it is merely the door
they enter through.

Conjoined with the sibling keys in the entries loop, matching the corrected form
#7846 landed across six doubles in this package an hour earlier. Not the
early-returning `if ($or) return …some(…)` shape those six carried before it:
that discards sibling keys, so `{ id, $or: [...] }` would stop constraining `id`
and could return some other commit whose org matched. This file was not among
#7846's six because it had no operator handling to correct, so it is a new
member of the #7620 lane rather than a regression of it.

`undefined` normalises to `null` on comparison, same as the six, because a
column a row never set reads as NULL out of a real driver.

@objectstack/objectql: 185 files / 3274 tests passing (was 184/3272 with the two
failures) — exactly the two cases restored, nothing else moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019hxiiv8qFCUmDThHU1k7HV
@huangyiirene
huangyiirene force-pushed the claude/issue-7819-org-scope-tier1 branch from 58bef02 to c21f09e Compare August 11, 2026 22:23
@huangyiirene
huangyiirene marked this pull request as ready for review August 11, 2026 22:45
@huangyiirene
huangyiirene added this pull request to the merge queue Aug 11, 2026
Merged via the queue into main with commit 756fd12 Aug 11, 2026
27 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-7819-org-scope-tier1 branch August 11, 2026 22:58
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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants