fix(spec,cli): conversion deprecation notices reach the author, not just os validate (#3855)

.changeset/conversion-notice-channel.md

@@ -0,0 +1,37 @@
+---
+"@objectstack/spec": minor
+"@objectstack/cli": patch
+---
+
+fix(spec,cli): conversion deprecation notices reach the author, not just `os validate` (#3855)
+
+The ADR-0087 D2 conversion layer rewrites an old-shape key to its canonical
+spelling at load and emits a structured `ConversionNotice` for each rewrite. The
+conversion 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 conversion pass discarded every notice:
+
+| Surface | Before | After |
+|---|---|---|
+| `os validate` | passed a sink, printed them | unchanged |
+| `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 |
+| `defineStack` | **passed no sink — notices discarded** | warns on the console, once per distinct conversion site |
+
+This 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 any 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.
+
+`defineStack` surfaces notices 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, so the gap cannot silently reopen.
+
+No behaviour change for a stack already on canonical shapes: nothing converts,
+so nothing warns.

packages/cli/src/commands/compile.ts

@@ -5,7 +5,7 @@ import path from 'path';
 import fs from 'fs';
 import chalk from 'chalk';
 import { ZodError } from 'zod';
-import { ObjectStackDefinitionSchema, normalizeStackInput, lintDeprecatedAliases } from '@objectstack/spec';
+import { ObjectStackDefinitionSchema, normalizeStackInput, lintDeprecatedAliases, type ConversionNotice } from '@objectstack/spec';
 import { loadConfig } from '../utils/config.js';
 import { lowerCallables } from '../utils/lower-callables.js';
 import { validateStackExpressions } from '@objectstack/lint';
@@ -92,8 +92,28 @@ export default class Compile extends Command {
  }
 
  // 2. Normalize map-formatted stack definition.
+ // The ADR-0087 D2 conversion layer runs here (inside normalizeStackInput).
+ // Each rewrite emits a structured deprecation notice, and this command
+ // used to drop every one of them: `os validate` passed a sink and
+ // surfaced them, `os build` passed none. That is the #3782 parity class
+ // — the two surfaces disagreeing about what an author is told — and it
+ // bites harder than it reads, because the notice is the ONLY warning an
+ // old-shape author gets before the conversion retires and their metadata
+ // stops loading. Five conversions are live today (protocol 11 and 15),
+ // so the gap is real, not hypothetical.
  if (!flags.json) printStep('Normalizing stack definition...');
- const normalized = normalizeStackInput(config as Record<string, unknown>);
+ const conversionNotices: ConversionNotice[] = [];
+ const normalized = normalizeStackInput(config as Record<string, unknown>, {
+ onConversionNotice: (n) => conversionNotices.push(n),
+ });
+ if (conversionNotices.length > 0 && !flags.json) {
+ console.log('');
+ for (const n of conversionNotices) {
+ printWarning(
+ `${n.path}: '${n.from}' → '${n.to}' (converted at load; conversion '${n.conversionId}', retires in protocol ${n.retiresIn})`,
+ );
+ }
+ }
 
  // 2a. [#3743] PRE-PARSE authoring lint. Everything in the post-parse lint
  // block (3d and below) reads `result.data`, which is the wrong side of
@@ -791,6 +811,9 @@ export default class Compile extends Command {
  runtimeModule: runtimeBundle?.outputFileName ?? null,
  runtimeModuleSize: runtimeBundle?.size ?? 0,
  warnings: widgetWarnings,
+ // Same key `os validate --json` uses, so a CI consumer reads one shape
+ // from either command rather than learning two.
+ conversions: conversionNotices,
  specVersionGap: specGap,
  stats,
  duration: timer.elapsed(),

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

@@ -77,4 +77,30 @@ describe('os validate is the read-only superset of os build (#3782)', () => {
  expect(validateGates.has(gate), `validate.ts must call ${gate}`).toBe(true);
  }
  });
+
+ /**
+ * The same drift, one layer down and easier to miss: not "does this command
+ * run the gate" but "does it LISTEN to what the gate says". The ADR-0087 D2
+ * conversion pass runs inside `normalizeStackInput` on both commands, so both
+ * always converted — but only `os validate` passed an `onConversionNotice`
+ * sink, so `os build` silently discarded every deprecation notice. A notice
+ * is the one warning an old-shape author gets before the conversion retires
+ * and their metadata stops loading, and five conversions are live today.
+ *
+ * Source-level for the same reason as the gate check above: it fails when the
+ * sink is dropped, which is the moment it is cheap to fix.
+ */
+ it('both commands pass a conversion-notice sink to normalizeStackInput', () => {
+ for (const file of ['compile.ts', 'validate.ts']) {
+ const src = readFileSync(join(COMMANDS_DIR, file), 'utf8');
+ const call = src.match(/normalizeStackInput\([\s\S]{0,400}?\)\s*;/);
+ expect(call, `${file} must call normalizeStackInput`).not.toBeNull();
+ expect(
+ call![0].includes('onConversionNotice'),
+ `${file} calls normalizeStackInput without an onConversionNotice sink, so every ADR-0087 ` +
+ `D2 deprecation notice it raises is discarded. Pass a sink and surface the notices ` +
+ `(mirror the other command).`,
+ ).toBe(true);
+ }
+ });
 });

packages/spec/src/stack.test.ts

@@ -1468,3 +1468,78 @@ describe('defineStack — deprecated alias warnings (#3743)', () => {
  expect('conditionalRequired' in field).toBe(false);
  });
 });
+
+// ── ADR-0087 D2 conversion notices reach the author ─────────────────────────
+//
+// A conversion is deliberately silent about FIXING the shape — zero consumer
+// action is the point. It is not supposed to be silent about having had to:
+// the notice carries "retires in protocol N", after which the old spelling
+// stops loading. `defineStack` passed no sink, so the author who wrote the old
+// shape heard nothing unless they happened to run `os validate` (`os build`
+// was deaf too). Five conversions are live today, so this was a real gap.
+describe('defineStack — ADR-0087 D2 conversion notices', () => {
+ const manifest = { id: 'p', version: '1.0.0', type: 'app' as const, name: 'P', namespace: 'demo' };
+ const obj = { name: 'demo_task', label: 'Task', fields: { title: { type: 'text' as const } } };
+
+ // A protocol-11 flow callout node type — `webhook` converts to `http`.
+ //
+ // The warn-once key is (conversionId, path, from, to) and the notice path is
+ // INDEX-based (`flows[i].nodes[j].type`) — the flow's name never enters it.
+ // Since the dedupe set is process-scoped by design, every case below has to
+ // occupy its own node index or it silently reuses an earlier case's key and
+ // asserts nothing.
+ const legacyFlowAt = (index: number) => ({
+ name: `conv_demo_${index}`,
+ nodes: [
+ ...Array.from({ length: index }, (_, i) => ({ id: `n_pad_${i}`, type: 'http' })),
+ { id: 'n_call', type: 'webhook' },
+ ],
+ });
+ // Non-strict throughout: the assertion is about the conversion sink alone,
+ // not about satisfying every downstream cross-reference check. The sink is
+ // wired on the single `normalizeStackInput` call that runs BEFORE the strict
+ // branch, so both modes go through it.
+ const define = (index: number) =>
+ defineStack({ manifest, objects: [obj], flows: [legacyFlowAt(index)] }, { strict: false });
+
+ let warn: ReturnType<typeof vi.spyOn>;
+ beforeEach(() => {
+ warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+ });
+ afterEach(() => {
+ warn.mockRestore();
+ });
+
+ it('warns when a conversion rewrites an old shape, naming the retirement major', () => {
+ define(0);
+
+ expect(warn).toHaveBeenCalledTimes(1);
+ const msg = warn.mock.calls[0][0] as string;
+ expect(msg).toContain(`'webhook' → 'http'`);
+ expect(msg).toContain('flow-node-http-callout-rename');
+ expect(msg).toContain('retires in protocol');
+ });
+
+ it('nags once per distinct conversion site', () => {
+ define(1);
+ define(1);
+ define(1);
+ expect(warn).toHaveBeenCalledTimes(1);
+ });
+
+ it('reports a second, distinct site separately', () => {
+ // Two different authored occurrences, each needing its own fix — dedupe
+ // must not swallow the second.
+ define(2);
+ define(3);
+ expect(warn).toHaveBeenCalledTimes(2);
+ });
+
+ it('stays quiet for a stack that is already canonical', () => {
+ defineStack(
+ { manifest, objects: [obj], flows: [{ name: 'conv_canonical', nodes: [{ id: 'n1', type: 'http' }] }] },
+ { strict: false },
+ );
+ expect(warn).not.toHaveBeenCalled();
+ });
+});

packages/spec/src/stack.zod.ts

@@ -11,6 +11,7 @@ import { hasPlatformObjectPrefix } from './system/constants/platform-object-name
 import { objectStackErrorMap, formatZodError } from './shared/error-map.zod';
 import { normalizeStackInput, type MetadataCollectionInput, type MapSupportedField } from './shared/metadata-collection.zod';
 import { lintDeprecatedAliases, formatDeprecatedAliasFinding } from './shared/deprecated-aliases';
+import type { ConversionNotice } from './conversions/types.js';
 
 // Data Protocol
 import { ObjectSchema, ObjectExtensionSchema } from './data/object.zod';
@@ -1068,15 +1069,48 @@ function warnDeprecatedAliases(normalized: Record<string, unknown>): void {
  }
 }
 
+/** Conversion notices already reported this process — same warn-once reason. */
+const warnedConversionNotices = new Set<string>();
+
+/**
+ * Surface the ADR-0087 D2 conversion notices raised while normalizing.
+ *
+ * A conversion is deliberately silent about *fixing* the shape — zero consumer
+ * action is the point — but it is not supposed to be silent about having HAD to.
+ * The notice is the one signal that says "this spelling retires in protocol N,
+ * and your metadata stops loading then", and `defineStack` is where the author
+ * who wrote the old shape actually is. Until now it passed no sink, so that
+ * author heard nothing unless they happened to run `os validate` — the same gap
+ * this change closes in `os build`.
+ *
+ * Advisory and warn-once: the conversion already produced a correct stack.
+ */
+function warnConversionNotice(notice: ConversionNotice): void {
+ const key = `${notice.conversionId} ${notice.path} ${notice.from} ${notice.to}`;
+ if (warnedConversionNotices.has(key)) return;
+ warnedConversionNotices.add(key);
+ console.warn(
+ `defineStack: ${notice.path}: '${notice.from}' → '${notice.to}' (converted at load; ` +
+ `conversion '${notice.conversionId}', retires in protocol ${notice.retiresIn}). ` +
+ `Update the source to the canonical shape — the conversion stops running then.`,
+ );
+}
+
 export function defineStack(
  config: ObjectStackDefinitionInput,
  options?: DefineStackOptions,
 ): ObjectStackDefinition {
  // Default to strict=true for safety (validate by default)
  const strict = options?.strict !== false;
 
- // Normalize map-formatted collections to arrays (key → name injection)
- const normalized = normalizeStackInput(config as Record<string, unknown>);
+ // Normalize map-formatted collections to arrays (key → name injection), and
+ // surface every ADR-0087 D2 conversion the pass had to apply. Unlike the alias
+ // warning below this runs in BOTH modes: a conversion happens whether or not
+ // we go on to parse, so `strict: false` does not make the old shape any less
+ // retiring.
+ const normalized = normalizeStackInput(config as Record<string, unknown>, {
+ onConversionNotice: warnConversionNotice,
+ });
 
  if (!strict) {
  // Non-strict mode: skip validation (advanced use cases only).