diff --git a/.changeset/doctor-organizations-hint-pinned.md b/.changeset/doctor-organizations-hint-pinned.md new file mode 100644 index 0000000000..94a89c0021 --- /dev/null +++ b/.changeset/doctor-organizations-hint-pinned.md @@ -0,0 +1,37 @@ +--- +"@objectstack/cli": patch +--- + +chore(cli): pin the multi-org runtime name `os doctor` prints, so a roster rename cannot drift it silently (#12464) + +`doctor.ts`'s `TENANCY_POSTURE_FIX_HINTS` repeated `serve`'s `isolated` posture +sentence verbatim, carrying its own bare `@objectstack/organizations` literal +under no check at all. PR #12463 single-sourced and pinned every operator-facing +occurrence in `serve.ts`; this copy was outside that card's file surface and was +correctly left alone. The defect it left behind is the same class: a roster-key +rename would leave `os doctor` naming a package that boot no longer resolves, +**with every gate green** — the roster pin only ever sees the declaration, and +nothing read this hint table's text. + +The `isolated` hint now interpolates a module-level `ORGANIZATIONS_RUNTIME_PKG` +in `doctor.ts`, and a new sibling test pins it on two legs: the **rendered** +bullet compared whitespace-included against text built from that declaration +(the #12463 shape — asserting what the operator sees, never that a constant +appears in source), and a **roster** leg asserting that declaration IS a key of +the spec-owned `PLATFORM_PLUGIN_WIRED_RUNTIMES`. The roster leg is the load-bearing +one: without it the hint and its expectation move together under a rename and +nothing goes red. + +**This deliberately does not single-source the spelling, and the constant's +docblock says so at the site.** The literal is still declared three times (the +roster key, `Serve.ORGANIZATIONS_RUNTIME_PKG`, and now this const). The roster +cannot supply the name — it is keyed BY package name, its row type carries no +`package` field by design, and its own header records that it is "not a +resolution registry" — and importing `serve`'s export into a diagnostic command +would be a worse coupling than the duplication it removes. What changes is that +this copy can no longer drift in silence. The duplication ends properly when a +shared tenancy-hint table lands (tracked at #12492); the docblock carries that +deletion condition. + +**No behaviour change.** The declared value is byte-identical to the literal it +replaces and the rendered bullet is unchanged. diff --git a/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts b/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts new file mode 100644 index 0000000000..55ac5bf5d5 --- /dev/null +++ b/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts @@ -0,0 +1,196 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os doctor`'s posture advice names the multi-org runtime from ONE + * declaration, and these pins read what it RENDERS (#12464). + * + * ── What this closes ───────────────────────────────────────────────────── + * + * PR #12463 (#12151) single-sourced every operator-facing occurrence in + * `serve.ts` onto `Serve.ORGANIZATIONS_RUNTIME_PKG` and pinned what those + * messages render. `doctor.ts` sat outside that card's file surface and kept + * its own bare `@objectstack/organizations` literal inside + * `TENANCY_POSTURE_FIX_HINTS`, under no check of any kind — so a roster-key + * rename left `os doctor` printing a package name that boot no longer + * resolves, with EVERY GATE GREEN: the roster pin only ever sees the + * declaration, and nothing read this hint table's text. + * + * The defect being closed is that SILENT DRIFT, not the duplication as such. + * The literal is still declared three times (the roster key, serve's static, + * doctor's const) and this file does not change that — see the const's own + * docblock for why the roster cannot supply the name, and for the deletion + * condition that ends the duplication properly. + * + * ── Two legs, and the second is the point ──────────────────────────────── + * + * (i) RENDERED — the `isolated` bullet is rendered through the real gate + * and compared, whitespace included, against text built from the + * declaration. + * (ii) ROSTER — that declaration IS a key of the spec-owned + * `PLATFORM_PLUGIN_WIRED_RUNTIMES`. + * + * ⭐ (ii) is the entire difference between a CHECKED duplicate and a third + * SILENT copy. Leg (i) on its own pins doctor against itself: rename the + * roster key and the hint and the expectation move together, so (i) stays + * green forever while `os doctor` names a package that no longer exists. + * (ii) is what makes that rename loud. Neither leg is optional. + * + * ── Why it reads the RENDERED text, not the source ─────────────────────── + * + * "No bare literal outside the declaration" is the tempting stronger form and + * is deliberately NOT built here, for the reason PR #12463 recorded: a source + * scan has to exclude COMMENTS — the const's docblock names the package + * repeatedly, and this file's own header does too — and that shape is easy to + * get wrong. What reaches the operator is what matters, so the pin renders the + * finding and compares the affected LINE with `toBe`, which is what turns + * "eyeballed" into "checked". + */ + +import { describe, it, expect, beforeEach, beforeAll, afterEach, afterAll } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { PLATFORM_PLUGIN_WIRED_RUNTIMES } from '@objectstack/spec/kernel'; + +import { + ORGANIZATIONS_RUNTIME_PKG, + resolveTenancyPostureOrFinding, + readDotenvFiles, + type DotenvReading, +} from './doctor.js'; + +/** The one declaration. Every expectation below is built from THIS, never from a literal. */ +const PKG = ORGANIZATIONS_RUNTIME_PKG; + +/** + * `chalk` may or may not emit SGR codes depending on TTY detection. + * + * The escape is written as `\x1b`, never as the byte itself: one raw control + * character makes grep treat the whole file as binary, and a test file nobody's + * `git grep` can find is a test file that stops being maintained (#4890/#5157). + */ +const SGR = /\x1b\[[0-9;]*m/g; +const plain = (s: string) => s.replace(SGR, ''); +const lines = (s: string) => plain(s).split('\n'); + +const TOUCHED = ['OS_TENANCY_POSTURE', 'OS_MULTI_ORG_ENABLED'] as const; +let saved: Record = {}; + +/** + * A real reading of a real directory holding no `.env*` file. + * + * Built by the REAL `readDotenvFiles()` rather than hand-rolled, matching + * `doctor-tenancy-posture-report.test.ts`: a fake reading here would let these + * cases keep passing if the real one started reporting files that do not exist. + */ +let emptyDir: string; +let shellOnly: DotenvReading; + +beforeAll(() => { + emptyDir = fs.mkdtempSync(path.join(os.tmpdir(), 'os-doctor-12464-noenv-')); + shellOnly = readDotenvFiles(emptyDir, 'production'); + expect(shellOnly.files).toEqual([]); +}); + +afterAll(() => { + fs.rmSync(emptyDir, { recursive: true, force: true }); +}); + +beforeEach(() => { + saved = Object.fromEntries(TOUCHED.map((k) => [k, process.env[k]])); + for (const k of TOUCHED) delete process.env[k]; +}); + +afterEach(() => { + for (const k of TOUCHED) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } +}); + +/** The `isolated` fix-list bullet, rendered through the real gate. */ +const renderIsolatedBullet = (): string => { + process.env.OS_TENANCY_POSTURE = 'not-a-posture'; + const reading = resolveTenancyPostureOrFinding(shellOnly); + expect(reading.ok, 'the gate accepted a value that is not a posture').toBe(false); + if (reading.ok) throw new Error('unreachable — guarded above'); + const bullet = lines(reading.result.fix ?? '').find((l) => l.includes('OS_TENANCY_POSTURE=isolated')); + expect(bullet, "the fix list no longer offers an `isolated` bullet at all").toBeDefined(); + return bullet as string; +}; + +describe('doctor — the posture description an operator reads names the declaration (#12464)', () => { + // LEG (i). Rendered through `resolveTenancyPostureOrFinding` rather than by + // reading the hint table: the bullet's assembly (indent, `• OS_TENANCY_POSTURE=`, + // the ` — ` separator) is part of what the operator sees, so it is part of + // what is pinned. + it('leg (i) — the `isolated` fix-list bullet names it, with the spacing intact', () => { + expect(renderIsolatedBullet()).toBe( + ` • OS_TENANCY_POSTURE=isolated — organization wall + the enterprise ${PKG} runtime ` + + "(the legacy spelling 'multi' is accepted and normalizes to this)", + ); + }); + + // LEG (ii) — the half that makes leg (i) mean something. Without this, a + // roster rename moves the hint and this file's expectation together and + // nothing anywhere goes red. + it('leg (ii) — the name doctor prints IS a key of the spec-owned roster, not a third unchecked copy', () => { + expect( + Object.keys(PLATFORM_PLUGIN_WIRED_RUNTIMES), + `os doctor tells operators about '${PKG}', which PLATFORM_PLUGIN_WIRED_RUNTIMES does not declare. ` + + 'The roster is the single source for whether an out-of-repo @objectstack/* package is real and ' + + 'where it ships from (#10921); a command that prints a package name at operators must name a row in it.', + ).toContain(PKG); + + // Provenance, read through doctor's own spelling rather than a literal: the + // row this advice describes is the enterprise one. + const row = PLATFORM_PLUGIN_WIRED_RUNTIMES[PKG]; + expect(row.edition, `edition drift for the runtime doctor names ('${PKG}')`).toBe('enterprise'); + }); + + it('no posture bullet an operator reads names any OTHER scoped package', () => { + // The sweep the excluded source-scan form was reaching for, done over the + // rendering instead — where comments cannot reach and no exclusion list is + // needed. + process.env.OS_TENANCY_POSTURE = 'not-a-posture'; + const reading = resolveTenancyPostureOrFinding(shellOnly); + expect(reading.ok).toBe(false); + if (reading.ok) return; + const names = plain(reading.result.fix ?? '').match(/@[a-z0-9-]+\/[a-z0-9._-]+/g) ?? []; + expect(names.length, 'the fix list names no package at all — it stopped telling operators which one') + .toBeGreaterThan(0); + for (const name of names) { + expect(name, `the fix list names '${name}', which is not the runtime serve loads`).toBe(PKG); + } + }); +}); + +describe('#12464 CONTROL — these pins can say no', () => { + it('a lost space around the interpolation fails the rendered comparison', () => { + // If this instrument could not tell the two apart, leg (i) would be + // decorative. Anchored on the exact regression interpolation invites. + const expected = + ` • OS_TENANCY_POSTURE=isolated — organization wall + the enterprise ${PKG} runtime ` + + "(the legacy spelling 'multi' is accepted and normalizes to this)"; + expect( + ` • OS_TENANCY_POSTURE=isolated — organization wall + the enterprise ${PKG} runtime` + + "(the legacy spelling 'multi' is accepted and normalizes to this)", + ).not.toBe(expected); + expect( + ` • OS_TENANCY_POSTURE=isolated — organization wall + the enterprise ${PKG} runtime ` + + "(the legacy spelling 'multi' is accepted and normalizes to this)", + ).not.toBe(expected); + // …and says yes to the real thing, so the two `not.toBe`s above are a + // reading rather than a pair of vacuous truths. + expect(renderIsolatedBullet()).toBe(expected); + }); + + it('the roster key check rejects a name the roster does not declare', () => { + // Anchored on a term that is NOT a substring of the one under test: a + // membership assertion is not a reading until the same instrument answers no. + const keys = Object.keys(PLATFORM_PLUGIN_WIRED_RUNTIMES); + expect(keys).not.toContain('@objectstack/legacy-orgs'); + expect(keys).not.toContain('@acme/thing'); + expect(keys).toContain(PKG); + }); +}); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 7026b08032..6ea7de7bd5 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -522,6 +522,54 @@ export function environmentSourcesCheck( // ─── Tenancy Posture ──────────────────────────────────────────────── +/** + * The `plugins[]`-wired multi-org runtime this command NAMES in its posture + * advice, spelled once so the sentence below cannot drift in silence (#12464). + * + * ⚠️ This is the THIRD declaration of this literal, and that is an accepted + * cost rather than an oversight. The roster KEY in `PLATFORM_PLUGIN_WIRED_RUNTIMES` + * is one; `Serve.ORGANIZATIONS_RUNTIME_PKG` (`serve.ts`, #11614) is another. + * What this const buys is NOT fewer copies — it is that this copy can no longer + * drift unnoticed: `doctor-organizations-message-spelling.test.ts` pins the + * RENDERED bullet against this value and pins this value as a roster key, so a + * roster rename turns a test red instead of leaving `os doctor` printing a + * package name that boot no longer resolves with every gate green. Three + * declarations is structurally worse than two; a duplicate that can drift + * silently and one that cannot are different things. + * + * ── Why this does not read the name from the spec roster ───────────────── + * + * Because the roster cannot supply it. `PLATFORM_PLUGIN_WIRED_RUNTIMES` is + * keyed BY package name, and its row type `PlatformPluginWiredRuntime` carries + * no `package` field — deliberately: *"here the package name is the KEY, so it + * cannot be `null` and cannot drift from a duplicate field"*. Its own header + * settles the rest: *"What this roster deliberately is NOT: a resolution + * registry … the rows record that fact as prose provenance, they do not encode + * it as a lookup."* Both rows are `edition: 'enterprise'`, so nothing + * machine-readable selects this one. The roster VALIDATES a name you already + * hold; it does not hand you one — which is why the pin reads it as a KEY + * CHECK, the only first-class read it actually offers. + * + * ⛔ Do NOT replace this with an import of `Serve.ORGANIZATIONS_RUNTIME_PKG`: a + * diagnostic command taking a dependency on a `serve` command's export, in + * order to spell a package name, is a worse coupling than the duplication it + * removes. + * + * ── Deletion condition ─────────────────────────────────────────────────── + * + * This const goes away the day a shared tenancy-hint table lands (tracked at + * #12492). The whole `TENANCY_POSTURE_FIX_HINTS` table below is duplicated + * between here and `serve.ts` — `single` and `group` byte-identical too, and + * those two touch no roster, so nothing could ever notice them drift. One + * shared table single-sources all three sentences AND this package name at + * once. This is a step toward that, not the end state. + * + * Exported for the same reason `serve` exposes its own as a static: one + * declaration, two readers — the sentence and the pin. A pin that read a copy + * of this value instead of this value would be pinning the test against itself. + */ +export const ORGANIZATIONS_RUNTIME_PKG = '@objectstack/organizations'; + /** * One-line descriptions of the accepted postures, keyed by the vocabulary * `@objectstack/spec/security` owns. A posture declared there but not described @@ -532,7 +580,7 @@ const TENANCY_POSTURE_FIX_HINTS: Readonly> = { single: 'one organization, no organization wall — the default', group: 'organization wall enforced by the open engine, one shared database', isolated: - 'organization wall + the enterprise @objectstack/organizations runtime ' + `organization wall + the enterprise ${ORGANIZATIONS_RUNTIME_PKG} runtime ` + "(the legacy spelling 'multi' is accepted and normalizes to this)", };