From b744d1520092a1d3afc71ca284b307d824b301f7 Mon Sep 17 00:00:00 2001 From: os-justin Date: Tue, 1 Sep 2026 15:03:33 +0000 Subject: [PATCH 1/3] fix(cli): read the upgrade advisory off manifest.engines.protocol (#13860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retire `specVersion` from the stack config's CLI vocabulary. The three commands that print the migration-guide advisory (validate / doctor / compile) read `manifest.specVersion`, a key `ManifestSchema` does not declare and silently drops, so the advisory has been dead for stack configs its whole life. It now reads `manifest.engines.protocol` — the declared, parsed and boot-enforced axis — and delegates the range verdict to `checkProtocolCompat`, the platform's single reader of that axis, rather than opening a second opinion on the grammar. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015YPiiDdw96RGS25WLctCQP --- .../cli-protocol-version-gap-advisory.md | 56 ++++++ packages/cli/package.json | 1 + packages/cli/src/commands/compile.ts | 18 +- packages/cli/src/commands/doctor.ts | 18 +- packages/cli/src/commands/validate.ts | 22 ++- .../src/utils/protocol-version-gap.test.ts | 161 ++++++++++++++++ .../cli/src/utils/protocol-version-gap.ts | 182 ++++++++++++++++++ packages/cli/src/utils/spec-version.test.ts | 52 ----- packages/cli/src/utils/spec-version.ts | 101 ---------- pnpm-lock.yaml | 3 + 10 files changed, 436 insertions(+), 178 deletions(-) create mode 100644 .changeset/cli-protocol-version-gap-advisory.md create mode 100644 packages/cli/src/utils/protocol-version-gap.test.ts create mode 100644 packages/cli/src/utils/protocol-version-gap.ts delete mode 100644 packages/cli/src/utils/spec-version.test.ts delete mode 100644 packages/cli/src/utils/spec-version.ts diff --git a/.changeset/cli-protocol-version-gap-advisory.md b/.changeset/cli-protocol-version-gap-advisory.md new file mode 100644 index 0000000000..1f56c4c48f --- /dev/null +++ b/.changeset/cli-protocol-version-gap-advisory.md @@ -0,0 +1,56 @@ +--- +"@objectstack/cli": minor +--- + +fix(cli): the upgrade advisory reads `manifest.engines.protocol`, the axis that is actually declared (#13860) + +`os validate`, `os doctor` and `os compile` all print a non-blocking advisory pointing +at the per-major migration guide when the installed platform has moved ahead of the app. +All three read it off `manifest.specVersion` — a key `ManifestSchema` does not declare. +`ManifestSchema` is not `.strict()`, so an author who wrote `specVersion` had it accepted +and dropped with nothing said, and the advisory could therefore only ever fire for a +manifest carrying a key the schema does not offer. It was dead for stack configs for its +whole life: the "breaking-change guidance the author should read before proceeding" that +its own header describes was never once delivered. + +The advisory now reads `manifest.engines.protocol` — declared by `PluginEnginesSchema`, +stamped by every scaffold and example (`engines: { protocol: '^17' }`), and already +enforced at boot by the ADR-0087 handshake. One declared version axis instead of one +declared axis and one phantom. + +`specVersion` is retired from the stack config's CLI vocabulary. It keeps its meaning on +the marketplace **template** manifest (`objectstack.manifest.json`, `cloud/TemplateManifest`), +which is a different surface and is untouched — the name means one thing in one place and +nothing in the other, which is the status quo stated honestly rather than a new debt. + +## The verdict comes from the platform's own handshake + +The range is judged by `checkProtocolCompat` from `@objectstack/metadata-core` rather +than by a leading-integer parse of the CLI's own. That module is the single reader of +this axis — it owns the source priority (`engines.protocol` → `engines.platform` → +legacy `engine.objectstack`) and the range grammar — and its header already records why: +two readers with two priority orders would be the "two opinions" defect. A private parse +would have been the third, and it would disagree exactly where it matters: `>=15 <18` +targets 15 but *admits* 17, so a naive reading advises an upgrade against a range that +already covers the installed platform. Delegating means the advisory fires precisely when +boot would refuse the app, which is what makes it guidance rather than noise. The +advisory names the key it actually read, so an author is never told to bump a key they +did not write. + +Comparing a protocol range against the `@objectstack/spec` resolved from the app's +`node_modules` is sound because `PROTOCOL_VERSION` is held in lockstep with that +package's major (`protocol-version.test.ts` fails on drift), which is also what keeps +the `docs/releases/v` link correct. + +## What changes for you + +Nothing is removed or renamed on a published surface, and no command's accept set or exit +status moves: the advisory is print-only, `os validate` keeps it outside `--strict` on +both faces, and `os doctor` never exited on warnings. The `--json` payload key stays +`specVersionGap` with its value shape unchanged. + +What does change is that the advisory now **fires**. An app whose `engines.protocol` is +behind the installed platform will start seeing the migration-guide pointer from all three +commands, and `os doctor` will summarise that run as "functional but has some warnings" +rather than "healthy". That is the check finally doing its job; if it speaks up, the drift +it names was already there. diff --git a/packages/cli/package.json b/packages/cli/package.json index 5f3233e4d4..b6c8166d07 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -66,6 +66,7 @@ "@objectstack/lint": "workspace:*", "@objectstack/mcp": "workspace:*", "@objectstack/metadata": "workspace:*", + "@objectstack/metadata-core": "workspace:*", "@objectstack/metadata-protocol": "workspace:*", "@objectstack/objectql": "workspace:^", "@objectstack/observability": "workspace:^", diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index b843fb6563..6ea30e7e2e 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -41,7 +41,7 @@ import { isExitSignal, errorCodeFields, } from '../utils/format.js'; -import { checkSpecVersionGap } from '../utils/spec-version.js'; +import { checkProtocolVersionGap } from '../utils/protocol-version-gap.js'; export default class Compile extends Command { static override description = 'Compile ObjectStack configuration to JSON artifact'; @@ -599,9 +599,9 @@ export default class Compile extends Command { const sizeKB = (jsonContent.length / 1024).toFixed(1); const stats = collectMetadataStats(config); - // Spec-version drift advisory (non-blocking): installed platform newer - // than the app declares → point at the migration guide. - const specGap = checkSpecVersionGap((config as { manifest?: { specVersion?: unknown } }).manifest); + // Protocol drift advisory (non-blocking): installed platform outside the + // app's declared `engines.protocol` range → point at the migration guide. + const protocolGap = checkProtocolVersionGap((config as { manifest?: unknown }).manifest); if (flags.json) { await emitJson({ @@ -680,7 +680,9 @@ export default class Compile extends Command { // Same key `os validate --json` uses, so a CI consumer reads one shape // from either command rather than learning two. conversions: conversionNotices, - specVersionGap: specGap, + // Published key name kept; the axis behind it moved to + // `manifest.engines.protocol` (#13860). See validate.ts. + specVersionGap: protocolGap, stats, duration: timer.elapsed(), }, 0, { compact: true }); @@ -711,10 +713,10 @@ export default class Compile extends Command { `${path.join(path.dirname(output), runtimeBundle.outputFileName)} ${chalk.dim(`(${runtimeKB} KB, ${lowering.count} handler${lowering.count === 1 ? '' : 's'})`)}`, ); } - if (specGap) { + if (protocolGap) { console.log(''); - console.log(chalk.yellow(` ⚠ ${specGap.message}`)); - console.log(chalk.dim(` → ${specGap.hint}`)); + console.log(chalk.yellow(` ⚠ ${protocolGap.message}`)); + console.log(chalk.dim(` → ${protocolGap.hint}`)); } console.log(''); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 3517c2d228..ebe5d66bea 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -9,7 +9,7 @@ import path from 'path'; import { normalizeStackInput } from '@objectstack/spec'; import { printHeader, printSuccess, printWarning, printError, printStep, printInfo } from '../utils/format.js'; import { loadConfig, configExists } from '../utils/config.js'; -import { checkSpecVersionGap } from '../utils/spec-version.js'; +import { checkProtocolVersionGap } from '../utils/protocol-version-gap.js'; // #5644 — "the optional package is not installed" and "it is installed and // will not load" are two facts, and one `catch` around `import()` cannot tell // them apart. That classification lives in one place, with the measurements @@ -1927,7 +1927,7 @@ export default class Doctor extends Command { // Here the honest report is no row at all. An application consumes // `@objectstack/spec` from `node_modules`, where "built" is not a state it // can be in — that dependency is covered by the `Dependencies` row above - // and by `checkSpecVersionGap()`. Inside the monorepo nothing changes: the + // and by `checkProtocolVersionGap()`. Inside the monorepo nothing changes: the // workspace is present, and an unbuilt `dist/` is still the real warning // it always was. const specWorkspaceDir = path.join(cwd, 'packages/spec'); @@ -2097,15 +2097,15 @@ export default class Doctor extends Command { const { config: rawConfig } = await withDotenvOverlayAsync(dotenvReading, () => loadConfig()); const config: any = normalizeStackInput(rawConfig as Record); - // Spec-version drift: installed platform newer than the app declares. - printStep('Checking platform spec version...'); - const specGap = checkSpecVersionGap(config.manifest); - if (specGap) { + // Protocol drift: installed platform outside the range the app declares. + printStep('Checking platform protocol version...'); + const protocolGap = checkProtocolVersionGap(config.manifest); + if (protocolGap) { hasWarnings = true; - printWarning(`Platform spec ${specGap.message}`); - console.log(chalk.dim(` → ${specGap.hint}`)); + printWarning(`Platform protocol ${protocolGap.message}`); + console.log(chalk.dim(` → ${protocolGap.hint}`)); } else { - printSuccess('Platform spec Declared specVersion is current with the installed platform'); + printSuccess('Platform protocol Declared engines.protocol covers the installed platform'); } // Circular dependency detection diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 690ab8202f..9bc527a104 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -34,7 +34,7 @@ import { isExitSignal, errorCodeFields, } from '../utils/format.js'; -import { checkSpecVersionGap } from '../utils/spec-version.js'; +import { checkProtocolVersionGap } from '../utils/protocol-version-gap.js'; export default class Validate extends Command { static override description = @@ -344,9 +344,10 @@ export default class Validate extends Command { // 4. Collect and display stats const stats = collectMetadataStats(config); - // Spec-version drift advisory (non-blocking): if the installed platform - // is a newer major than the app declares, point at the migration guide. - const specGap = checkSpecVersionGap(config.manifest); + // Protocol drift advisory (non-blocking): if the installed platform is a + // newer major than the app's declared `engines.protocol` range admits, + // point at the migration guide. + const protocolGap = checkProtocolVersionGap(config.manifest); // 4b. Structural advisories (non-blocking) — computed HERE, above the // `if (flags.json)` branch, for exactly the reason `unknownKeyWarnings` @@ -443,7 +444,12 @@ export default class Validate extends Command { // this one. warnings: warningsSoFar(), conversions: conversionNotices, - specVersionGap: specGap, + // The payload key keeps its published name. The AXIS it reports + // moved from the undeclared `manifest.specVersion` to + // `manifest.engines.protocol` (#13860), but this is a machine face + // with pinned consumers, and renaming it is a break nobody asked + // for. Its value shape is unchanged. + specVersionGap: protocolGap, duration: timer.elapsed(), }, // `--strict` means one thing — "treat warnings as errors" — and it now @@ -507,10 +513,10 @@ export default class Validate extends Command { } // Non-blocking upgrade advisory — never gated by --strict. - if (specGap) { + if (protocolGap) { console.log(''); - console.log(chalk.yellow(` ⚠ ${specGap.message}`)); - console.log(chalk.dim(` → ${specGap.hint}`)); + console.log(chalk.yellow(` ⚠ ${protocolGap.message}`)); + console.log(chalk.dim(` → ${protocolGap.hint}`)); } console.log(''); diff --git a/packages/cli/src/utils/protocol-version-gap.test.ts b/packages/cli/src/utils/protocol-version-gap.test.ts new file mode 100644 index 0000000000..84dc45d0ed --- /dev/null +++ b/packages/cli/src/utils/protocol-version-gap.test.ts @@ -0,0 +1,161 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { ManifestSchema } from '@objectstack/spec/kernel'; +import { checkProtocolVersionGap } from './protocol-version-gap.js'; + +/** A minimal manifest the schema accepts, plus whatever the case is about. */ +function manifest(extra: Record): Record { + return { + id: 'com.acme.crm', + namespace: 'acme', + version: '1.0.0', + type: 'app', + name: 'Acme CRM', + ...extra, + }; +} + +describe('checkProtocolVersionGap', () => { + // ── The reachability this advisory never had ─────────────────────────── + // + // Before the axis moved, the advisory read `manifest.specVersion` — a key + // `ManifestSchema` does not declare, on a schema that is not `.strict()`. + // It was therefore DEAD for stack configs: it could only fire for a manifest + // carrying a key the schema does not offer. These two cases are the pins for + // that: the axis it reads now survives the schema parse, and the axis it + // retired does not. Assert them THROUGH `ManifestSchema.parse` rather than on + // a hand-built literal — a literal would prove the function works on input + // the platform never produces, which is exactly the state being repaired. + + it('FIRES for a gap on a manifest the schema actually declares', () => { + const parsed = ManifestSchema.parse(manifest({ engines: { protocol: '^16' } })); + expect(parsed.engines?.protocol, 'engines.protocol must survive the parse').toBe('^16'); + + const gap = checkProtocolVersionGap(parsed, '17.2.0'); + expect(gap, 'a declared, schema-visible range behind the platform must advise').not.toBeNull(); + expect(gap!.declaredMajor).toBe(16); + expect(gap!.installedMajor).toBe(17); + expect(gap!.installedVersion).toBe('17.2.0'); + expect(gap!.url).toBe('https://objectstack.ai/docs/releases/v17'); + expect(gap!.message).toContain("engines.protocol '^16'"); + expect(gap!.hint).toContain('https://objectstack.ai/docs/releases/v17'); + }); + + it('is silent for the RETIRED `specVersion` key, which the schema drops', () => { + // The pin the old suite spent on `checkSpecVersionGap({ specVersion: + // '^12.0.0' }, '14.7.0')`, re-aimed at the contract that replaced it. + // `ManifestSchema` is not `.strict()`, so the key is accepted and dropped + // with nothing said — the silence that made the old advisory unreachable. + const parsed = ManifestSchema.parse(manifest({ specVersion: '^12.0.0' })) as Record; + expect(parsed.specVersion, 'ManifestSchema does not declare specVersion').toBeUndefined(); + expect(checkProtocolVersionGap({ specVersion: '^12.0.0' }, '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap(parsed, '17.2.0')).toBeNull(); + }); + + it('is silent when the declared range covers the installed platform', () => { + // The shape every scaffold and example stamps today. + const parsed = ManifestSchema.parse(manifest({ engines: { protocol: '^17' } })); + expect(checkProtocolVersionGap(parsed, '17.2.0')).toBeNull(); + }); + + // ── Direction, target and guide ──────────────────────────────────────── + + it('points at the guide for the INSTALLED major, not the declared one', () => { + // Two-major jump (15 → 17): the guide must be v17, the version on disk. + const gap = checkProtocolVersionGap({ engines: { protocol: '^15' } }, '17.0.0'); + expect(gap!.url).toBe('https://objectstack.ai/docs/releases/v17'); + expect(gap!.declaredMajor).toBe(15); + }); + + it('is silent when the app targets a NEWER major (stale install, out of scope)', () => { + expect(checkProtocolVersionGap({ engines: { protocol: '^18' } }, '17.2.0')).toBeNull(); + }); + + it('is silent when no compatibility range is declared', () => { + expect(checkProtocolVersionGap(manifest({}), '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap({ engines: {} }, '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap({}, '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap(undefined, '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap(null, '17.2.0')).toBeNull(); + }); + + it('is silent when the installed version cannot be resolved', () => { + expect(checkProtocolVersionGap({ engines: { protocol: '^15' } }, null)).toBeNull(); + }); + + // ── Range grammar is the handshake's, not a second opinion ───────────── + + it('advises across the range spellings that pin a single older major', () => { + for (const range of ['^15', '^15.0.0', '~15.3.0', '15.x', '15.0.0']) { + const gap = checkProtocolVersionGap({ engines: { protocol: range } }, '17.0.0'); + expect(gap, range).not.toBeNull(); + expect(gap!.declaredMajor, range).toBe(15); + } + }); + + it('stays silent on a multi-major range that ADMITS the installed platform', () => { + // The case a leading-integer parse of our own would get wrong: `>=15 <18` + // targets 15 but explicitly covers 17, so there is nothing to advise. The + // verdict comes from `checkProtocolCompat`, the same reader the boot + // handshake uses — the advisory cannot disagree with what the loader does. + expect(checkProtocolVersionGap({ engines: { protocol: '>=15 <18' } }, '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap({ engines: { protocol: '15 - 17' } }, '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap({ engines: { protocol: '*' } }, '17.2.0')).toBeNull(); + }); + + it('advises on a bounded range that EXCLUDES the installed platform', () => { + const gap = checkProtocolVersionGap({ engines: { protocol: '>=15 <17' } }, '17.2.0'); + expect(gap).not.toBeNull(); + expect(gap!.declaredMajor).toBe(15); + expect(gap!.installedMajor).toBe(17); + }); + + it('is silent on a range shape the handshake cannot parse', () => { + // `unparsed-range` is admitted at load with a warning, never refused, so + // the advisory must not speak where the loader does not. + expect(checkProtocolVersionGap({ engines: { protocol: 'not-a-range' } }, '17.2.0')).toBeNull(); + }); + + // ── Source priority, named honestly ──────────────────────────────────── + + it('falls back to engines.platform and names the key it actually read', () => { + const gap = checkProtocolVersionGap({ engines: { platform: '^15' } }, '17.2.0'); + expect(gap).not.toBeNull(); + expect(gap!.message).toContain("engines.platform '^15'"); + expect(gap!.hint).toContain('bumping engines.platform'); + }); + + it('falls back to the legacy engine.objectstack and names it', () => { + const gap = checkProtocolVersionGap({ engine: { objectstack: '^15.0.0' } }, '17.2.0'); + expect(gap).not.toBeNull(); + expect(gap!.message).toContain("engine.objectstack '^15.0.0'"); + }); + + it('prefers engines.protocol over the other two sources', () => { + const gap = checkProtocolVersionGap( + { engines: { protocol: '^15', platform: '^9' }, engine: { objectstack: '^3' } }, + '17.2.0', + ); + expect(gap!.declaredMajor).toBe(15); + expect(gap!.message).toContain('engines.protocol'); + }); + + // ── The advisory must never be what fails a command ──────────────────── + + it('tolerates a non-string range without throwing', () => { + // `doctor` hands over an unvalidated `normalizeStackInput` result, so this + // reaches the util as raw JSON. `resolveDeclaredRange` calls `.trim()`; an + // advisory that throws here would turn a print-only check into a command + // failure. + expect(() => checkProtocolVersionGap({ engines: { protocol: 17 } }, '17.2.0')).not.toThrow(); + expect(checkProtocolVersionGap({ engines: { protocol: 17 } }, '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap({ engines: null }, '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap({ engines: 'nope' }, '17.2.0')).toBeNull(); + expect(checkProtocolVersionGap('not an object', '17.2.0')).toBeNull(); + }); + + it('raises no advisory when the installed version is unreadable', () => { + expect(checkProtocolVersionGap({ engines: { protocol: '^15' } }, 'not-a-version')).toBeNull(); + }); +}); diff --git a/packages/cli/src/utils/protocol-version-gap.ts b/packages/cli/src/utils/protocol-version-gap.ts new file mode 100644 index 0000000000..c8816ef6d3 --- /dev/null +++ b/packages/cli/src/utils/protocol-version-gap.ts @@ -0,0 +1,182 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { createRequire } from 'module'; + +import { checkProtocolCompat, type ProtocolHandshakeManifest } from '@objectstack/metadata-core'; + +/** + * Protocol-version drift advisory. + * + * Surfaces the one thing an AI agent (or human) upgrading a third-party app + * almost never finds on its own: the curated per-major migration guide. When + * an app's declared compatibility range EXCLUDES the `@objectstack/spec` + * actually installed in its `node_modules`, and the platform is the newer + * side, the platform moved ahead of the app and there is breaking-change + * guidance the author should read before proceeding. Every major from v12 on + * is guaranteed a `content/docs/releases/v.mdx` page (enforced by + * `scripts/check-release-notes.mjs`), so the URL below never 404s. + * + * The check is advisory-only — it never fails a build/validate/doctor. It + * exists so the release notes are discoverable at the exact moment of the + * upgrade, instead of being reverse-engineered from per-package + * `CHANGELOG.md` files. + * + * ## Which axis this reads, and why it is `engines.protocol` + * + * This advisory used to read `manifest.specVersion`. `ManifestSchema` + * (`packages/spec/src/kernel/manifest.zod.ts`) declares no such member and is + * not `.strict()`, so an authored `specVersion` was accepted and dropped with + * nothing said — which made the advisory DEAD for stack configs: it could only + * ever fire for a manifest carrying a key the schema does not offer. The axis + * is now `manifest.engines.protocol` — declared (`PluginEnginesSchema`), + * stamped by every scaffold and example, and genuinely enforced at boot + * (`assertProtocolCompat`, raising `OS_PROTOCOL_INCOMPATIBLE`). `specVersion` + * is retired from the stack config's CLI vocabulary; it keeps its meaning on + * the unrelated marketplace TEMPLATE manifest + * (`packages/spec/src/cloud/template-manifest.zod.ts`), which is a different + * surface and is untouched. + * + * ## Why the range is judged by `checkProtocolCompat` and not re-parsed here + * + * `@objectstack/metadata-core`'s handshake is the platform's single reader of + * that range: it owns the source priority (`engines.protocol` → + * `engines.platform` → legacy `engine.objectstack`) and the range grammar. Its + * own header records why `resolveDeclaredRange` was exported — "two readers of + * `engines.protocol` with two priority orders would be the 'two opinions' + * defect". A leading-integer parse of our own would be the third such opinion, + * and it would disagree in exactly the cases that matter: `'>=15 <18'` targets + * 15 but ADMITS 17, so a naive parse would advise an upgrade against a range + * that already covers the installed platform. Delegating means the advisory + * fires precisely when boot would refuse the app — which is what makes it + * "guidance to read before proceeding" rather than noise. + * + * ## Why the installed *package* version is a sound runtime version to compare + * + * `PROTOCOL_VERSION` is held in lockstep with the `@objectstack/spec` package + * major (`packages/spec/src/kernel/protocol-version.test.ts` fails on drift), + * so the protocol major and the installed package major are the same integer. + * Comparing against the version resolved from the APP's `node_modules` — not + * the CLI's compiled-in constant — is deliberate: a globally linked CLI must + * still report the platform the app actually installed. It also keeps the + * `docs/releases/v` URL correct, since release majors are package + * majors. + */ + +const RELEASES_BASE = 'https://objectstack.ai/docs/releases'; + +export interface ProtocolVersionGap { + /** Major of the `@objectstack/spec` resolved from the app's node_modules. */ + installedMajor: number; + /** Major the app's declared compatibility range targets. */ + declaredMajor: number; + /** Full installed spec version (e.g. `17.2.0`). */ + installedVersion: string; + /** Canonical migration guide for the installed major. */ + url: string; + /** Ready-to-print one-line advisory. */ + message: string; + /** Ready-to-print follow-up pointing at the guide. */ + hint: string; +} + +/** Resolve the installed `@objectstack/spec` version from the app being operated on. */ +function resolveInstalledSpecVersion(): string | null { + try { + // Resolve relative to the CWD (the app), not the CLI install, so a globally + // linked CLI still reports the app's locked spec version. Fall back to the + // CLI's own resolution if the app doesn't hoist spec to its root. + const requireFromApp = createRequire(`${process.cwd()}/package.json`); + const pkg = requireFromApp('@objectstack/spec/package.json') as { version?: string }; + if (typeof pkg.version === 'string') return pkg.version; + } catch { + // ignore — try the CLI-relative resolution below + } + try { + const requireFromCli = createRequire(import.meta.url); + const pkg = requireFromCli('@objectstack/spec/package.json') as { version?: string }; + if (typeof pkg.version === 'string') return pkg.version; + } catch { + // ignore — spec not resolvable, no advisory + } + return null; +} + +/** A field of the manifest slice, only if it is actually a string. */ +function str(value: unknown): string | undefined { + return typeof value === 'string' ? value : undefined; +} + +/** + * Narrow an unvalidated config manifest to the slice the handshake reads. + * + * The commands that call this hand over a manifest that has NOT necessarily + * been through `ManifestSchema` yet (`doctor` runs on `normalizeStackInput` + * output typed `any`), so `engines.protocol` can be any JSON value at this + * point. `resolveDeclaredRange` calls `.trim()` on it — a non-string would + * throw, and an advisory that can throw is an advisory that changes what the + * command REJECTS. Narrowing to `string | undefined` here keeps the failure + * mode at "no advisory". This narrows types only; the range itself is still + * interpreted exclusively by the handshake. + */ +function toHandshakeManifest(manifest: unknown): ProtocolHandshakeManifest | null { + if (!manifest || typeof manifest !== 'object') return null; + const m = manifest as { id?: unknown; engines?: unknown; engine?: unknown }; + const engines = m.engines && typeof m.engines === 'object' ? (m.engines as Record) : undefined; + const engine = m.engine && typeof m.engine === 'object' ? (m.engine as Record) : undefined; + return { + id: str(m.id), + engines: engines ? { protocol: str(engines.protocol), platform: str(engines.platform) } : undefined, + engine: engine ? { objectstack: str(engine.objectstack) } : undefined, + }; +} + +/** + * Compute a protocol-version drift advisory for the given app manifest, or + * `null` when there is nothing to say: spec unresolvable, no compatibility + * range declared, a range shape the handshake does not recognize, a range that + * already admits the installed platform, or a range that targets a NEWER major + * than the platform on disk (a stale/mismatched install — a different problem, + * out of scope for release-note discoverability). + */ +export function checkProtocolVersionGap( + manifest: unknown, + /** Injectable for tests; defaults to the spec resolved from the app on disk. */ + installedVersion: string | null = resolveInstalledSpecVersion(), +): ProtocolVersionGap | null { + if (!installedVersion) return null; + const slice = toHandshakeManifest(manifest); + if (!slice) return null; + + // The platform's own handshake decides compatibility. Anything other than a + // positive incompatibility — `ok`, `no-range`, `unparsed-range` — is a case + // the loader admits, so the advisory stays silent rather than second-guessing + // it. + const result = checkProtocolCompat(slice, installedVersion); + if (result.status !== 'incompatible') return null; + + const declaredMajor = result.diagnostic.targetMajor; + const installedMajor = result.runtimeMajor; + if (declaredMajor === null) return null; + // Only the upgrade case: the platform on disk is newer than the app targets. + // This guard also covers an unreadable `installedVersion`, for which the + // handshake reports `runtimeMajor: 0` — no non-negative declared major can be + // below it, so a garbled version can never raise a false advisory. + if (declaredMajor >= installedMajor) return null; + + const url = `${RELEASES_BASE}/v${installedMajor}`; + // Name the key the range was actually read from. The handshake falls back to + // `engines.platform` and legacy `engine.objectstack`, and an advisory that + // told an author to bump `engines.protocol` when it had read `engine` would + // be sending them to a key they never wrote. + const source = result.source; + return { + installedMajor, + declaredMajor, + installedVersion, + url, + message: + `Installed @objectstack/spec is v${installedVersion} but this app declares ` + + `${source} '${result.requiredRange}', which targets protocol v${declaredMajor}.`, + hint: `Review the v${installedMajor} migration guide before bumping ${source}: ${url}`, + }; +} diff --git a/packages/cli/src/utils/spec-version.test.ts b/packages/cli/src/utils/spec-version.test.ts deleted file mode 100644 index 568716b03d..0000000000 --- a/packages/cli/src/utils/spec-version.test.ts +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. - -import { describe, it, expect } from 'vitest'; -import { checkSpecVersionGap } from './spec-version.js'; - -describe('checkSpecVersionGap', () => { - it('flags an app declaring an older major than the installed platform', () => { - const gap = checkSpecVersionGap({ specVersion: '^12.0.0' }, '14.7.0'); - expect(gap).not.toBeNull(); - expect(gap!.declaredMajor).toBe(12); - expect(gap!.installedMajor).toBe(14); - expect(gap!.installedVersion).toBe('14.7.0'); - expect(gap!.url).toBe('https://objectstack.ai/docs/releases/v14'); - expect(gap!.hint).toContain('https://objectstack.ai/docs/releases/v14'); - }); - - it('points at the guide for the INSTALLED major, not the declared one', () => { - // Two-major jump (12 → 14): the guide must be v14, the version on disk. - const gap = checkSpecVersionGap({ specVersion: '^12.0.0' }, '14.0.0'); - expect(gap!.url).toBe('https://objectstack.ai/docs/releases/v14'); - }); - - it('is silent when declared major matches the installed platform', () => { - expect(checkSpecVersionGap({ specVersion: '^14.0.0' }, '14.7.0')).toBeNull(); - }); - - it('is silent when the app declares a NEWER major (stale install, out of scope)', () => { - expect(checkSpecVersionGap({ specVersion: '^15.0.0' }, '14.7.0')).toBeNull(); - }); - - it('is silent when no specVersion is declared', () => { - expect(checkSpecVersionGap({}, '14.7.0')).toBeNull(); - expect(checkSpecVersionGap(undefined, '14.7.0')).toBeNull(); - expect(checkSpecVersionGap(null, '14.7.0')).toBeNull(); - }); - - it('is silent when the installed version cannot be resolved', () => { - expect(checkSpecVersionGap({ specVersion: '^12.0.0' }, null)).toBeNull(); - }); - - it('parses the major out of assorted range spellings', () => { - for (const range of ['^12.0.0', '>=12', '12.x', '~12.3.0', '12 || 13']) { - const gap = checkSpecVersionGap({ specVersion: range }, '14.0.0'); - expect(gap, range).not.toBeNull(); - expect(gap!.declaredMajor, range).toBe(12); - } - }); - - it('ignores a non-string specVersion', () => { - expect(checkSpecVersionGap({ specVersion: 12 as unknown as string }, '14.0.0')).toBeNull(); - }); -}); diff --git a/packages/cli/src/utils/spec-version.ts b/packages/cli/src/utils/spec-version.ts deleted file mode 100644 index edbef44003..0000000000 --- a/packages/cli/src/utils/spec-version.ts +++ /dev/null @@ -1,101 +0,0 @@ -// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. - -import { createRequire } from 'module'; - -/** - * Spec-version drift advisory. - * - * Surfaces the one thing an AI agent (or human) upgrading a third-party app - * almost never finds on its own: the curated per-major migration guide. When - * an app's authored `manifest.specVersion` declares an OLDER major than the - * `@objectstack/spec` actually installed in its `node_modules`, the platform - * moved ahead of the app and there is breaking-change guidance the author - * should read before proceeding. Every major from v12 on is guaranteed a - * `content/docs/releases/v.mdx` page (enforced by - * `scripts/check-release-notes.mjs`), so the URL below never 404s. - * - * The check is advisory-only — it never fails a build/validate. It exists so - * the release notes are discoverable at the exact moment of the upgrade, - * instead of being reverse-engineered from per-package `CHANGELOG.md` files. - */ - -const RELEASES_BASE = 'https://objectstack.ai/docs/releases'; - -export interface SpecVersionGap { - /** Major of the `@objectstack/spec` resolved from the app's node_modules. */ - installedMajor: number; - /** Major declared by the app's `manifest.specVersion` range. */ - declaredMajor: number; - /** Full installed spec version (e.g. `14.7.0`). */ - installedVersion: string; - /** Canonical migration guide for the installed major. */ - url: string; - /** Ready-to-print one-line advisory. */ - message: string; - /** Ready-to-print follow-up pointing at the guide. */ - hint: string; -} - -/** Parse the leading major integer out of a semver range like `^12.0.0`, `>=13`, `14.x`. */ -function parseMajor(range: unknown): number | null { - if (typeof range !== 'string') return null; - const m = range.match(/\d+/); - if (!m) return null; - const major = Number.parseInt(m[0], 10); - return Number.isFinite(major) ? major : null; -} - -/** Resolve the installed `@objectstack/spec` version from the app being operated on. */ -function resolveInstalledSpecVersion(): string | null { - try { - // Resolve relative to the CWD (the app), not the CLI install, so a globally - // linked CLI still reports the app's locked spec version. Fall back to the - // CLI's own resolution if the app doesn't hoist spec to its root. - const requireFromApp = createRequire(`${process.cwd()}/package.json`); - const pkg = requireFromApp('@objectstack/spec/package.json') as { version?: string }; - if (typeof pkg.version === 'string') return pkg.version; - } catch { - // ignore — try the CLI-relative resolution below - } - try { - const requireFromCli = createRequire(import.meta.url); - const pkg = requireFromCli('@objectstack/spec/package.json') as { version?: string }; - if (typeof pkg.version === 'string') return pkg.version; - } catch { - // ignore — spec not resolvable, no advisory - } - return null; -} - -/** - * Compute a spec-version drift advisory for the given app manifest, or `null` - * when there is nothing to say (spec unresolvable, no `specVersion` declared, - * or the declared major already matches / leads the installed platform). - */ -export function checkSpecVersionGap( - manifest: { specVersion?: unknown } | undefined | null, - /** Injectable for tests; defaults to the spec resolved from the app on disk. */ - installedVersion: string | null = resolveInstalledSpecVersion(), -): SpecVersionGap | null { - const declaredMajor = parseMajor(manifest?.specVersion); - if (declaredMajor == null) return null; - - if (!installedVersion) return null; - const installedMajor = parseMajor(installedVersion); - if (installedMajor == null) return null; - - // Only the upgrade case: the platform on disk is newer than what the app - // declares. (declaredMajor > installedMajor is a stale/mismatched install — - // a different problem, out of scope for release-note discoverability.) - if (declaredMajor >= installedMajor) return null; - - const url = `${RELEASES_BASE}/v${installedMajor}`; - return { - installedMajor, - declaredMajor, - installedVersion, - url, - message: `Installed @objectstack/spec is v${installedVersion} but this app declares specVersion for v${declaredMajor}.`, - hint: `Review the v${installedMajor} migration guide before bumping specVersion: ${url}`, - }; -} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b4bf90a7b5..3a146440b1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -448,6 +448,9 @@ importers: '@objectstack/metadata': specifier: workspace:* version: link:../metadata + '@objectstack/metadata-core': + specifier: workspace:* + version: link:../metadata-core '@objectstack/metadata-protocol': specifier: workspace:* version: link:../metadata-protocol From ff98f1885cb7eff0752b85a44af88f28709a749d Mon Sep 17 00:00:00 2001 From: os-justin Date: Tue, 1 Sep 2026 15:52:15 +0000 Subject: [PATCH 2/3] test(cli): resolve @objectstack/metadata-core from source in vitest (#13860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check:test-source-alias flagged the new advisory test as importing the protocol handshake through `exports` -> dist. The advisory is a thin direction check over `checkProtocolCompat`, so without the alias the range grammar it pins would be whatever was last compiled — green against a stale artifact with nothing in the output saying so. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015YPiiDdw96RGS25WLctCQP --- packages/cli/vitest.config.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 07d443b7be..3d4e6f8929 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -528,6 +528,18 @@ export default defineConfig({ find: /^@objectstack\/plugin-auth$/, replacement: path.resolve(__dirname, '../plugins/plugin-auth/src/index.ts'), }, + // `src/utils/protocol-version-gap.test.ts` (#13860) exercises the upgrade + // advisory, whose verdict comes from `checkProtocolCompat` — the platform's + // single reader of `engines.protocol`. The advisory is a thin direction + // check over that handshake, so a test resolving the handshake through + // `exports` to metadata-core's **dist** would be a verdict about build + // state: the range grammar it actually pins would be whatever was last + // compiled, and the dangerous half is not an error but a green run against + // a stale artifact. + { + find: /^@objectstack\/metadata-core$/, + replacement: path.resolve(__dirname, '../metadata-core/src/index.ts'), + }, ], }, test: { From fb3643cf32d187e3bbdb04b4a8df42b75a74a441 Mon Sep 17 00:00:00 2001 From: os-justin Date: Tue, 1 Sep 2026 17:28:52 +0000 Subject: [PATCH 3/3] fix(cli): re-aim doctor's label consumers at the renamed protocol row (#13860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renaming doctor's row from `Platform spec` to `Platform protocol` left two consumers behind, and only a repo-wide grep for the old string found them — the suites the rename touched were the wrong population to search. - `doctor-config-env-overlay.test.ts` pinned the old label as a LIVENESS proxy ("the config checks did not merely stop failing — they RAN"), not as a label check. Re-aimed at the new label, intent preserved: the row prints on either branch, so its presence still proves the check executed whatever it concluded. A comment now records that these strings are row labels that must track `doctor.ts` through a rename rather than be relaxed. - `configLoadFailureCheck`'s operator-visible sentence enumerated the skipped config-aware checks as "spec version", naming a row that no longer exists. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015YPiiDdw96RGS25WLctCQP --- .changeset/cli-protocol-version-gap-advisory.md | 5 +++++ .../cli/src/commands/doctor-config-env-overlay.test.ts | 9 ++++++++- packages/cli/src/commands/doctor.ts | 4 ++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.changeset/cli-protocol-version-gap-advisory.md b/.changeset/cli-protocol-version-gap-advisory.md index 1f56c4c48f..d92344aca6 100644 --- a/.changeset/cli-protocol-version-gap-advisory.md +++ b/.changeset/cli-protocol-version-gap-advisory.md @@ -49,6 +49,11 @@ status moves: the advisory is print-only, `os validate` keeps it outside `--stri both faces, and `os doctor` never exited on warnings. The `--json` payload key stays `specVersionGap` with its value shape unchanged. +`os doctor`'s row is renamed with the axis, from `Platform spec` to `Platform protocol`, and the +sentence it prints when a config cannot be loaded — the one enumerating which config-aware checks +were skipped — names the row by its new name too, so an operator reading either is pointed at a +row that exists. + What does change is that the advisory now **fires**. An app whose `engines.protocol` is behind the installed platform will start seeing the migration-guide pointer from all three commands, and `os doctor` will summarise that run as "functional but has some warnings" diff --git a/packages/cli/src/commands/doctor-config-env-overlay.test.ts b/packages/cli/src/commands/doctor-config-env-overlay.test.ts index 36febe192b..1fbf8b9e02 100644 --- a/packages/cli/src/commands/doctor-config-env-overlay.test.ts +++ b/packages/cli/src/commands/doctor-config-env-overlay.test.ts @@ -289,7 +289,14 @@ describe('os doctor, end to end, against a config that reads .env at top level', // `.env` before bundling, boots this exact directory. expect(run.out).not.toContain('Could not load config for analysis'); // The config checks did not merely stop failing — they RAN. - expect(run.out).toContain('Platform spec'); + // Both strings below are ROW LABELS used as liveness evidence, not verdicts: + // the row is printed on either branch, so its presence proves the check + // executed whatever it concluded. That is why they must track `doctor.ts`'s + // labels through a rename rather than be relaxed — `Platform spec` became + // `Platform protocol` when the advisory moved onto `engines.protocol` + // (#13860), and this assertion is the one consumer that lived outside the + // suites that rename touched. + expect(run.out).toContain('Platform protocol'); expect(run.out).toContain('No circular references detected'); expect(run.exitCode).toBeUndefined(); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index ebe5d66bea..57caef8987 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -1434,8 +1434,8 @@ export function configLoadFailureCheck(err: unknown): HealthCheckResult { '`os serve` loads this same file the same way — bundle-require, under the `.env*`\n' + ' cascade named above (#5397) — and prints this error in full, so a config that\n' + ' lands here is one the server cannot boot either.\n' - + ' The config-aware checks were SKIPPED, not passed: spec version, circular\n' - + ' dependencies, unused objects, orphan views, dashboard integrity.\n' + + ' The config-aware checks were SKIPPED, not passed: platform protocol,\n' + + ' circular dependencies, unused objects, orphan views, dashboard integrity.\n' + ` cause: ${indentUnderGutter(cause)}`, }; }