fix(metadata-protocol): the default /diff range labels its to side with the active row's own version

.changeset/20397-diff-default-range-labels.md

@@ -0,0 +1,11 @@
+---
+'@objectstack/metadata-protocol': patch
+---
+
+fix(metadata-protocol): `diffMetaItem`'s default range labels its to side with the active row's own version, so `GET /meta/:type/:name/diff` with no `from` / `to` names the versions it compares while a draft is pending (#20397)
+
+With no `toVersion`, the to side is the current active `sys_metadata` row. Its body was compared, but `toVersion` came from the newest `sys_metadata_history` row, which is a draft save whenever a draft is pending: every draft save appends a history row. The labels and the bodies then named different rows. Measured on the real REST stack, an app with one active save and two draft saves answered `fromVersion 2 → toVersion 3` over its version-1 body, and a view with one active save and one draft save answered "no changes" labelled `1 → 2` while version 2 differs.
+
+- **Now:** `toVersion` is the active row's own `version`, read in the same read as its body. The default `fromVersion` is still the history version immediately before that label. An item whose active row is version 2 with a draft pending answers `1 → 2`, the same answer as `?from=1&to=2`.
+- **No active row** (a draft-only item, or a deleted one): the to side is absent, and both labels are `null` with empty buckets, as the response schema declares for an absent side. Before, a draft-only item was labelled with its newest draft save, and its from side could be an earlier draft save's body. A deleted item was labelled `N-1 → N` up to its tombstone. That deletion is still read by naming its versions (`?from=N-1&to=N`).
+- Unchanged: the response shape, explicit `from` / `to` ranges, and the default range of an item with no draft pending.

packages/metadata-protocol/src/protocol.diff-dead-history-read.test.ts

@@ -282,3 +282,157 @@ describe('#8798 — a history-table outage now answers the same way for every ty
         expect(err.status).toBe(503);
     });
 });
+
+/**
+ * [#20397] The DEFAULT range (no `toVersion`) labels its to side with the
+ * version whose body it compares.
+ *
+ * The to-side body is the active `sys_metadata` row; the label used to be the
+ * newest `sys_metadata_history` row's version, which is a draft save whenever a
+ * draft is pending (every draft save appends a history row). The label is now
+ * the active row's own `version` column, the one `SysMetadataRepository.put`
+ * stamps with the version of the history row it appends in the same
+ * transaction. Pinned here over seeded rows, beside this file's double, which
+ * already honours the `where` on both tables; the same readings through the
+ * real REST stack are `packages/rest/src/meta-diff-default-range-labels.test.ts`.
+ */
+describe('#20397 — the default range labels the to side with the active row\'s own version', () => {
+    type Seed = { version: number; op: string; body: Record<string, unknown> | null };
+    /** History rows in version order, plus the active and draft rows they left behind. */
+    function seedLineage(
+        tables: Record<string, Array<Record<string, unknown>>>,
+        type: string,
+        name: string,
+        history: Seed[],
+        rows: { active?: Seed; draft?: Seed },
+    ) {
+        const base = { organization_id: null, type, name };
+        history.forEach((h, i) => {
+            tables.sys_metadata_history!.push({
+                ...base,
+                id: `h_${h.version}`,
+                version: h.version,
+                event_seq: i + 1,
+                operation_type: h.op,
+                metadata: h.body == null ? null : JSON.stringify(h.body),
+                checksum: h.body == null ? null : hashSpec(h.body),
+                recorded_at: new Date(i + 1).toISOString(),
+            });
+        });
+        for (const state of ['active', 'draft'] as const) {
+            const row = rows[state];
+            if (!row) continue;
+            tables.sys_metadata!.push({
+                ...base,
+                id: `m_${state}`,
+                state,
+                version: row.version,
+                metadata: JSON.stringify(row.body),
+                checksum: hashSpec(row.body!),
+            });
+        }
+    }
+
+    for (const type of [ORDINARY_TYPE, 'app']) {
+        it(`${type}: with a draft pending, toVersion is the active row's version and the answer is the explicit range's`, async () => {
+            const { engine, tables } = makeStubEngine();
+            const one = { name: 'item', label: 'One' };
+            const two = { name: 'item', label: 'Two' };
+            const pending = { name: 'item', label: 'Pending draft' };
+            seedLineage(tables, type, 'item', [
+                { version: 1, op: 'create', body: one },
+                { version: 2, op: 'update', body: two },
+                { version: 3, op: 'create', body: pending },
+            ], { active: { version: 2, op: 'update', body: two }, draft: { version: 3, op: 'create', body: pending } });
+            const protocol = new ObjectStackProtocolImplementation(engine);
+
+            const res: any = await protocol.diffMetaItem({ type, name: 'item' });
+            const explicit: any = await protocol.diffMetaItem({ type, name: 'item', fromVersion: 1, toVersion: 2 });
+
+            expect(res).toEqual({
+                type,
+                name: 'item',
+                fromVersion: 1,
+                toVersion: 2,
+                added: [],
+                removed: [],
+                changed: [{ path: 'label', from: 'One', to: 'Two' }],
+            });
+            expect(res).toEqual(explicit);
+        });
+    }
+
+    it('the card\'s app reading: one active save and two draft saves label the to side 1 (was 3), with no from side before it', async () => {
+        const { engine, tables } = makeStubEngine();
+        const v1 = { name: 'atlas', label: 'Atlas v1' };
+        const d2 = { name: 'atlas', label: 'Atlas v2 draft' };
+        const d3 = { name: 'atlas', label: 'Atlas v3 draft' };
+        seedLineage(tables, 'app', 'atlas', [
+            { version: 1, op: 'create', body: v1 },
+            { version: 2, op: 'create', body: d2 },
+            { version: 3, op: 'update', body: d3 },
+        ], { active: { version: 1, op: 'create', body: v1 }, draft: { version: 3, op: 'update', body: d3 } });
+        const protocol = new ObjectStackProtocolImplementation(engine);
+
+        const res: any = await protocol.diffMetaItem({ type: 'app', name: 'atlas' });
+
+        expect(res).toEqual({
+            type: 'app',
+            name: 'atlas',
+            fromVersion: null,
+            toVersion: 1,
+            added: [{ path: 'name', value: 'atlas' }, { path: 'label', value: 'Atlas v1' }],
+            removed: [],
+            changed: [],
+        });
+    });
+
+    it('no active row (draft saves only): the to side is absent, so its label is null and no draft body is either side', async () => {
+        const { engine, tables } = makeStubEngine();
+        const d1 = { name: 'item', label: 'Only draft one' };
+        const d2 = { name: 'item', label: 'Only draft two' };
+        seedLineage(tables, ORDINARY_TYPE, 'item', [
+            { version: 1, op: 'create', body: d1 },
+            { version: 2, op: 'update', body: d2 },
+        ], { draft: { version: 2, op: 'update', body: d2 } });
+        const protocol = new ObjectStackProtocolImplementation(engine);
+
+        const res: any = await protocol.diffMetaItem({ type: ORDINARY_TYPE, name: 'item' });
+
+        expect(res).toEqual({
+            type: ORDINARY_TYPE, name: 'item', fromVersion: null, toVersion: null, added: [], removed: [], changed: [],
+        });
+    });
+
+    it('no active row (deleted): the default range answers null on both sides; the deletion stays readable by naming its versions', async () => {
+        // The same rule as the draft-only case: the to side is absent, so is
+        // its label. Before #20397 this one case happened to agree (the newest
+        // history row was the tombstone, whose body is also absent); the
+        // deletion itself is still one explicit range away.
+        const { engine, tables } = makeStubEngine();
+        const one = { name: 'item', label: 'One' };
+        const two = { name: 'item', label: 'Two' };
+        seedLineage(tables, ORDINARY_TYPE, 'item', [
+            { version: 1, op: 'create', body: one },
+            { version: 2, op: 'update', body: two },
+            { version: 3, op: 'delete', body: null },
+        ], {});
+        const protocol = new ObjectStackProtocolImplementation(engine);
+
+        const res: any = await protocol.diffMetaItem({ type: ORDINARY_TYPE, name: 'item' });
+        const deletion: any = await protocol.diffMetaItem({ type: ORDINARY_TYPE, name: 'item', fromVersion: 2, toVersion: 3 });
+
+        expect(res).toEqual({
+            type: ORDINARY_TYPE, name: 'item', fromVersion: null, toVersion: null, added: [], removed: [], changed: [],
+        });
+        expect(deletion).toEqual({
+            type: ORDINARY_TYPE,
+            name: 'item',
+            fromVersion: 2,
+            toVersion: 3,
+            added: [],
+            removed: [{ path: 'name', value: 'item' }, { path: 'label', value: 'Two' }],
+            changed: [],
+        });
+    });
+});

packages/metadata-protocol/src/protocol.ts

@@ -21286,9 +21286,10 @@ export class ObjectStackProtocolImplementation implements
     /**
      * Compute a shallow structural diff between two historical
      * versions of a metadata item. Either side may be omitted: when
-     * `toVersion` is undefined the current active body is used; when
-     * `fromVersion` is undefined the immediately previous history row
-     * is used. Returns `{ added, removed, changed }` keyed by JSON
+     * `toVersion` is undefined the current active body is used, labelled
+     * with that active row's own `version` (`null` when there is no active
+     * row); when `fromVersion` is undefined the immediately previous history
+     * row is used. Returns `{ added, removed, changed }` keyed by JSON
      * pointer-style paths for primitive leaves; nested objects/arrays
      * are reported as a single change record.
      *
@@ -21391,12 +21392,6 @@ export class ObjectStackProtocolImplementation implements
         // `catch` below is this function's only stated intent for that failure,
         // so removing the call makes every type take it. Pinned in
         // `protocol.diff-dead-history-read.test.ts`.
-        const repo = this.getOverlayRepo(orgId);
-        const fullRef = {
-            type: singularType,
-            name: request.name,
-            org: orgId ?? 'env',
-        } as { type: string; name: string; org: string };
         const histRows: Array<{ version: number; body: Record<string, unknown> | null }> = [];
         try {
             const engineAny = this.engine as any;
@@ -21467,9 +21462,40 @@ export class ObjectStackProtocolImplementation implements
             toVersion = request.toVersion;
             toBody = byVersion.get(request.toVersion) ?? null;
         } else {
-            const current = await repo.get(fullRef as any, { state: 'active' });
-            toBody = current ? (current.body as Record<string, unknown>) : null;
-            toVersion = histRows.length ? histRows[histRows.length - 1]!.version : null;
+            // [#20397] The default `to` side is the CURRENT ACTIVE ROW, and ONE
+            // read of that row supplies both of its facts: the body compared and
+            // the `version` it is labelled with. `SysMetadataRepository.put`
+            // stamps that column in the same transaction that appends the history
+            // row carrying the same body, so it names the version this body is.
+            //
+            // The label used to come from the NEWEST `sys_metadata_history` row
+            // instead, which is a draft save whenever a draft is pending (every
+            // draft save appends a row too). Body and label then named different
+            // rows: on the real REST stack an app answered `2 → 3` over its
+            // version-1 body, and a view answered "no changes" labelled `1 → 2`
+            // while version 2 differs.
+            //
+            // Read here, not through `SysMetadataRepository.get`: its
+            // `MetadataItem` projection carries the row's content hash but not
+            // its lineage `version`. Same predicate as that read (active state, no
+            // package scope), and VERBATIM like the history bodies it is compared
+            // against: no ADR-0087 conversion on either side.
+            //
+            // No active row (a draft-only item, a deleted one) ⇒ that side is
+            // absent and so is its label: `null`, as `DiffMetaItemResponseSchema`
+            // declares, never the number of a row whose body is not the one
+            // compared. ⛔ Do not recover a number by matching bodies or hashes
+            // against history: a publish and a revert both write rows whose
+            // bodies repeat earlier ones.
+            const current = (await this.engine.findOne('sys_metadata', {
+                where: { organization_id: orgId, type: singularType, name: request.name, state: 'active' },
+            })) as { metadata?: unknown; version?: unknown } | null;
+            toBody = current?.metadata == null
+                ? null
+                : (typeof current.metadata === 'string'
+                    ? JSON.parse(current.metadata)
+                    : current.metadata as Record<string, unknown>);
+            toVersion = current && typeof current.version === 'number' ? current.version : null;
         }
         if (request.fromVersion !== undefined) {
             fromVersion = request.fromVersion;