Skip to content

Commit 0bfdf46

Browse files
authored
fix(spec,cli): conversion deprecation notices reach the author, not just os validate (#3855) (#3872)
The ADR-0087 D2 conversion layer rewrites an old-shape key to canonical at load and emits a structured `ConversionNotice` per rewrite. Being silent about FIXING the shape is the point — zero consumer action. Being silent about having HAD to is not: the notice is the one signal that says "this spelling retires in protocol N, and your metadata stops loading then". Two of the three surfaces that run the pass discarded every notice: os validate — passed a sink, printed them. os build — passed NO sink. Every notice dropped. defineStack — passed NO sink. Every notice dropped. That is the #3782 parity class one layer down: not "does this command run the gate" but "does it listen to what the gate says". Five conversions are live today (protocol 11 and 15), so an author on one of those shapes was told by one command and not the other two — and `defineStack` is where that author actually is, since it runs inside their own config module. `os build` now prints the notices and includes a `conversions` array in `--json` under the same key `os validate --json` already uses. `defineStack` warns once per distinct conversion site, in BOTH strict and non-strict mode: the conversion happens on the shared `normalizeStackInput` call before the strict branch, and `strict: false` does not make the old shape any less retiring. A new assertion in `validate-build-gate-parity.test.ts` fails if either command calls `normalizeStackInput` without a sink — verified non-vacuous by stripping the sink and watching it go red. The `defineStack` tests document a sharp edge: the notice path is index-based (`flows[i].nodes[j].type`) and the warn-once set is process-scoped, so fixtures must occupy distinct node indices or they silently reuse an earlier case's dedupe key and assert nothing. No behaviour change for a stack already on canonical shapes — nothing converts, so nothing warns.
1 parent 41642b0 commit 0bfdf46

5 files changed

Lines changed: 199 additions & 4 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/cli": patch
4+
---
5+
6+
fix(spec,cli): conversion deprecation notices reach the author, not just `os validate` (#3855)
7+
8+
The ADR-0087 D2 conversion layer rewrites an old-shape key to its canonical
9+
spelling at load and emits a structured `ConversionNotice` for each rewrite. The
10+
conversion being silent about *fixing* the shape is the point — zero consumer
11+
action. Being silent about having **had** to is not: the notice is the one signal
12+
that says *this spelling retires in protocol N, and your metadata stops loading
13+
then*.
14+
15+
Two of the three surfaces that run the conversion pass discarded every notice:
16+
17+
| Surface | Before | After |
18+
|---|---|---|
19+
| `os validate` | passed a sink, printed them | unchanged |
20+
| `os build` / `os compile` | **passed no sink — notices discarded** | prints them, and includes a `conversions` array in `--json` under the same key `os validate --json` uses |
21+
| `defineStack` | **passed no sink — notices discarded** | warns on the console, once per distinct conversion site |
22+
23+
This is the #3782 parity class one layer down: not "does this command run the
24+
gate" but "does it listen to what the gate says". Five conversions are live
25+
today (protocol 11 and 15), so an author on any of those shapes was told by one
26+
command and not the other two — and `defineStack` is where that author actually
27+
is, since it runs inside their own config module.
28+
29+
`defineStack` surfaces notices in **both** strict and non-strict mode: the
30+
conversion happens on the shared `normalizeStackInput` call before the strict
31+
branch, and `strict: false` does not make the old shape any less retiring.
32+
33+
A new assertion in `validate-build-gate-parity.test.ts` fails if either command
34+
calls `normalizeStackInput` without a sink, so the gap cannot silently reopen.
35+
36+
No behaviour change for a stack already on canonical shapes: nothing converts,
37+
so nothing warns.

packages/cli/src/commands/compile.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import path from 'path';
55
import fs from 'fs';
66
import chalk from 'chalk';
77
import { ZodError } from 'zod';
8-
import { ObjectStackDefinitionSchema, normalizeStackInput, lintDeprecatedAliases } from '@objectstack/spec';
8+
import { ObjectStackDefinitionSchema, normalizeStackInput, lintDeprecatedAliases, type ConversionNotice } from '@objectstack/spec';
99
import { loadConfig } from '../utils/config.js';
1010
import { lowerCallables } from '../utils/lower-callables.js';
1111
import { validateStackExpressions } from '@objectstack/lint';
@@ -92,8 +92,28 @@ export default class Compile extends Command {
9292
}
9393

9494
// 2. Normalize map-formatted stack definition.
95+
// The ADR-0087 D2 conversion layer runs here (inside normalizeStackInput).
96+
// Each rewrite emits a structured deprecation notice, and this command
97+
// used to drop every one of them: `os validate` passed a sink and
98+
// surfaced them, `os build` passed none. That is the #3782 parity class
99+
// — the two surfaces disagreeing about what an author is told — and it
100+
// bites harder than it reads, because the notice is the ONLY warning an
101+
// old-shape author gets before the conversion retires and their metadata
102+
// stops loading. Five conversions are live today (protocol 11 and 15),
103+
// so the gap is real, not hypothetical.
95104
if (!flags.json) printStep('Normalizing stack definition...');
96-
const normalized = normalizeStackInput(config as Record<string, unknown>);
105+
const conversionNotices: ConversionNotice[] = [];
106+
const normalized = normalizeStackInput(config as Record<string, unknown>, {
107+
onConversionNotice: (n) => conversionNotices.push(n),
108+
});
109+
if (conversionNotices.length > 0 && !flags.json) {
110+
console.log('');
111+
for (const n of conversionNotices) {
112+
printWarning(
113+
`${n.path}: '${n.from}' → '${n.to}' (converted at load; conversion '${n.conversionId}', retires in protocol ${n.retiresIn})`,
114+
);
115+
}
116+
}
97117

98118
// 2a. [#3743] PRE-PARSE authoring lint. Everything in the post-parse lint
99119
// block (3d and below) reads `result.data`, which is the wrong side of
@@ -791,6 +811,9 @@ export default class Compile extends Command {
791811
runtimeModule: runtimeBundle?.outputFileName ?? null,
792812
runtimeModuleSize: runtimeBundle?.size ?? 0,
793813
warnings: widgetWarnings,
814+
// Same key `os validate --json` uses, so a CI consumer reads one shape
815+
// from either command rather than learning two.
816+
conversions: conversionNotices,
794817
specVersionGap: specGap,
795818
stats,
796819
duration: timer.elapsed(),

packages/cli/test/validate-build-gate-parity.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,30 @@ describe('os validate is the read-only superset of os build (#3782)', () => {
7777
expect(validateGates.has(gate), `validate.ts must call ${gate}`).toBe(true);
7878
}
7979
});
80+
81+
/**
82+
* The same drift, one layer down and easier to miss: not "does this command
83+
* run the gate" but "does it LISTEN to what the gate says". The ADR-0087 D2
84+
* conversion pass runs inside `normalizeStackInput` on both commands, so both
85+
* always converted — but only `os validate` passed an `onConversionNotice`
86+
* sink, so `os build` silently discarded every deprecation notice. A notice
87+
* is the one warning an old-shape author gets before the conversion retires
88+
* and their metadata stops loading, and five conversions are live today.
89+
*
90+
* Source-level for the same reason as the gate check above: it fails when the
91+
* sink is dropped, which is the moment it is cheap to fix.
92+
*/
93+
it('both commands pass a conversion-notice sink to normalizeStackInput', () => {
94+
for (const file of ['compile.ts', 'validate.ts']) {
95+
const src = readFileSync(join(COMMANDS_DIR, file), 'utf8');
96+
const call = src.match(/normalizeStackInput\([\s\S]{0,400}?\)\s*;/);
97+
expect(call, `${file} must call normalizeStackInput`).not.toBeNull();
98+
expect(
99+
call![0].includes('onConversionNotice'),
100+
`${file} calls normalizeStackInput without an onConversionNotice sink, so every ADR-0087 ` +
101+
`D2 deprecation notice it raises is discarded. Pass a sink and surface the notices ` +
102+
`(mirror the other command).`,
103+
).toBe(true);
104+
}
105+
});
80106
});

packages/spec/src/stack.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,3 +1468,78 @@ describe('defineStack — deprecated alias warnings (#3743)', () => {
14681468
expect('conditionalRequired' in field).toBe(false);
14691469
});
14701470
});
1471+
1472+
// ── ADR-0087 D2 conversion notices reach the author ─────────────────────────
1473+
//
1474+
// A conversion is deliberately silent about FIXING the shape — zero consumer
1475+
// action is the point. It is not supposed to be silent about having had to:
1476+
// the notice carries "retires in protocol N", after which the old spelling
1477+
// stops loading. `defineStack` passed no sink, so the author who wrote the old
1478+
// shape heard nothing unless they happened to run `os validate` (`os build`
1479+
// was deaf too). Five conversions are live today, so this was a real gap.
1480+
describe('defineStack — ADR-0087 D2 conversion notices', () => {
1481+
const manifest = { id: 'p', version: '1.0.0', type: 'app' as const, name: 'P', namespace: 'demo' };
1482+
const obj = { name: 'demo_task', label: 'Task', fields: { title: { type: 'text' as const } } };
1483+
1484+
// A protocol-11 flow callout node type — `webhook` converts to `http`.
1485+
//
1486+
// The warn-once key is (conversionId, path, from, to) and the notice path is
1487+
// INDEX-based (`flows[i].nodes[j].type`) — the flow's name never enters it.
1488+
// Since the dedupe set is process-scoped by design, every case below has to
1489+
// occupy its own node index or it silently reuses an earlier case's key and
1490+
// asserts nothing.
1491+
const legacyFlowAt = (index: number) => ({
1492+
name: `conv_demo_${index}`,
1493+
nodes: [
1494+
...Array.from({ length: index }, (_, i) => ({ id: `n_pad_${i}`, type: 'http' })),
1495+
{ id: 'n_call', type: 'webhook' },
1496+
],
1497+
});
1498+
// Non-strict throughout: the assertion is about the conversion sink alone,
1499+
// not about satisfying every downstream cross-reference check. The sink is
1500+
// wired on the single `normalizeStackInput` call that runs BEFORE the strict
1501+
// branch, so both modes go through it.
1502+
const define = (index: number) =>
1503+
defineStack({ manifest, objects: [obj], flows: [legacyFlowAt(index)] }, { strict: false });
1504+
1505+
let warn: ReturnType<typeof vi.spyOn>;
1506+
beforeEach(() => {
1507+
warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
1508+
});
1509+
afterEach(() => {
1510+
warn.mockRestore();
1511+
});
1512+
1513+
it('warns when a conversion rewrites an old shape, naming the retirement major', () => {
1514+
define(0);
1515+
1516+
expect(warn).toHaveBeenCalledTimes(1);
1517+
const msg = warn.mock.calls[0][0] as string;
1518+
expect(msg).toContain(`'webhook' → 'http'`);
1519+
expect(msg).toContain('flow-node-http-callout-rename');
1520+
expect(msg).toContain('retires in protocol');
1521+
});
1522+
1523+
it('nags once per distinct conversion site', () => {
1524+
define(1);
1525+
define(1);
1526+
define(1);
1527+
expect(warn).toHaveBeenCalledTimes(1);
1528+
});
1529+
1530+
it('reports a second, distinct site separately', () => {
1531+
// Two different authored occurrences, each needing its own fix — dedupe
1532+
// must not swallow the second.
1533+
define(2);
1534+
define(3);
1535+
expect(warn).toHaveBeenCalledTimes(2);
1536+
});
1537+
1538+
it('stays quiet for a stack that is already canonical', () => {
1539+
defineStack(
1540+
{ manifest, objects: [obj], flows: [{ name: 'conv_canonical', nodes: [{ id: 'n1', type: 'http' }] }] },
1541+
{ strict: false },
1542+
);
1543+
expect(warn).not.toHaveBeenCalled();
1544+
});
1545+
});

packages/spec/src/stack.zod.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { hasPlatformObjectPrefix } from './system/constants/platform-object-name
1111
import { objectStackErrorMap, formatZodError } from './shared/error-map.zod';
1212
import { normalizeStackInput, type MetadataCollectionInput, type MapSupportedField } from './shared/metadata-collection.zod';
1313
import { lintDeprecatedAliases, formatDeprecatedAliasFinding } from './shared/deprecated-aliases';
14+
import type { ConversionNotice } from './conversions/types.js';
1415

1516
// Data Protocol
1617
import { ObjectSchema, ObjectExtensionSchema } from './data/object.zod';
@@ -1068,15 +1069,48 @@ function warnDeprecatedAliases(normalized: Record<string, unknown>): void {
10681069
}
10691070
}
10701071

1072+
/** Conversion notices already reported this process — same warn-once reason. */
1073+
const warnedConversionNotices = new Set<string>();
1074+
1075+
/**
1076+
* Surface the ADR-0087 D2 conversion notices raised while normalizing.
1077+
*
1078+
* A conversion is deliberately silent about *fixing* the shape — zero consumer
1079+
* action is the point — but it is not supposed to be silent about having HAD to.
1080+
* The notice is the one signal that says "this spelling retires in protocol N,
1081+
* and your metadata stops loading then", and `defineStack` is where the author
1082+
* who wrote the old shape actually is. Until now it passed no sink, so that
1083+
* author heard nothing unless they happened to run `os validate` — the same gap
1084+
* this change closes in `os build`.
1085+
*
1086+
* Advisory and warn-once: the conversion already produced a correct stack.
1087+
*/
1088+
function warnConversionNotice(notice: ConversionNotice): void {
1089+
const key = `${notice.conversionId} ${notice.path} ${notice.from} ${notice.to}`;
1090+
if (warnedConversionNotices.has(key)) return;
1091+
warnedConversionNotices.add(key);
1092+
console.warn(
1093+
`defineStack: ${notice.path}: '${notice.from}' → '${notice.to}' (converted at load; ` +
1094+
`conversion '${notice.conversionId}', retires in protocol ${notice.retiresIn}). ` +
1095+
`Update the source to the canonical shape — the conversion stops running then.`,
1096+
);
1097+
}
1098+
10711099
export function defineStack(
10721100
config: ObjectStackDefinitionInput,
10731101
options?: DefineStackOptions,
10741102
): ObjectStackDefinition {
10751103
// Default to strict=true for safety (validate by default)
10761104
const strict = options?.strict !== false;
10771105

1078-
// Normalize map-formatted collections to arrays (key → name injection)
1079-
const normalized = normalizeStackInput(config as Record<string, unknown>);
1106+
// Normalize map-formatted collections to arrays (key → name injection), and
1107+
// surface every ADR-0087 D2 conversion the pass had to apply. Unlike the alias
1108+
// warning below this runs in BOTH modes: a conversion happens whether or not
1109+
// we go on to parse, so `strict: false` does not make the old shape any less
1110+
// retiring.
1111+
const normalized = normalizeStackInput(config as Record<string, unknown>, {
1112+
onConversionNotice: warnConversionNotice,
1113+
});
10801114

10811115
if (!strict) {
10821116
// Non-strict mode: skip validation (advanced use cases only).

0 commit comments

Comments
 (0)