From 92e47b7ca21e28a6bac28db1d988048fab9cbd62 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 07:37:54 +0000 Subject: [PATCH] fix(cli): os doctor stops guessing the installed-package ledger directory when the authority export is missing (#5996) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx --- .changeset/doctor-ledger-dir-authority.md | 13 + .../doctor-ledger-dir-authority.test.ts | 428 ++++++++++++++++++ packages/cli/src/commands/doctor.ts | 176 ++++++- 3 files changed, 601 insertions(+), 16 deletions(-) create mode 100644 .changeset/doctor-ledger-dir-authority.md create mode 100644 packages/cli/src/commands/doctor-ledger-dir-authority.test.ts diff --git a/.changeset/doctor-ledger-dir-authority.md b/.changeset/doctor-ledger-dir-authority.md new file mode 100644 index 0000000000..406ad0496f --- /dev/null +++ b/.changeset/doctor-ledger-dir-authority.md @@ -0,0 +1,13 @@ +--- +"@objectstack/cli": patch +--- + +fix(cli): `os doctor` stops guessing the installed-package ledger directory when the authority export is missing (#5996) + +Hardens a diagnosis boundary; not a live defect. `DEFAULT_INSTALLED_PACKAGES_DIR` — `@objectstack/cloud-connection`'s export, the single authority on what the ledger directory is called — exists in every version ever shipped, so the consumer-side `??` fallback this change deletes had never fired. It sat two lines above the #5413 comment forbidding exactly that tolerant read (Prime Directive #12), and it answered the wrong state: a reader that LOADS without declaring the export would have doctor silently reading a hard-coded path while the runtime keeps reading wherever the package decides — two reports, potentially two directories, no line saying so. + +Three changes, one authority: + +- The `??` fallback is gone. The export is type-checked before `path.join()` ever sees it, which also retires the old misreport where a non-string export was absorbed by the config `catch` and surfaced as "Could not load config for analysis". +- A reader that loads without declaring the directory (as a string) is now its own named report row — "The installed-package ledger reader does not declare the ledger directory (installed packages NOT checked)" — the last cell of the edge #5644 carved: that issue split "present but unloadable" out of absence's silence; this row is "loaded but unrecognizable". While it shows, doctor reads no directory at all — guessed or otherwise — and the ADR-0120 D5e advisory withholds its `✓ Unique scope` line, because its ledger half never ran. +- `installedPackageLedgerSkippedEntriesCheck`'s fix now quotes the directory doctor actually read — resolved from the real export and carried on the reading — instead of re-hardcoding the ``Under `.objectstack/installed-packages/`:`` literal, which was the same guess in prose. diff --git a/packages/cli/src/commands/doctor-ledger-dir-authority.test.ts b/packages/cli/src/commands/doctor-ledger-dir-authority.test.ts new file mode 100644 index 0000000000..5acad42170 --- /dev/null +++ b/packages/cli/src/commands/doctor-ledger-dir-authority.test.ts @@ -0,0 +1,428 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os doctor` does not GUESS the installed-package ledger's directory (#5996). + * + * ── What this file pins, honestly ──────────────────────────────────────── + * + * Not a live defect. `DEFAULT_INSTALLED_PACKAGES_DIR` is an export of + * `@objectstack/cloud-connection` in every version ever shipped, so the `??` + * fallback this change deletes had never fired. What is being hardened is a + * DIAGNOSIS BOUNDARY — the last cell of the edge #5644 carved. That issue + * split "present but unloadable" out of absence's silence; this one closes + * "loaded but unrecognizable": the module evaluates fine and does not declare, + * as a string, the one export that is the single authority on what the ledger + * directory is called. + * + * The old answer to that state was a consumer-side `??` reading a hard-coded + * `.objectstack/installed-packages` — two lines above the #5413 comment that + * forbids exactly that accommodation (Prime Directive #12). The runtime keeps + * reading wherever the package decides, so the guess could put doctor and the + * runtime on two different directories, both silent: a report over the wrong + * directory is the same false PASS as this family's other three rows, wearing + * a plausible path. The new answer is a named row, and NO read — of the + * guessed directory or any other — while the state holds. + * + * ── The second half of the same authority (#5996 part 3) ───────────────── + * + * `installedPackageLedgerSkippedEntriesCheck`'s `fix` used to re-hardcode + * ``Under `.objectstack/installed-packages/`:`` — the same guess in prose. At + * the moment that row is built doctor HOLDS the directory it actually read + * (resolved from the real export), so the row now takes it as a required + * parameter and quotes it. The unit case with a NON-default directory is the + * one a re-hardcoded literal cannot pass. + * + * ── Simulation notes ───────────────────────────────────────────────────── + * + * The authority-missing state is simulated through the loader seam + * (`../utils/optional-package.js`), the same doubling idiom the #5644 cases + * next door use: the fake module's `LocalManifestSource` RECORDS construction + * and would hand back a listing a guessed read WOULD have found (a + * global-unique entry plus a skipped file). Reaching any of that in the + * report therefore means doctor read a directory nobody authorized — those + * `not.toContain` assertions, with the constructor-call count, are what carry + * the reverse direction; restoring the `??` turns them red. + */ + +import { describe, it, expect, beforeAll, 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, { + installedPackageLedgerDirAuthorityMissingCheck, + installedPackageLedgerSkippedEntriesCheck, +} 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 success line that must NOT appear when the ledger half never ran. */ +const CLEAN_BILL = 'No unconfirmed installation-wide uniques'; + +/** The DIRECTORY-level row (#5412) — must not be confused with the new one. */ +const LEDGER_HEADLINE = 'Could not read the installed-package ledger'; + +/** The ENTRY-level row (#5413). */ +const SKIPPED_HEADLINE = 'installed-package ledger entr'; + +/** The READER-level row (#5644) — the neighbouring cell, not this one. */ +const READER_HEADLINE = 'Could not load the installed-package ledger reader'; + +/** + * The head of the AUTHORITY row (#5996). + * + * Deliberately not a superstring of any sibling headline (and vice versa), so + * every `not.toContain` in this family keeps meaning what it says whichever + * row is on screen: "read" (#5412) / "entr" (#5413) / "load … reader" (#5644) + * / "declare … directory" (here) are four different verbs for four facts. + */ +const AUTHORITY_HEADLINE = + 'The installed-package ledger reader does not declare the ledger directory'; + +/** The name column all four readability rows share since #5429. */ +const LEDGER_ROW_NAME = 'Installed packages'; + +describe('installedPackageLedgerDirAuthorityMissingCheck — loaded but unrecognizable (#5996)', () => { + it('takes the `Installed packages` name column and stays a warning, like its three siblings', () => { + const check = installedPackageLedgerDirAuthorityMissingCheck(undefined); + + expect(check.name).toBe(LEDGER_ROW_NAME); + expect(check.status).toBe('warning'); + }); + + it('says WHICH fact this is: the export is not there, and nothing was checked', () => { + const check = installedPackageLedgerDirAuthorityMissingCheck(undefined); + + expect(check.message).toContain(AUTHORITY_HEADLINE); + expect(check.message).toContain('installed packages NOT checked'); + expect(check.message).toContain('not among its exports'); + // One row is one line. + expect(check.message).not.toContain('\n'); + }); + + it('names the received value when the export exists but is not a string', () => { + const check = installedPackageLedgerDirAuthorityMissingCheck(42); + + expect(check.message).toContain('not a string'); + expect(check.message).toContain('got number: 42'); + }); + + it('is none of its three siblings — four facts, four headlines', () => { + const check = installedPackageLedgerDirAuthorityMissingCheck(undefined); + + expect(check.message).not.toContain(LEDGER_HEADLINE); + expect(check.message).not.toContain(SKIPPED_HEADLINE); + expect(check.message).not.toContain(READER_HEADLINE); + }); + + it('says the package is present AND loaded — the two things that separate it from its kin', () => { + const fix = installedPackageLedgerDirAuthorityMissingCheck(undefined).fix ?? ''; + + // "IS installed" separates it from absence's silence; "DID load" separates + // it from the #5644 row one cell over. Without both, the reader's first + // move would be to rebuild a package that builds and loads fine. + expect(fix).toContain('IS installed here and it DID load'); + expect(fix).toContain('authority on what the ledger directory is called'); + // And it names the remedy: the two ends of this contract disagree. + expect(fix).toContain('Align their versions'); + }); + + it('claims no directory path, anywhere — naming one is the knowledge this row just lost', () => { + const check = installedPackageLedgerDirAuthorityMissingCheck(undefined); + + // The one name doctor had for the ledger is the export that came back + // non-string. A path in the row — the old `??` guess included — would be + // doctor asserting what it cannot know (#5644's option-B rejection, one + // cell over). + expect(check.message).not.toContain('.objectstack'); + expect(check.fix ?? '').not.toContain('.objectstack'); + }); +}); + +describe('installedPackageLedgerSkippedEntriesCheck — names the directory it actually read (#5996)', () => { + const oneSkipped = [{ file: 'broken.json', cause: new Error('Unexpected end of JSON input') }]; + + it('quotes the resolved directory in the fix', () => { + const check = installedPackageLedgerSkippedEntriesCheck( + oneSkipped, + '/srv/app/.objectstack/installed-packages', + ); + + expect(check.fix).toContain('Under `/srv/app/.objectstack/installed-packages`:'); + expect(check.fix).toContain('broken.json'); + }); + + it('a NON-default authority value flows through — the assertion a re-hardcoded literal cannot pass', () => { + // The directory parameter exists precisely so this row tracks the + // producer's export instead of restating the consumer's old guess. If the + // literal ever creeps back, this is the case that goes red. + const check = installedPackageLedgerSkippedEntriesCheck(oneSkipped, '/srv/app/.objectstack/pkgs-v2'); + + expect(check.fix).toContain('Under `/srv/app/.objectstack/pkgs-v2`:'); + expect(check.fix).not.toContain('.objectstack/installed-packages'); + }); +}); + +/** + * ── End to end: the export is missing, and doctor reads NOTHING ────────── + * + * Simulated through the loader seam, like the #5644 cause case next door: the + * mock hands doctor a module that LOADED (`state: 'loaded'`) and carries no + * `DEFAULT_INSTALLED_PACKAGES_DIR`. No preflight (`assertLedgerReaderIsBuilt`) + * in this describe, and that is not a rollback of #5612: nothing here reads + * the real package at all — the seam is mocked — so there is no accident for + * a preflight to catch. + */ +describe('os doctor, end to end, against a reader that declares no ledger directory (#5996)', () => { + afterEach(() => { + vi.doUnmock('../utils/optional-package.js'); + vi.resetModules(); + }); + + /** A listing a guessed read WOULD have found — see the header. */ + const guessedListing = () => ({ + entries: [ + { + manifestId: 'billing', + manifest: { + objects: [ + { + name: 'invoice', + label: 'Invoice', + fields: [{ name: 'code', type: 'text', label: 'Code', unique: 'global' }], + }, + ], + }, + }, + ], + skipped: [{ file: 'broken.json', cause: new Error('Unexpected end of JSON input') }], + }); + + async function runAgainstModule( + fakeModule: Record, + argv: string[], + ): Promise<{ out: string; exitCode: number | undefined }> { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'os-doctor-5996-e2e-')); + // Without `node_modules/` doctor's Dependencies check is an `error` on its + // own and exits 1 — the trap PR #5390 wrote down. + fs.mkdirSync(path.join(tmp, 'node_modules')); + fs.writeFileSync( + path.join(tmp, 'objectstack.config.ts'), + [ + 'export default {', + " manifest: { name: 'os5996', label: 'No Authority', version: '1.0.0' },", + " objects: [{ name: 'account', label: 'Account', fields: [{ name: 'name', type: 'text', label: 'Name' }] }],", + '};', + '', + ].join('\n'), + ); + + const savedPosture = process.env.OS_TENANCY_POSTURE; + // D5e's advisory only runs under `isolated`; withholding its clean bill is + // one of the facts under test. + process.env.OS_TENANCY_POSTURE = 'isolated'; + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmp); + + vi.resetModules(); + vi.doMock('../utils/optional-package.js', () => ({ + loadOptionalPackage: async () => ({ state: 'loaded', module: fakeModule }), + })); + const { default: FreshDoctor } = await import('./doctor.js'); + + 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 FreshDoctor.run(argv, { root: CLI_ROOT }); + } catch (err) { + if (!(err instanceof Error) || !err.message.startsWith('__PROCESS_EXIT__')) throw err; + } finally { + logSpy.mockRestore(); + exitSpy.mockRestore(); + cwdSpy.mockRestore(); + if (savedPosture === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = savedPosture; + fs.rmSync(tmp, { recursive: true, force: true }); + } + return { out: plain(logs.join('\n')), exitCode }; + } + + it('reports the named row and reads NO directory — guessed or otherwise', async () => { + const ctorCalls: string[] = []; + const fakeModule = { + // Deliberately NO `DEFAULT_INSTALLED_PACKAGES_DIR` — the state under test. + LocalManifestSource: class { + constructor(dir: string) { + ctorCalls.push(dir); + } + list() { + return guessedListing(); + } + }, + }; + + const plainRun = await runAgainstModule(fakeModule, []); + const verboseRun = await runAgainstModule(fakeModule, ['--verbose']); + + // ① THE assertion of #5996: no read happened. The old `??` constructed the + // source over a guessed path; the fake records every construction. + expect(ctorCalls).toHaveLength(0); + // ② …so nothing a guessed read would have found is in the report: not the + // listing's global-unique finding, not its skipped file. + expect(plainRun.out).not.toContain('invoice.code'); + expect(plainRun.out).not.toContain('broken.json'); + expect(plainRun.out).not.toContain(SKIPPED_HEADLINE); + // ③ The state is its own named row, under the family's name column… + expect(plainRun.out).toContain(AUTHORITY_HEADLINE); + expect(plainRun.out).toContain(LEDGER_ROW_NAME); + // ④ …and none of its three siblings': the directory was never touched and + // the reader loaded fine. + expect(plainRun.out).not.toContain(LEDGER_HEADLINE); + expect(plainRun.out).not.toContain(READER_HEADLINE); + // ⑤ The D5e advisory withholds its clean bill — its ledger half never ran. + expect(plainRun.out).not.toContain(CLEAN_BILL); + // ⑥ Warning-tier detail follows the family rule: only under --verbose. + expect(plainRun.out).not.toContain('saw:'); + expect(verboseRun.out).toContain('saw:'); + expect(verboseRun.out).toContain('IS installed here and it DID load'); + // Gauge: warning, the report finishes, exit stays 0. + expect(plainRun.out).toContain('Environment is functional but has some warnings'); + expect(plainRun.exitCode).toBeUndefined(); + expect(verboseRun.exitCode).toBeUndefined(); + }, 120_000); + + it('a non-string export is the same fact — never a path.join over it', async () => { + const ctorCalls: string[] = []; + const fakeModule = { + // Present, and not a string: the shape whose `path.join` throw the old + // code mis-reported through the config `catch` (see the function's + // header in doctor.ts). + DEFAULT_INSTALLED_PACKAGES_DIR: 42, + LocalManifestSource: class { + constructor(dir: string) { + ctorCalls.push(dir); + } + list() { + return guessedListing(); + } + }, + }; + + const run = await runAgainstModule(fakeModule, ['--verbose']); + + expect(ctorCalls).toHaveLength(0); + expect(run.out).toContain(AUTHORITY_HEADLINE); + expect(run.out).toContain('not a string'); + expect(run.out).toContain('got number: 42'); + expect(run.out).not.toContain(CLEAN_BILL); + expect(run.out).not.toContain('invoice.code'); + expect(run.exitCode).toBeUndefined(); + }, 120_000); +}); + +/** + * ── End to end: the skipped-entries row names the REAL directory ───────── + * + * Against the real `@objectstack/cloud-connection`, so this describe needs the + * #5612 preflight: in a worktree where that package is unbuilt, doctor's + * deliberate silence would turn the case below into an assertion diff that + * reads like the report face regressed. + */ +describe('os doctor, end to end, skipped-entries fix quotes the resolved directory (#5996)', () => { + beforeAll(async () => { + let mod: Record; + try { + mod = await import('@objectstack/cloud-connection'); + } catch (err) { + throw new Error( + 'Preflight failed (#5612): `@objectstack/cloud-connection` did not load, so the case ' + + 'below cannot observe the ledger row at all. Build the dependency graph first:\n' + + " pnpm --workspace-concurrency=2 --filter '@objectstack/cli^...' build\n\n" + + `cause: ${err instanceof Error ? err.message : String(err)}`, + ); + } + // A dist built before #5413 returns a bare array and derails into the + // directory-level row — the §9 stale-artefact trap, made loud. + const listing = new mod.LocalManifestSource(path.join(os.tmpdir(), 'os-5996-preflight-absent')).list(); + if (!Array.isArray(listing?.entries) || !Array.isArray(listing?.skipped)) { + throw new Error( + 'Preflight failed (#5612): the built `@objectstack/cloud-connection` predates the ' + + '#5413 `{ entries, skipped }` listing. Rebuild it.', + ); + } + }); + + it('the verbose fix says Under ``, never the re-hardcoded default', async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'os-doctor-5996-dir-')); + fs.mkdirSync(path.join(tmp, 'node_modules')); + fs.writeFileSync( + path.join(tmp, 'objectstack.config.ts'), + [ + 'export default {', + " manifest: { name: 'os5996d', label: 'Real Dir', version: '1.0.0' },", + " objects: [{ name: 'account', label: 'Account', fields: [{ name: 'name', type: 'text', label: 'Name' }] }],", + '};', + '', + ].join('\n'), + ); + // The directory the REAL export resolves to under this cwd. Unique per + // run (mkdtemp), which is what makes "the resolved dir, not the literal" + // assertable at all: the literal can never contain it. + const ledgerDir = path.join(tmp, '.objectstack/installed-packages'); + fs.mkdirSync(ledgerDir, { recursive: true }); + fs.writeFileSync(path.join(ledgerDir, 'broken.json'), '{"manifestId":"broken"'); + + const savedPosture = process.env.OS_TENANCY_POSTURE; + process.env.OS_TENANCY_POSTURE = 'isolated'; + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmp); + + const logs: string[] = []; + const logSpy = vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { + logs.push(a.join(' ')); + }); + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`__PROCESS_EXIT__:${code}`); + }) as never); + + try { + await Doctor.run(['--verbose'], { root: CLI_ROOT }); + } catch (err) { + if (!(err instanceof Error) || !err.message.startsWith('__PROCESS_EXIT__')) throw err; + } finally { + logSpy.mockRestore(); + exitSpy.mockRestore(); + cwdSpy.mockRestore(); + if (savedPosture === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = savedPosture; + fs.rmSync(tmp, { recursive: true, force: true }); + } + + const out = plain(logs.join('\n')); + + // The row itself is unchanged — pinned next door in + // `doctor-ledger-read-failure.test.ts`. What #5996 changed is WHERE the + // fix says the files live: the directory doctor actually read. + expect(out).toContain(SKIPPED_HEADLINE); + expect(out).toContain('broken.json'); + expect(out).toContain(`Under \`${ledgerDir}\`:`); + expect(out).not.toContain('Under `.objectstack/installed-packages/`'); + }, 120_000); +}); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 518efda8ff..6b4434eca1 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -899,6 +899,30 @@ export interface InstalledPackageLedgerReading { * directory name is the missing package's own constant. */ readerFailure?: { cause: unknown }; + /** + * Present ONLY when the reader package LOADED and its + * `DEFAULT_INSTALLED_PACKAGES_DIR` export is not a string (#5996). + * + * A FOURTH fact — the last cell of the edge #5644 carved. `readerFailure` is + * "present but unloadable"; this is "loaded but unrecognizable". That export + * is the single authority on where the ledger lives, so a reading in this + * state carries no `dir` and read no directory at all: the `??` fallback + * that used to guess `.objectstack/installed-packages` in its place was the + * tolerant consumer read Prime Directive #12 forbids, two lines from the + * #5413 comment saying so. `received` is what the export actually was. + */ + dirAuthorityMissing?: { received: unknown }; + /** + * The resolved ledger directory — `cwd` joined with the producer's + * `DEFAULT_INSTALLED_PACKAGES_DIR` export (#5996). Present exactly when that + * export was read successfully, whatever happened afterwards (a `failure` + * reading still carries it); absent when the reader never loaded + * (`readerFailure`) or loaded without declaring it (`dirAuthorityMissing`). + * Rows that name the directory take it from HERE, never from a re-hardcoded + * literal: the literal is the consumer restating what only the producer + * decides. + */ + dir?: string; } /** @@ -957,15 +981,29 @@ interface SkippedLedgerEntry { * separated by `loadOptionalPackage()` (`utils/optional-package.ts` carries how, * and the measurements behind it); only the genuinely-absent half stays silent. * + * And a FIFTH — the cell the fourth's edge left uncovered (#5996). `loaded` is + * still not RECOGNIZED: the ledger directory's name is the module's own + * `DEFAULT_INSTALLED_PACKAGES_DIR` export — the single authority on that name — + * and a module that evaluates fine while declaring no such string is a reader + * this doctor does not know how to follow. The consumer-side `??` that used to + * sit on that export answered the state with a hard-coded guess, two lines + * above the #5413 comment that forbids exactly that accommodation (Prime + * Directive #12), while the runtime kept reading wherever the package decides — + * two reports, potentially two directories, no line saying so. The guess is + * gone: the state comes back as `dirAuthorityMissing`, and no directory — + * guessed or otherwise — is read while it holds. + * * 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". + * else. It must therefore report rather than throw for anything it can hit, + * which is why the export is TYPE-CHECKED before `path.join()` ever sees it + * (#5996): the join over two known strings cannot throw. Before the check + * existed, the same hazard was handled positionally — the join lived INSIDE the + * guarded block, because 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". The named reading replaces both the guess and the throw. */ async function readInstalledPackageEntries(cwd: string): Promise { // Dynamic, like serve.ts's cloud-connection load: `os doctor` must still run @@ -984,32 +1022,54 @@ async function readInstalledPackageEntries(cwd: string): Promise ({ file: s.file, cause: describeThrown(s.cause) })); const n = described.length; @@ -1419,7 +1486,7 @@ export function installedPackageLedgerSkippedEntriesCheck( + ' the kernel and does not appear in the console\'s installed-apps list — so an\n' + ' app missing from this environment is very likely one of the files below.\n' + ' Repair the JSON, or delete the file to uninstall the package for real.\n' - + ' Under `.objectstack/installed-packages/`:\n' + + ` Under \`${dir}\`:\n` + described .map((s) => ` ${s.file}\n cause: ${indentUnderGutter(s.cause).replace(/\n/g, '\n ')}`) .join('\n'), @@ -1478,6 +1545,69 @@ export function installedPackageLedgerReaderFailureCheck(err: unknown): HealthCh }; } +/** + * Say what the directory-authority export actually was, for the one row that + * reports it missing (#5996). Nothing was THROWN in this state — the module + * loaded fine — so this is a sibling of `describeThrown` for a received value + * rather than a caught one. + */ +function describeMissingDirAuthority(received: unknown): string { + if (received === undefined) { + return 'DEFAULT_INSTALLED_PACKAGES_DIR is not among its exports'; + } + return `DEFAULT_INSTALLED_PACKAGES_DIR is not a string (got ${typeof received}: ${String(received)})`; +} + +/** + * What doctor reports when the package it reads ledgers through LOADED and + * does not declare where the ledger lives (#5996). + * + * The fourth sibling of `installedPackageLedgerFailureCheck`, and the last + * cell of the edge #5644 carved. That issue split "present but unloadable" out + * of absence's silence; this row is "loaded but unrecognizable": the module + * evaluated fine and its `DEFAULT_INSTALLED_PACKAGES_DIR` export — the single + * authority on what the ledger directory is called — is not a string. Like its + * three siblings the row takes the {@link LEDGER_ROW_NAME} name column, makes + * the D5e advisory withhold its `✓ Unique scope` line, and stays a warning. + * + * What the row replaces is not silence but a GUESS. A consumer-side `??` used + * to answer this state by reading a hard-coded `.objectstack/installed-packages` + * instead — two lines above the #5413 comment prohibiting exactly that + * accommodation (Prime Directive #12). The runtime keeps reading wherever the + * package decides, so the guess could have doctor and the runtime reporting on + * two different directories with neither saying so; a report over the wrong + * directory is the same false PASS as this family's other three, wearing a + * plausible path. + * + * What is deliberately NOT here, in the row or behind it: any directory path. + * The one name doctor had for the ledger is the export that just came back + * non-string, so naming a path — any path — would be doctor claiming the + * knowledge whose loss this row reports. Same ground as #5644's option-B + * rejection, one cell over: while this row shows, no directory is read at all. + */ +export function installedPackageLedgerDirAuthorityMissingCheck(received: unknown): HealthCheckResult { + const saw = describeMissingDirAuthority(received); + return { + name: LEDGER_ROW_NAME, + status: 'warning', + message: + 'The installed-package ledger reader does not declare the ledger directory (installed packages NOT checked) — ' + + reportRowHeadline(saw), + fix: + '`@objectstack/cloud-connection` IS installed here and it DID load — but it does not\n' + + ' export `DEFAULT_INSTALLED_PACKAGES_DIR` as a string, and that export is the single\n' + + ' authority on what the ledger directory is called. Doctor will not read a guessed\n' + + ' path in its place: the ledger’s location belongs to that package — the runtime\n' + + ' reads wherever it decides — so a guess here could have doctor and the runtime\n' + + ' reporting on two different directories with neither saying so. Nothing about the\n' + + ' installed packages was read; whether a ledger even exists went unasked.\n' + + ' A package that loads without this export is not one this doctor recognizes: the\n' + + ' installed `@objectstack/cloud-connection` and this `os` CLI disagree about that\n' + + ' package’s export surface. Align their versions (upgrade the older of the two).\n' + + ` saw: ${indentUnderGutter(saw)}`, + }; +} + /** * Every readability finding one ledger reading produced — the whole of the * posture-independent check #5429 promoted out of the D5e block. @@ -1533,18 +1663,32 @@ 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 + // reached — mutually exclusive with the rows below rather than another // independent one (#5644). if (reading.readerFailure) { return [installedPackageLedgerReaderFailureCheck(reading.readerFailure.cause)]; } + // The reader loaded and never said where the ledger is (#5996) — nothing + // below the export read was reached, so mutually exclusive with both + // remaining rows for the same reason as above. + if (reading.dirAuthorityMissing) { + return [installedPackageLedgerDirAuthorityMissingCheck(reading.dirAuthorityMissing.received)]; + } 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)); + if (reading.skipped.length > 0) { + // A reading with skipped entries always carries the directory they were + // skipped IN: `skipped` is per-file fallout of an enumeration only the + // resolved `dir` makes possible, and `readInstalledPackageEntries()` sets + // both on the same return. The `!` states that invariant where the flat + // reading shape cannot (#5996) — the parameter stays required so the row + // can never quietly fall back to a guessed literal. + out.push(installedPackageLedgerSkippedEntriesCheck(reading.skipped, reading.dir!)); + } return out; }