diff --git a/.changeset/ledger-readability-leaves-the-posture-gate.md b/.changeset/ledger-readability-leaves-the-posture-gate.md new file mode 100644 index 0000000000..925dc5fbee --- /dev/null +++ b/.changeset/ledger-readability-leaves-the-posture-gate.md @@ -0,0 +1,40 @@ +--- +'@objectstack/cli': patch +--- + +`os doctor` reports an unreadable installed-package ledger under EVERY tenancy posture + +The three rows that say doctor could not read `.objectstack/installed-packages/` +— the directory could not be enumerated, a file inside it would not parse, or +the package that reads ledgers would not load — were all produced inside the +ADR-0120 D5e unique-scope advisory, whose entry condition is +`postureGatesGlobalUniques(posture)`. That is true only for `isolated` (and its +legacy alias `multi`), so under `single` and `group` the ledger was never read +at all and `os doctor` said nothing about it. `OS_TENANCY_POSTURE` unset +resolves to `single`, so the silent posture was the default one. + +Whether an environment's `unique: 'global'` is dangerous IS a posture question, +and that gate is unchanged. Whether a file in the ledger can be read is not: it +is equally true under every posture and means the same thing under every one — +that installed app is dropped at boot, absent from the kernel and from the +console's installed-apps list. + +Ledger readability is now its own check, run unconditionally, independent of +both the posture and of whether an `objectstack.config.ts` loaded. The D5e block +keeps the unique-scope judgment alone, consuming the same reading rather than +taking a second one, so one bad ledger produces one row under `isolated` too. An +incomplete reading still withholds `✓ Unique scope` there — that line is a claim +about both halves of the advisory and only one of them ran. + +**Report face:** the three readability rows now take the `Installed packages` +name column instead of `Unique scope`, and drop `for installation-wide uniques` +from the message's parenthetical — under `single` and `group` there is no +unique-scope check to name. This supersedes the sentence in the pending +`quiet-ledgers-speak-up` changeset that called it a `Unique scope` warning row. +The `Unique scope` name still exists, under the D5e block, for the unique-scope +verdict alone. A readable ledger prints nothing new under any posture. + +This is the diagnostic-command half only. The runtime's own signal — the +`rehydrate()` warning per dropped entry at boot — is unchanged and stays +posture-independent; the two are separate channels for separate moments and +neither substitutes for the other. diff --git a/packages/cli/src/commands/doctor-ledger-posture-independence.test.ts b/packages/cli/src/commands/doctor-ledger-posture-independence.test.ts new file mode 100644 index 0000000000..0993ce9c0f --- /dev/null +++ b/packages/cli/src/commands/doctor-ledger-posture-independence.test.ts @@ -0,0 +1,427 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os doctor` reports an unreadable installed-package ledger under EVERY + * tenancy posture — not only `isolated` (#5429). + * + * ── The defect ─────────────────────────────────────────────────────────── + * + * Three ledger-readability rows existed before this file — the directory-level + * one (#5412), the entry-level one (#5413) and the reader-level one (#5644) — + * and all three were produced inside the ADR-0120 D5e advisory block, whose + * entry condition is: + * + * if (postureReading.ok && postureGatesGlobalUniques(postureReading.posture)) { + * + * with `findUnscopedGlobalUniques()` re-asserting the same gate on its first + * line. `postureGatesGlobalUniques()` is true for `isolated` (and its legacy + * alias `multi`) and false for `single` and `group`, so under those two + * postures `readInstalledPackageEntries()` was never called at all: whether the + * ledger could be read, and whether any file in it was corrupt, was a question + * `os doctor` did not ask and said nothing about. + * + * The default makes it worse than "two of four postures". `resolveTenancyPosture()` + * returns `single` when `OS_TENANCY_POSTURE` is unset and `OS_MULTI_ORG_ENABLED` + * is not truthy (`packages/types/src/env.ts:161`), so the SILENT posture is the + * out-of-the-box one. A developer who never set the variable got the blind + * report. + * + * ── Why posture is the wrong gate for this fact ────────────────────────── + * + * "Is this environment's `unique: 'global'` dangerous?" IS a posture question — + * `'global'` is unambiguous under `single` (one customer) and `group` (the + * installation is the customer), and only `isolated` makes it a finding. Gating + * THAT on the posture is correct and stays. + * + * "Is there a file under `.objectstack/installed-packages/` that cannot be + * read?" is not. It is equally true under every posture and it means the same + * thing under every posture: that installed app is dropped at boot — not + * registered with the kernel, absent from the console's installed-apps list. + * The maintainer's 2026-08-06 ruling on #5429 (option A) promoted it to its own + * posture-independent check under its own `Installed packages` name, leaving the + * D5e block with the unique-scope judgment alone. + * + * ── The other half of the ruling: no double reporting ──────────────────── + * + * Under `isolated` both things are live at once, so the same bad ledger could + * easily be reported twice — once by the standalone check and once by the D5e + * block. It is not: the ledger is read ONCE per run and the D5e advisory + * consumes that same reading instead of taking its own. What D5e keeps is the + * consequence for its own verdict — an incomplete reading still withholds the + * `✓ Unique scope` clean bill, because that line is a claim about both halves + * of the advisory and only one of them ran. + * + * ── Division of labour with the boot side (deliberate, not redundant) ──── + * + * `rehydrate()` warns per corrupt entry at boot, posture-independently, and + * that is a different channel for a different moment: the runtime telling an + * operator what it just dropped while starting. This file is about the + * DIAGNOSTIC command — the thing someone runs on purpose, before or after a + * boot, to ask what is wrong. Both are correct; neither substitutes for the + * other, and the diagnostic being posture-blind is what #5429 is about. + */ + +import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import Doctor from './doctor.js'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +/** `packages/cli` — the oclif root the real command is loaded against below. */ +const CLI_ROOT = path.resolve(HERE, '..', '..'); + +/** + * 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 no + * `git grep` can find stops being maintained (#4890 / #5157). + */ +const SGR = /\x1b\[[0-9;]*m/g; +const plain = (s: string) => s.replace(SGR, ''); + +/** The D5e success line — a claim about BOTH halves of that advisory. */ +const CLEAN_BILL = 'No unconfirmed installation-wide uniques'; + +/** The DIRECTORY-level finding (#5412). */ +const LEDGER_HEADLINE = 'Could not read the installed-package ledger'; +/** Its ENTRY-level sibling (#5413). */ +const SKIPPED_HEADLINE = 'installed-package ledger entr'; +/** The name column all ledger-readability rows take since #5429. */ +const LEDGER_ROW_NAME = 'Installed packages'; + +/** The postures that gate the D5e advisory OFF — where doctor used to be blind. */ +const BLIND_POSTURES = ['single', 'group'] as const; + +/** + * The same preflight `doctor-ledger-read-failure.test.ts` carries, and for the + * same reason (#5612): doctor reaches the ledger through a dynamic + * `import('@objectstack/cloud-connection')` whose absent-package branch is + * deliberately silent, so in a worktree where that package is unbuilt every + * case below fails with an assertion diff that reads exactly like this feature + * having been reverted. Nothing here is relaxed by the guard — in a correctly + * built worktree it is a no-op; it only replaces a misleading red with an + * accurate one. + */ +const PREFLIGHT_HINT = [ + 'Preflight failed: the ledger cases below cannot observe anything.', + '', + '`@objectstack/cloud-connection` is the package doctor reads the ledger through, and it is', + 'either not built or built from a source older than #5413 in this worktree.', + '', + 'Build the dependency graph first:', + " pnpm --workspace-concurrency=2 --filter '@objectstack/cli^...' build", +].join('\n'); + +async function assertLedgerReaderIsBuilt(): Promise { + let mod: Record; + try { + mod = await import('@objectstack/cloud-connection'); + } catch (err) { + throw new Error(`${PREFLIGHT_HINT}\n\ncause: ${err instanceof Error ? err.message : String(err)}`); + } + if (typeof mod.LocalManifestSource !== 'function') { + throw new Error(`${PREFLIGHT_HINT}\n\ncause: the module loaded but exports no LocalManifestSource.`); + } + const listing = new mod.LocalManifestSource(path.join(os.tmpdir(), 'os-5429-preflight-absent')).list(); + if (!Array.isArray(listing?.entries) || !Array.isArray(listing?.skipped)) { + throw new Error( + `${PREFLIGHT_HINT}\n\ncause: LocalManifestSource.list() returned ${JSON.stringify(listing)}, ` + + 'not the { entries, skipped } listing #5413 introduced — the built artefact predates it.', + ); + } +} + +/** Both variables that decide the posture — the second one owns the default. */ +const TOUCHED = ['OS_TENANCY_POSTURE', 'OS_MULTI_ORG_ENABLED'] as const; + +describe('os doctor reports ledger readability under every posture (#5429)', () => { + beforeAll(assertLedgerReaderIsBuilt); + + /** + * `node_modules/` exists in the temp cwd on purpose — without it doctor's + * `Dependencies` check is itself an `error` and exits 1 on its own, which + * would make an assertion pass for a reason having nothing to do with this + * change (the trap PR #5390 wrote down). + */ + let tmp: string; + let cwdSpy: ReturnType; + const saved: Partial> = {}; + + beforeEach(() => { + for (const key of TOUCHED) saved[key] = process.env[key]; + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'os-doctor-5429-')); + fs.mkdirSync(path.join(tmp, 'node_modules')); + cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmp); + }); + + afterEach(() => { + cwdSpy.mockRestore(); + for (const key of TOUCHED) { + if (saved[key] === undefined) delete process.env[key]; + else process.env[key] = saved[key]!; + } + fs.rmSync(tmp, { recursive: true, force: true }); + }); + + const ledgerPath = () => path.join(tmp, '.objectstack/installed-packages'); + + const writeConfig = () => + fs.writeFileSync( + path.join(tmp, 'objectstack.config.ts'), + [ + 'export default {', + " manifest: { name: 'os5429', label: 'Ledger Posture', version: '1.0.0' },", + " objects: [{ name: 'account', label: 'Account', fields: [{ name: 'name', type: 'text', label: 'Name' }] }],", + '};', + '', + ].join('\n'), + ); + + /** The ledger PATH exists but is not a directory — `readdirSync` throws ENOTDIR. */ + const writeUnreadableLedgerDirectory = () => { + fs.mkdirSync(path.dirname(ledgerPath()), { recursive: true }); + fs.writeFileSync(ledgerPath(), 'this is a file, not the ledger directory\n'); + }; + + /** A readable directory holding one truncated entry — #5413's repro verbatim. */ + const writeCorruptLedgerEntry = () => { + fs.mkdirSync(ledgerPath(), { recursive: true }); + fs.writeFileSync( + path.join(ledgerPath(), 'broken.json'), + '{"manifestId":"broken","manifest":{"objects":[{"name":"acct"', + ); + }; + + const setPosture = (posture: string | undefined) => { + if (posture === undefined) { + delete process.env.OS_TENANCY_POSTURE; + // The default is only `single` while multi-org is off, and that default + // is the whole point of the unset case. + delete process.env.OS_MULTI_ORG_ENABLED; + } else { + process.env.OS_TENANCY_POSTURE = posture; + } + }; + + async function runDoctor(argv: string[] = []): Promise<{ out: string; exitCode: number | undefined }> { + const logs: string[] = []; + const logSpy = vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { + logs.push(a.join(' ')); + }); + let exitCode: number | undefined; + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + exitCode = code; + throw new Error(`__PROCESS_EXIT__:${code}`); + }) as never); + + try { + await Doctor.run(argv, { root: CLI_ROOT }); + } catch (err) { + if (!(err instanceof Error) || !err.message.startsWith('__PROCESS_EXIT__')) throw err; + } finally { + logSpy.mockRestore(); + exitSpy.mockRestore(); + } + return { out: plain(logs.join('\n')), exitCode }; + } + + /** How many times a finding's headline reaches the terminal. */ + const occurrences = (haystack: string, needle: string): number => haystack.split(needle).length - 1; + + // ① The acceptance criterion: both ledger findings have their own row under + // the postures that gate the D5e advisory off. + describe.each(BLIND_POSTURES)("under the '%s' posture", (posture) => { + it('reports a ledger DIRECTORY it could not read, with the cause', async () => { + setPosture(posture); + writeConfig(); + writeUnreadableLedgerDirectory(); + + const run = await runDoctor(); + + // Before #5429 this output contained none of the three: the read never + // happened, because the posture gate returned first. + expect(run.out).toContain(LEDGER_HEADLINE); + expect(run.out).toContain('ENOTDIR'); + expect(run.out).toContain(LEDGER_ROW_NAME); + // The D5e advisory itself stays posture-gated — this posture asks no + // unique-scope question at all, and none is answered. + expect(run.out).not.toContain(CLEAN_BILL); + expect(run.out).not.toContain("Checking unique scopes"); + // Gauge: a warning, the report finishes, exit stays 0. + expect(run.out).toContain('Environment is functional but has some warnings'); + expect(run.exitCode).toBeUndefined(); + }, 60_000); + + it('names a corrupt ENTRY inside a readable ledger', async () => { + setPosture(posture); + writeConfig(); + writeCorruptLedgerEntry(); + + const run = await runDoctor(); + + expect(run.out).toContain(SKIPPED_HEADLINE); + expect(run.out).toContain('broken.json'); + expect(run.out).toContain(LEDGER_ROW_NAME); + // Two distinct facts, two distinct headlines: the directory read fine. + expect(run.out).not.toContain(LEDGER_HEADLINE); + expect(run.out).toContain('Environment is functional but has some warnings'); + expect(run.exitCode).toBeUndefined(); + }, 60_000); + + it('expands the per-file cause under --verbose, like every other warning row', async () => { + setPosture(posture); + writeConfig(); + writeCorruptLedgerEntry(); + + const plainRun = await runDoctor(); + const verboseRun = await runDoctor(['--verbose']); + + expect(plainRun.out).not.toContain('cause:'); + expect(verboseRun.out).toContain('cause:'); + expect(verboseRun.out).toContain('Repair the JSON, or delete the file'); + }, 60_000); + }); + + it('reports with OS_TENANCY_POSTURE UNSET — the default posture is the blind one', async () => { + // `resolveTenancyPosture()` returns `single` when the variable is unset and + // multi-org is off, so before #5429 the out-of-the-box configuration was + // exactly the one that said nothing. This case is the issue's real blast + // radius: not an exotic posture, the default. + setPosture(undefined); + writeConfig(); + writeCorruptLedgerEntry(); + + const run = await runDoctor(); + + expect(run.out).toContain(SKIPPED_HEADLINE); + expect(run.out).toContain('broken.json'); + expect(run.exitCode).toBeUndefined(); + }, 60_000); + + it('reports without an objectstack.config.ts — ledger readability is not a config-derived fact', async () => { + // The ledger half never needed the config: it reads a directory. Leaving + // the check inside the config-analysis block would have replaced a posture + // gate with a config gate — the same silence one condition along, which is + // the widening #5382 made for the posture reader for the same reason. + setPosture('single'); + expect(fs.existsSync(path.join(tmp, 'objectstack.config.ts'))).toBe(false); + writeUnreadableLedgerDirectory(); + + const run = await runDoctor(); + + expect(run.out).toContain(LEDGER_HEADLINE); + expect(run.out).toContain(LEDGER_ROW_NAME); + }, 60_000); + + // ② Dedup: `isolated` has both the standalone check and the D5e advisory + // live, and one bad ledger must still produce one finding. + describe("under the 'isolated' posture, where both checks are live", () => { + it('reports an unreadable DIRECTORY exactly once, and still withholds the ✓', async () => { + setPosture('isolated'); + writeConfig(); + writeUnreadableLedgerDirectory(); + + const run = await runDoctor(); + + expect(occurrences(run.out, LEDGER_HEADLINE)).toBe(1); + // The D5e success line is a claim about both halves of that advisory and + // only one of them ran — unchanged from #5412. + expect(run.out).not.toContain(CLEAN_BILL); + expect(run.exitCode).toBeUndefined(); + }, 60_000); + + it('reports a corrupt ENTRY exactly once, and still withholds the ✓', async () => { + setPosture('isolated'); + writeConfig(); + writeCorruptLedgerEntry(); + + const run = await runDoctor(); + + expect(occurrences(run.out, SKIPPED_HEADLINE)).toBe(1); + expect(occurrences(run.out, 'broken.json')).toBe(1); + expect(run.out).not.toContain(CLEAN_BILL); + }, 60_000); + + it('still reports the unique-scope advisory itself — the D5e half is untouched', async () => { + setPosture('isolated'); + writeConfig(); + fs.mkdirSync(ledgerPath(), { recursive: true }); + fs.writeFileSync( + path.join(ledgerPath(), 'billing.json'), + JSON.stringify({ + manifestId: 'billing', + manifest: { + objects: [ + { + name: 'invoice', + label: 'Invoice', + fields: [{ name: 'code', type: 'text', label: 'Code', unique: 'global' }], + }, + ], + }, + }), + ); + + const run = await runDoctor(); + + // The advisory reads the entries the standalone check already loaded. + expect(run.out).toContain('invoice.code'); + expect(run.out).toContain("installed package 'billing'"); + expect(run.out).toContain('Unique scope'); + // A readable ledger produces no readability row at all. + expect(run.out).not.toContain(LEDGER_ROW_NAME); + }, 60_000); + }); + + // ③ Zero noise. A healthy ledger — and a project that never installed + // anything — must leave the report exactly as it was, under every posture. + describe.each([...BLIND_POSTURES, 'isolated'] as const)("a healthy ledger is silent under '%s'", (posture) => { + it('says nothing about the ledger when every entry reads clean', async () => { + setPosture(posture); + writeConfig(); + fs.mkdirSync(ledgerPath(), { recursive: true }); + fs.writeFileSync( + path.join(ledgerPath(), 'clean.json'), + JSON.stringify({ manifestId: 'clean', manifest: { objects: [] } }), + ); + + const run = await runDoctor(['--verbose']); + + expect(run.out).not.toContain(LEDGER_HEADLINE); + expect(run.out).not.toContain(SKIPPED_HEADLINE); + expect(run.out).not.toContain(LEDGER_ROW_NAME); + expect(run.exitCode).toBeUndefined(); + }, 60_000); + + it('says nothing when nothing was ever installed', async () => { + setPosture(posture); + writeConfig(); + expect(fs.existsSync(ledgerPath())).toBe(false); + + const run = await runDoctor(['--verbose']); + + expect(run.out).not.toContain(LEDGER_HEADLINE); + expect(run.out).not.toContain(SKIPPED_HEADLINE); + expect(run.out).not.toContain(LEDGER_ROW_NAME); + expect(run.exitCode).toBeUndefined(); + }, 60_000); + }); + + it("keeps the D5e clean bill under 'isolated' when the ledger is healthy", async () => { + // The success line still belongs to the advisory, not to the new check — + // the two are separate rows with separate subjects. + setPosture('isolated'); + writeConfig(); + fs.mkdirSync(ledgerPath(), { recursive: true }); + fs.writeFileSync( + path.join(ledgerPath(), 'clean.json'), + JSON.stringify({ manifestId: 'clean', manifest: { objects: [] } }), + ); + + const run = await runDoctor(['--verbose']); + + expect(run.out).toContain(CLEAN_BILL); + }, 60_000); +}); diff --git a/packages/cli/src/commands/doctor-ledger-read-failure.test.ts b/packages/cli/src/commands/doctor-ledger-read-failure.test.ts index 83ad7cc49c..f7ce207e3b 100644 --- a/packages/cli/src/commands/doctor-ledger-read-failure.test.ts +++ b/packages/cli/src/commands/doctor-ledger-read-failure.test.ts @@ -66,6 +66,20 @@ * it produces, and the absent contract is pinned separately through the loader * seam. Both facts are still here — one of them changed how it is spelled, * because the fact it used to spell was the defect. + * + * ── What #5429 changed under this file ─────────────────────────────────── + * + * All three rows above lived inside the D5e advisory block, so they only + * existed under the `isolated` posture — which is why every case here sets + * `OS_TENANCY_POSTURE=isolated` and why they all still do. #5429 promoted the + * readability check out from under that gate, and the rows moved with it: their + * name column is `Installed packages` rather than `Unique scope`, because under + * `single` and `group` there is no unique-scope check to name. Every other + * assertion in this file is unchanged, and deliberately so — the isolated-posture + * report is the one that must NOT drift while the check becomes reachable from + * the other postures. That the rows now also appear under those postures, and + * appear only once when both checks are live, is pinned next door in + * `doctor-ledger-posture-independence.test.ts`. */ import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest'; @@ -109,6 +123,17 @@ const SKIPPED_HEADLINE = 'installed-package ledger entr'; */ const READER_HEADLINE = 'Could not load the installed-package ledger reader'; +/** + * The name column all three rows take since #5429. + * + * It was `Unique scope` while these rows only existed inside the D5e advisory: + * the row an operator scans for had to be present rather than missing. Once the + * check became posture-independent that name stopped being true — `single` and + * `group` run no unique-scope check at all — so the rows say what they are + * about instead. + */ +const LEDGER_ROW_NAME = 'Installed packages'; + /** * ── Why this file needs a preflight (#5612) ────────────────────────────── * @@ -197,13 +222,19 @@ describe('installedPackageLedgerFailureCheck — the finding the shared catch us expect(check.fix).toContain("scandir '/p/.objectstack'"); }); - it('takes the `Unique scope` name column, so the row is present rather than missing', () => { + it('takes the `Installed packages` name column, so the row is present rather than missing', () => { const check = installedPackageLedgerFailureCheck(new Error('boom')); // Load-bearing, not cosmetic. An operator scans the report by its name - // column; a separately-named row would leave `Unique scope` simply absent, - // which is the same silence this issue is about wearing a different hat. - expect(check.name).toBe('Unique scope'); + // column, and this row has to be somewhere findable rather than absent — + // absence is the silence this issue is about wearing a different hat. + // + // #5429 moved WHICH column. It was `Unique scope`, correct while the row + // could only be produced inside the D5e advisory; now that the check runs + // under every posture, a row named for a check that does not exist under + // `single` or `group` would be its own small lie. The `Unique scope` name + // still exists and still belongs to the unique-scope verdict alone. + expect(check.name).toBe(LEDGER_ROW_NAME); }); it('stays a warning — the environment runs, doctor’s sight of it is what broke', () => { @@ -253,13 +284,13 @@ describe('installedPackageLedgerReaderFailureCheck — the finding one boundary expect(check.fix).toContain('dist/index.js'); }); - it('takes the `Unique scope` name column and stays a warning, like its two siblings', () => { + it('takes the `Installed packages` name column and stays a warning, like its two siblings', () => { const check = installedPackageLedgerReaderFailureCheck(new Error('boom')); - // Same reasoning as #5412: an operator scans the report by its name column, - // and a row under a different name leaves `Unique scope` simply missing — - // the silence this family of issues is about, wearing a different hat. - expect(check.name).toBe('Unique scope'); + // All three readability rows share one name column, so an operator scanning + // for the ledger finds it in one place whichever of the three fired + // (#5429 moved that column off `Unique scope`; see the sibling case above). + expect(check.name).toBe(LEDGER_ROW_NAME); // The environment still runs; what broke is doctor's sight of part of it. expect(check.status).toBe('warning'); }); @@ -393,7 +424,7 @@ describe('os doctor, end to end, against an unreadable installed-package ledger' expect(run.out).toContain(LEDGER_HEADLINE); expect(run.out).toContain('ENOTDIR'); // Rendered through the ONE renderer, so it carries a name column. - expect(run.out).toContain('Unique scope'); + expect(run.out).toContain(LEDGER_ROW_NAME); // Gauge: warning, the report finishes, exit stays 0. expect(run.out).toContain('Environment is functional but has some warnings'); expect(run.exitCode).toBeUndefined(); @@ -524,9 +555,10 @@ describe('os doctor, end to end, against an unreadable installed-package ledger' expect(run.out).toContain('broken.json'); // ② With the parser's own words, not a summary doctor invented. expect(run.out).toMatch(/JSON/i); - // ③ Under the `Unique scope` name column, like its directory-level sibling, - // so the row an operator scans for is present rather than missing. - expect(run.out).toContain('Unique scope'); + // ③ Under the `Installed packages` name column, like its directory-level + // sibling, so the row an operator scans for is present rather than + // missing. + expect(run.out).toContain(LEDGER_ROW_NAME); // ④ NOT the directory-level row: the directory read fine. Two distinct // facts, two distinct headlines (#5412 vs #5413). expect(run.out).not.toContain(LEDGER_HEADLINE); @@ -685,7 +717,7 @@ describe('the optional package INSTALLED BUT UNLOADABLE is reported (#5644)', () expect(out).not.toContain(CLEAN_BILL); // ② …replaced by a row that names what could not be loaded. expect(out).toContain(READER_HEADLINE); - expect(out).toContain('Unique scope'); + expect(out).toContain(LEDGER_ROW_NAME); // ③ NOT the directory-level row: the directory was never reached, and its // text asserts a ledger exists — which doctor cannot know from here. expect(out).not.toContain(LEDGER_HEADLINE); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 4f3da9bee0..518efda8ff 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -874,7 +874,7 @@ interface UniqueScopeAdvisory { * carries a `failure` alongside it. The caller distinguishes them; before * #5412 nobody could, because one `catch` returned `[]` for both. */ -interface InstalledPackageLedgerReading { +export interface InstalledPackageLedgerReading { entries: any[]; /** * Ledger files that exist but could not be turned into entries (#5413). @@ -956,6 +956,16 @@ interface SkippedLedgerEntry { * function's history: now over a reader it could not even start. The two are * separated by `loadOptionalPackage()` (`utils/optional-package.ts` carries how, * and the measurements behind it); only the genuinely-absent half stays silent. + * + * Called UNCONDITIONALLY since #5429 — from `run()`, outside the tenancy-posture + * gate and outside the config-analysis block. Two consequences worth knowing: + * every finding above is now reachable under every posture (which is the whole + * point), and this function no longer sits inside a `try` belonging to somebody + * else. It must therefore report rather than throw for anything it can hit, so + * the directory path is resolved INSIDE the guarded block: `DEFAULT_INSTALLED_ + * PACKAGES_DIR` comes from a dynamically loaded module, and a `path.join()` over + * a non-string export used to be absorbed by the config `catch` and misreported + * as "Could not load config for analysis". */ async function readInstalledPackageEntries(cwd: string): Promise { // Dynamic, like serve.ts's cloud-connection load: `os doctor` must still run @@ -974,8 +984,8 @@ async function readInstalledPackageEntries(cwd: string): Promise { - if (!postureGatesGlobalUniques(posture)) return { advisories: [], skippedLedgerEntries: [] }; + ledgerEntries: any[], +): UniqueScopeAdvisory[] { + if (!postureGatesGlobalUniques(posture)) return []; const out: UniqueScopeAdvisory[] = []; for (const finding of collectGlobalUniques(config?.objects)) { out.push({ source: 'this project’s metadata', finding }); } - const ledger = await readInstalledPackageEntries(cwd); - for (const entry of ledger.entries) { + for (const entry of ledgerEntries) { const findings = collectGlobalUniques(entry?.manifest?.objects); // Subtract what the install ceremony already answered for — an attested // install must not be re-reported, or the advisory becomes the recurring @@ -1053,12 +1056,7 @@ async function findUnscopedGlobalUniques( out.push({ source: `installed package '${entry?.manifestId ?? entry?.packageId}'`, finding }); } } - return { - advisories: out, - skippedLedgerEntries: ledger.skipped, - ...(ledger.failure ? { ledgerFailure: ledger.failure } : {}), - ...(ledger.readerFailure ? { ledgerReaderFailure: ledger.readerFailure } : {}), - }; + return out; } // ─── Filesystem Checks ────────────────────────────────────────────── @@ -1302,6 +1300,26 @@ export function configLoadFailureCheck(err: unknown): HealthCheckResult { }; } +/** + * The name column every installed-package-ledger readability row takes (#5429). + * + * It used to be `Unique scope`, and that was right while these rows only + * existed inside the ADR-0120 D5e advisory: the row an operator scanned for had + * to be PRESENT and not a `✓`, so a readability failure wore the advisory's + * name rather than leaving it missing. + * + * #5429 made the readability check posture-independent, and `Unique scope` + * stopped being true. Under `single` and `group` there IS no unique-scope + * question — `'global'` is unambiguous there — so a row named for it would + * announce a check that does not run under that posture and never did. These + * rows now say what they are actually about: the installed packages, and + * whether doctor could read them at all. + * + * The `Unique scope` name still exists, still under the D5e block, and still + * belongs to the unique-scope verdict alone. + */ +const LEDGER_ROW_NAME = 'Installed packages'; + /** * What doctor reports when the installed-package ledger cannot be read (#5412). * @@ -1324,10 +1342,9 @@ export function configLoadFailureCheck(err: unknown): HealthCheckResult { * • **Warning, not error.** The environment still runs; what is broken is * doctor's ability to see part of it. Doctor keeps going and keeps exiting * 0, exactly as it does for a config it cannot load. - * • **It takes the `Unique scope` name column.** Not a new label: the point - * is that the row an operator scans for is PRESENT and not a `✓`. A - * separately-named row would leave `Unique scope` simply missing, which is - * the silence this issue is about wearing a different hat. + * • **It takes the {@link LEDGER_ROW_NAME} name column** — `Unique scope` + * until #5429 moved the check out from under the posture gate; see that + * constant for why the name had to move with it. * • **The cause is quoted, not paraphrased** (#5390 / #5403). `ENOTDIR: not * a directory, scandir '…'` names the file that is in the way; no sentence * doctor could invent would beat it. @@ -1335,19 +1352,21 @@ export function configLoadFailureCheck(err: unknown): HealthCheckResult { export function installedPackageLedgerFailureCheck(err: unknown): HealthCheckResult { const cause = describeThrown(err); return { - name: 'Unique scope', + name: LEDGER_ROW_NAME, status: 'warning', - message: - 'Could not read the installed-package ledger (installed packages NOT checked for ' - + `installation-wide uniques) — ${reportRowHeadline(cause)}`, + message: `Could not read the installed-package ledger (installed packages NOT checked) — ${reportRowHeadline(cause)}`, fix: - 'This check has two halves and only one of them ran. Uniques declared by THIS\n' - + ' project’s metadata were checked and are reported above; uniques declared by\n' - + ' INSTALLED PACKAGES were not looked at, so an installed app carrying an\n' - + ' installation-wide `unique` would not have appeared.\n' - + ' The ledger is the `.objectstack/installed-packages/` directory under the\n' + 'The ledger is the `.objectstack/installed-packages/` directory under the\n' + ' project root; it exists here, which is why this is reported rather than\n' - + ' treated as "nothing was ever installed".\n' + + ' treated as "nothing was ever installed". Every package it lists is one\n' + + ' this runtime ALSO cannot rehydrate at boot — not registered with the\n' + + ' kernel, absent from the console’s installed-apps list — so an app missing\n' + + ' from this environment is very likely in there.\n' + + ' Under the `isolated` tenancy posture it costs the ADR-0120 D5e advisory\n' + + ' half its input too: that check has two halves and only one of them ran.\n' + + ' Uniques declared by THIS project’s metadata were checked; uniques declared\n' + + ' by INSTALLED PACKAGES were not looked at, which is why no `✓ Unique scope`\n' + + ' line appears.\n' + ` cause: ${indentUnderGutter(cause)}`, }; } @@ -1360,8 +1379,8 @@ export function installedPackageLedgerFailureCheck(err: unknown): HealthCheckRes * fires when the ledger DIRECTORY could not be read at all; this one fires when * the directory read fine and some of the files in it did not. Both produce the * same false PASS if unreported — `✓ Unique scope` over manifests doctor never - * parsed — and both therefore take the `Unique scope` name column and withhold - * the success line, for the reasons written out above. + * parsed — and both therefore take the {@link LEDGER_ROW_NAME} name column and + * withhold that success line, for the reasons written out above. * * Why entry-level corruption is a finding at all, rather than something the * producer just handles: skipping a corrupt file IS correct — one truncated @@ -1389,11 +1408,11 @@ export function installedPackageLedgerSkippedEntriesCheck( const head = described[0]!; const more = n > 1 ? ` (+${n - 1} more)` : ''; return { - name: 'Unique scope', + name: LEDGER_ROW_NAME, status: 'warning', message: - `${n} installed-package ledger ${noun} could not be read (those packages NOT checked ` - + `for installation-wide uniques) — ${reportRowHeadline(`${head.file}: ${head.cause}`)}${more}`, + `${n} installed-package ledger ${noun} could not be read (those packages NOT checked) — ` + + `${reportRowHeadline(`${head.file}: ${head.cause}`)}${more}`, fix: 'The ledger directory was read fine; these files inside it were not. Each one is an\n' + ' installed package this runtime ALSO drops at boot — it is not registered with\n' @@ -1416,8 +1435,9 @@ export function installedPackageLedgerSkippedEntriesCheck( * `installedPackageLedgerSkippedEntriesCheck` fires when individual files in it * would not parse; this one fires when the reader itself never started. All * three produce the identical false PASS if unreported — `✓ Unique scope` over - * installed packages nobody looked at — so all three take the `Unique scope` - * name column, hold back the success line, and stay warnings. + * installed packages nobody looked at — so all three take the + * {@link LEDGER_ROW_NAME} name column, hold back that success line, and stay + * warnings. * * What is deliberately NOT a condition here: whether * `.objectstack/installed-packages/` exists. Doctor does not know that it does @@ -1438,16 +1458,17 @@ export function installedPackageLedgerSkippedEntriesCheck( export function installedPackageLedgerReaderFailureCheck(err: unknown): HealthCheckResult { const cause = describeThrown(err); return { - name: 'Unique scope', + name: LEDGER_ROW_NAME, status: 'warning', message: - 'Could not load the installed-package ledger reader (installed packages NOT checked ' - + `for installation-wide uniques) — ${reportRowHeadline(cause)}`, + 'Could not load the installed-package ledger reader (installed packages NOT checked) — ' + + `${reportRowHeadline(cause)}`, fix: '`@objectstack/cloud-connection` IS installed here — its specifier resolves — and loading\n' + ' it threw. That package is how `os doctor` reads `.objectstack/installed-packages/`,\n' - + ' so this half of the check never started: an installed app declaring an\n' - + ' installation-wide `unique` would not have appeared. Doctor cannot even tell you\n' + + ' so nothing about the installed packages was read: whether any of them is\n' + + ' unreadable — and, under the `isolated` posture, whether any declares an\n' + + ' installation-wide `unique` — went unasked. Doctor cannot even tell you\n' + ' whether a ledger is present — the directory’s name is one of that package’s\n' + ' exports.\n' + ' A checkout that never installed the package says nothing at all, so this row means\n' @@ -1457,6 +1478,76 @@ export function installedPackageLedgerReaderFailureCheck(err: unknown): HealthCh }; } +/** + * Every readability finding one ledger reading produced — the whole of the + * posture-independent check #5429 promoted out of the D5e block. + * + * ── What was wrong ─────────────────────────────────────────────────────── + * + * All three rows above were built inside the ADR-0120 D5e advisory, whose entry + * condition is `postureGatesGlobalUniques(posture)` — true for `isolated` (and + * its legacy alias `multi`), false for `single` and `group` — and + * `findUnscopedGlobalUniques()` re-asserted the same gate on its first line. So + * under `single` and `group`, {@link readInstalledPackageEntries} was never + * called: a ledger that could not be read, or a file inside it that would not + * parse, was a question `os doctor` did not ask. + * + * The default is the blind one. `resolveTenancyPosture()` returns `single` when + * `OS_TENANCY_POSTURE` is unset and multi-org is off, so out of the box doctor + * said nothing at all about a broken ledger. + * + * ── Why the posture is the wrong gate for THIS fact ────────────────────── + * + * "Is this environment's `unique: 'global'` dangerous?" is genuinely a posture + * question: `'global'` is unambiguous under `single` (one customer) and `group` + * (the installation IS the customer), and only `isolated` makes it a finding. + * That gate is correct and stays exactly where it is. + * + * "Is there a file under `.objectstack/installed-packages/` that cannot be + * read?" is not. It is equally true under every posture, and it means the same + * thing under every posture: that installed app is dropped at boot. The + * maintainer's 2026-08-06 ruling on #5429 (option A) split the two apart — + * these rows became their own check, the D5e block kept the unique-scope + * verdict alone. + * + * ── Dedup, structurally rather than by a flag ──────────────────────────── + * + * Under `isolated` both are live, and the same bad ledger must still be + * reported once. It is, because the ledger is read ONCE per run: `run()` reads + * it, renders these rows, and hands the same reading to + * {@link findUnscopedGlobalUniques}, which no longer reads or reports anything + * about readability. There is no second read to disagree and no second row to + * suppress. What D5e keeps is the CONSEQUENCE for its own verdict — an + * incomplete reading still withholds its `✓ Unique scope`, because that line is + * a claim about both halves of the advisory and only one of them ran. + * + * ── Not a duplicate of the boot-side warning ───────────────────────────── + * + * `rehydrate()` warns per corrupt entry at boot, posture-independently — the + * runtime saying what it just dropped while starting. This is the DIAGNOSTIC + * command: what someone runs on purpose, possibly without ever booting, to ask + * what is wrong. #4801 / cloud#1020 are about those two faces disagreeing; + * before #5429 they did. + */ +export function installedPackageLedgerChecks( + reading: InstalledPackageLedgerReading, +): HealthCheckResult[] { + // The reader never loaded, so neither the directory nor any entry was + // reached — mutually exclusive with both rows below rather than a third + // independent one (#5644). + if (reading.readerFailure) { + return [installedPackageLedgerReaderFailureCheck(reading.readerFailure.cause)]; + } + const out: HealthCheckResult[] = []; + if (reading.failure) out.push(installedPackageLedgerFailureCheck(reading.failure.cause)); + // Independent of the row above, not an `else`: `failure` means the directory + // could not be enumerated at all, `skipped` means it enumerated fine and + // named files inside it would not parse. Each names packages the other does + // not (#5412 vs #5413). + if (reading.skipped.length > 0) out.push(installedPackageLedgerSkippedEntriesCheck(reading.skipped)); + return out; +} + // ─── Command ──────────────────────────────────────────────────────── export default class Doctor extends Command { @@ -1642,6 +1733,24 @@ export default class Doctor extends Command { results.push(postureReading.result); } + // ── Installed-package ledger readability (#5429) ───────────────── + // + // Read ONCE per run, here, and unconditionally. + // + // Placement is the fix, exactly as it was for the posture reader above. + // These rows used to be built inside the ADR-0120 D5e advisory, so + // `readInstalledPackageEntries()` only ran under `isolated` and only when a + // config loaded. Whether a file under `.objectstack/installed-packages/` can + // be read is neither of those things: it is equally true under every + // posture, it needs no config to answer, and it means the same thing every + // time — that installed app is dropped at boot. Under `single` (the DEFAULT + // posture) doctor said nothing about it at all. + // + // The same reading is handed to the D5e advisory further down instead of + // being read a second time, which is what keeps one bad ledger to one row. + const ledgerReading = await readInstalledPackageEntries(cwd); + results.push(...installedPackageLedgerChecks(ledgerReading)); + // Display environment results let hasErrors = false; let hasWarnings = false; @@ -1765,12 +1874,12 @@ export default class Doctor extends Command { // so nothing is silently lost. if (postureReading.ok && postureGatesGlobalUniques(postureReading.posture)) { printStep("Checking unique scopes against the 'isolated' tenancy posture..."); - const { - advisories, - ledgerFailure, - ledgerReaderFailure, - skippedLedgerEntries, - } = await findUnscopedGlobalUniques(cwd, config, postureReading.posture); + // #5429 — the entries were read once, at the top of the run, and the + // three ways that read can fail are already reported as their own + // `Installed packages` rows. What is left here is the unique-scope + // judgment itself, which is the only part of this block that depends + // on the posture. + const advisories = findUnscopedGlobalUniques(config, postureReading.posture, ledgerReading.entries); if (advisories.length > 0) { hasWarnings = true; for (const { source, finding } of advisories) { @@ -1778,39 +1887,21 @@ export default class Doctor extends Command { } console.log(chalk.dim(` → ${GLOBAL_UNIQUE_ISOLATED_PRESCRIPTION}`)); } - // #5412 — the success line is a claim about BOTH halves of the - // advisory, so it may only be printed when both halves ran. A ledger - // that exists and could not be read is reported in its place; a - // false `✓` here is worse than a missing check, because it is the - // one thing that stops the operator looking further. + // #5412 / #5413 / #5644 — the success line is a claim about BOTH + // halves of the advisory, so it may only be printed when the ledger + // half was read in full: a directory that could not be enumerated, a + // file inside it that would not parse, and a reader that never loaded + // each leave installed packages unexamined, and any of them makes a + // `✓` here a false PASS. That is worse than a missing check, because + // it is the one thing that stops the operator looking further. // - // #5413 — entry-level corruption is the same claim failing one layer - // down, so it gates the `✓` in exactly the same way. The two are - // reported independently rather than as an either/or: a directory - // that read fine can still hold three unparseable files, and each - // names a different package the advisory could not look at. - if (skippedLedgerEntries.length > 0) { - hasWarnings = true; - renderHealthCheckResult( - installedPackageLedgerSkippedEntriesCheck(skippedLedgerEntries), - flags.verbose, - ); - } - // #5644 — the same claim failing one boundary UP: the reader - // package is installed and would not load, so neither the directory - // nor the entries were ever reached. Mutually exclusive with the two - // above (nothing downstream of a reader that never loaded can also - // fail), so it is an `else if` rather than a fourth independent row. - if (ledgerReaderFailure) { - hasWarnings = true; - renderHealthCheckResult( - installedPackageLedgerReaderFailureCheck(ledgerReaderFailure.cause), - flags.verbose, - ); - } else if (ledgerFailure) { - hasWarnings = true; - renderHealthCheckResult(installedPackageLedgerFailureCheck(ledgerFailure.cause), flags.verbose); - } else if (advisories.length === 0 && skippedLedgerEntries.length === 0) { + // #5429 — withholding it is ALL that survives here of those three + // issues. The findings themselves are rendered once, above, by the + // posture-independent check; re-rendering them here would report the + // same bad ledger twice under `isolated` and not at all under the + // other postures, which is the pair of defects this arrangement + // replaced. + if (advisories.length === 0 && ledgerReadingIsComplete(ledgerReading)) { printSuccess("Unique scope No unconfirmed installation-wide uniques for this 'isolated' environment"); } }