fix(metadata-protocol): let an org-scoped caller revert an env-wide commit (#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

Author: claude

.changeset/revert-commit-org-scope.md

@@ -0,0 +1,98 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+fix(metadata-protocol): let an org-scoped caller revert an env-wide commit (#7819, tier 1)
+
+`revertCommit` and `rollbackToPackageCommit`'s target lookup each resolved
+their target commit with a strict `organization_id` equality:
+
+```ts
+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. Both lookups now accept org-scoped **or** env-wide rows, the same `$or`
+`deletePackage` (#7705) and `listCommits` (#7779) already carry.
+
+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`
+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 converted this
+from silent to loud (pre-#7814: `{success: true, revertedCommits: []}` with the
+changes still live; after it: `success: false` naming the commit), so this
+closes a blocked-but-attributable operation rather than a silent data defect.
+
+## Why the `$or` here, and not the other two remedies
+
+Unlike the earlier members 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** rather than scan scoping, 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. The org never gates the call.
+2. The `organizationId` that arrives is the session's *active org selection*
+   from `resolveActiveOrganizationId`, 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, not 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 that one.
+
+The decisive in-code evidence is that the **body already accepted what the
+lookup refused**: #7559 made `revertCommit` resolve each item's scope from the
+row rather than the request, precisely because "a batch legitimately mixes an
+env-wide artifact with an org overlay". `rollbackToPackageCommit` made the
+contradiction self-evident — since #7814 it plans from `listCommits` (org +
+env-wide) and fed each id straight back into a lookup that refused half of them.
+
+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.
+
+## Pin
+
+`packages/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, 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: the premise measured out of
+SQLite, the positive for each site, **both** negative directions (another
+organization's commit refused on each site; another package's commits not
+reached by the planner), and the no-org door on each site. Refusals are asserted
+on `code` **and** `status` per ADR-0112, never on "it threw".
+
+`package-list-commits-org-scope.integration.test.ts` (#7814) carried the handoff
+assertion that pinned this defect as known-incomplete
+(`rollback.success === false`, `failed == [c2]`); it now asserts the rollback
+succeeds and reverts `c2`, and survives as the family's end-to-end case.
+
+**Reverse verification**, direction predicted before running: restoring the
+strict equality turns exactly the two positive cases red plus the updated
+handoff assertion, and leaves both negative directions and both no-org doors
+green, since strict equality is *narrower* than the `$or`. Measured: 3 failed |
+11 passed, exactly those three.
+
+## Scope
+
+Tier 1 of #7819 only. The two remaining strict equalities in this file —
+`duplicatePackage` and `reassignOrphanedMetadata`, a different table
+(`sys_metadata`) whose step one is the unanswered "are these states even
+reachable" — are deliberately untouched, and #7819 stays open to carry them.

packages/metadata-protocol/src/protocol.ts

@@ -12212,7 +12212,52 @@ export class ObjectStackProtocolImplementation implements
         await this.ensureOverlayIndex();
         const orgId = request.organizationId ?? null;
         const where: Record<string, unknown> = { id: request.commitId };
-        if (request.organizationId) where.organization_id = request.organizationId;
+        // [#7819] Resolve BOTH org-scoped and env-wide (`organization_id IS
+        // NULL`) commit rows for an org-scoped caller — the same defect and
+        // remedy as the sibling {@link listCommits} (#7779) and {@link
+        // deletePackage} (#7705). `organization_id = <org>` matches no NULL
+        // column, so this answered `COMMIT_NOT_FOUND` (404) for a row that
+        // demonstrably exists and that the SAME caller's `listCommits`
+        // returns.
+        //
+        // ⚠️ This site is NOT the family's plain scan-scoping, and the `$or`
+        // was chosen over the two alternatives rather than copied. `where` is
+        // keyed on `id`, so the predicate reads like an AUTHORIZATION filter
+        // layered on a unique key. Measured against the only door, it is not
+        // one: authorization on `POST /packages/:id/commits/:commitId/revert`
+        // is `requireManageMetadata`, checked before this call; the
+        // `organizationId` that arrives is the session's *active org
+        // selection* from `resolveActiveOrganizationId`, whose body is
+        // entirely `catch`-wrapped and answers `undefined` on any auth-seam
+        // throw — and `undefined` omits this predicate, which is the WIDEST
+        // reading (every organization's commits). A boundary that fails OPEN
+        // is not a boundary, so there is no authz here to make precise; that
+        // rules out "keep it but distinguish 'not yours' from 'no such
+        // commit'". Dropping the predicate outright is defensible on an id
+        // lookup, but 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 that one.
+        //
+        // The body already agreed with this reading before the lookup did:
+        // #7559 made each item resolve its scope FROM THE ROW ({@link
+        // resolveMetaItemOrgScope}) precisely because "a batch legitimately
+        // mixes an env-wide artifact with an org overlay", so the loop below
+        // processes env-wide items for an org caller while the lookup above
+        // refused to hand them over. {@link rollbackToPackageCommit} made the
+        // contradiction self-evident: since #7814 it plans from `listCommits`
+        // (org + env-wide) and fed each id straight back into this lookup.
+        //
+        // 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.
+        if (request.organizationId) {
+            where.$or = [
+                { organization_id: request.organizationId },
+                { organization_id: null },
+            ];
+        }
         const row = (await this.engine.findOne('sys_metadata_commit', { where })) as any;
         if (!row) {
             const err: any = new Error(`[commit_not_found] No commit '${request.commitId}'.`);
@@ -12487,7 +12532,20 @@ export class ObjectStackProtocolImplementation implements
         failed: Array<{ commitId: string; error: string }>;
     }> {
         const where: Record<string, unknown> = { id: request.commitId };
-        if (request.organizationId) where.organization_id = request.organizationId;
+        // [#7819] Same widening as the {@link revertCommit} lookup above, and
+        // for the sharper reason: this function PLANS from {@link listCommits},
+        // which since #7814 returns org-scoped and env-wide rows alike to an
+        // org caller. With the strict equality here, an org-scoped rollback
+        // whose TARGET happened to be recorded env-wide answered 404 before it
+        // planned anything at all — for a commit the caller's own timeline had
+        // just listed. The rationale for the `$or` over the alternatives, and
+        // for leaving the no-org branch un-narrowed, is stated in full there.
+        if (request.organizationId) {
+            where.$or = [
+                { organization_id: request.organizationId },
+                { organization_id: null },
+            ];
+        }
         const target = (await this.engine.findOne('sys_metadata_commit', { where })) as any;
         if (!target) {
             const err: any = new Error(`[commit_not_found] No commit '${request.commitId}'.`);

packages/runtime/src/package-list-commits-org-scope.integration.test.ts

@@ -303,21 +303,30 @@ describe('#7779 — org-scoped listCommits must not hide env-wide commit rows',
     const commits = await p.listCommits({ packageId: PKG, organizationId: ACTIVE_ORG });
     expect(idsOf(commits)).toEqual([c1, c2].sort());
 
-    // ⚠️ KNOWN REMAINING GAP, measured and reported on #7779 rather than fixed
-    // here — `packages/metadata-protocol/src/protocol.ts` is serialized and
-    // this card holds it for `listCommits` alone.
+    // [#7819 tier 1] ⭐ THE GAP THIS SUITE HANDED ON IS NOW CLOSED — these
+    // lines are the handoff, and they changed exactly as it predicted.
     //
-    // `revertCommit` (its own `findOne`) and `rollbackToPackageCommit` (its
-    // target lookup) still carry the byte-identical strict equality. So the
-    // planner now SEES C2 and asks `revertCommit` to undo it, and that lookup
-    // still cannot find an env-wide row: the rollback reports
-    // `success: false` naming C2, instead of the silent `success: true` it
-    // reported before. That is strictly better — the failure is now loud,
-    // attributable and non-destructive rather than invisible — but it is not
-    // the whole repair, and this assertion is here so the remaining half
-    // cannot drift unnoticed before its own card lands.
+    // What they asserted until #7819: `revertCommit` (its own `findOne`) and
+    // `rollbackToPackageCommit` (its target lookup) still carried the
+    // byte-identical strict equality, because `protocol.ts` is serialized and
+    // #7779 held it for `listCommits` alone. So the planner SAW C2 and asked
+    // `revertCommit` to undo it, and that lookup could not resolve an env-wide
+    // row — the rollback answered `success: false` naming C2. Already strictly
+    // better than the silent `success: true` of before #7814 (loud,
+    // attributable, non-destructive), but still a legitimate operation
+    // blocked; the assertion existed so the remaining half could not drift
+    // unnoticed before its own card landed.
+    //
+    // #7819 tier 1 widened both lookups to the same `$or` this suite pinned
+    // for `listCommits`, so C2 now resolves and is actually undone. The case
+    // survives as the family's END-TO-END pin: the planner sees the env-wide
+    // commit (asserted above) AND can now act on it — the only combination
+    // under which an org-scoped rollback past an env-wide publish does what it
+    // reports. Its own negative directions live in the sibling
+    // `package-revert-commit-org-scope.integration.test.ts`.
     const rollback = await p.rollbackToPackageCommit({ commitId: c1, organizationId: ACTIVE_ORG });
-    expect(rollback.success).toBe(false);
-    expect(rollback.failed.map((f: any) => f.commitId)).toEqual([c2]);
+    expect(rollback.failed).toEqual([]);
+    expect(rollback.success).toBe(true);
+    expect(rollback.revertedCommits).toEqual([c2]);
   });
 });