From f0c77829b11b647bad7bcd65190b153cae8929b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 07:50:53 +0000 Subject: [PATCH 1/2] refactor(cli): single-source the tenancy posture hint table shared by serve and doctor (#12492) --- ...tor-organizations-message-spelling.test.ts | 91 ++++++++++++--- packages/cli/src/commands/doctor.ts | 74 +++---------- ...rve-organizations-message-spelling.test.ts | 84 +++++++++++++- packages/cli/src/commands/serve.ts | 34 +++--- .../cli/src/utils/tenancy-posture-hints.ts | 104 ++++++++++++++++++ 5 files changed, 296 insertions(+), 91 deletions(-) create mode 100644 packages/cli/src/utils/tenancy-posture-hints.ts diff --git a/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts b/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts index 55ac5bf5d5..55f55529aa 100644 --- a/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts +++ b/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts @@ -16,18 +16,26 @@ * 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 ──────────────────────────────── + * ── Retargeted by #12492, not rewritten ────────────────────────────────── * - * (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`. + * That deletion condition has since been met. `doctor.ts` no longer declares + * `ORGANIZATIONS_RUNTIME_PKG` or its own hint table: both moved to + * `../utils/tenancy-posture-hints.ts`, which `os serve` reads too, so the + * literal is now declared twice (the roster key and that module) instead of + * three times. These pins moved with the declaration — ⛔ none of them was + * dropped, because what they measure did not change: what `os doctor` RENDERS. + * Leg (ii) especially, which is the load-bearing one (see below). + * + * ── Three 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`. + * (iii) SHARED TABLE (#12492) — every posture bullet, `single` and `group` + * included, renders the shared table's entry verbatim. * * ⭐ (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 @@ -35,6 +43,14 @@ * green forever while `os doctor` names a package that no longer exists. * (ii) is what makes that rename loud. Neither leg is optional. * + * (iii) covers the half neither of the other two can reach. `single` and + * `group` carry no package literal, so no roster leg is possible for them and + * nothing ever watched them — that is the defect #12492 filed. Leg (iii) does + * not check the PROSE (a reword moves the shared table and this expectation + * together, and leg (i)'s hard-coded text is what reddens then); it checks that + * doctor renders THE SHARED TABLE. Re-grow a module-local copy in `doctor.ts` — + * exactly the state this card found — and it goes red on every posture at once. + * * ── Why it reads the RENDERED text, not the source ─────────────────────── * * "No bare literal outside the declaration" is the tempting stronger form and @@ -51,9 +67,16 @@ 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 { TENANCY_POSTURES } from '@objectstack/spec/security'; +// The declaration and the table both live here since #12492 — `os serve` reads +// the same module, which is what makes leg (iii) below a reading of the SHARING +// rather than of one command talking to itself. import { ORGANIZATIONS_RUNTIME_PKG, + TENANCY_POSTURE_FIX_HINTS, +} from '../utils/tenancy-posture-hints.js'; +import { resolveTenancyPostureOrFinding, readDotenvFiles, type DotenvReading, @@ -108,17 +131,20 @@ afterEach(() => { } }); -/** The `isolated` fix-list bullet, rendered through the real gate. */ -const renderIsolatedBullet = (): string => { +/** One posture's fix-list bullet, rendered through the real gate. */ +const renderPostureBullet = (posture: string): 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(); + const bullet = lines(reading.result.fix ?? '').find((l) => l.includes(`OS_TENANCY_POSTURE=${posture}`)); + expect(bullet, `the fix list no longer offers a '${posture}' bullet at all`).toBeDefined(); return bullet as string; }; +/** The `isolated` fix-list bullet, rendered through the real gate. */ +const renderIsolatedBullet = (): string => renderPostureBullet('isolated'); + 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=`, @@ -148,6 +174,28 @@ describe('doctor — the posture description an operator reads names the declara expect(row.edition, `edition drift for the runtime doctor names ('${PKG}')`).toBe('enterprise'); }); + // LEG (iii) — the half legs (i) and (ii) cannot reach (#12492). `single` and + // `group` carry no package literal, so no roster leg is possible for them; + // before this card nothing anywhere read their text at either command, and a + // reword of one command's copy drifted from the other in total silence. What + // closes that is not a pin on the PROSE — it is this: the bullets an operator + // reads here are assembled from the SHARED table, the same one `os serve` + // renders. A module-local hint table re-grown in `doctor.ts` reddens this. + it('leg (iii) — every posture bullet renders the SHARED hint table verbatim, `single` and `group` included', () => { + for (const posture of TENANCY_POSTURES) { + const hint = TENANCY_POSTURE_FIX_HINTS[posture]; + expect(renderPostureBullet(posture)).toBe( + ` • OS_TENANCY_POSTURE=${posture}${hint ? ` — ${hint}` : ''}`, + ); + } + // …and the sweep above actually swept. A posture vocabulary that went empty + // would satisfy every assertion inside the loop without reading anything — + // the two entries this card is ABOUT are named explicitly for that reason. + expect(TENANCY_POSTURES).toContain('single'); + expect(TENANCY_POSTURES).toContain('group'); + expect(TENANCY_POSTURES).toContain('isolated'); + }); + 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 @@ -185,6 +233,21 @@ describe('#12464 CONTROL — these pins can say no', () => { expect(renderIsolatedBullet()).toBe(expected); }); + it('the shared-table comparison rejects a bullet whose hint was reworded (#12492)', () => { + // If leg (iii) could not tell a reworded hint from the shared one it would + // be decorative. Anchored on `group` — one of the two entries that had + // nothing watching them at all before this card, and deliberately NOT a + // substring game: 'closed engine' is a different claim, not a truncation. + const real = ` • OS_TENANCY_POSTURE=group — ${TENANCY_POSTURE_FIX_HINTS.group}`; + expect( + ' • OS_TENANCY_POSTURE=group — organization wall enforced by the closed engine, one shared database', + ).not.toBe(real); + expect(' • OS_TENANCY_POSTURE=group').not.toBe(real); + // …and says yes to the real thing, so the two `not.toBe`s are a reading + // rather than a pair of vacuous truths. + expect(renderPostureBullet('group')).toBe(real); + }); + 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. diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 6ea7de7bd5..3517c2d228 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -15,6 +15,7 @@ import { checkSpecVersionGap } from '../utils/spec-version.js'; // them apart. That classification lives in one place, with the measurements // behind it written down there. import { loadOptionalPackage } from '../utils/optional-package.js'; +import { TENANCY_POSTURE_FIX_HINTS } from '../utils/tenancy-posture-hints.js'; import { validateWidgetBindings } from '@objectstack/lint'; import { resolveTenancyPosture, @@ -523,66 +524,21 @@ 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. + * The posture prose below comes from `../utils/tenancy-posture-hints.ts`, which + * `os serve` reads too (#12492). + * + * #12464 added a module-local `ORGANIZATIONS_RUNTIME_PKG` here and a + * `TENANCY_POSTURE_FIX_HINTS` table that repeated `serve`'s, byte for byte. That + * const's docblock named THIS card as its deletion condition, so it is gone: the + * declaration moved to the shared module and doctor reads it from there. ⛔ Not + * from `serve.ts` — a diagnostic command depending on a `serve` command's export + * in order to spell a package name is the coupling that docblock ruled out, and + * a neutral utility both commands sit above is not that. + * + * Doctor keeps its OWN bullet assembly (indent, `• OS_TENANCY_POSTURE=`, the + * ` — ` separator): serve renders the same hints at a different indent inside a + * FATAL refusal, and only the TABLE was ever duplicated. */ -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 - * here is still listed by the fix list (bare, without prose) rather than - * silently dropped — the advice can go terse, never stale. - */ -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 ${ORGANIZATIONS_RUNTIME_PKG} runtime ` - + "(the legacy spelling 'multi' is accepted and normalizes to this)", -}; /** * What doctor's tenancy-posture read decided (#5382). diff --git a/packages/cli/src/commands/serve-organizations-message-spelling.test.ts b/packages/cli/src/commands/serve-organizations-message-spelling.test.ts index 802832abdd..a7de2ba73e 100644 --- a/packages/cli/src/commands/serve-organizations-message-spelling.test.ts +++ b/packages/cli/src/commands/serve-organizations-message-spelling.test.ts @@ -33,11 +33,31 @@ * from a literal on purpose. Rename the roster key and these keep passing — * the prose moved with it. Re-spell it inline in any of these messages and the * rendered line stops matching the declaration, which is the exact drift this - * file exists to catch. + * file exists to catch. (The roster leg that makes that safe is not here: it is + * `test/serve-capability-vocabulary.test.ts`, which pins the same static as a + * roster KEY. Both halves are required; they simply live in two files.) + * + * ── Widened by #12492, not rewritten ───────────────────────────────────── + * + * `serve.ts` and `doctor.ts` each carried a byte-identical `TENANCY_POSTURE_FIX_HINTS` + * table. It now lives once, in `../utils/tenancy-posture-hints.ts`, and + * `Serve.ORGANIZATIONS_RUNTIME_PKG` re-exports that module's declaration rather + * than declaring the literal itself — so every pin above still reads ONE + * declaration, through the handle `serve` exposes. ⛔ Nothing here was dropped. + * + * Site 7 is new and is the reading that makes the sharing REAL rather than + * textual: it asserts that the bullets `serve` renders come from that shared + * table, `single` and `group` included — the two entries that touch no roster + * and that, before #12492, nothing at either command ever read. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import type { HostDeclaration } from '@objectstack/types/node'; +import { TENANCY_POSTURES } from '@objectstack/spec/security'; + +// The table `doctor` renders too (#12492). Read here so site 7 measures the +// SHARING, not this command talking to itself. +import { TENANCY_POSTURE_FIX_HINTS } from '../utils/tenancy-posture-hints.js'; import Serve, { formatDegradedTenancyWarning, @@ -47,7 +67,15 @@ import Serve, { resolveTenancyPostureOrRefusal, } from './serve.js'; -/** The one declaration. Every expectation below is built from THIS, never from a literal. */ +/** + * The one declaration, read through the handle `serve` exposes. + * + * Since #12492 this static is a RE-EXPORT of + * `../utils/tenancy-posture-hints.ts`'s `ORGANIZATIONS_RUNTIME_PKG`, so this is + * still one declaration and not a copy of one. Reading it as `Serve.…` is + * deliberate: what these pins are about is what THIS COMMAND puts in front of an + * operator, and the static is the seam the boot path and the roster pin address. + */ const PKG = Serve.ORGANIZATIONS_RUNTIME_PKG; /** @@ -176,6 +204,32 @@ describe('serve — the posture description an operator reads names the declarat + "(the legacy spelling 'multi' is accepted and normalizes to this)", ); }); + + // #12492. Site 6 pins the `isolated` PROSE, hard-coded, and that is what + // reddens on a reword. This pins something site 6 cannot: that the bullets + // come from the table `os doctor` renders too — `single` and `group` + // included, the two entries no roster touches and nothing anywhere read + // before this card. Re-grow a module-local table in `serve.ts` and this goes + // red on every posture at once. + it('site 7 — every posture bullet renders the SHARED hint table verbatim, `single` and `group` included', () => { + process.env.OS_TENANCY_POSTURE = 'not-a-posture'; + const verdict = resolveTenancyPostureOrRefusal(); + expect(verdict.ok, 'the gate accepted a value that is not a posture').toBe(false); + if (verdict.ok) return; + const rendered = lines(verdict.fatal); + for (const posture of TENANCY_POSTURES) { + const hint = TENANCY_POSTURE_FIX_HINTS[posture]; + expect(rendered).toContain( + ` • set OS_TENANCY_POSTURE=${posture}${hint ? ` — ${hint}` : ''}`, + ); + } + // …and the sweep above actually swept. An empty posture vocabulary would + // satisfy every assertion inside the loop without reading a thing — the two + // entries this card is ABOUT are named explicitly for that reason. + expect(TENANCY_POSTURES).toContain('single'); + expect(TENANCY_POSTURES).toContain('group'); + expect(TENANCY_POSTURES).toContain('isolated'); + }); }); describe('#12151 CONTROL — these pins can say no', () => { @@ -189,6 +243,32 @@ describe('#12151 CONTROL — these pins can say no', () => { expect(lines(remedyUndeclared())[0]).toBe(expected); }); + it('the shared-table comparison rejects a bullet whose hint was reworded (#12492)', () => { + // If site 7 could not tell a reworded hint from the shared one it would be + // decorative. Anchored on `group`, one of the two entries that had nothing + // watching them before this card, and against a term that is NOT a + // substring of the one under test: 'closed engine' is a different claim. + const real = ` • set OS_TENANCY_POSTURE=group — ${TENANCY_POSTURE_FIX_HINTS.group}`; + expect( + ' • set OS_TENANCY_POSTURE=group — organization wall enforced by the closed engine, one shared database', + ).not.toBe(real); + expect(' • set OS_TENANCY_POSTURE=group').not.toBe(real); + // …and says yes to the real thing — rendered through the real gate, not + // restated. `expect(X).toBe(X)` here would be a vacuous truth dressed as a + // control, which is the failure mode a control exists to rule out. + const saved = process.env.OS_TENANCY_POSTURE; + try { + process.env.OS_TENANCY_POSTURE = 'not-a-posture'; + const verdict = resolveTenancyPostureOrRefusal(); + expect(verdict.ok).toBe(false); + if (verdict.ok) return; + expect(lines(verdict.fatal)).toContain(real); + } finally { + if (saved === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = saved; + } + }); + it('the scoped-name reader returns a positive on a foreign package and empty on none', () => { // Anchored on a term that is NOT a substring of the one under test: a // zero-hit sweep is not a reading until the same instrument answers yes. diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 3b93962e6b..98f8ccd40e 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -52,6 +52,12 @@ import { LOG_LEVELS, resolveLogLevel, readLogLevelEnv } from '../utils/log-level import { BootLogCapture, isVerboseBootLevel } from '../utils/boot-log-capture.js'; import { graftAuthoredRuntimeMembers, isAppPluginLike } from '../utils/graft-runtime-hooks.js'; import { redactConnectionUrl, describeDriverConnection } from '../utils/connection-display.js'; +// The posture prose `os serve` and `os doctor` BOTH print, declared once (#12492). +// Aliased on the way in only so the static re-export below reads unambiguously. +import { + ORGANIZATIONS_RUNTIME_PKG as SHARED_ORGANIZATIONS_RUNTIME_PKG, + TENANCY_POSTURE_FIX_HINTS, +} from '../utils/tenancy-posture-hints.js'; // Shared with @objectstack/verify and the dogfood multi-org probes (#4700) — // node-only, hence the `/node` subpath rather than the edge-safe root export. import { @@ -495,7 +501,7 @@ export default class Serve extends Command { /** * The `plugins[]`-wired multi-org runtime this command loads when the - * resolved tenancy posture is walled (ADR-0105 D12) — declared ONCE, here. + * resolved tenancy posture is walled (ADR-0105 D12). * * `serve` is the only runtime that prints this package name AT OPERATORS (the * install remedy, the fatal refusal, the degraded-boot warning), and the @@ -514,11 +520,21 @@ export default class Serve extends Command { * Exposed as a static for the same reason ALWAYS_ON_CAPABILITIES above is: * one declaration, two readers — the boot path and the pin. * + * ⚠️ Since #12492 this is a RE-EXPORT, not the declaration. The literal now + * lives in `../utils/tenancy-posture-hints.ts`, beside the posture prose that + * interpolates it, because `os doctor` prints that same name and needed a + * source it could read WITHOUT depending on a `serve` command's export. It is + * kept as a static for the same reason ALWAYS_ON_CAPABILITIES above is kept as + * one: `Serve.ORGANIZATIONS_RUNTIME_PKG` is the stable handle the boot path + * and two sibling pins (`serve-capability-vocabulary.test.ts`, + * `serve-cluster-host-resolution.test.ts`) already address, and a re-export + * keeps every one of those readings pointed at one declaration. + * * This single-sources the SPELLING and nothing else. Which postures load the * runtime, and what each of the two failure stages means, stay exactly where * they are; the roster is deliberately not a resolution registry. */ - static readonly ORGANIZATIONS_RUNTIME_PKG = '@objectstack/organizations'; + static readonly ORGANIZATIONS_RUNTIME_PKG = SHARED_ORGANIZATIONS_RUNTIME_PKG; /** * Auto-registered plugin tiers. Plugins explicitly listed in @@ -4264,20 +4280,6 @@ export type TenancyPostureGateVerdict = | { ok: true; posture: TenancyPosture } | { ok: false; fatal: string }; -/** - * One-line prescriptions for the accepted postures, keyed by the vocabulary - * `@objectstack/spec/security` owns. A posture added there but not described - * here still gets listed by the gate (bare, without prose) rather than silently - * dropped from the advice — the fix list can go terse, never stale. - */ -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 ${Serve.ORGANIZATIONS_RUNTIME_PKG} runtime ` - + "(the legacy spelling 'multi' is accepted and normalizes to this)", -}; - /** * Resolve the deployment's requested tenancy posture, or produce the FATAL text * that refuses the boot (#5359). diff --git a/packages/cli/src/utils/tenancy-posture-hints.ts b/packages/cli/src/utils/tenancy-posture-hints.ts new file mode 100644 index 0000000000..592e632076 --- /dev/null +++ b/packages/cli/src/utils/tenancy-posture-hints.ts @@ -0,0 +1,104 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The one-line posture descriptions `os serve` and `os doctor` BOTH put in front + * of an operator, declared ONCE (#12492). + * + * ── What this closes ────────────────────────────────────────────────────── + * + * `serve.ts` and `doctor.ts` each carried their own `TENANCY_POSTURE_FIX_HINTS` + * table, and the two tables were byte-identical (modulo the expression that + * spells the package name) under no cross-check of any kind. `isolated` at + * least carried a package literal the spec-owned roster could be pinned against + * (#12464 / PR #12496); `single` and `group` touch no roster, so there was + * nothing anywhere that could ever have noticed those two drift apart — reword + * one command's copy and every gate stays green while the two commands describe + * the same posture differently to the same operator. + * + * ⭐ The two UNCOVERED entries were the worse half, which is why all three moved + * here together. Single-sourcing only `isolated` would have closed the half that + * was already covered and left the other two exactly as silent as before. + * + * ── Why this is CLI-INTERNAL and not `packages/spec` ───────────────────── + * + * `packages/spec` owns the posture VOCABULARY (`TENANCY_POSTURES`, + * `TenancyPosture`) and these hints are keyed by it. The prose is not part of + * that contract: it is operator advice, in the imperative voice of a terminal, + * read by exactly two CLI commands and by nothing else. That was measured + * rather than assumed before this module was placed here — no reader outside + * `packages/cli` consumes any of the three strings (the near hits are a comment + * in `plugin-auth` that happens to phrase `group` similarly, and ADR-0105's own + * different sentence about a shared database). + * + * Publishing it from `packages/spec` instead would widen public surface to buy + * nothing: it would export operator prose to every consumer of the protocol + * contract, and the drift this closes is entirely internal to two files that + * sit three directories apart. A posture declared in the spec but not described + * here is still listed by both commands — bare, without prose — rather than + * silently dropped, so the vocabulary stays the authority and this table stays + * an optional gloss on it. The advice can go terse; it can never go stale. + * + * ── What is shared here, and what deliberately is not ──────────────────── + * + * Only the TABLE. Each command keeps its own bullet assembly, because the two + * renderings genuinely differ and always did: doctor emits + * ` • OS_TENANCY_POSTURE=

— ` inside a health-check `fix`, serve + * emits ` • set OS_TENANCY_POSTURE=

— ` inside a FATAL refusal, at + * different indents. Hoisting those too would force one of the two messages to + * change shape, and this change is a refactor with byte-identical output. + * + * ⛔ Do not add rendering, chalk, or posture POLICY here. What loads the + * multi-org runtime, and what each failure stage means, stay where they are. + */ + +/** + * The `plugins[]`-wired multi-org runtime both commands NAME in their posture + * advice, spelled once (#11614 → #12464 → #12492). + * + * ⚠️ This is now the SECOND declaration of the literal, down from three, and the + * remaining duplicate is load-bearing: the other is the KEY of the spec-owned + * `PLATFORM_PLUGIN_WIRED_RUNTIMES` roster, and the roster cannot supply the + * name. It is keyed BY package name and its row type `PlatformPluginWiredRuntime` + * carries no `package` field — deliberately, so the name "cannot be `null` and + * cannot drift from a duplicate field" — and its own header records that it is + * "not a resolution registry". Both organization 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. That is why the pins over this value + * read it as a roster KEY CHECK, the only first-class read the roster offers. + * + * `Serve.ORGANIZATIONS_RUNTIME_PKG` is no longer a third declaration — it reads + * this one. It stays a static because `serve`'s boot path and two sibling pins + * (`serve-capability-vocabulary.test.ts`, `serve-cluster-host-resolution.test.ts`) + * address it there, and because a command's own resolution seam is a reasonable + * thing for that command to expose. `doctor.ts` no longer declares it at all: + * the const #12464 added carried its deletion condition in its own docblock, + * naming this card, and that condition is met here. Doctor reads this module + * rather than `serve.ts` — the coupling that const's docblock ⛔ ruled out was a + * diagnostic command depending on a `serve` command's export, not on a neutral + * utility both commands sit above. + */ +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 here is still listed by both + * commands (bare, without prose) rather than silently dropped — both call sites + * read this table with an optional-hint guard, and that is contract, not + * defensiveness. + * + * ⚠️ Reword an entry here and BOTH commands change together. That is the point, + * and it is what the rendered pins in `serve-organizations-message-spelling.test.ts` + * and `doctor-organizations-message-spelling.test.ts` measure: those two files + * hold hard-coded expectations of this prose, so a reword here reddens a pin on + * each command from one edit. If only one ever reddened, the table would be + * shared in name only. + */ +export 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 ${ORGANIZATIONS_RUNTIME_PKG} runtime ` + + "(the legacy spelling 'multi' is accepted and normalizes to this)", +}; From bbefdb008bcc8e0fed2acb605473b371f948b166 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 08:17:34 +0000 Subject: [PATCH 2/2] =?UTF-8?q?refactor(cli):=20keep=20serve's=20package?= =?UTF-8?q?=20literal=20in=20place=20=E2=80=94=20the=20host-anchoring=20sw?= =?UTF-8?q?eep=20resolves=20through=20it=20(#12492)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .changeset/shared-tenancy-posture-hints.md | 42 +++++++++++ ...tor-organizations-message-spelling.test.ts | 19 +++-- ...rve-organizations-message-spelling.test.ts | 57 +++++++++++---- packages/cli/src/commands/serve.ts | 43 ++++++----- .../cli/src/utils/tenancy-posture-hints.ts | 71 +++++++++++++------ 5 files changed, 174 insertions(+), 58 deletions(-) create mode 100644 .changeset/shared-tenancy-posture-hints.md diff --git a/.changeset/shared-tenancy-posture-hints.md b/.changeset/shared-tenancy-posture-hints.md new file mode 100644 index 0000000000..240b445977 --- /dev/null +++ b/.changeset/shared-tenancy-posture-hints.md @@ -0,0 +1,42 @@ +--- +"@objectstack/cli": patch +--- + +refactor(cli): single-source the tenancy posture hint table `os serve` and `os doctor` both print (#12492) + +`serve.ts` and `doctor.ts` each declared their own `TENANCY_POSTURE_FIX_HINTS`, +and the two tables were **byte-identical** — sha256 `97497ea8…` on both, modulo +the expression spelling the package name — under no cross-check of any kind. + +The `isolated` entry at least carried a package literal the spec-owned +`PLATFORM_PLUGIN_WIRED_RUNTIMES` roster could be pinned against (#12464 / PR +#12496). **`single` and `group` were the worse half**: they touch no roster, so +nothing anywhere could ever have noticed them drift apart. A reword of one +command's copy left the other describing the same posture differently to the +same operator, with every gate green. + +Both tables now come from one CLI-internal module, +`packages/cli/src/utils/tenancy-posture-hints.ts`, which also holds the single +`ORGANIZATIONS_RUNTIME_PKG` declaration. `Serve.ORGANIZATIONS_RUNTIME_PKG` +becomes a **re-export** of it rather than a second declaration, keeping the +stable handle `serve`'s boot path and the roster pin already address; the +module-local const #12464 added to `doctor.ts` is **deleted**, which is the +deletion condition that const's own docblock recorded against this card. The +literal is now declared twice (the roster key and this module) instead of three +times. + +Each command keeps its **own bullet assembly** — doctor renders +`• OS_TENANCY_POSTURE=

— ` inside a health-check fix list, serve renders +`• set OS_TENANCY_POSTURE=

— ` inside a FATAL refusal, at different +indents. Only the table was ever duplicated. + +The two sibling spelling pins are **retargeted, not dropped** — including the +roster leg, which is the load-bearing one (a rename of the package value leaves +the rendered leg green; only the roster leg catches it). Each gains one new leg +asserting that every posture bullet, `single` and `group` included, renders the +**shared** table's entry verbatim — so a command that re-grows a local copy goes +red instead of drifting in silence. + +**No behaviour change.** The rendered text is byte-identical before and after +for all three postures at both commands, verified by capturing both commands' +full rendered fix lists on `origin/main` and on this branch and diffing them. diff --git a/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts b/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts index 55f55529aa..aa8d0384b3 100644 --- a/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts +++ b/packages/cli/src/commands/doctor-organizations-message-spelling.test.ts @@ -21,11 +21,20 @@ * * That deletion condition has since been met. `doctor.ts` no longer declares * `ORGANIZATIONS_RUNTIME_PKG` or its own hint table: both moved to - * `../utils/tenancy-posture-hints.ts`, which `os serve` reads too, so the - * literal is now declared twice (the roster key and that module) instead of - * three times. These pins moved with the declaration — ⛔ none of them was - * dropped, because what they measure did not change: what `os doctor` RENDERS. - * Leg (ii) especially, which is the load-bearing one (see below). + * `../utils/tenancy-posture-hints.ts`, which `os serve` reads too. These pins + * moved with the declaration — ⛔ none of them was dropped, because what they + * measure did not change: what `os doctor` RENDERS. Leg (ii) especially, which + * is the load-bearing one (see below). + * + * ⚠️ The literal is still declared three times, not two: the roster key, the + * shared module this file now reads, and `Serve.ORGANIZATIONS_RUNTIME_PKG`, + * which must stay a string LITERAL in `serve.ts` or the host-anchoring sweep in + * `serve-cluster-host-resolution.test.ts` can no longer resolve which package + * that command's `import()` names. What changed is that no copy can drift in + * silence any more: the serve↔shared pair is pinned equal by site 8 of + * `serve-organizations-message-spelling.test.ts`, and each copy is separately + * pinned as a roster key. Ending the duplication needs that sweep's resolver to + * follow one more hop — a file this card does not own. * * ── Three legs, and the second is the point ────────────────────────────── * diff --git a/packages/cli/src/commands/serve-organizations-message-spelling.test.ts b/packages/cli/src/commands/serve-organizations-message-spelling.test.ts index a7de2ba73e..68b66e502d 100644 --- a/packages/cli/src/commands/serve-organizations-message-spelling.test.ts +++ b/packages/cli/src/commands/serve-organizations-message-spelling.test.ts @@ -40,15 +40,23 @@ * ── Widened by #12492, not rewritten ───────────────────────────────────── * * `serve.ts` and `doctor.ts` each carried a byte-identical `TENANCY_POSTURE_FIX_HINTS` - * table. It now lives once, in `../utils/tenancy-posture-hints.ts`, and - * `Serve.ORGANIZATIONS_RUNTIME_PKG` re-exports that module's declaration rather - * than declaring the literal itself — so every pin above still reads ONE - * declaration, through the handle `serve` exposes. ⛔ Nothing here was dropped. + * table. It now lives once, in `../utils/tenancy-posture-hints.ts`, which both + * commands read. ⛔ Nothing here was dropped; two pins were added. * - * Site 7 is new and is the reading that makes the sharing REAL rather than - * textual: it asserts that the bullets `serve` renders come from that shared - * table, `single` and `group` included — the two entries that touch no roster - * and that, before #12492, nothing at either command ever read. + * **Site 7** is the reading that makes the sharing REAL rather than textual: the + * bullets `serve` renders come from that shared table, `single` and `group` + * included — the two entries that touch no roster and that, before #12492, + * nothing at either command ever read. + * + * **Site 8** covers what the shared table could NOT absorb. + * `Serve.ORGANIZATIONS_RUNTIME_PKG` deliberately stays a string LITERAL in + * `serve.ts` (its docblock says why: `serve-cluster-host-resolution.test.ts` + * resolves the organizations `import()` through that static and needs the + * literal in that file, or the load drops out of the host-anchoring sweep + * silently). So the spelling is declared twice inside `packages/cli`, and site 8 + * is what keeps that duplication CHECKED instead of silent — it asserts the two + * declarations are equal. Each is separately pinned as a roster key, here via + * `test/serve-capability-vocabulary.test.ts` and there via doctor's leg (ii). */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; @@ -57,7 +65,10 @@ import { TENANCY_POSTURES } from '@objectstack/spec/security'; // The table `doctor` renders too (#12492). Read here so site 7 measures the // SHARING, not this command talking to itself. -import { TENANCY_POSTURE_FIX_HINTS } from '../utils/tenancy-posture-hints.js'; +import { + ORGANIZATIONS_RUNTIME_PKG as SHARED_ORGANIZATIONS_RUNTIME_PKG, + TENANCY_POSTURE_FIX_HINTS, +} from '../utils/tenancy-posture-hints.js'; import Serve, { formatDegradedTenancyWarning, @@ -70,11 +81,11 @@ import Serve, { /** * The one declaration, read through the handle `serve` exposes. * - * Since #12492 this static is a RE-EXPORT of - * `../utils/tenancy-posture-hints.ts`'s `ORGANIZATIONS_RUNTIME_PKG`, so this is - * still one declaration and not a copy of one. Reading it as `Serve.…` is - * deliberate: what these pins are about is what THIS COMMAND puts in front of an - * operator, and the static is the seam the boot path and the roster pin address. + * Reading it as `Serve.…` is deliberate: what these pins are about is what THIS + * COMMAND puts in front of an operator, and the static is the seam the boot path + * and the roster pin already address. It is a literal in `serve.ts`, not a + * re-export of the shared hints module — site 8 below is what holds the two + * equal. */ const PKG = Serve.ORGANIZATIONS_RUNTIME_PKG; @@ -230,6 +241,24 @@ describe('serve — the posture description an operator reads names the declarat expect(TENANCY_POSTURES).toContain('group'); expect(TENANCY_POSTURES).toContain('isolated'); }); + + // #12492. The one thing the shared table could not absorb: the package name is + // declared twice inside `packages/cli` on purpose — see + // `Serve.ORGANIZATIONS_RUNTIME_PKG`'s docblock, and the shared module's. This + // is the assertion that makes that duplication a CHECKED one. Drift either + // copy and `os serve` and `os doctor` start naming different packages at + // operators; without this, nothing anywhere would say so. + it('site 8 — serve\'s literal and the shared hints module declare the SAME package', () => { + expect( + PKG, + 'Serve.ORGANIZATIONS_RUNTIME_PKG and the shared tenancy-hints module disagree about the ' + + 'multi-org runtime, so `os serve` and `os doctor` now name different packages at operators', + ).toBe(SHARED_ORGANIZATIONS_RUNTIME_PKG); + + // …and the isolated hint an operator reads is built from the shared copy, so + // the equality above is load-bearing rather than a spare assertion. + expect(TENANCY_POSTURE_FIX_HINTS.isolated).toContain(SHARED_ORGANIZATIONS_RUNTIME_PKG); + }); }); describe('#12151 CONTROL — these pins can say no', () => { diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 98f8ccd40e..0cc49f8ae7 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -53,11 +53,7 @@ import { BootLogCapture, isVerboseBootLevel } from '../utils/boot-log-capture.js import { graftAuthoredRuntimeMembers, isAppPluginLike } from '../utils/graft-runtime-hooks.js'; import { redactConnectionUrl, describeDriverConnection } from '../utils/connection-display.js'; // The posture prose `os serve` and `os doctor` BOTH print, declared once (#12492). -// Aliased on the way in only so the static re-export below reads unambiguously. -import { - ORGANIZATIONS_RUNTIME_PKG as SHARED_ORGANIZATIONS_RUNTIME_PKG, - TENANCY_POSTURE_FIX_HINTS, -} from '../utils/tenancy-posture-hints.js'; +import { TENANCY_POSTURE_FIX_HINTS } from '../utils/tenancy-posture-hints.js'; // Shared with @objectstack/verify and the dogfood multi-org probes (#4700) — // node-only, hence the `/node` subpath rather than the edge-safe root export. import { @@ -501,7 +497,7 @@ export default class Serve extends Command { /** * The `plugins[]`-wired multi-org runtime this command loads when the - * resolved tenancy posture is walled (ADR-0105 D12). + * resolved tenancy posture is walled (ADR-0105 D12) — declared ONCE, here. * * `serve` is the only runtime that prints this package name AT OPERATORS (the * install remedy, the fatal refusal, the degraded-boot warning), and the @@ -520,21 +516,36 @@ export default class Serve extends Command { * Exposed as a static for the same reason ALWAYS_ON_CAPABILITIES above is: * one declaration, two readers — the boot path and the pin. * - * ⚠️ Since #12492 this is a RE-EXPORT, not the declaration. The literal now - * lives in `../utils/tenancy-posture-hints.ts`, beside the posture prose that - * interpolates it, because `os doctor` prints that same name and needed a - * source it could read WITHOUT depending on a `serve` command's export. It is - * kept as a static for the same reason ALWAYS_ON_CAPABILITIES above is kept as - * one: `Serve.ORGANIZATIONS_RUNTIME_PKG` is the stable handle the boot path - * and two sibling pins (`serve-capability-vocabulary.test.ts`, - * `serve-cluster-host-resolution.test.ts`) already address, and a re-export - * keeps every one of those readings pointed at one declaration. + * ⛔ This MUST stay a string LITERAL in this file — do not turn it into a + * re-export of `../utils/tenancy-posture-hints.ts`, however tempting that is + * now that the posture hints live there (#12492). + * + * `serve-cluster-host-resolution.test.ts` sweeps this file for every dynamic + * `import()` and proves each app-declarable one is host-anchored. To do that + * it must know WHICH package a load site names, and the organizations load is + * spelled `const organizationsPkg = Serve.ORGANIZATIONS_RUNTIME_PKG;` — so the + * sweep resolves the identifier one hop, to this static, and then reads the + * literal. Written as `= SOME_IMPORTED_CONST`, the literal is no longer in + * this file, the specifier becomes unresolvable, and that load drops OUT of + * the swept population rather than failing inside it. That is the exact + * silent-vacuity failure #11614 already paid for once; the named half of that + * sweep's vacuity guard is what catches it, and it does (measured on this + * card, which tried the re-export and was refused by it). + * + * The consequence is that the spelling is declared twice inside this package + * — here, and in the shared hints module `os doctor` also reads. That + * duplication is CHECKED, not silent: `serve-organizations-message-spelling.test.ts` + * asserts the two are equal, and each is independently pinned as a key of the + * spec-owned roster (here via `test/serve-capability-vocabulary.test.ts`, + * there via `doctor-organizations-message-spelling.test.ts`). Single-sourcing + * it properly needs that sweep's resolver to follow one more hop, into a + * sibling module — an edit to a file this card does not own. * * This single-sources the SPELLING and nothing else. Which postures load the * runtime, and what each of the two failure stages means, stay exactly where * they are; the roster is deliberately not a resolution registry. */ - static readonly ORGANIZATIONS_RUNTIME_PKG = SHARED_ORGANIZATIONS_RUNTIME_PKG; + static readonly ORGANIZATIONS_RUNTIME_PKG = '@objectstack/organizations'; /** * Auto-registered plugin tiers. Plugins explicitly listed in diff --git a/packages/cli/src/utils/tenancy-posture-hints.ts b/packages/cli/src/utils/tenancy-posture-hints.ts index 592e632076..eb4b32afee 100644 --- a/packages/cli/src/utils/tenancy-posture-hints.ts +++ b/packages/cli/src/utils/tenancy-posture-hints.ts @@ -19,6 +19,10 @@ * here together. Single-sourcing only `isolated` would have closed the half that * was already covered and left the other two exactly as silent as before. * + * What moved is the TABLE. The package name the `isolated` sentence interpolates + * is declared here too, but `serve.ts` still holds its own copy for a reason + * recorded at that const below — read it before "finishing the job". + * * ── Why this is CLI-INTERNAL and not `packages/spec` ───────────────────── * * `packages/spec` owns the posture VOCABULARY (`TENANCY_POSTURES`, @@ -53,29 +57,50 @@ /** * The `plugins[]`-wired multi-org runtime both commands NAME in their posture - * advice, spelled once (#11614 → #12464 → #12492). - * - * ⚠️ This is now the SECOND declaration of the literal, down from three, and the - * remaining duplicate is load-bearing: the other is the KEY of the spec-owned - * `PLATFORM_PLUGIN_WIRED_RUNTIMES` roster, and the roster cannot supply the - * name. It is keyed BY package name and its row type `PlatformPluginWiredRuntime` - * carries no `package` field — deliberately, so the name "cannot be `null` and - * cannot drift from a duplicate field" — and its own header records that it is - * "not a resolution registry". Both organization 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. That is why the pins over this value - * read it as a roster KEY CHECK, the only first-class read the roster offers. - * - * `Serve.ORGANIZATIONS_RUNTIME_PKG` is no longer a third declaration — it reads - * this one. It stays a static because `serve`'s boot path and two sibling pins - * (`serve-capability-vocabulary.test.ts`, `serve-cluster-host-resolution.test.ts`) - * address it there, and because a command's own resolution seam is a reasonable - * thing for that command to expose. `doctor.ts` no longer declares it at all: - * the const #12464 added carried its deletion condition in its own docblock, - * naming this card, and that condition is met here. Doctor reads this module - * rather than `serve.ts` — the coupling that const's docblock ⛔ ruled out was a - * diagnostic command depending on a `serve` command's export, not on a neutral - * utility both commands sit above. + * advice, spelled once FOR THIS TABLE (#11614 → #12464 → #12492). + * + * ⚠️ This is not the only declaration of the literal inside `packages/cli`, and + * that is a measured constraint rather than an oversight. `serve.ts` keeps its + * own `Serve.ORGANIZATIONS_RUNTIME_PKG` literal because + * `serve-cluster-host-resolution.test.ts` sweeps `serve.ts` for every dynamic + * `import()` and resolves the organizations load site through that static to a + * LITERAL IN THAT FILE. Rewrite the static as a re-export of this const and the + * specifier stops resolving, so that load drops OUT of the host-anchoring sweep + * instead of failing inside it — the silent-vacuity mode #11614 already paid + * for. This card tried exactly that and the sweep's named vacuity guard refused + * it, by name, which is the guard working. + * + * ⭐ So the spelling is declared twice in this package, and both copies are + * CHECKED rather than silent — a duplicate that can drift unnoticed and one + * that cannot are different things: + * + * · `serve-organizations-message-spelling.test.ts` asserts the two are EQUAL. + * · this one is a key of the spec-owned `PLATFORM_PLUGIN_WIRED_RUNTIMES` + * (`doctor-organizations-message-spelling.test.ts`, leg (ii)). + * · serve's is a key of the same roster (`test/serve-capability-vocabulary.test.ts`). + * + * Ending the duplication for real needs that sweep's `resolveIdentifier()` to + * follow one further hop — an import alias into a sibling module — which is an + * edit to a file #12492 does not own, and which was in flight elsewhere when + * this landed. It is a two-line change to a resolver whose own docblock records + * that resolving one hop further "strictly WIDENS what the sweep judges; it can + * never excuse a load". + * + * ⛔ Do NOT close the gap by importing `Serve.ORGANIZATIONS_RUNTIME_PKG` here + * instead: this module is read by `os doctor`, and 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 (#12464's ruling, unchanged). + * + * ── Why neither copy can 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, so the name "cannot be `null` and cannot drift + * from a duplicate field" — and its own header records that it is "not a + * resolution registry". Both organization 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. That is why every pin over this value + * reads it as a roster KEY CHECK, the only first-class read the roster offers. */ export const ORGANIZATIONS_RUNTIME_PKG = '@objectstack/organizations';