From 72d35e35e6bd9034f02aed62e095ce5598ace864 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 20:34:27 +0000 Subject: [PATCH 1/2] refactor(cli): render the ADR-0087 conversion notice from one source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The human face of an ADR-0087 D2 conversion notice was written out three times, verbatim, in os build / os validate / os lint. The three template literals were byte-identical (measured: one distinct literal across the three) and held equal by convention alone — the parity guard asserted each command PASSES an onConversionNotice sink, never that they SAY the same thing once they have one, so a reword in one command diverged silently with every gate green. Hoist the sentence into formatConversionNotice() in src/utils/format.ts and render all three through it. It is a formatter, not a printer, which is what makes one implementation possible: the three call sites genuinely differ in DISPOSITION (build/lint print behind !flags.json, validate pushes into the --strict warnings list) but not in what they say, so the difference costs the function no parameter. Output is byte-identical. Extend packages/cli/test/validate-build-gate-parity.test.ts with the rule the old guard could not see: every authoring command renders through the one formatter and none spells the sentence out inline, with a positive control so it cannot pass vacuously on a CLI that says nothing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- packages/cli/src/commands/compile.ts | 5 +- packages/cli/src/commands/lint.ts | 5 +- packages/cli/src/commands/validate.ts | 3 +- .../utils/format.conversion-notice.test.ts | 65 +++++++++++++++ packages/cli/src/utils/format.ts | 42 +++++++++- .../test/validate-build-gate-parity.test.ts | 81 ++++++++++++++++++- 6 files changed, 192 insertions(+), 9 deletions(-) create mode 100644 packages/cli/src/utils/format.conversion-notice.test.ts diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index c1f3d0453f..ceca3d63a5 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -28,6 +28,7 @@ import { printError, printStep, printWarning, + formatConversionNotice, printAuthoringAdvisories, printAuthoringRuleErrors, printDocIssueErrors, @@ -234,9 +235,7 @@ export default class Compile extends Command { 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})`, - ); + printWarning(formatConversionNotice(n)); } } diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 85e8018828..43ace89d8e 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -19,6 +19,7 @@ import { printHeader, printSuccess, printWarning, + formatConversionNotice, printError, printInfo, printStep, @@ -576,9 +577,7 @@ export default class Lint extends Command { 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})`, - ); + printWarning(formatConversionNotice(n)); } } const issues = lintConfig(normalized, { sduiManifest: resolveSduiManifest() }); diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 9bc527a104..e7293ad2a0 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -23,6 +23,7 @@ import { printSuccess, printError, printStep, + formatConversionNotice, printAuthoringRuleErrors, printDocIssueErrors, JSON_FULL_LIST_REMEDY, @@ -406,7 +407,7 @@ export default class Validate extends Command { // was auto-converted at load. No action is required to keep loading, but // the notice steers the author to the canonical key before it retires. for (const n of conversionNotices) { - warnings.push(`${n.path}: '${n.from}' → '${n.to}' (converted at load; conversion '${n.conversionId}', retires in protocol ${n.retiresIn})`); + warnings.push(formatConversionNotice(n)); } // Every advisory the registry raised. All of them feed `--strict` now: diff --git a/packages/cli/src/utils/format.conversion-notice.test.ts b/packages/cli/src/utils/format.conversion-notice.test.ts new file mode 100644 index 0000000000..a32b1ac7ab --- /dev/null +++ b/packages/cli/src/utils/format.conversion-notice.test.ts @@ -0,0 +1,65 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, expect, it } from 'vitest'; +import { CONVERSION_NOTICE_CODE, type ConversionNotice } from '@objectstack/spec'; +import { formatConversionNotice } from './format.js'; + +/** + * The VALUE pin for the ADR-0087 D2 conversion notice's human face (#13743). + * + * Its sibling in `test/validate-build-gate-parity.test.ts` is structural: it + * holds the three authoring commands to ONE formatter so no copy can drift. + * That rule says nothing about what the one source actually says — and the + * sentence is the point. A conversion rewrites the old shape and asks the + * author for nothing, so this notice is the only warning they get before the + * conversion retires from the load path and their metadata stops loading. + * + * ⭐ It is also the measurement that made the #13743 extraction safe to do at + * all. `os build`, `os validate` and `os lint` each carried a verbatim copy of + * this template; the copies were byte-identical (one distinct template literal + * across the three), so hoisting them onto one function changes no output. The + * expected string below is that literal's rendering, transcribed from the + * pre-extraction source — if the extraction had altered one byte, this fails. + */ +describe('formatConversionNotice (#13743)', () => { + const notice: ConversionNotice = { + code: CONVERSION_NOTICE_CODE, + conversionId: 'page-jsx-to-html', + surface: 'page.kind', + toMajor: 15, + retiresIn: 16, + from: 'jsx', + to: 'html', + path: 'pages[0].kind', + message: '[protocol] converted page.kind at pages[0].kind …', + }; + + it('renders the four fields an author needs, in the shipped wording', () => { + expect(formatConversionNotice(notice)).toBe( + "pages[0].kind: 'jsx' → 'html' (converted at load; conversion 'page-jsx-to-html', retires in protocol 16)", + ); + }); + + /** + * The expiry is what separates this notice from an ordinary deprecation + * warning: it names the protocol major in which the source STOPS LOADING. + * Pinned separately from the whole-string assertion above so a future reword + * cannot drop it while still looking like a reword. + */ + it('always names the retiring major — the part that makes it actionable', () => { + expect(formatConversionNotice({ ...notice, retiresIn: 21 })).toContain('retires in protocol 21'); + }); + + /** + * ⛔ NOT the notice's own `message`. `ConversionNotice.message` is a second, + * longer prose form built in `packages/spec/src/conversions/apply.ts`, and it + * is what the `--json` payloads carry under `conversions`. The text face has + * always rendered its own terser sentence from the structured fields instead. + * Pinned so the difference is a recorded fact rather than something the next + * reader discovers and "fixes" in one command only — which is exactly the + * divergence this card is about. + */ + it('is derived from the structured fields, not from notice.message', () => { + expect(formatConversionNotice({ ...notice, message: 'REPLACED' })).not.toContain('REPLACED'); + }); +}); diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index 167200f413..53d51fe1c7 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -2,7 +2,7 @@ import chalk from 'chalk'; import type { ZodError } from 'zod'; -import { formatZodIssue } from '@objectstack/spec'; +import { formatZodIssue, type ConversionNotice } from '@objectstack/spec'; import type { TenancyPosture } from '@objectstack/spec/security'; import { writeStdoutDirect } from './json-stdout.js'; @@ -1395,3 +1395,43 @@ export function printBulletList( remedy: options.remedy, }); } + +// ─── ADR-0087 D2 conversion notices ───────────────────────────────── + +/** + * The human face of one ADR-0087 D2 conversion notice — ONE implementation of + * that sentence, deliberately, for the same reason as + * {@link printTruncationNotice} above. + * + * The sentence is close to a contract. A conversion rewrites an old-shape key + * at load and asks the author for nothing, so this notice is the ONLY warning + * they get before the conversion retires and their metadata stops loading — + * and an author who runs two of the three authoring commands over one tree is + * meant to be told the same thing in the same words. It was written out three + * times, verbatim, in `compile.ts`, `validate.ts` and `lint.ts` (#13743), held + * equal by convention alone: the parity guard in + * `test/validate-build-gate-parity.test.ts` asserted that each command PASSES + * an `onConversionNotice` sink, never that they SAY the same thing once they + * have one — so a reword in one command drifted from the other two with every + * gate green. + * + * ⛔ A formatter, not a printer, and that is what makes one implementation + * possible at all. The three call sites are genuinely not interchangeable in + * what they DO with the string — `os build` and `os lint` hand it to + * {@link printWarning} behind `!flags.json`, while `os validate` pushes it + * into the `warnings` list that `--strict` then judges — but they were + * byte-identical in what they SAY (measured: one distinct template literal + * across the three). The whole difference lives in the disposition of the + * returned string, so it costs this function no parameter. + * + * ⛔ NOT the `defineStack` face, which is a fourth rendering of these same + * fields and deliberately a different sentence: `warnConversionNotice` in + * `packages/spec/src/stack.zod.ts` adds a `defineStack:` prefix and a trailing + * "update the source" instruction, because that seam warns once per process + * while an author is composing, not inside a command's report. It also cannot + * import this: `@objectstack/cli` depends on `@objectstack/spec`, not the + * reverse. Hoisting all four onto one source is a separate question. + */ +export function formatConversionNotice(notice: ConversionNotice): string { + return `${notice.path}: '${notice.from}' → '${notice.to}' (converted at load; conversion '${notice.conversionId}', retires in protocol ${notice.retiresIn})`; +} diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index 6035dd8c54..e12b3f90a5 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -76,6 +76,26 @@ const BUILD_ONLY_GATES: Readonly> = { const sourceOf = (file: string) => readFileSync(join(COMMANDS_DIR, file), 'utf8'); +const UTILS_DIR = join(__dirname, '..', 'src', 'utils'); + +/** + * The three authoring commands, as one list. Named once so a rule below cannot + * quietly cover a subset of the class it describes — the #12297 failure the + * sink guard at the bottom of this file records. + */ +const AUTHORING_COMMANDS: readonly string[] = ['compile.ts', 'validate.ts', 'lint.ts']; + +/** + * The prose fingerprint of the ADR-0087 D2 conversion notice — the part of the + * sentence that is neither interpolation nor punctuation, so it survives a + * rename of the loop variable and does NOT survive a reword. Matching on this + * rather than the whole template is deliberate: a divergence that only reworded + * the tail would still be caught by the formatter-call assertion, and a + * whole-template match would go vacuously green the day someone reflowed a + * line. + */ +const NOTICE_PROSE = 'converted at load; conversion'; + /** Every `lintFoo(`/`validateFoo(` call site in a command's source. */ function gateCallsIn(file: string): Set { const calls = sourceOf(file).match(/\b(?:lint|validate)[A-Z]\w*(?=\s*\()/g) ?? []; @@ -154,7 +174,7 @@ describe('os validate is the read-only superset of os build (#3782, #4409)', () * sink is dropped, which is the moment it is cheap to fix. */ it('all three authoring commands pass a conversion-notice sink to normalizeStackInput', () => { - for (const file of ['compile.ts', 'validate.ts', 'lint.ts']) { + for (const file of AUTHORING_COMMANDS) { const src = sourceOf(file); const call = src.match(/normalizeStackInput\([\s\S]{0,400}?\)\s*;/); expect(call, `${file} must call normalizeStackInput`).not.toBeNull(); @@ -166,4 +186,63 @@ describe('os validate is the read-only superset of os build (#3782, #4409)', () ).toBe(true); } }); + + /** + * The same drift again, one step past the sink: not "does this command hear + * the notice" but "does it SAY THE SAME THING once it has one". + * + * ⭐ [#13743] The sink guard above is blind here by construction. It asserts + * each command PASSES an `onConversionNotice` sink; once all three had one, + * each rendered the sentence from its own verbatim copy of the template, held + * equal by convention alone. A reword in one command diverged it from the + * other two and EVERY GATE STAYED GREEN — including this file, which is the + * one place that would have been expected to notice. + * + * That sentence is close to a contract: a conversion asks the author for + * nothing at load, so the notice is the ONLY warning they get before the + * conversion retires and their metadata stops loading. An author who runs two + * of the three commands over one tree must be told the same thing in the same + * words. + * + * The rule is therefore structural rather than comparative — the three + * copies are gone, and what is asserted is that they cannot come back: every + * authoring command renders through the ONE formatter, and none of them + * spells the sentence out inline. Comparing three literals for equality would + * have locked today's three copies together while leaving a fourth free to + * appear; requiring the single source forecloses both. + */ + it('all three authoring commands render the conversion notice through ONE formatter', () => { + // Positive control FIRST: the sentence must still exist in the formatter. + // Without this, deleting `formatConversionNotice` and every inline copy + // would satisfy every "no inline copy" assertion below — a rule that is + // green precisely when the notice has been silenced. + const formatter = readFileSync(join(UTILS_DIR, 'format.ts'), 'utf8'); + expect( + /export function formatConversionNotice\b/.test(formatter), + 'src/utils/format.ts must export formatConversionNotice — if it moved, move this guard with it.', + ).toBe(true); + expect( + formatter.includes(NOTICE_PROSE), + `src/utils/format.ts no longer carries the notice wording ("${NOTICE_PROSE}"), so the ` + + `assertions below would pass vacuously on a CLI that says nothing at all.`, + ).toBe(true); + + for (const file of AUTHORING_COMMANDS) { + expect( + calls(file, 'formatConversionNotice'), + `${file} must render its ADR-0087 D2 conversion notices with formatConversionNotice() ` + + `from src/utils/format.ts. The three commands dispose of the string differently — ` + + `os build and os lint print it, os validate pushes it into the --strict warnings list ` + + `— but they must SAY the same thing, so the sentence has exactly one source.`, + ).toBe(true); + expect( + sourceOf(file).includes(NOTICE_PROSE), + `${file} spells the ADR-0087 D2 conversion notice out inline instead of calling ` + + `formatConversionNotice(). That is the #13743 divergence: this sentence is the only ` + + `warning an old-shape author gets before the conversion retires and their metadata ` + + `stops loading, and a copy here drifts from the other commands silently. Edit the ` + + `wording in src/utils/format.ts, where all three read it.`, + ).toBe(false); + } + }); }); From 5858beb0b5031beb4531122d819cc8882e1fbc4f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 21:03:59 +0000 Subject: [PATCH 2/2] chore(changeset): patch @objectstack/cli for the conversion-notice consolidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of #13743. `@objectstack/cli` publishes `dist`, compiled from the edited `src`, so this diff changes the published package even though it changes nothing an author can observe — `patch`, not `skip-changeset`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- .../cli-conversion-notice-one-source.md | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 .changeset/cli-conversion-notice-one-source.md diff --git a/.changeset/cli-conversion-notice-one-source.md b/.changeset/cli-conversion-notice-one-source.md new file mode 100644 index 0000000000..2a3969cd76 --- /dev/null +++ b/.changeset/cli-conversion-notice-one-source.md @@ -0,0 +1,72 @@ +--- +"@objectstack/cli": patch +--- + +refactor(cli): the ADR-0087 conversion notice has one source, and a gate that holds it (#13743) + +**No output change.** The sentence `os build`, `os validate` and `os lint` +print for an ADR-0087 D2 conversion is byte-for-byte what it was. What changed +is that there is now exactly one copy of it, and a guard that keeps it that +way. + +## Why a changeset at all + +`@objectstack/cli` publishes `dist`, which is compiled from the edited `src`, +so this diff changes the published package even though it changes nothing an +author can observe. It is graded `patch` rather than skipped: nothing is added +to the package's public export surface (`src/utils/format.ts` is internal — +the package exports only `.` and `./console`), no CLI flag, payload key or +exit code moves, and no wording moves. + +## What was duplicated + +The human face of a conversion notice was written out three times, verbatim: + +``` +packages/cli/src/commands/compile.ts printWarning(`…`) +packages/cli/src/commands/lint.ts printWarning(`…`) +packages/cli/src/commands/validate.ts warnings.push(`…`) +``` + +Measured on the branch point: one distinct template literal across the three, +124 bytes each. They were held equal by convention alone. The parity guard in +`packages/cli/test/validate-build-gate-parity.test.ts` asserts that each +command **passes** an `onConversionNotice` sink to `normalizeStackInput` — it +never asserted they **say the same thing** once they have one, so a reword in +one command diverged from the other two with every gate green. + +That matters more than ordinary duplication because the sentence is close to a +contract: a conversion rewrites the old shape and asks the author for nothing, +so this notice is the only warning they get before the conversion retires from +the load path and their metadata stops loading. An author who runs two of the +three commands over one tree is meant to be told the same thing in the same +words. + +## What changed + +`formatConversionNotice(notice)` in `src/utils/format.ts` is now the single +source of the sentence, and all three commands render through it. + +It is a **formatter, not a printer**, and that is what makes one +implementation possible. The three call sites are genuinely not +interchangeable in what they DO with the string — `os build` and `os lint` +hand it to `printWarning` behind `!flags.json`, while `os validate` pushes it +into the `warnings` list that `--strict` then judges — but they were identical +in what they SAY. The whole difference lives in the disposition of the +returned string, so it costs the function no parameter. + +The parity guard gains the rule it could not see: every authoring command +renders through the one formatter, and none spells the sentence out inline — +with a positive control, so it cannot go green on a CLI that says nothing at +all. `src/utils/format.conversion-notice.test.ts` pins the rendered sentence +itself. + +## What is deliberately NOT unified + +`ConversionNotice.message` (built in `packages/spec/src/conversions/apply.ts`) +and the `defineStack:` warning (`packages/spec/src/stack.zod.ts`) are two +further renderings of the same fields, in different registers and for +different audiences. Neither is touched here, and neither can read this +function — `@objectstack/cli` depends on `@objectstack/spec`, not the reverse. +Whether all of them should descend from one source is a separate question, +filed separately.