Skip to content

Commit 8cdbe0c

Browse files
fix(metadata-protocol): the default /diff range labels its to side with the active row's own version (#20443)
Fixes #20397 Clause-②: no ## What changes `diffMetaItem` (`packages/metadata-protocol/src/protocol.ts`), its default range only. 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. That row is a draft save whenever a draft is pending, because every draft save appends a history row too. So the labels and the bodies named different rows. Now one read of the active row supplies both facts: the body compared, and the row's own `version` as the label. `SysMetadataRepository.put` stamps that column in the same transaction that appends the history row carrying the same body. The `fromVersion` default is unchanged: the history version immediately before the label. With no active row, the to side is absent, so `toVersion` is `null`, as `DiffMetaItemResponseSchema` declares for an absent side. The hunk also deletes the two locals the old read used (`repo`, `fullRef`; nothing else in the function read them), and adds one clause to the method's docblock. Nothing else in `protocol.ts` moves. `rest-server.ts`, `packages/spec` and `sys_metadata_history` are untouched. The response shape is unchanged: `@objectstack/metadata-protocol` `patch` changeset. ## Measured before and after, on the real REST stack Real routes and a real `ObjectStackProtocolImplementation` over better-sqlite3 `:memory:` with the real `sys_metadata*` objects. Each read is `GET /meta/:type/:name/diff` with no `from` / `to`, as an author (`manage_metadata`). "Before" is `origin/main` `dbddf02c1` (the dist carried the old label line). "After" is `53cad078f`. The probe was a scratch file, deleted and never committed. | lineage (v = history version) | before | after | |:--|:--|:--| | app: v1 active, v2 draft, v3 draft (the card) | `2 → 3`, `label` changed "Atlas v2 draft" → "Atlas v1": the to side is the v1 body | `null → 1`, the v1 body as added | | view: v1 active, v2 draft (the card) | "no changes", labelled `1 → 2` | `null → 1`, the v1 body as added | | view: v1 active, v2 active, v3 draft | `2 → 3`, no changes | `1 → 2`, `label` A → B, byte-equal to `?from=1&to=2` | | app: the same lineage | `2 → 3`, no changes | `1 → 2`, byte-equal to `?from=1&to=2` | | view: v1 draft, v2 draft (never published) | `1 → 2`, every v1 key under removed (a draft body served as the from side) | `null → null`, empty | | view: v1 draft only | `null → 1`, empty | `null → null`, empty | | app: draft only | 404 (the REST handler's absence for a gated type) | 404, unchanged | | view: v1, v2 active, then deleted (v3 tombstone) | `2 → 3`, every v2 key under removed | `null → null`, empty (see Acceptance notes) | | view: deleted, then a draft save v4 | `3 → 4`, no changes | `null → null`, empty | | view: v1 active, v2 draft, v3 publish (± a pending v4 draft) | `2 → 3` no changes (`3 → 4` with v4 pending) | `2 → 3` no changes in both cases (see Acceptance notes) | ## The order's mechanism hypotheses - **H1, confirmed.** It reproduced as the table's first two rows. Source: the `request.toVersion === undefined` arm read the body through `repo.get(..., { state: 'active' })` and the label from `histRows[histRows.length - 1].version`. - **H2, confirmed at the row, not at the projection.** The `sys_metadata` row carries `version`, equal to the history row whose body it is. Measured in every lineage above: active v1 = history v1, active v2 = history v2, the published row v3 = the `publish` history row v3. The `MetadataItem` that `repo.get` returns does not carry it. `rowToItem` builds `ref` from `fullRef` (no `version`) and exposes only the content `hash`. So `diffMetaItem` reads the row itself, with the same predicate `repo.get` uses (active state, no package scope). The same file has two precedents: the ADR-0067 commit capture in `publishPackageDrafts` reads the raw active row's `version` as `prevVersion`, and `resolveOverlayPackageBinding` reads the raw row rather than widening `MetadataItem`. Nothing looks a version up by body or hash. - **H3, measured.** The corrected `toVersion` does not change the from rule. With v1 active, v2 draft, v3 publish, the default answers `2 → 3` "no changes". The from side is the unpublished draft save v2, whose body is the one v3 published. The answer is the same with a v4 draft pending. The previously published v1 differs and is not the from side. Which history rows count as versions is not changed here. See Acceptance notes. - **H4, pinned.** No active row ⇒ `null → null`, empty buckets, and no draft body on either side. Before: labelled with the newest draft save, and with two draft saves the first one's body was served as the from side. ## Tests - `packages/metadata-protocol/src/protocol.diff-dead-history-read.test.ts`: a new `#20397` describe block, 5 cases over seeded rows. It sits beside that file's already-pinned engine double, which honours the `where` on both tables, so the engine-double ledger is untouched. The cases: a pending draft for `view` and for `app` equals the explicit range; the card's app reading; draft-only; deleted, with the deletion still readable as `2 → 3`. - `packages/rest/src/meta-diff-default-range-labels.test.ts` (new, test side only): 6 cases through the real routes and the real writes. `toVersion` equals the active row's `version`, read past every door, and the answer equals `?from=1&to=2` for an `app` and a `view`. The two card readings, draft-only, and a lit control with no draft pending, which answers `1 → 2` both before and after. **Ablation**, from the committed fix (`53cad078f`), through `scripts/ablation-replace.mjs`. The mutation puts the old label line back (anchor 1 → 0, blob `3bb7041da297` → `7e3ce0508bd8`). After a `@objectstack/metadata-protocol` rebuild, `ablation-dist-preflight` found the marker in 2 built files, exit 0. `protocol.diff-dead-history-read.test.ts`: 5 failed, 6 passed (the 5 new cases red, the 6 older ones green). `meta-diff-default-range-labels.test.ts`: 5 failed, 1 passed (the lit control). The restore leg brought the blob back to the HEAD blob with `git diff HEAD` empty. After a rebuild, `--absent` found the marker absent from all 24 built files and the tree clean (exit 0). The re-runs gave 11/11 and 6/6. Direction: red, as predicted. (A first run proved the same mutation and the same 5 + 5 reds. Its preflight tree reading refused only because the source spells the marker with a `!` that the build drops, so it was re-run with `--source-marker`.) ## Gates, at the measured head `fd767fec0` (after a true merge of `origin/main`) - `pnpm --filter @objectstack/metadata-protocol exec vitest run`: 189 files passed, 3 skipped; 2750 tests passed, 19 skipped. `typecheck` (tsc --noEmit) exit 0. Its program includes the edited test file (`--listFilesOnly`: 1). - `pnpm --filter @objectstack/rest exec vitest run --project local`: 215 files passed; 3898 tests passed, 26 skipped. `typecheck` (tsc --noEmit plus `check:test-typecheck`, 0 debt) exit 0. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 62 commands. All 62 exit 0. `--ran` reconciliation: 62 derived, 62 run, 0 NOT-MEASURED, a derived zero with every exit code recorded. `check:dual-build-cjs-loads` and `check:type-check-debt` first answered PREREQUISITE NOT MET (exit 3). They were re-run exit 0 after `turbo run build --filter='./packages/*' --filter='./packages/*/*'`, and the dist-reading gates were re-run on that full build. - eslint, narrowed to the 3 touched TypeScript files: `--no-inline-config --format json` gave 3 results, 0 errors, 0 warnings. All 3 are in eslint's own population (`isPathIgnored` false for each). This config enables no type-aware linting (no `parserOptions.project`), so the diff cannot move an untouched file's verdict. The repo-wide `pnpm lint` is CI's. ## Acceptance notes - **H3, carrier #20378.** The default from side is the history row immediately before the label, draft saves included. So after a draft-then-publish, the default range answers "no changes" (`N-1 → N`) against the draft that was published, not against the previous published version. That is the documented rule ("the immediately previous history row"), and this PR does not change which history rows count as versions. The order names #20378 as the carrier. Its ruling (5865708652) took B and declined A (a state column on `sys_metadata_history`), so as the thread stands no card holds that question. - **Deleted items.** The default range of a deleted item was `N-1 → N` up to its tombstone, every key removed. It is now `null → null`, the same rule as a draft-only item: the to side is absent. The deletion is still one explicit range away (`?from=N-1&to=N`, pinned). - **A stale caller mention, outside this surface.** `SysMetadataRepository.get`'s docblock (`sys-metadata-repository.ts`) names "`diffMeta` compares this body against `sys_metadata_history` bodies" as a reason `get` stays verbatim. `diffMetaItem` no longer calls `get`. It reads the same row verbatim itself, and its comment carries the same no-conversion rule. The bullet's reasoning holds, but it names a caller that is gone. Carrier: none; noted, not filed. - Consumers: `/diff` has no in-repo caller of its default range (`client.meta.diffItem` has one docs example). objectui's `MetadataClient.diff` has zero callers at the pin, as the #20378 round measured. --- _Generated by [Claude Code](https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7fa3e3e commit 8cdbe0c

4 files changed

Lines changed: 451 additions & 12 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
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)
6+
7+
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.
8+
9+
- **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`.
10+
- **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`).
11+
- 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‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,3 +282,157 @@ describe('#8798 — a history-table outage now answers the same way for every ty
282282
expect(err.status).toBe(503);
283283
});
284284
});
285+
286+
/**
287+
* [#20397] The DEFAULT range (no `toVersion`) labels its to side with the
288+
* version whose body it compares.
289+
*
290+
* The to-side body is the active `sys_metadata` row; the label used to be the
291+
* newest `sys_metadata_history` row's version, which is a draft save whenever a
292+
* draft is pending (every draft save appends a history row). The label is now
293+
* the active row's own `version` column, the one `SysMetadataRepository.put`
294+
* stamps with the version of the history row it appends in the same
295+
* transaction. Pinned here over seeded rows, beside this file's double, which
296+
* already honours the `where` on both tables; the same readings through the
297+
* real REST stack are `packages/rest/src/meta-diff-default-range-labels.test.ts`.
298+
*/
299+
describe('#20397 — the default range labels the to side with the active row\'s own version', () => {
300+
type Seed = { version: number; op: string; body: Record<string, unknown> | null };
301+
/** History rows in version order, plus the active and draft rows they left behind. */
302+
function seedLineage(
303+
tables: Record<string, Array<Record<string, unknown>>>,
304+
type: string,
305+
name: string,
306+
history: Seed[],
307+
rows: { active?: Seed; draft?: Seed },
308+
) {
309+
const base = { organization_id: null, type, name };
310+
history.forEach((h, i) => {
311+
tables.sys_metadata_history!.push({
312+
...base,
313+
id: `h_${h.version}`,
314+
version: h.version,
315+
event_seq: i + 1,
316+
operation_type: h.op,
317+
metadata: h.body == null ? null : JSON.stringify(h.body),
318+
checksum: h.body == null ? null : hashSpec(h.body),
319+
recorded_at: new Date(i + 1).toISOString(),
320+
});
321+
});
322+
for (const state of ['active', 'draft'] as const) {
323+
const row = rows[state];
324+
if (!row) continue;
325+
tables.sys_metadata!.push({
326+
...base,
327+
id: `m_${state}`,
328+
state,
329+
version: row.version,
330+
metadata: JSON.stringify(row.body),
331+
checksum: hashSpec(row.body!),
332+
});
333+
}
334+
}
335+
336+
for (const type of [ORDINARY_TYPE, 'app']) {
337+
it(`${type}: with a draft pending, toVersion is the active row's version and the answer is the explicit range's`, async () => {
338+
const { engine, tables } = makeStubEngine();
339+
const one = { name: 'item', label: 'One' };
340+
const two = { name: 'item', label: 'Two' };
341+
const pending = { name: 'item', label: 'Pending draft' };
342+
seedLineage(tables, type, 'item', [
343+
{ version: 1, op: 'create', body: one },
344+
{ version: 2, op: 'update', body: two },
345+
{ version: 3, op: 'create', body: pending },
346+
], { active: { version: 2, op: 'update', body: two }, draft: { version: 3, op: 'create', body: pending } });
347+
const protocol = new ObjectStackProtocolImplementation(engine);
348+
349+
const res: any = await protocol.diffMetaItem({ type, name: 'item' });
350+
const explicit: any = await protocol.diffMetaItem({ type, name: 'item', fromVersion: 1, toVersion: 2 });
351+
352+
expect(res).toEqual({
353+
type,
354+
name: 'item',
355+
fromVersion: 1,
356+
toVersion: 2,
357+
added: [],
358+
removed: [],
359+
changed: [{ path: 'label', from: 'One', to: 'Two' }],
360+
});
361+
expect(res).toEqual(explicit);
362+
});
363+
}
364+
365+
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 () => {
366+
const { engine, tables } = makeStubEngine();
367+
const v1 = { name: 'atlas', label: 'Atlas v1' };
368+
const d2 = { name: 'atlas', label: 'Atlas v2 draft' };
369+
const d3 = { name: 'atlas', label: 'Atlas v3 draft' };
370+
seedLineage(tables, 'app', 'atlas', [
371+
{ version: 1, op: 'create', body: v1 },
372+
{ version: 2, op: 'create', body: d2 },
373+
{ version: 3, op: 'update', body: d3 },
374+
], { active: { version: 1, op: 'create', body: v1 }, draft: { version: 3, op: 'update', body: d3 } });
375+
const protocol = new ObjectStackProtocolImplementation(engine);
376+
377+
const res: any = await protocol.diffMetaItem({ type: 'app', name: 'atlas' });
378+
379+
expect(res).toEqual({
380+
type: 'app',
381+
name: 'atlas',
382+
fromVersion: null,
383+
toVersion: 1,
384+
added: [{ path: 'name', value: 'atlas' }, { path: 'label', value: 'Atlas v1' }],
385+
removed: [],
386+
changed: [],
387+
});
388+
});
389+
390+
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 () => {
391+
const { engine, tables } = makeStubEngine();
392+
const d1 = { name: 'item', label: 'Only draft one' };
393+
const d2 = { name: 'item', label: 'Only draft two' };
394+
seedLineage(tables, ORDINARY_TYPE, 'item', [
395+
{ version: 1, op: 'create', body: d1 },
396+
{ version: 2, op: 'update', body: d2 },
397+
], { draft: { version: 2, op: 'update', body: d2 } });
398+
const protocol = new ObjectStackProtocolImplementation(engine);
399+
400+
const res: any = await protocol.diffMetaItem({ type: ORDINARY_TYPE, name: 'item' });
401+
402+
expect(res).toEqual({
403+
type: ORDINARY_TYPE, name: 'item', fromVersion: null, toVersion: null, added: [], removed: [], changed: [],
404+
});
405+
});
406+
407+
it('no active row (deleted): the default range answers null on both sides; the deletion stays readable by naming its versions', async () => {
408+
// The same rule as the draft-only case: the to side is absent, so is
409+
// its label. Before #20397 this one case happened to agree (the newest
410+
// history row was the tombstone, whose body is also absent); the
411+
// deletion itself is still one explicit range away.
412+
const { engine, tables } = makeStubEngine();
413+
const one = { name: 'item', label: 'One' };
414+
const two = { name: 'item', label: 'Two' };
415+
seedLineage(tables, ORDINARY_TYPE, 'item', [
416+
{ version: 1, op: 'create', body: one },
417+
{ version: 2, op: 'update', body: two },
418+
{ version: 3, op: 'delete', body: null },
419+
], {});
420+
const protocol = new ObjectStackProtocolImplementation(engine);
421+
422+
const res: any = await protocol.diffMetaItem({ type: ORDINARY_TYPE, name: 'item' });
423+
const deletion: any = await protocol.diffMetaItem({ type: ORDINARY_TYPE, name: 'item', fromVersion: 2, toVersion: 3 });
424+
425+
expect(res).toEqual({
426+
type: ORDINARY_TYPE, name: 'item', fromVersion: null, toVersion: null, added: [], removed: [], changed: [],
427+
});
428+
expect(deletion).toEqual({
429+
type: ORDINARY_TYPE,
430+
name: 'item',
431+
fromVersion: 2,
432+
toVersion: 3,
433+
added: [],
434+
removed: [{ path: 'name', value: 'item' }, { path: 'label', value: 'Two' }],
435+
changed: [],
436+
});
437+
});
438+
});

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21286,9 +21286,10 @@ export class ObjectStackProtocolImplementation implements
2128621286
/**
2128721287
* Compute a shallow structural diff between two historical
2128821288
* versions of a metadata item. Either side may be omitted: when
21289-
* `toVersion` is undefined the current active body is used; when
21290-
* `fromVersion` is undefined the immediately previous history row
21291-
* is used. Returns `{ added, removed, changed }` keyed by JSON
21289+
* `toVersion` is undefined the current active body is used, labelled
21290+
* with that active row's own `version` (`null` when there is no active
21291+
* row); when `fromVersion` is undefined the immediately previous history
21292+
* row is used. Returns `{ added, removed, changed }` keyed by JSON
2129221293
* pointer-style paths for primitive leaves; nested objects/arrays
2129321294
* are reported as a single change record.
2129421295
*
@@ -21391,12 +21392,6 @@ export class ObjectStackProtocolImplementation implements
2139121392
// `catch` below is this function's only stated intent for that failure,
2139221393
// so removing the call makes every type take it. Pinned in
2139321394
// `protocol.diff-dead-history-read.test.ts`.
21394-
const repo = this.getOverlayRepo(orgId);
21395-
const fullRef = {
21396-
type: singularType,
21397-
name: request.name,
21398-
org: orgId ?? 'env',
21399-
} as { type: string; name: string; org: string };
2140021395
const histRows: Array<{ version: number; body: Record<string, unknown> | null }> = [];
2140121396
try {
2140221397
const engineAny = this.engine as any;
@@ -21467,9 +21462,40 @@ export class ObjectStackProtocolImplementation implements
2146721462
toVersion = request.toVersion;
2146821463
toBody = byVersion.get(request.toVersion) ?? null;
2146921464
} else {
21470-
const current = await repo.get(fullRef as any, { state: 'active' });
21471-
toBody = current ? (current.body as Record<string, unknown>) : null;
21472-
toVersion = histRows.length ? histRows[histRows.length - 1]!.version : null;
21465+
// [#20397] The default `to` side is the CURRENT ACTIVE ROW, and ONE
21466+
// read of that row supplies both of its facts: the body compared and
21467+
// the `version` it is labelled with. `SysMetadataRepository.put`
21468+
// stamps that column in the same transaction that appends the history
21469+
// row carrying the same body, so it names the version this body is.
21470+
//
21471+
// The label used to come from the NEWEST `sys_metadata_history` row
21472+
// instead, which is a draft save whenever a draft is pending (every
21473+
// draft save appends a row too). Body and label then named different
21474+
// rows: on the real REST stack an app answered `2 → 3` over its
21475+
// version-1 body, and a view answered "no changes" labelled `1 → 2`
21476+
// while version 2 differs.
21477+
//
21478+
// Read here, not through `SysMetadataRepository.get`: its
21479+
// `MetadataItem` projection carries the row's content hash but not
21480+
// its lineage `version`. Same predicate as that read (active state, no
21481+
// package scope), and VERBATIM like the history bodies it is compared
21482+
// against: no ADR-0087 conversion on either side.
21483+
//
21484+
// No active row (a draft-only item, a deleted one) ⇒ that side is
21485+
// absent and so is its label: `null`, as `DiffMetaItemResponseSchema`
21486+
// declares, never the number of a row whose body is not the one
21487+
// compared. ⛔ Do not recover a number by matching bodies or hashes
21488+
// against history: a publish and a revert both write rows whose
21489+
// bodies repeat earlier ones.
21490+
const current = (await this.engine.findOne('sys_metadata', {
21491+
where: { organization_id: orgId, type: singularType, name: request.name, state: 'active' },
21492+
})) as { metadata?: unknown; version?: unknown } | null;
21493+
toBody = current?.metadata == null
21494+
? null
21495+
: (typeof current.metadata === 'string'
21496+
? JSON.parse(current.metadata)
21497+
: current.metadata as Record<string, unknown>);
21498+
toVersion = current && typeof current.version === 'number' ? current.version : null;
2147321499
}
2147421500
if (request.fromVersion !== undefined) {
2147521501
fromVersion = request.fromVersion;

0 commit comments

Comments
 (0)