diff --git a/.changeset/cli-banner-tenancy-posture.md b/.changeset/cli-banner-tenancy-posture.md new file mode 100644 index 0000000000..38898173f1 --- /dev/null +++ b/.changeset/cli-banner-tenancy-posture.md @@ -0,0 +1,53 @@ +--- +"@objectstack/cli": patch +--- + +fix(cli): the boot banner's `Tenancy:` row now reports the resolved posture, not the superseded boolean (#4801) + +`printServerReady` printed `Tenancy: multi-tenant | single-tenant` from a boolean +`multiTenant` that `serve` filled with `resolveMultiOrgEnabled()` — i.e. from +`OS_MULTI_ORG_ENABLED`. [ADR-0105 D1] replaced that knob with +`OS_TENANCY_POSTURE`, keeping the boolean only as the fallback +`resolveTenancyPosture()` consults when the posture is unset, and **the runtime +wiring in `serve` already keys off the posture**. So the banner and the server it +describes read two different sources for one fact, and they drifted exactly where +it hurts: booting with `OS_TENANCY_POSTURE=isolated` and `OS_MULTI_ORG_ENABLED` +unset printed + +``` + Tenancy: single-tenant + Plugins: 40 loaded + …, Organizations, … +``` + +— the banner claiming single-org one line above the plugin table that proves the +organization wall is up (observed on a real boot in cloud#1020, where the lie was +only caught by hand-comparing the plugin list). + +This is not cosmetic. It is the "declared ≠ enforced" class (ADR-0049) landing on +the **diagnostic** surface, which is the worst place for it: a banner that can be +wrong costs every later investigation an extra lap proving whether it is. + +**What changes for users.** The row now prints the posture verbatim — `Tenancy: +single`, `Tenancy: group`, `Tenancy: isolated` — sourced from the same +`resolveTenancyPosture()` call the runtime wiring uses. The old `multi-tenant` / +`single-tenant` vocabulary is gone. That vocabulary was itself part of the defect: +tenancy has been a three-valued spectrum since ADR-0105, and a boolean has no +spelling for `group` at all, so a `group` deployment could only ever be +misreported. + +**The internal `multiTenant` option is removed, not deprecated.** With the posture +authoritative, a retained boolean could only ever be a field the printer ignores — +and a field that exists but cannot be believed is precisely how this bug was +authored in the first place. `ServerReadyOptions.tenancyPosture` is typed as +`TenancyPosture`, so re-wiring the banner to the legacy boolean now fails to +compile (`resolveMultiOrgEnabled()` returns `boolean`) instead of producing a +plausible-looking wrong line. The interface is package-internal — `format.ts` is +not re-exported from `@objectstack/cli`'s entry point — so no consumer code needs +a change. + +Regression-pinned in `packages/cli/src/utils/format.tenancy.test.ts`, which asserts +the printed token **is** `resolveTenancyPosture()`'s answer across the cases that +made the old code wrong: posture set with the boolean unset, posture unset with the +boolean true, both set and contradicting (either direction), the legacy `multi` +spelling, and `group`. diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 7a2ae9148a..9a2a42f32d 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -10,7 +10,11 @@ import { loadConfig, BUNDLE_REQUIRE_EXTERNALS } from '../utils/config.js'; import { mergeBootConfig } from '../utils/merge-boot-config.js'; import { isHostConfig, shouldBootWithLibrary } from '../utils/plugin-detection.js'; import { resolveDriverType, resolveStorageDefinition, UnsupportedDriverError } from '../utils/storage-driver.js'; -import { readEnvWithDeprecation, resolveMultiOrgEnabled, resolveTenancyPosture, resolveAllowDegradedTenancy, isMcpServerEnabled, stampSearchPinyinEnabled, isModuleNotFoundError } from '@objectstack/types'; +// [ADR-0105 D1] `resolveMultiOrgEnabled` is deliberately NOT imported here: the +// posture is the authoritative knob and `resolveTenancyPosture()` already folds +// the legacy boolean in as its unset-fallback. serve's last direct reader of the +// boolean was the banner, and that was exactly the drift #4801 fixed. +import { readEnvWithDeprecation, resolveTenancyPosture, resolveAllowDegradedTenancy, isMcpServerEnabled, stampSearchPinyinEnabled, isModuleNotFoundError } from '@objectstack/types'; import { PLATFORM_CAPABILITY_TOKENS, PLATFORM_ALWAYS_ON_CAPABILITIES } from '@objectstack/spec/kernel'; import { missingProviderMessage } from '../utils/capability-preflight.js'; import { resolveObjectStackHome } from '@objectstack/runtime'; @@ -2688,7 +2692,15 @@ export default class Serve extends Command { consolePath: loadedPlugins.includes('ConsoleUI') ? CONSOLE_PATH : undefined, driverLabel: resolvedDriverLabel, databaseUrl: resolvedDatabaseUrl ? redactConnectionUrl(resolvedDatabaseUrl) : undefined, - multiTenant: resolveMultiOrgEnabled(), + // [ADR-0105 D1] #4801 — the banner reads the SAME resolver the runtime + // wiring above keys off (`resolveTenancyPosture()`), not the legacy + // boolean `resolveMultiOrgEnabled()`. With `OS_TENANCY_POSTURE=isolated` + // and `OS_MULTI_ORG_ENABLED` unset, the boolean says `false` while the + // wall is up — the banner printed `single-tenant` on the same screen + // that listed `Organizations` in the plugin table (cloud#1020). A + // diagnostic surface that disagrees with the runtime costs every later + // investigation an extra lap. + tenancyPosture: resolveTenancyPosture(), seededAdmin, automation: automationSummary, seeds: seedSummary, diff --git a/packages/cli/src/utils/format.tenancy.test.ts b/packages/cli/src/utils/format.tenancy.test.ts new file mode 100644 index 0000000000..3bc7508394 --- /dev/null +++ b/packages/cli/src/utils/format.tenancy.test.ts @@ -0,0 +1,181 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { resolveTenancyPosture } from '@objectstack/types'; +import type { TenancyPosture } from '@objectstack/spec/security'; +import { printServerReady, type ServerReadyOptions } from './format.js'; + +/** + * framework#4801 — the boot banner's `Tenancy:` row. + * + * [ADR-0105 D1] `OS_TENANCY_POSTURE` is the authoritative knob; the boolean + * `OS_MULTI_ORG_ENABLED` survives only as its unset-fallback, folded in by + * `resolveTenancyPosture()`. serve wires the runtime off that resolver, but the + * banner printed a boolean sourced from `resolveMultiOrgEnabled()` — a second + * source for one fact. Booting with `OS_TENANCY_POSTURE=isolated` alone printed + * `Tenancy: single-tenant` on the same screen where the plugin table listed + * `Organizations` (observed in cloud#1020): the banner said single-org while the + * organization wall was up. + * + * So the property under test is not "the line looks right", it is **the banner + * and the resolver cannot disagree**. Every case below computes + * `resolveTenancyPosture()` from the environment and asserts the printed token + * IS that value — including the cases that made the old code wrong: a posture + * set with the boolean unset, a posture that contradicts the boolean, and + * `group`, which a boolean cannot express at all. + */ +describe('printServerReady Tenancy row (#4801, ADR-0105 D1)', () => { + const base: ServerReadyOptions = { + port: 3000, + configFile: 'objectstack.config.ts', + isDev: true, + pluginCount: 1, + }; + + const originalPosture = process.env.OS_TENANCY_POSTURE; + const originalMultiOrg = process.env.OS_MULTI_ORG_ENABLED; + + let lines: string[]; + let spy: ReturnType; + + beforeEach(() => { + lines = []; + spy = vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + // Strip SGR escapes so assertions hold whether or not chalk colors. + lines.push(args.join(' ').replace(/\u001b\[[0-9;]*m/g, '')); + }); + }); + + afterEach(() => { + spy.mockRestore(); + if (originalPosture === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = originalPosture; + if (originalMultiOrg === undefined) delete process.env.OS_MULTI_ORG_ENABLED; + else process.env.OS_MULTI_ORG_ENABLED = originalMultiOrg; + }); + + /** The single `Tenancy:` row, ANSI-stripped and trimmed. */ + const tenancyLine = (): string | undefined => { + const rows = lines.filter((l) => l.includes('Tenancy:')); + expect(rows.length).toBeLessThanOrEqual(1); + return rows[0]?.trim(); + }; + + /** The token the banner printed after `Tenancy:`. */ + const printedPosture = (): string | undefined => tenancyLine()?.replace(/^Tenancy:\s*/, ''); + + /** Set the two knobs, exactly (unset = absent, not empty). */ + const env = (posture: string | undefined, multiOrg: string | undefined) => { + if (posture === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = posture; + if (multiOrg === undefined) delete process.env.OS_MULTI_ORG_ENABLED; + else process.env.OS_MULTI_ORG_ENABLED = multiOrg; + }; + + /** + * The invariant, applied to whatever the environment currently says: boot the + * banner the way serve does — `tenancyPosture: resolveTenancyPosture()` — and + * assert the row names precisely the resolver's answer. + */ + const expectBannerAgreesWithResolver = (): TenancyPosture => { + const resolved = resolveTenancyPosture(); + printServerReady({ ...base, tenancyPosture: resolved }); + expect(printedPosture()).toBe(resolved); + return resolved; + }; + + describe('the banner never disagrees with resolveTenancyPosture()', () => { + it('posture explicitly isolated, boolean unset — the cloud#1020 lie', () => { + // The regression case. `resolveMultiOrgEnabled()` is false here, so the + // old banner printed `single-tenant` while serve mounted the wall. + env('isolated', undefined); + expect(expectBannerAgreesWithResolver()).toBe('isolated'); + expect(tenancyLine()).toBe('Tenancy: isolated'); + expect(tenancyLine()).not.toContain('single-tenant'); + }); + + it('posture unset, boolean true — the legacy fallback still reads isolated', () => { + env(undefined, 'true'); + expect(expectBannerAgreesWithResolver()).toBe('isolated'); + expect(tenancyLine()).toBe('Tenancy: isolated'); + }); + + it('posture single while the boolean says true — posture wins', () => { + // Contradiction, the direction the old code got right by accident: the + // boolean would have said `multi-tenant`, the runtime runs unwalled. + env('single', 'true'); + expect(expectBannerAgreesWithResolver()).toBe('single'); + expect(tenancyLine()).toBe('Tenancy: single'); + }); + + it('posture isolated while the boolean says false — posture wins', () => { + // The same contradiction pointing the other way, which the old code got + // wrong: banner `single-tenant`, runtime walled. + env('isolated', 'false'); + expect(expectBannerAgreesWithResolver()).toBe('isolated'); + expect(tenancyLine()).toBe('Tenancy: isolated'); + }); + + it('prints group — the posture a boolean can only misreport', () => { + // `group` is why the field is not a boolean: flattened, it must print + // either `multi-tenant` (wrong wall shape) or `single-tenant` (no wall). + env('group', undefined); + expect(expectBannerAgreesWithResolver()).toBe('group'); + expect(tenancyLine()).toBe('Tenancy: group'); + }); + + it('posture unset and boolean unset — single, the default', () => { + env(undefined, undefined); + expect(expectBannerAgreesWithResolver()).toBe('single'); + expect(tenancyLine()).toBe('Tenancy: single'); + }); + + it("accepts the legacy 'multi' spelling and prints its canonical name", () => { + env('multi', undefined); + expect(expectBannerAgreesWithResolver()).toBe('isolated'); + expect(tenancyLine()).toBe('Tenancy: isolated'); + }); + }); + + it('prints the posture verbatim for every posture the spec defines', () => { + for (const posture of ['single', 'group', 'isolated'] as const) { + lines = []; + printServerReady({ ...base, tenancyPosture: posture }); + expect(tenancyLine()).toBe(`Tenancy: ${posture}`); + } + }); + + it('omits the row entirely when the caller has no posture to report', () => { + printServerReady({ ...base }); + expect(tenancyLine()).toBeUndefined(); + }); + + it('never flattens the posture back into multi-/single-tenant wording', () => { + // The old vocabulary is the tell that a boolean crept back in: it has no + // spelling for `group`, so its return means the spectrum was squashed again. + for (const posture of ['single', 'group', 'isolated'] as const) { + lines = []; + printServerReady({ ...base, tenancyPosture: posture }); + // Assert the row EXISTS before asserting what it does not say — a + // `not.toMatch` over a banner with no Tenancy row at all passes + // vacuously, which reads identical to the bug this file exists to catch. + expect(tenancyLine()).toBeDefined(); + expect(lines.join('\n')).not.toMatch(/multi-tenant|single-tenant/); + } + }); + + it('rejects the boolean shape at COMPILE time, not at review time', () => { + // These two directives are the structural half of the fix and are checked + // by `pnpm typecheck` (tests are type-checked — AGENTS.md), not by the + // assertions in this file. `resolveMultiOrgEnabled()` returns `boolean`, so + // re-wiring the banner to the legacy knob can no longer compile, and the + // retired field name can no longer be passed in silently ignored. + // @ts-expect-error — tenancyPosture is a TenancyPosture, never a boolean. + printServerReady({ port: 1, configFile: 'c', isDev: true, pluginCount: 0, tenancyPosture: true }); + // @ts-expect-error — `multiTenant` was removed with #4801; nothing reads it. + printServerReady({ port: 1, configFile: 'c', isDev: true, pluginCount: 0, multiTenant: true }); + // @ts-expect-error — and an arbitrary string is not a posture. + printServerReady({ port: 1, configFile: 'c', isDev: true, pluginCount: 0, tenancyPosture: 'multi' }); + expect(lines.filter((l) => l.includes('Tenancy:'))).toHaveLength(2); + }); +}); diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index 86fb656f97..91df3cdc72 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -2,6 +2,7 @@ import chalk from 'chalk'; import type { ZodError } from 'zod'; +import type { TenancyPosture } from '@objectstack/spec/security'; // ─── Constants ────────────────────────────────────────────────────── export const CLI_NAME = 'objectstack'; @@ -285,8 +286,31 @@ export interface ServerReadyOptions { driverLabel?: string; /** Resolved DB URL with credentials redacted. */ databaseUrl?: string; - /** Whether the SecurityPlugin was wired in multi-tenant mode (default true). */ - multiTenant?: boolean; + /** + * [ADR-0105 D1] The deployment's resolved tenancy posture — printed verbatim. + * + * This is the SAME fact serve wires the runtime from: it must be + * `resolveTenancyPosture()` (`@objectstack/types`), never a re-derivation. + * The banner used to take a boolean `multiTenant` sourced from + * `resolveMultiOrgEnabled()`, i.e. from `OS_MULTI_ORG_ENABLED` — the knob + * `OS_TENANCY_POSTURE` superseded, and which `resolveTenancyPosture()` now + * consults only as a fallback when the posture is unset. So a deployment + * booted with `OS_TENANCY_POSTURE=isolated` alone printed + * `Tenancy: single-tenant` while the organization wall was actually up and + * `Organizations` was in the plugin table one line below (framework#4801, + * observed in cloud#1020). Two sources for one fact, drifting. + * + * The field is the posture, not a boolean, for the same reason: tenancy is a + * three-valued spectrum (`single` | `group` | `isolated`), and a boolean + * cannot say `group` at all — it would have to lie, and the flattening is + * where the drift hides. Typing it as {@link TenancyPosture} also makes the + * old wiring a COMPILE error rather than a wrong-but-plausible line of + * output: `resolveMultiOrgEnabled()` returns `boolean` and no longer fits. + * + * Omitted → no `Tenancy:` row (unchanged: a caller with nothing to say says + * nothing, rather than guessing a posture). + */ + tenancyPosture?: TenancyPosture; /** * Credentials of the dev admin seeded on an empty DB this boot (dev only). * When present, the banner surfaces them so backend debugging never has to @@ -409,8 +433,10 @@ export function printServerReady(opts: ServerReadyOptions) { const dbInfo = opts.databaseUrl ? `${opts.driverLabel} ${chalk.dim('→')} ${opts.databaseUrl}` : opts.driverLabel; console.log(chalk.dim(` Driver: ${dbInfo}`)); } - if (opts.multiTenant !== undefined) { - console.log(chalk.dim(` Tenancy: ${opts.multiTenant ? 'multi-tenant' : 'single-tenant'}`)); + // [ADR-0105 D1] Print the posture verbatim — see `tenancyPosture` above for + // why this is not a boolean and why it must be the resolver's answer. + if (opts.tenancyPosture !== undefined) { + console.log(chalk.dim(` Tenancy: ${opts.tenancyPosture}`)); } console.log(chalk.dim(` Plugins: ${opts.pluginCount} loaded`)); if (opts.pluginNames && opts.pluginNames.length > 0) {