Skip to content

Commit 159dbad

Browse files
claude[bot]claude
andauthored
fix(platform-objects): attestFreshDatastore looks its os migrate remedy up, never defaults it (#16180)
* fix(platform-objects): attestFreshDatastore looks the remedy up, never defaults it (#16067) `attestFreshDatastore`'s contradiction warning built its `os migrate` sentence from a two-way branch: the file-references id got `files-to-references` and EVERY other id got `value-shapes` by default. `CREATION_ATTESTED_MIGRATION_IDS` has had three members since the ADR-0030 cut-over id joined it, and for that third id the default is a wrong prescription — `os migrate value-shapes --apply` neither attests nor clears it, and there is no `os migrate notification-event` to send an operator to at all (measured: no such sub-command exists under `packages/cli/src/commands/migrate/`). Replaced with an explicit id -> remedy register that is TOTAL over the ids a value-shape tally can contradict, and the loop now asks it instead of falling into an arm: an id with no value-shape contract is never-contradictable by this evidence and is attested on the birth observation. A new member therefore inherits NO remedy — adding a third arm that happened to be right today would only have moved the same defect onto the fourth member. A `Map` rather than an object literal: `id` arrives from a caller-supplied array and an object literal would answer `'toString'` with a function. Pins the default, which is where the defect lived: a contradiction fed for a non-ADR-0104 id asserts the warning does not name `value-shapes`, and a case total over `CREATION_ATTESTED_MIGRATION_IDS` asserts no id is ever handed another migration's command. A pin over only the two known ids passed on the broken code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * chore(changeset): patch for the attestFreshDatastore remedy register (#16067) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 89cf4d6 commit 159dbad

3 files changed

Lines changed: 158 additions & 2 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
'@objectstack/platform-objects': patch
3+
---
4+
5+
`attestFreshDatastore` looks its `os migrate` remedy up instead of defaulting it
6+
7+
When a fresh datastore's own boot has already admitted a value that contradicts a
8+
migration's contract, that id is not attested and the operator is told what closes
9+
the gate on real evidence. The sentence used to be built from a two-way branch: the
10+
file-references id got `files-to-references`, and **every other id** got
11+
`value-shapes` by default.
12+
13+
`CREATION_ATTESTED_MIGRATION_IDS` has three members. For the third —
14+
`adr-0030-notification-event` — that default is a wrong prescription: `os migrate
15+
value-shapes --apply` neither attests nor clears it, and there is no `os migrate
16+
notification-event` sub-command to send an operator to at all (that cut-over is an
17+
operator call with no self-check).
18+
19+
The branch is now an explicit id-to-remedy register, total over the ids a
20+
value-shape tally can contradict. The loop asks it rather than falling into an arm,
21+
so an id with no value-shape contract is never-contradictable by that evidence and
22+
is attested on the birth observation as before. A new member therefore inherits no
23+
remedy: adding a third arm that happened to be right today would only have moved the
24+
same defect onto the fourth member.
25+
26+
No behaviour changes for the two ADR-0104 ids, which is where every reachable path
27+
runs today: the shipped engine keys its admitted-violation tally from a closed
28+
`'media' | 'value-shape'` union, so it cannot name a third id.

packages/platform-objects/src/system/migration-flag.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import { describe, it, expect, vi } from 'vitest';
1111
import { assertEngineUpdateDispatch } from '@objectstack/metadata-core';
1212
import {
1313
CREATION_ATTESTED_MIGRATION_IDS,
14+
FILE_REFERENCES_MIGRATION_ID,
1415
NOTIFICATION_EVENT_MIGRATION_ID,
16+
VALUE_SHAPES_MIGRATION_ID,
1517
} from '@objectstack/spec/system';
1618
import {
1719
readDataMigrationFlag,
@@ -301,5 +303,89 @@ describe('fresh-datastore attestation (ADR-0104, 2026-07-30 addendum)', () => {
301303

302304
expect(await attestFreshDatastore(engine)).toEqual([...CREATION_ATTESTED_MIGRATION_IDS]);
303305
});
306+
307+
/**
308+
* #16067. The remedy sentence used to be a two-way branch whose `else`
309+
* gave `os migrate value-shapes` to every id that was not the file one.
310+
* These pins test that DEFAULT, which is where the defect lived — a pin
311+
* that only exercised the two ADR-0104 ids passed on the broken code, and
312+
* still would.
313+
*
314+
* ⚠️ Reachability, measured rather than assumed: the SHIPPED engine cannot
315+
* key this tally with a third id (`ObjectQL.noteAdmittedValueShapeViolation`
316+
* derives the key from a closed `'media' | 'value-shape'` union, and it is
317+
* the only writer of the map). But `valueShapeViolationsAdmitted` is an
318+
* OPTIONAL, duck-typed member of {@link MigrationFlagEngine} returning an
319+
* open `Record<string, …>` — any other engine satisfies it, as the doubles
320+
* in this very file do. So the default was one non-ObjectQL producer away
321+
* from being read by an operator, and "unreachable" was never a property
322+
* of the seam.
323+
*/
324+
describe('the remedy is looked up, never defaulted (#16067)', () => {
325+
/** The mapping the operator-facing sentence must obey, restated here so
326+
* a change to the production map has to be made twice, on purpose. */
327+
const REMEDY_BY_ID: Record<string, string> = {
328+
[FILE_REFERENCES_MIGRATION_ID]: 'files-to-references',
329+
[VALUE_SHAPES_MIGRATION_ID]: 'value-shapes',
330+
};
331+
332+
it('an id with NO value-shape contract is never-contradictable, and is told to run nothing', async () => {
333+
const engine = fakeEngine();
334+
engine.valueShapeViolationsAdmitted = () => ({
335+
[NOTIFICATION_EVENT_MIGRATION_ID]: VIOLATED,
336+
});
337+
const logger = { info: vi.fn(), warn: vi.fn() };
338+
339+
const attested = await attestFreshDatastore(engine, { logger });
340+
341+
// A value-shape tally is evidence about value shapes. The ADR-0030
342+
// cut-over's fact — no legacy per-user inbox row here — is not one, so
343+
// this counterexample disproves nothing about it and the birth
344+
// observation still settles it.
345+
expect(attested).toContain(NOTIFICATION_EVENT_MIGRATION_ID);
346+
expect(await isDataMigrationVerified(engine, NOTIFICATION_EVENT_MIGRATION_ID)).toBe(true);
347+
348+
const warnings = logger.warn.mock.calls.map((c) => String(c[0] ?? '')).join('\n');
349+
// ⭐ The card's pin: the operator is NOT sent to `os migrate
350+
// value-shapes`, which neither attests nor clears this id. There is no
351+
// `os migrate notification-event` to send them to either — that
352+
// cut-over is an operator call with no self-check — so the correct
353+
// sentence here is no sentence.
354+
expect(warnings).not.toContain('value-shapes');
355+
expect(warnings).toBe('');
356+
});
357+
358+
/**
359+
* Total over the array, so a FOURTH member is judged the moment it is
360+
* added instead of inheriting whatever the last branch happened to say.
361+
*/
362+
it.each([...CREATION_ATTESTED_MIGRATION_IDS])(
363+
'a contradiction for %s is never handed another migration\'s command',
364+
async (id) => {
365+
const engine = fakeEngine();
366+
engine.valueShapeViolationsAdmitted = () => ({ [id]: VIOLATED });
367+
const logger = { info: vi.fn(), warn: vi.fn() };
368+
369+
const attested = await attestFreshDatastore(engine, { logger });
370+
const warnings = logger.warn.mock.calls.map((c) => String(c[0] ?? '')).join('\n');
371+
const own = REMEDY_BY_ID[id];
372+
373+
if (own === undefined) {
374+
expect(attested).toContain(id);
375+
expect(warnings).toBe('');
376+
} else {
377+
expect(attested).not.toContain(id);
378+
expect(warnings).toContain(`os migrate ${own} --apply`);
379+
}
380+
381+
// The half a bigger ternary would still get wrong: no id may ever be
382+
// prescribed a command that belongs to a different id.
383+
for (const [other, command] of Object.entries(REMEDY_BY_ID)) {
384+
if (other === id) continue;
385+
expect(warnings).not.toContain(`os migrate ${command}`);
386+
}
387+
},
388+
);
389+
});
304390
});
305391
});

packages/platform-objects/src/system/migration-flag.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
DATA_MIGRATION_FLAG_OBJECT,
77
FILE_REFERENCES_MIGRATION_ID,
88
isDataMigrationFlagVerified,
9+
VALUE_SHAPES_MIGRATION_ID,
910
type DataMigrationFlag,
1011
} from '@objectstack/spec/system';
1112

@@ -205,6 +206,42 @@ export async function recordDataMigrationRun(
205206
return flag;
206207
}
207208

209+
/**
210+
* The `os migrate` sub-command that re-earns each id a boot's own admitted
211+
* value can CONTRADICT — and, by having no row for anything else, the register
212+
* of which ids that is.
213+
*
214+
* ## Why a map and not a branch
215+
*
216+
* The counterexample this reads comes from
217+
* {@link MigrationFlagEngine.valueShapeViolationsAdmitted}, whose whole subject
218+
* is ADR-0104 value shapes. So membership here is one question — *does this id
219+
* stand for a value-shape contract a stored value can disprove?* — and the
220+
* answer for every id that has one is a DIFFERENT command. A two-way branch
221+
* answered both at once: it read the file id and gave every other id
222+
* `value-shapes` by default. That default was silently wrong the moment a third
223+
* id joined {@link CREATION_ATTESTED_MIGRATION_IDS} — `adr-0030-notification-event`
224+
* would have been told to run `os migrate value-shapes --apply`, a command that
225+
* does not attest it, does not clear it, and has nothing to do with it (there is
226+
* no `os migrate notification-event` at all; that cut-over is an operator call
227+
* with no self-check, ruled on `NOTIFICATION_EVENT_MIGRATION_ID`'s docblock).
228+
*
229+
* ⛔ So a new member must NOT inherit a remedy. An id absent from this map is
230+
* never-contradictable *by this evidence* — a value-shape tally says nothing
231+
* about a fact that is not about value shapes — and it is attested on the birth
232+
* observation like any other. Adding a third arm that happens to be right today
233+
* would only move the same defect onto the fourth member; adding a ROW is a
234+
* deliberate act, and its absence prescribes nothing rather than prescribing
235+
* the wrong thing.
236+
*
237+
* A `Map`, not an object literal: `id` reaches this from a caller-supplied
238+
* array, and an object would answer `'toString'` with a function.
239+
*/
240+
const VALUE_SHAPE_CONTRACT_REMEDY: ReadonlyMap<string, string> = new Map([
241+
[FILE_REFERENCES_MIGRATION_ID, 'files-to-references'],
242+
[VALUE_SHAPES_MIGRATION_ID, 'value-shapes'],
243+
]);
244+
208245
/** Marker written into a creation-attested row's `details`, so an operator
209246
* reading a verified flag can tell evidence-by-scan from evidence-by-birth. */
210247
export const CREATION_ATTESTATION_DETAIL = { attested: 'datastore-created-empty' } as const;
@@ -291,7 +328,12 @@ export async function attestFreshDatastore(
291328
try {
292329
if (await readDataMigrationFlag(engine, id)) continue; // not ours to write
293330
// #4769 — a boot may not prove a contract it has already broken.
294-
const contradiction = admitted[id];
331+
// Asked of the register, not of a default: an id with no value-shape
332+
// contract cannot be contradicted by a value-shape tally, so it is
333+
// attested on the birth observation instead of being handed another
334+
// migration's remedy. See VALUE_SHAPE_CONTRACT_REMEDY.
335+
const remedy = VALUE_SHAPE_CONTRACT_REMEDY.get(id);
336+
const contradiction = remedy === undefined ? undefined : admitted[id];
295337
if (contradiction && contradiction.count > 0) {
296338
const at = contradiction.first;
297339
const where = at?.object && at?.field ? `${at.object}.${at.field}` : 'a record';
@@ -301,7 +343,7 @@ export async function attestFreshDatastore(
301343
`(${where}${at?.detail ? `: ${at.detail}` : ''}). The store was created empty, but it ` +
302344
'is no longer empty and what it now holds contradicts the claim — the gate stays ' +
303345
'open (warn-first). Fix the data, then run `os migrate ' +
304-
(id === FILE_REFERENCES_MIGRATION_ID ? 'files-to-references' : 'value-shapes') +
346+
remedy +
305347
' --apply` to close it on real evidence (ADR-0104).',
306348
);
307349
continue;

0 commit comments

Comments
 (0)