Skip to content

Commit f761666

Browse files
committed
fix(migrate): advertise the value-shape gate, and stop counting fields nothing enforces (#3438)
The scan shipped with no way to find out it exists. ADR-0104's rollout asked for two advertisements and neither was built: `os migrate meta` — the command a 16→17 upgrade already runs — never mentioned the data migrations, and a warn-first deployment booted silently. The command landed and the posture it governs never changed, because a gate nobody is told about is served by nobody. `os migrate meta` now closes by naming both gated data migrations with what each covers and the gate it records, on the human path and in `--json` alike, including the run that rewrote nothing — an upgrade needing no source change is exactly where "already done" is easy to assume. A kernel booting with covered fields and no verified `adr-0104-value-shapes` row logs one line naming the command that ends warn mode; every case where that line would be false or useless (an env switch already settled the posture, no covered field is registered, the gate is already open) answers null and stays silent. Deliberately WITHOUT reading gate status from the datastore inside `os migrate meta`, which the ADR's sketch of this advisory assumed it would show. That command rewrites metadata SOURCE and has never opened a database — its e2e test spawns the real CLI against a project in a tmpdir. Booting one to decorate a hint would hand a read-only command a `.objectstack/` side effect and a new way to hang on an unreachable database, a poor trade for two words of status. Status is served where a datastore is already open: both commands report before they write anything, and the boot line carries the live verdict. Building the advisory surfaced a defect in the gate that shipped with the scan. `objectHasCoveredValueField` — the dormancy short-circuit meant to spare an object with no covered field the flag query — tested raw type membership, while the registry INJECTS covered-type fields into every object it registers: `organization_id` and `owner_id` (both `system`), `created_by` and `updated_by` (both in `SKIP_FIELDS`), four `lookup`s. `validateRecord` skips every one of them before the value-shape check, so the short-circuit answered true for literally every object, never fired, and its cache memoized a constant. It now uses the validator's own `isScannableValueShapeField`, the predicate the scanner already imports — three readings of "a covered field" drifting by one clause is how a gate ends up governing fields nothing enforces. The flag read was already memoized per process, so this corrects the property the ADR states rather than a user-visible cost. The silent-when-uncovered test pins it: an object declaring only text still carries those four injected lookups, and counting by type made the advisory fire on every deployment that exists. Verified: objectql 1326, cli 625, spec 7154, platform-objects 253, service-storage 252, driver-memory 265, runtime 954; check:generated 8/8; release-notes and doc-authoring gates pass; docs site and all 71 packages build; ESLint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016zgA8CQMJeJbFEjnbib1Uv
1 parent 6e357ed commit f761666

8 files changed

Lines changed: 383 additions & 8 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/objectql": patch
3+
"@objectstack/cli": patch
4+
---
5+
6+
fix(migrate): the value-shape gate advertises itself, and stops counting fields nothing enforces (#3438)
7+
8+
The scan landed with no way to find out it exists. ADR-0104's rollout asked for
9+
two advertisements and neither was built: `os migrate meta` — the command a
10+
16→17 upgrade already runs — never mentioned the data migrations, and a
11+
warn-first deployment booted silently. A gate nobody is told about is served by
12+
nobody, so the command shipped and the posture it governs never changed.
13+
14+
`os migrate meta` now ends by naming both gated data migrations
15+
(`files-to-references`, `value-shapes`) with what each covers and the gate it
16+
records, on the human path and in `--json` alike, including the run that
17+
rewrote nothing — an upgrade needing no source change is exactly where "already
18+
done" is easy to assume. A kernel that boots with covered fields and no
19+
verified `adr-0104-value-shapes` row now logs one line naming the command that
20+
ends warn mode.
21+
22+
Deliberately WITHOUT reading gate status from the datastore inside `os migrate
23+
meta`, which the ADR's sketch assumed. That command rewrites metadata SOURCE
24+
and has never opened a database — its e2e test runs the real CLI against a
25+
project in a tmpdir. Booting one to decorate a hint would hand a read-only
26+
command a `.objectstack/` side effect and a new way to hang on an unreachable
27+
database. Status is served where a datastore is already open: both commands
28+
report before they write anything, and the boot line carries the live verdict.
29+
30+
The advisory also found a defect in the gate that shipped with the scan.
31+
`objectHasCoveredValueField` — the dormancy short-circuit that is supposed to
32+
spare an object with no covered field the flag query — tested raw type
33+
membership, while the registry INJECTS covered-type fields into every object it
34+
registers: `organization_id` and `owner_id` (both `system`), `created_by` and
35+
`updated_by` (both in `SKIP_FIELDS`), four `lookup`s. `validateRecord` skips
36+
every one before the value-shape check, so the short-circuit answered `true` for
37+
literally every object, never fired, and its cache memoized a constant. It now
38+
uses the validator's own `isScannableValueShapeField`, the same predicate the
39+
scanner imports — three readings of "a covered field" drifting by one clause is
40+
how a gate ends up governing fields nothing enforces. Behaviour is otherwise
41+
unchanged: the flag read was already memoized per process, so this corrects the
42+
property the ADR states rather than a user-visible cost.

content/docs/releases/v17.mdx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,6 +1012,14 @@ can correct — so it reports the object, field, type, count, sample record ids
10121012
and parse issue, and you re-run until it is clean. `OS_ALLOW_LAX_VALUE_SHAPES=1`
10131013
re-opens leniency while diagnosing.
10141014

1015+
**You are told about both, rather than having to find this page.** `os migrate
1016+
meta` — the command a 16→17 upgrade already runs — ends by naming the two data
1017+
migrations and the gate each records, including on a run that rewrote nothing,
1018+
since canonical metadata says nothing about stored values. A deployment that
1019+
boots holding covered fields with no verified `adr-0104-value-shapes` row logs
1020+
one line naming the command that ends warn mode. Neither reads as "done":
1021+
until a gate is recorded, the classes it covers keep warning.
1022+
10151023
**A database created by 17 attests both flags at creation** (#3438), so a new
10161024
deployment enforces from its first boot instead of waiting for someone to run a
10171025
migration that, for an empty store, does nothing. The platform attests only a

packages/cli/src/commands/migrate/meta.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,50 @@ import {
2424
emitJson,
2525
} from '../../utils/format.js';
2626

27+
/**
28+
* The gated ADR-0104 DATA migrations, named at the end of every `os migrate
29+
* meta` run (#3438). This command is the one a 16→17 upgrade already runs, so
30+
* it is where a deployment should learn that rewriting metadata is only half
31+
* of the upgrade — a gate nobody is told about is served by nobody.
32+
*
33+
* Deliberately WITHOUT reading each gate's status from the datastore, which
34+
* the ADR's sketch of this advisory assumed it would show. `os migrate meta`
35+
* rewrites metadata SOURCE and has never opened a database — its e2e test
36+
* spawns the real CLI against a project in a tmpdir. Booting one to decorate a
37+
* hint would hand a read-only command a `.objectstack/` side effect and a new
38+
* way to hang on an unreachable database, which is a poor trade for two words
39+
* of status. Status is served where a datastore is already open: both commands
40+
* report before they write anything, and a warn-first boot logs the
41+
* value-shape line itself.
42+
*/
43+
const GATED_DATA_MIGRATIONS = [
44+
{
45+
command: 'os migrate files-to-references',
46+
covers: 'file / image / avatar / video / audio values',
47+
gate: 'adr-0104-file-references',
48+
},
49+
{
50+
command: 'os migrate value-shapes',
51+
covers: 'lookup / user / location / address / repeater / … value shapes',
52+
gate: 'adr-0104-value-shapes',
53+
},
54+
] as const;
55+
56+
/** Print the data-migration advisory that closes a human-readable run. */
57+
function printDataMigrationAdvisory(): void {
58+
console.log(chalk.bold(' Data migrations — metadata is only half of an upgrade:'));
59+
for (const m of GATED_DATA_MIGRATIONS) {
60+
console.log(` • ${chalk.white(m.command)}`);
61+
console.log(chalk.dim(` covers: ${m.covers}`));
62+
console.log(chalk.dim(` gate: ${m.gate}`));
63+
}
64+
console.log(
65+
chalk.dim(' Each reports what it would do and writes nothing until `--apply`. Until a\n') +
66+
chalk.dim(' deployment records a gate, the classes it covers stay warn-first (ADR-0104).'),
67+
);
68+
console.log('');
69+
}
70+
2771
/**
2872
* `os migrate meta --from N` — replay the ADR-0087 D3 migration chain.
2973
*
@@ -112,6 +156,9 @@ export default class MigrateMeta extends Command {
112156
: undefined,
113157
specChanges,
114158
schemaValid: parsed.success,
159+
// The same advisory the human path prints, so an upgrading agent
160+
// reading only JSON is told about the data half too (#3438).
161+
dataMigrations: GATED_DATA_MIGRATIONS,
115162
duration: timer.elapsed(),
116163
});
117164
if (flags.out) writeFileSync(resolve(flags.out), JSON.stringify(result.stack, null, 2));
@@ -124,6 +171,11 @@ export default class MigrateMeta extends Command {
124171

125172
if (result.applied.length === 0 && result.todos.length === 0) {
126173
printSuccess('Nothing to migrate — the metadata is already canonical for this range.');
174+
// Still advertise: canonical METADATA says nothing about whether this
175+
// deployment's stored VALUES have been scanned, and an upgrade that
176+
// needed no source rewrite is exactly where that is easy to assume.
177+
console.log('');
178+
printDataMigrationAdvisory();
127179
return;
128180
}
129181

@@ -171,6 +223,7 @@ export default class MigrateMeta extends Command {
171223
);
172224
}
173225
console.log('');
226+
printDataMigrationAdvisory();
174227
} catch (error: any) {
175228
if (error instanceof MigrationFloorError) {
176229
if (flags.json) {

packages/cli/test/migrate-meta.e2e.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { tmpdir } from 'node:os';
2222
import { join, resolve } from 'node:path';
2323
import { fileURLToPath } from 'node:url';
2424
import { ObjectStackDefinitionSchema } from '@objectstack/spec';
25+
import { PROTOCOL_MAJOR } from '@objectstack/spec/kernel';
2526

2627
const execFileP = promisify(execFile);
2728
const HERE = resolve(fileURLToPath(import.meta.url), '..');
@@ -233,6 +234,41 @@ describe('os migrate meta --from 16 (e2e over the real CLI)', () => {
233234
}
234235
}, 120_000);
235236

237+
/**
238+
* The upgrade-path advertisement (ADR-0104 rollout / #3438). Metadata is
239+
* only half of a 16→17 upgrade, and this is the command that half runs
240+
* through — so both surfaces name the data migrations, and both are pinned:
241+
* an advertisement that silently stops appearing leaves the gate exactly as
242+
* undiscoverable as having built no gate at all.
243+
*/
244+
it('names both gated data migrations in the machine output', () => {
245+
const commands = (out.parsed.dataMigrations ?? []).map((m: any) => m.command);
246+
expect(commands).toContain('os migrate files-to-references');
247+
expect(commands).toContain('os migrate value-shapes');
248+
const gates = (out.parsed.dataMigrations ?? []).map((m: any) => m.gate);
249+
expect(gates).toContain('adr-0104-value-shapes');
250+
});
251+
252+
it('ends a human-readable run with the same advisory', async () => {
253+
const stdout = await runMeta(['--from', '16'], dir);
254+
expect(stdout).toContain('os migrate value-shapes');
255+
expect(stdout).toContain('os migrate files-to-references');
256+
// The advisory must not read as something the chain just did.
257+
expect(stdout).toMatch(/only half of an upgrade/i);
258+
}, 120_000);
259+
260+
it('advertises even when the chain had nothing to rewrite', async () => {
261+
// The easiest run to mistake for "upgrade complete", and the one that
262+
// returns before the normal tail: an empty chain (`--from` already at the
263+
// runtime's major). Canonical METADATA says nothing about whether this
264+
// deployment's stored VALUES have been scanned, so the advisory has to
265+
// survive the early return too.
266+
const stdout = await runMeta(['--from', String(PROTOCOL_MAJOR)], dir);
267+
expect(stdout).toMatch(/Nothing to migrate/i);
268+
expect(stdout).toContain('os migrate value-shapes');
269+
expect(stdout).toContain('os migrate files-to-references');
270+
}, 120_000);
271+
236272
it('refuses a --from below the support floor with the structured error', async () => {
237273
let failed = false;
238274
try {

packages/objectql/src/engine.ts

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
type DroppedFieldsEvent
1313
} from '@objectstack/spec/data';
1414
import type { WriteObservabilityOptions } from '@objectstack/spec/contracts';
15-
import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES, STRUCTURED_JSON_TYPES, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY } from '@objectstack/spec/data';
15+
import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY } from '@objectstack/spec/data';
1616
import {
1717
DATA_MIGRATION_FLAG_OBJECT,
1818
FILE_REFERENCES_MIGRATION_ID,
@@ -69,7 +69,7 @@ import { ExpressionEngine } from '@objectstack/formula';
6969
import type { Expression } from '@objectstack/spec';
7070
import { isAggregatedViewContainer, expandViewContainer } from '@objectstack/spec';
7171
import { bindHooksToEngine } from './hook-binder.js';
72-
import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError } from './validation/record-validator.js';
72+
import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError, valueShapePostureSetByEnv, isScannableValueShapeField } from './validation/record-validator.js';
7373
import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, stripReadonlyFields } from './validation/rule-validator.js';
7474
import { applyInMemoryAggregation } from './in-memory-aggregation.js';
7575

@@ -2149,17 +2149,28 @@ export class ObjectQL implements IDataEngine {
21492149
}
21502150

21512151
/**
2152-
* Does this object declare a reference or structured-JSON field? Same
2153-
* per-schema cache and same dormancy rule as {@link objectHasMediaField}: an
2154-
* object holding none of these types can hold no violation of them, so it
2155-
* must not pay a query to learn that.
2152+
* Does this object declare a reference or structured-JSON field that a write
2153+
* would actually check? Same per-schema cache and same dormancy rule as
2154+
* {@link objectHasMediaField}: an object holding none can hold no violation
2155+
* of them, so it must not pay a query to learn that.
2156+
*
2157+
* The membership test is `isScannableValueShapeField` — the validator's own
2158+
* — and not raw type membership, because the registry INJECTS covered-type
2159+
* fields into every object it registers: `organization_id` and `owner_id`
2160+
* (both `system`), plus `created_by` / `updated_by` (both in `SKIP_FIELDS`),
2161+
* are all `lookup`s. `validateRecord` skips every one of them before it ever
2162+
* reaches the value-shape check, so counting them made this answer `true` for
2163+
* literally every object — the dormancy rule above never fired, and this
2164+
* cache memoized a constant. Same predicate as the scanner for the same
2165+
* reason the scanner imports it: three readings of "a covered field" drifting
2166+
* by one clause is how a gate ends up governing fields nothing enforces.
21562167
*/
21572168
private objectHasCoveredValueField(objectSchema: any): boolean {
21582169
if (!objectSchema?.fields) return false;
21592170
const cached = ObjectQL.coveredValueFieldPresence.get(objectSchema);
21602171
if (cached !== undefined) return cached;
2161-
const present = Object.values(objectSchema.fields).some(
2162-
(def: any) => def && (REFERENCE_VALUE_TYPES.has(def.type) || STRUCTURED_JSON_TYPES.has(def.type)),
2172+
const present = Object.entries(objectSchema.fields).some(
2173+
([name, def]: [string, any]) => isScannableValueShapeField(name, def),
21632174
);
21642175
ObjectQL.coveredValueFieldPresence.set(objectSchema, present);
21652176
return present;
@@ -2224,6 +2235,37 @@ export class ObjectQL implements IDataEngine {
22242235
return this.valueShapesMigrationVerified;
22252236
}
22262237

2238+
/**
2239+
* How many registered objects would benefit from being told about the
2240+
* value-shape scan at boot — or `null` when there is nothing worth saying.
2241+
*
2242+
* ADR-0104's rollout asks a warn-first deployment to log one startup line
2243+
* naming the command that ends warn mode, on the reasoning that a gate
2244+
* nobody is told about is served by nobody. Everything that would make that
2245+
* line false or useless answers `null` instead, cheapest test first so a
2246+
* kernel with nothing to advertise never reaches the flag query:
2247+
*
2248+
* - an env switch already settled the posture (see
2249+
* {@link valueShapePostureSetByEnv});
2250+
* - no registered object declares a covered field, so the gate is moot here;
2251+
* - the flag is verified, so the gate is already open.
2252+
*
2253+
* Read-only and advisory: it opens no gate and changes no posture. The count
2254+
* it returns is the reason the line is worth printing, so the operator can
2255+
* see the advice applies to their data rather than to the platform in
2256+
* general.
2257+
*/
2258+
async valueShapeScanAdvisory(): Promise<number | null> {
2259+
if (valueShapePostureSetByEnv()) return null;
2260+
let objects: any[] = [];
2261+
try { objects = (this._registry as any).getAllObjects?.() ?? []; } catch { objects = []; }
2262+
let covered = 0;
2263+
for (const o of objects) if (this.objectHasCoveredValueField(o)) covered++;
2264+
if (covered === 0) return null;
2265+
if (await this.isValueShapesMigrationVerified()) return null;
2266+
return covered;
2267+
}
2268+
22272269
/**
22282270
* Read one deployment migration flag and answer whether it authorises its
22292271
* consumers. Shared by both flags so the "every way of not knowing answers

packages/objectql/src/plugin.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,25 @@ export class ObjectQLPlugin implements Plugin {
543543
objectsRegistered: this.ql?.registry?.getAllObjects?.()?.length || 0
544544
});
545545

546+
// [ADR-0104 D1 / #3438] Tell a warn-first deployment that the gate exists.
547+
// The engine decides whether there is anything to say (covered fields
548+
// present, no env switch already settling the posture, flag unverified);
549+
// every other case answers null and this stays silent. Wrapped because an
550+
// advisory that fails a boot is worse than an advisory nobody reads — the
551+
// flag read touches the database and a kernel mid-recovery must still start.
552+
try {
553+
const covered = await this.ql?.valueShapeScanAdvisory?.();
554+
if (covered) {
555+
ctx.logger.info(
556+
`[value-shape] ${covered} registered object(s) declare reference or structured-JSON ` +
557+
'fields and this deployment has no verified `adr-0104-value-shapes` row, so malformed ' +
558+
'values WARN instead of being rejected. `os migrate value-shapes` reports what strict ' +
559+
'mode would reject without writing anything; `--apply` records the gate once it is ' +
560+
'clean (ADR-0104 / #3438).',
561+
);
562+
}
563+
} catch { /* never fail a boot for an advisory */ }
564+
546565
// ADR-0057: arm the periodic lifecycle sweep once the engine is live.
547566
this.lifecycleService?.start();
548567
}

packages/objectql/src/validation/record-validator.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,25 @@ function valueShapeStrictEffective(deploymentVerified: boolean): boolean {
594594
return deploymentVerified;
595595
}
596596

597+
/**
598+
* Has an environment switch already settled the reference / structured-JSON
599+
* posture, so the boot advisory for the scan (#3438) would be untrue or
600+
* useless?
601+
*
602+
* `OS_DATA_VALUE_SHAPE_STRICT_ENABLED=1` means enforcement is already on for
603+
* every class, so a line announcing "warn mode is in effect" would be false.
604+
* `OS_ALLOW_LAX_VALUE_SHAPES=1` means the operator opted out deliberately —
605+
* running the scan would not change what they get, so pointing at it is noise.
606+
*
607+
* Exported rather than re-read in the engine for the same reason the scanner
608+
* imports `valueShapeViolation`: two readings of these variables drifting by
609+
* one clause is how a deployment gets told to run a migration that would not
610+
* change its posture, or gets told nothing while it still would.
611+
*/
612+
export function valueShapePostureSetByEnv(): boolean {
613+
return LAX_VALUE_SHAPES() || VALUE_SHAPE_STRICT();
614+
}
615+
597616
/**
598617
* Is `value` a violation of `def`'s declared stored shape — the SAME question
599618
* `validateOne` asks, exported so the `os migrate value-shapes` scanner counts

0 commit comments

Comments
 (0)