|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// Real-engine regression for #7860 — `protocol.revertCommit` recorded its |
| 4 | +// compensating commit under the REQUEST's organization |
| 5 | +// (`recordPackageCommit({ orgId: request.organizationId ?? null })`) rather |
| 6 | +// than under the scope of the commit it was reverting. |
| 7 | + |
| 8 | +import { describe, it, expect, afterEach } from 'vitest'; |
| 9 | +import { mkdtempSync, rmSync } from 'node:fs'; |
| 10 | +import { tmpdir } from 'node:os'; |
| 11 | +import { join } from 'node:path'; |
| 12 | +import { ObjectQL } from '@objectstack/objectql'; |
| 13 | +import { SqlDriver } from '@objectstack/driver-sql'; |
| 14 | +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; |
| 15 | +import { |
| 16 | + SysMetadataObject, |
| 17 | + SysMetadataHistoryObject, |
| 18 | + SysMetadataAuditObject, |
| 19 | + SysMetadataCommitObject, |
| 20 | +} from '@objectstack/metadata-core'; |
| 21 | + |
| 22 | +/** |
| 23 | + * The invariant, stated once: a revert commit is visible to exactly the |
| 24 | + * readers who can see the commit it reverts. |
| 25 | + * |
| 26 | + * --------------------------------------------------------------------------- |
| 27 | + * Why this was a REPORTING defect and not a design question |
| 28 | + * --------------------------------------------------------------------------- |
| 29 | + * The card (#7860) was filed explicitly NOT as a defect — the behaviour is |
| 30 | + * self-consistent for the caller who performed the revert, and it asked for a |
| 31 | + * measurement first: after an org-scoped revert of an env-wide commit, what |
| 32 | + * does a DIFFERENT organization's `listCommits` show, and what does the no-org |
| 33 | + * (direct-mount REST) door see? Measured here, on a real driver, before any |
| 34 | + * edit: |
| 35 | + * |
| 36 | + * actor (`org_active`) → [revert, apply] coherent |
| 37 | + * different org → [apply] the env-wide publish, with NO |
| 38 | + * compensation anywhere after it |
| 39 | + * no-org REST → [revert, apply] coherent |
| 40 | + * |
| 41 | + * The middle row is the defect, and what makes it more than cosmetic is the |
| 42 | + * artifact state measured alongside it: `sys_metadata` held NO row for the |
| 43 | + * reverted view afterwards. `revertCommit` resolves each item's scope from the |
| 44 | + * ROW (#7559), so the artifact really was removed ENV-WIDE — the effect is |
| 45 | + * global while the record documenting it was private. A reader in another |
| 46 | + * organization saw an `apply` commit that was never compensated, for an |
| 47 | + * artifact that had in fact already been withdrawn underneath it. And since |
| 48 | + * #7814, `rollbackToPackageCommit` PLANS from `listCommits`, so this list is |
| 49 | + * not merely an observability surface. |
| 50 | + * |
| 51 | + * The mirror direction is the same mismatch pointed the other way and is |
| 52 | + * pinned below: a no-org caller reverting an ORG-SCOPED commit stamped the |
| 53 | + * revert env-wide, so every other organization read a dangling `Revert: …` |
| 54 | + * whose `parentCommitId` names a commit that door cannot see. |
| 55 | + * |
| 56 | + * --------------------------------------------------------------------------- |
| 57 | + * Why a REAL engine and a REAL driver |
| 58 | + * --------------------------------------------------------------------------- |
| 59 | + * Every assertion here turns on whether `organization_id = 'org'` matches a |
| 60 | + * NULL column — a property of the driver's SQL, not of a stub's `filter()`. |
| 61 | + * The suites in this family that stub `engine.find` are structurally unable to |
| 62 | + * see any of it. This file seeds through the REAL publish path, exactly as its |
| 63 | + * siblings `package-revert-commit-org-scope.integration.test.ts` (#7819) and |
| 64 | + * `package-list-commits-org-scope.integration.test.ts` (#7814), and lives in |
| 65 | + * `packages/runtime` for the same reason they do: `metadata-protocol` cannot |
| 66 | + * import `objectql` (dependency cycle). |
| 67 | + * |
| 68 | + * ⚠️ These suites resolve `metadata-protocol` through its `dist` while stack |
| 69 | + * traces are source-mapped back to `src`, so an edited-but-unbuilt `src` looks |
| 70 | + * like it is running while the old bytes execute. Every number above was taken |
| 71 | + * with a rebuild between measurements. |
| 72 | + * |
| 73 | + * --------------------------------------------------------------------------- |
| 74 | + * Reachability |
| 75 | + * --------------------------------------------------------------------------- |
| 76 | + * Only since #7819 tier 1. Before it the target lookup answered |
| 77 | + * `COMMIT_NOT_FOUND` (404) for an env-wide row, so an org-scoped caller could |
| 78 | + * not reach the attribution line with a mismatched scope at all. |
| 79 | + */ |
| 80 | + |
| 81 | +const PKG = 'com.repro.attrib'; |
| 82 | +const PLATFORM_PKG = '@objectstack/platform-objects'; |
| 83 | +const ACTIVE_ORG = 'org_active'; |
| 84 | +const OTHER_ORG = 'org_other'; |
| 85 | + |
| 86 | +let cleanup: Array<() => void> = []; |
| 87 | +afterEach(() => { |
| 88 | + for (const c of cleanup) c(); |
| 89 | + cleanup = []; |
| 90 | +}); |
| 91 | + |
| 92 | +/** REAL ObjectQL wired to a REAL SqlDriver over on-disk better-sqlite3. */ |
| 93 | +async function boot() { |
| 94 | + const dir = mkdtempSync(join(tmpdir(), 'os-7860-')); |
| 95 | + cleanup.push(() => rmSync(dir, { recursive: true, force: true })); |
| 96 | + |
| 97 | + const driver = new SqlDriver({ |
| 98 | + client: 'better-sqlite3', |
| 99 | + connection: { filename: join(dir, 'data.sqlite') }, |
| 100 | + useNullAsDefault: true, |
| 101 | + }); |
| 102 | + const objects = [ |
| 103 | + SysMetadataObject, |
| 104 | + SysMetadataHistoryObject, |
| 105 | + SysMetadataAuditObject, |
| 106 | + SysMetadataCommitObject, |
| 107 | + ] as any[]; |
| 108 | + await driver.initObjects(objects); |
| 109 | + |
| 110 | + const engine = new ObjectQL(); |
| 111 | + engine.registerDriver(driver as any, true); |
| 112 | + await engine.init(); |
| 113 | + for (const o of objects) engine.registry.registerObject(o, PLATFORM_PKG); |
| 114 | + cleanup.push(() => { void engine.destroy(); }); |
| 115 | + |
| 116 | + // `'package-author'` is the control-plane assembly's channel — the #4463 |
| 117 | + // runtime authoring gate would otherwise refuse the seeding saves below. |
| 118 | + const protocol = new ObjectStackProtocolImplementation( |
| 119 | + engine as any, undefined, undefined, 'package-author', |
| 120 | + ); |
| 121 | + return { engine, protocol }; |
| 122 | +} |
| 123 | + |
| 124 | +const viewBody = (name: string) => ({ |
| 125 | + name, |
| 126 | + label: name, |
| 127 | + type: 'grid', |
| 128 | + object: 'anything', // [#7741] the inline arm requires the object binding pair |
| 129 | + viewKind: 'list', |
| 130 | + data: { provider: 'object', object: 'anything' }, |
| 131 | + columns: ['id'], |
| 132 | +}); |
| 133 | + |
| 134 | +/** |
| 135 | + * Author one draft and publish it — what records ONE commit row. The commit's |
| 136 | + * `organization_id` is the PUBLISH REQUEST's org (`?? null`), so omitting |
| 137 | + * `organizationId` reproduces exactly what the dispatcher sends when the |
| 138 | + * session has no active organization. |
| 139 | + */ |
| 140 | +async function publishOne( |
| 141 | + protocol: any, |
| 142 | + args: { view: string; packageId: string; organizationId?: string; message: string }, |
| 143 | +): Promise<string> { |
| 144 | + await protocol.saveMetaItem({ |
| 145 | + type: 'view', |
| 146 | + name: args.view, |
| 147 | + item: viewBody(args.view), |
| 148 | + packageId: args.packageId, |
| 149 | + mode: 'draft', |
| 150 | + }); |
| 151 | + const res = await protocol.publishPackageDrafts({ |
| 152 | + packageId: args.packageId, |
| 153 | + ...(args.organizationId ? { organizationId: args.organizationId } : {}), |
| 154 | + message: args.message, |
| 155 | + }); |
| 156 | + expect(res.success).toBe(true); |
| 157 | + expect(res.commitId).toBeTruthy(); |
| 158 | + return res.commitId as string; |
| 159 | +} |
| 160 | + |
| 161 | +const ops = (commits: any[]) => commits.map((c) => c.operation); |
| 162 | + |
| 163 | +describe('#7860 — a revert commit is attributed to what it reverted, not to who asked', () => { |
| 164 | + it('an org caller reverting an ENV-WIDE commit records the revert env-wide', async () => { |
| 165 | + const { engine, protocol } = await boot(); |
| 166 | + const p = protocol as any; |
| 167 | + const envWide = await publishOne(p, { |
| 168 | + view: 'attr_env', packageId: PKG, message: 'env-wide publish', |
| 169 | + }); |
| 170 | + |
| 171 | + const result = await p.revertCommit({ commitId: envWide, organizationId: ACTIVE_ORG }); |
| 172 | + expect(result.success).toBe(true); |
| 173 | + |
| 174 | + // Straight out of SQLite. Pre-fix this row carried `org_active`. |
| 175 | + const rows = (await engine.find('sys_metadata_commit', { where: {} })) as any[]; |
| 176 | + expect(rows.map((r) => ({ op: r.operation, org: r.organization_id ?? null }))).toEqual([ |
| 177 | + { op: 'apply', org: null }, |
| 178 | + { op: 'revert', org: null }, |
| 179 | + ]); |
| 180 | + }); |
| 181 | + |
| 182 | + it('a DIFFERENT organization sees the compensation, not a bare uncompensated publish', async () => { |
| 183 | + const { engine, protocol } = await boot(); |
| 184 | + const p = protocol as any; |
| 185 | + const envWide = await publishOne(p, { |
| 186 | + view: 'attr_env', packageId: PKG, message: 'env-wide publish', |
| 187 | + }); |
| 188 | + await p.revertCommit({ commitId: envWide, organizationId: ACTIVE_ORG }); |
| 189 | + |
| 190 | + // THE defect this card was opened to measure. Pre-fix: `['apply']` — the |
| 191 | + // env-wide publish alone, with nothing recording that it was undone. |
| 192 | + const asOther = await p.listCommits({ packageId: PKG, organizationId: OTHER_ORG }); |
| 193 | + expect(ops(asOther)).toEqual(['revert', 'apply']); |
| 194 | + expect(asOther[0].parentCommitId).toBe(envWide); |
| 195 | + |
| 196 | + // Why the omission mattered: the artifact really is gone ENV-WIDE (items |
| 197 | + // revert in the ROW's scope, #7559), so the reader above was being shown |
| 198 | + // an `apply` that had already been withdrawn underneath it. |
| 199 | + const meta = (await engine.find('sys_metadata', { where: { name: 'attr_env' } })) as any[]; |
| 200 | + expect(meta).toEqual([]); |
| 201 | + }); |
| 202 | + |
| 203 | + it('the actor and the no-org REST door keep the timeline they already had', async () => { |
| 204 | + const { protocol } = await boot(); |
| 205 | + const p = protocol as any; |
| 206 | + const envWide = await publishOne(p, { |
| 207 | + view: 'attr_env', packageId: PKG, message: 'env-wide publish', |
| 208 | + }); |
| 209 | + await p.revertCommit({ commitId: envWide, organizationId: ACTIVE_ORG }); |
| 210 | + |
| 211 | + // Both were coherent BEFORE the fix and must stay so after it — the change |
| 212 | + // may only ADD the missing reader, never trade one blind spot for another. |
| 213 | + expect(ops(await p.listCommits({ packageId: PKG, organizationId: ACTIVE_ORG }))) |
| 214 | + .toEqual(['revert', 'apply']); |
| 215 | + expect(ops(await p.listCommits({ packageId: PKG }))).toEqual(['revert', 'apply']); |
| 216 | + }); |
| 217 | + |
| 218 | + it('mirror — the no-org door reverting an ORG-SCOPED commit records the revert in THAT org', async () => { |
| 219 | + const { engine, protocol } = await boot(); |
| 220 | + const p = protocol as any; |
| 221 | + const owned = await publishOne(p, { |
| 222 | + view: 'attr_owned', packageId: PKG, organizationId: ACTIVE_ORG, message: 'org_active publish', |
| 223 | + }); |
| 224 | + |
| 225 | + const result = await p.revertCommit({ commitId: owned }); |
| 226 | + expect(result.success).toBe(true); |
| 227 | + |
| 228 | + const rows = (await engine.find('sys_metadata_commit', { where: {} })) as any[]; |
| 229 | + expect(rows.map((r) => ({ op: r.operation, org: r.organization_id ?? null }))).toEqual([ |
| 230 | + { op: 'apply', org: ACTIVE_ORG }, |
| 231 | + { op: 'revert', org: ACTIVE_ORG }, |
| 232 | + ]); |
| 233 | + |
| 234 | + // Pre-fix the revert was stamped env-wide, so an unrelated organization |
| 235 | + // read a DANGLING `Revert: …` whose parent it cannot see. The owning org |
| 236 | + // and the no-org door still see the pair. |
| 237 | + expect(await p.listCommits({ packageId: PKG, organizationId: OTHER_ORG })).toEqual([]); |
| 238 | + expect(ops(await p.listCommits({ packageId: PKG, organizationId: ACTIVE_ORG }))) |
| 239 | + .toEqual(['revert', 'apply']); |
| 240 | + expect(ops(await p.listCommits({ packageId: PKG }))).toEqual(['revert', 'apply']); |
| 241 | + }); |
| 242 | +}); |
0 commit comments