Skip to content

Commit 76a72c8

Browse files
committed
fix(objectql): the open-gate announcement stops speaking where it is false (#3438)
The boot line that names an open value-shape gate (#4253) fired on every deployment there is, and stated something untrue on any deployment that had already settled the posture with an environment switch. Both are one failure: an advisory that speaks where it does not apply is exactly how readers learn to ignore it, and the line's whole value is that a warn-first deployment is told once, credibly, which command ends warn mode. Neither defect is reachable from the suite that shipped with the announcement. `engine.test.ts` mocks `./registry`, and a mocked registry hands the engine exactly the fields the test wrote — so these cases get a sibling file built on the genuine `registerApp → registry` path, which is the only way a test sees what a deployment sees. `objectHasCoveredValueField` — the dormancy short-circuit meant to spare an object with no covered field the flag query — tested raw type membership, while the real 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 it reaches the value-shape check, so the short-circuit answered `true` for literally every object, never fired, and its WeakMap memoized a constant. It now counts by 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, which is the shape #4235 argued against. The announcement also consulted no environment switch, though both postures it reports on short-circuit ahead of the deployment flag it reads. Under `OS_DATA_VALUE_SHAPE_STRICT_ENABLED` enforcement is already on for both classes, so "checked but NOT enforced here" was false; under either opt-out the operator chose leniency deliberately, so naming a migration that cannot change what they get is noise. Each gate consults its own pair — `mediaPostureSetByEnv` and `valueShapePostureSetByEnv`, siblings for the reason `mediaStrictEffective` and `valueShapeStrictEffective` are siblings, since the opt-outs are per-class while the opt-in opens both. Cheapest test first, so a kernel with nothing to say still reaches no flag query. Enforcement is unchanged: same gates, same flags, same default. The flag read was already memoized per process, so what this corrects is the property ADR-0104 states, not a user-visible cost. The predicate fix is pinned by reverting it: with raw-type membership restored, `stays silent when no object declares a covered field` fails, because a `note` declaring only text still carries the four injected lookups. Verified on a full workspace build: objectql 1345 (7 new), cli 628, spec check:generated 8/8, check:release-notes and check:doc-authoring pass, ESLint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016zgA8CQMJeJbFEjnbib1Uv
1 parent bcf1112 commit 76a72c8

5 files changed

Lines changed: 307 additions & 11 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): the boot gate announcement stops firing where it is false, and stops counting fields nothing enforces (#3438)
6+
7+
The startup line that names an open value-shape gate (#4253) fired on every
8+
deployment there is, and said something untrue on any deployment that had
9+
already settled the question with an environment switch. Both are the same
10+
failure — an advisory that speaks where it does not apply is how readers learn
11+
to ignore it — and neither is reachable from the suite that shipped with it,
12+
because `engine.test.ts` mocks the registry away and a mocked registry hands
13+
the engine exactly the fields the test wrote.
14+
15+
`objectHasCoveredValueField` — the dormancy short-circuit that is supposed to
16+
spare an object with no covered field the flag query — tested raw type
17+
membership, while the real registry INJECTS covered-type fields into every
18+
object it registers: `organization_id` and `owner_id` (both `system`),
19+
`created_by` and `updated_by` (both in `SKIP_FIELDS`), four `lookup`s.
20+
`validateRecord` skips every one of them before it reaches the value-shape
21+
check, so the short-circuit answered `true` for literally every object, never
22+
fired, and its WeakMap memoized a constant. Counting is now by the validator's
23+
own `isScannableValueShapeField`, the predicate the scanner already imports —
24+
three readings of "a covered field" drifting by one clause is how a gate ends
25+
up governing fields nothing enforces.
26+
27+
The announcement also consulted no environment switch, while both postures it
28+
reports on short-circuit ahead of the deployment flag. Under
29+
`OS_DATA_VALUE_SHAPE_STRICT_ENABLED` enforcement is already on for both
30+
classes, so "checked but NOT enforced here" was simply false; under either
31+
opt-out the operator chose leniency deliberately, so naming a migration that
32+
cannot change what they get is noise. Each gate now consults its own pair
33+
(`mediaPostureSetByEnv` / `valueShapePostureSetByEnv`, siblings for the reason
34+
`mediaStrictEffective` and `valueShapeStrictEffective` are siblings), since the
35+
opt-outs are per-class while the opt-in opens both. Cheapest test first, so a
36+
kernel with nothing to say still reaches no flag query.
37+
38+
Enforcement is unchanged: same gates, same flags, same default. The flag read
39+
was already memoized per process, so what this corrects is the property
40+
ADR-0104 states, not a user-visible cost.

content/docs/releases/v17.mdx

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

1028+
**You are told about both, rather than having to find this page.** `os migrate
1029+
meta` — the command a 16→17 upgrade already runs — ends by naming the two data
1030+
migrations and the gate each records, including on a run that rewrote nothing,
1031+
since canonical metadata says nothing about stored values. A deployment that
1032+
boots holding covered fields with no verified gate row logs one line naming the
1033+
command that ends warn mode. Neither reads as "done": until a gate is recorded,
1034+
the classes it covers keep warning.
1035+
1036+
That boot line is scoped to the deployments it can actually help. It counts only
1037+
fields a write would check — not the `lookup`s the registry injects into every
1038+
object — and says nothing once an environment switch has already settled the
1039+
posture, since `OS_DATA_VALUE_SHAPE_STRICT_ENABLED` enforces both classes
1040+
outright and either `OS_ALLOW_LAX_*` opt-out is a deliberate choice the scan
1041+
would not change.
1042+
10281043
**A database created by 17 attests both flags at creation** (#3438), so a new
10291044
deployment enforces from its first boot instead of waiting for someone to run a
10301045
migration that, for an empty store, does nothing. The platform attests only a

packages/objectql/src/engine.ts

Lines changed: 35 additions & 11 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, mediaPostureSetByEnv, 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;
@@ -2282,6 +2293,15 @@ export class ObjectQL implements IDataEngine {
22822293
* own metadata says the gate is about something it stores; the verified case
22832294
* already logs from the flag read.
22842295
*
2296+
* A gate whose posture an environment switch has already settled says
2297+
* nothing either, because the flag this line reports on is not what decides
2298+
* that deployment's posture: under `OS_DATA_VALUE_SHAPE_STRICT_ENABLED`
2299+
* enforcement is already on, so "checked but NOT enforced here" is simply
2300+
* false, and under either opt-out the operator chose leniency deliberately,
2301+
* so naming a migration that would not change what they get is noise. Each
2302+
* gate consults its own pair, since the opt-outs are per-class. Cheapest
2303+
* test first: a kernel with nothing to say never reaches the flag query.
2304+
*
22852305
* Costs one flag read per applicable gate (both memoized, and both already
22862306
* paid by the first write to such an object), and nothing at all for an app
22872307
* that declares neither class of field.
@@ -2290,12 +2310,16 @@ export class ObjectQL implements IDataEngine {
22902310
if (this.migrationGatesAnnounced) return;
22912311
this.migrationGatesAnnounced = true;
22922312
try {
2313+
const mediaByEnv = mediaPostureSetByEnv();
2314+
const coveredByEnv = valueShapePostureSetByEnv();
2315+
if (mediaByEnv && coveredByEnv) return;
2316+
22932317
let media = false;
22942318
let covered = false;
22952319
for (const obj of this._registry.getAllObjects() as any[]) {
2296-
if (!media && this.objectHasMediaField(obj)) media = true;
2297-
if (!covered && this.objectHasCoveredValueField(obj)) covered = true;
2298-
if (media && covered) break;
2320+
if (!media && !mediaByEnv && this.objectHasMediaField(obj)) media = true;
2321+
if (!covered && !coveredByEnv && this.objectHasCoveredValueField(obj)) covered = true;
2322+
if ((media || mediaByEnv) && (covered || coveredByEnv)) break;
22992323
}
23002324

23012325
if (media && !(await this.isFileReferencesMigrationVerified())) {

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,41 @@ 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+
* The predicate is exactly the pair {@link valueShapeStrictEffective} consults
613+
* ahead of the deployment flag — the announcement stays silent in precisely
614+
* the cases where the flag it reports on cannot decide the posture.
615+
*/
616+
export function valueShapePostureSetByEnv(): boolean {
617+
return LAX_VALUE_SHAPES() || VALUE_SHAPE_STRICT();
618+
}
619+
620+
/**
621+
* The media counterpart, standing in the same relation to
622+
* {@link mediaStrictEffective}. A sibling rather than a parameter for the
623+
* reason the two `*StrictEffective` functions are siblings: each gate names
624+
* the switches it answers to at its own call site, and the two sets differ by
625+
* more than a name — `OS_DATA_VALUE_SHAPE_STRICT_ENABLED` opens both, while
626+
* each opt-out reaches only its own class.
627+
*/
628+
export function mediaPostureSetByEnv(): boolean {
629+
return LAX_MEDIA_VALUES() || VALUE_SHAPE_STRICT();
630+
}
631+
597632
/**
598633
* Is `value` a violation of `def`'s declared stored shape — the SAME question
599634
* `validateOne` asks, exported so the `os migrate value-shapes` scanner counts
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `announceOpenMigrationGates` against the REAL registry (ADR-0104 D1 rollout /
5+
* #3438, correcting #4235 and #4253).
6+
*
7+
* The announcement itself is covered in `engine.test.ts`, which mocks
8+
* `./registry` away. That mock is why these cases live in their own file: the
9+
* genuine `registerApp → registry` path INJECTS covered-type fields into every
10+
* object it registers, and a mocked registry hands the engine exactly the
11+
* fields the test wrote. The two defects pinned below are both invisible under
12+
* the mock and unconditional in production, so the suite that can see them has
13+
* to build its objects the way a deployment does.
14+
*
15+
* The risk with any such line is not silence, it is speaking where the line is
16+
* untrue or useless — that trains readers to ignore it. So each silent case is
17+
* pinned, not just the speaking one.
18+
*/
19+
20+
import { describe, it, expect, afterEach, vi } from 'vitest';
21+
import { ObjectQL } from './engine';
22+
import type { IDataDriver } from '@objectstack/spec/contracts';
23+
24+
/** Minimal driver whose `find` serves whatever rows the test seeded. */
25+
function makeDriver(seed: Record<string, Array<Record<string, unknown>>> = {}): IDataDriver {
26+
const store = new Map<string, Array<Record<string, unknown>>>(Object.entries(seed));
27+
return {
28+
name: 'default',
29+
version: '1.0.0',
30+
async connect() {},
31+
async disconnect() {},
32+
async find(object: string) { return store.get(object) ?? []; },
33+
async findOne() { return null; },
34+
async count() { return 0; },
35+
async create(_o: string, data: any) { return data; },
36+
async update(_o: string, id: string, data: any) { return { ...data, id }; },
37+
async delete() { return true; },
38+
async bulkCreate(_o: string, rows: any[]) { return rows; },
39+
async syncSchema() {},
40+
async dropTable() {},
41+
} as unknown as IDataDriver;
42+
}
43+
44+
const COVERED = {
45+
name: 'contact',
46+
fields: {
47+
id: { type: 'text' },
48+
account: { type: 'lookup', reference: 'account' },
49+
geo: { type: 'location' },
50+
},
51+
};
52+
53+
/** Declares no covered field of its own — but the registry will inject four. */
54+
const UNCOVERED = {
55+
name: 'note',
56+
fields: { id: { type: 'text' }, body: { type: 'textarea' } },
57+
};
58+
59+
/** The `sys_migration` shape the flag reader needs to find a row at all. */
60+
const FLAG_OBJECT = {
61+
name: 'sys_migration',
62+
fields: {
63+
id: { type: 'text' },
64+
last_run_at: { type: 'datetime' },
65+
verified_at: { type: 'datetime' },
66+
blocking: { type: 'number' },
67+
},
68+
};
69+
70+
function makeEngine(opts: {
71+
objects?: any[];
72+
flagRows?: Array<Record<string, unknown>>;
73+
} = {}) {
74+
const engine = new ObjectQL();
75+
engine.registerDriver(
76+
makeDriver(opts.flagRows ? { sys_migration: opts.flagRows } : {}),
77+
true,
78+
);
79+
engine.registerApp({
80+
id: 'advisory_pkg',
81+
name: 'Advisory',
82+
objects: opts.objects ?? [COVERED, UNCOVERED],
83+
} as any);
84+
return engine;
85+
}
86+
87+
/** Everything the announcement logged, as one string. */
88+
async function announce(engine: ObjectQL): Promise<string> {
89+
const info = vi.spyOn((engine as any).logger, 'info');
90+
await engine.announceOpenMigrationGates();
91+
return info.mock.calls.map((c: any[]) => String(c[0])).join('\n');
92+
}
93+
94+
afterEach(() => {
95+
vi.restoreAllMocks();
96+
delete process.env.OS_DATA_VALUE_SHAPE_STRICT_ENABLED;
97+
delete process.env.OS_ALLOW_LAX_VALUE_SHAPES;
98+
delete process.env.OS_ALLOW_LAX_MEDIA_VALUES;
99+
});
100+
101+
describe('announceOpenMigrationGates, real registry (ADR-0104 D1 / #3438)', () => {
102+
it('names the command that ends warn mode on a warn-first deployment', async () => {
103+
expect(await announce(makeEngine())).toMatch(/os migrate value-shapes/);
104+
});
105+
106+
it('stays silent when no object declares a covered field — the gate is moot here', async () => {
107+
// The regression guard for the predicate the dormancy short-circuit counts
108+
// with. `note` declares only text, but the REGISTRY injects
109+
// `organization_id` and `owner_id` (both `system`) plus `created_by` /
110+
// `updated_by` (both in `SKIP_FIELDS`) — four `lookup`s, a covered TYPE
111+
// apiece. `validateRecord` skips every one of them before it reaches the
112+
// value-shape check, so counting by raw type answered true for literally
113+
// every object that exists: the short-circuit never fired, its WeakMap
114+
// memoized a constant, and this line fired on every deployment there is.
115+
// Counting by the validator's own `isScannableValueShapeField` — the
116+
// predicate the scanner already imports — is what makes it mean something.
117+
//
118+
// Unreachable through `engine.test.ts`: under its registry mock `note`
119+
// carries the two fields written above and nothing else.
120+
const said = await announce(makeEngine({ objects: [UNCOVERED] }));
121+
expect(said).not.toMatch(/os migrate/);
122+
});
123+
124+
it('stays silent under the all-class opt-in — "NOT enforced here" would be false', async () => {
125+
// `VALUE_SHAPE_STRICT()` short-circuits both `*StrictEffective` functions
126+
// ahead of the deployment flag, so enforcement is already on and the flag
127+
// this line reports on decides nothing.
128+
process.env.OS_DATA_VALUE_SHAPE_STRICT_ENABLED = '1';
129+
expect(await announce(makeEngine())).not.toMatch(/os migrate/);
130+
});
131+
132+
it('stays silent under the value-shape escape hatch — the operator opted out on purpose', async () => {
133+
// Running the scan cannot change this deployment's posture, so naming it is
134+
// noise, not help.
135+
process.env.OS_ALLOW_LAX_VALUE_SHAPES = '1';
136+
expect(await announce(makeEngine())).not.toMatch(/os migrate value-shapes/);
137+
});
138+
139+
it('applies each opt-out to its own class only', async () => {
140+
// The two opt-outs are per-class, so silencing one must not silence the
141+
// other. `contact` declares a `location`; `note` gets a `file` here so both
142+
// gates are live and exactly one is switched off.
143+
process.env.OS_ALLOW_LAX_MEDIA_VALUES = '1';
144+
const said = await announce(makeEngine({
145+
objects: [COVERED, { name: 'note', fields: { id: { type: 'text' }, doc: { type: 'file' } } }],
146+
}));
147+
expect(said).not.toMatch(/os migrate files-to-references/);
148+
expect(said).toMatch(/os migrate value-shapes/);
149+
});
150+
151+
it('stays silent once the gate is open (verified row)', async () => {
152+
const said = await announce(makeEngine({
153+
objects: [COVERED, UNCOVERED, FLAG_OBJECT],
154+
flagRows: [
155+
{
156+
id: 'adr-0104-value-shapes',
157+
last_run_at: '2026-07-30T00:00:00.000Z',
158+
verified_at: '2026-07-30T00:00:00.000Z',
159+
blocking: 0,
160+
},
161+
],
162+
}));
163+
expect(said).not.toMatch(/os migrate value-shapes/);
164+
});
165+
166+
it('still speaks when a row exists but did not pass its self-check', async () => {
167+
// A failed run is not permission — the same "absent evidence is not
168+
// permission" rule the gate itself runs on, so the advice still applies.
169+
const said = await announce(makeEngine({
170+
objects: [COVERED, UNCOVERED, FLAG_OBJECT],
171+
flagRows: [
172+
{
173+
id: 'adr-0104-value-shapes',
174+
last_run_at: '2026-07-30T00:00:00.000Z',
175+
verified_at: null,
176+
blocking: 3,
177+
},
178+
],
179+
}));
180+
expect(said).toMatch(/os migrate value-shapes/);
181+
});
182+
});

0 commit comments

Comments
 (0)