fix(cli): doctor 的账本读取失败行改报真实解析出的目录,install 提示注明其字面量为描述性 (#6643)

.changeset/doctor-ledger-failure-dir.md

@@ -0,0 +1,12 @@
+---
+"@objectstack/cli": patch
+---
+
+fix(cli): `os doctor`'s ledger-failure row names the directory it actually read (#6643)
+
+Removes divergence surface; not a live defect. `DEFAULT_INSTALLED_PACKAGES_DIR` — `@objectstack/cloud-connection`'s export, the single authority on what the installed-package ledger directory is called — exists in every version ever shipped and has never changed value, so the literal this change deletes currently agrees with it. What it buys is that the agreement stops being a coincidence nobody would notice breaking.
+
+The residue of #5996, which fixed the same restatement one row over (`installedPackageLedgerSkippedEntriesCheck`) and enumerated the rest rather than widening in place:
+
+- `installedPackageLedgerFailureCheck` takes the resolved `dir` — already carried on the reading since #5996 — and quotes it. Its `fix` used to open with a re-hardcoded ``.objectstack/installed-packages/`` "under the project root", which was the consumer restating a value only the producer decides, and a vaguer answer than the one doctor was holding: the reading's `dir` is `cwd`-joined and absolute. Under `--verbose` the row now reads ``The ledger is `/srv/app/.objectstack/installed-packages`;`` and drops the now-redundant project-root hedge. The parameter is required, so the row cannot quietly fall back to a guess.
+- `os package install`'s post-install hint keeps its literal, now with the reasons written down. That sentence describes the **remote** runtime host's directory: the CLI never touches that disk, the host's directory is configurable (`MarketplaceInstallLocalPlugin` builds `new LocalManifestSource(config.storageDir)`, so the export is only its default), and the install response carries no `storageDir` to quote. Resolving it locally would state this machine's default as an observed fact about another one — so the literal stays, marked as the description of a convention rather than a consumer read.

packages/cli/src/commands/doctor-ledger-read-failure.test.ts

@@ -210,11 +210,20 @@ async function assertLedgerReaderIsBuilt(): Promise<void> {
 }
 
 describe('installedPackageLedgerFailureCheck — the finding the shared catch used to eat', () => {
+ /**
+ * The resolved ledger directory the reading carries (#5996), which this row
+ * has taken as a required parameter since #6643. Deliberately rooted at
+ * `/srv/app` rather than left relative: the old hard-coded literal was
+ * `.objectstack/installed-packages/` with no root, so any assertion naming
+ * this path is one the literal cannot satisfy.
+ */
+ const DIR = '/srv/app/.objectstack/installed-packages';
+
  it('quotes what was thrown, in the row AND in the verbose detail', () => {
  const err = Object.assign(new Error("ENOTDIR: not a directory, scandir '/p/.objectstack'"), {
  code: 'ENOTDIR',
  });
- const check = installedPackageLedgerFailureCheck(err);
+ const check = installedPackageLedgerFailureCheck(err, DIR);
 
  // Before #5412 this text existed nowhere in any doctor output under any
  // flag — the error object was discarded at the point it was caught.
@@ -223,7 +232,7 @@ describe('installedPackageLedgerFailureCheck — the finding the shared catch us
  });
 
  it('takes the `Installed packages` name column, so the row is present rather than missing', () => {
- const check = installedPackageLedgerFailureCheck(new Error('boom'));
+ const check = installedPackageLedgerFailureCheck(new Error('boom'), DIR);
 
  // Load-bearing, not cosmetic. An operator scans the report by its name
  // column, and this row has to be somewhere findable rather than absent —
@@ -238,22 +247,25 @@ describe('installedPackageLedgerFailureCheck — the finding the shared catch us
  });
 
  it('stays a warning — the environment runs, doctor’s sight of it is what broke', () => {
- expect(installedPackageLedgerFailureCheck(new Error('boom')).status).toBe('warning');
+ expect(installedPackageLedgerFailureCheck(new Error('boom'), DIR).status).toBe('warning');
  });
 
  it('says WHICH half did not run, rather than that "something" failed', () => {
- const check = installedPackageLedgerFailureCheck(new Error('boom'));
+ const check = installedPackageLedgerFailureCheck(new Error('boom'), DIR);
  const fix = check.fix ?? '';
 
  // The harm the issue names is a reader treating a partial check as a whole
  // one. The row has to state its own incompleteness in both channels.
  expect(check.message).toContain('installed packages NOT checked');
  expect(fix).toContain('two halves and only one of them ran');
- expect(fix).toContain('.objectstack/installed-packages/');
+ // Re-pointed by #6643. This used to assert the bare
+ // `.objectstack/installed-packages/` literal the fix restated; the row now
+ // names the directory doctor actually read, so the assertion follows it.
+ expect(fix).toContain(DIR);
  });
 
  it('folds a multi-line cause onto the row and keeps it whole in the detail', () => {
- const check = installedPackageLedgerFailureCheck(new Error('line one\nline two: the reason'));
+ const check = installedPackageLedgerFailureCheck(new Error('line one\nline two: the reason'), DIR);
 
  expect(check.message).toContain('line two: the reason');
  // One row is one line.
@@ -262,15 +274,37 @@ describe('installedPackageLedgerFailureCheck — the finding the shared catch us
  });
 
  it('never trails off into nothing for an Error with no message', () => {
- const check = installedPackageLedgerFailureCheck(new TypeError());
+ const check = installedPackageLedgerFailureCheck(new TypeError(), DIR);
 
  expect(check.message.endsWith('— ')).toBe(false);
  expect(check.message).toContain('TypeError');
  });
 
  it('reports a thrown non-Error rather than swallowing it', () => {
- expect(installedPackageLedgerFailureCheck('boom').message).toContain('boom');
- expect(installedPackageLedgerFailureCheck(42).message).toContain('42');
+ expect(installedPackageLedgerFailureCheck('boom', DIR).message).toContain('boom');
+ expect(installedPackageLedgerFailureCheck(42, DIR).message).toContain('42');
+ });
+
+ it('a NON-default directory flows through — the assertion a re-hardcoded literal cannot pass', () => {
+ // #6643, the sibling of #5996's identical case on
+ // `installedPackageLedgerSkippedEntriesCheck`. The parameter exists so this
+ // row tracks the producer's `DEFAULT_INSTALLED_PACKAGES_DIR` instead of
+ // restating the consumer's old guess. A directory sharing NO substring with
+ // that guess is what separates the two: if the literal ever creeps back,
+ // both assertions below go red at once.
+ const check = installedPackageLedgerFailureCheck(new Error('boom'), '/var/lib/os-ledger');
+
+ expect(check.fix).toContain('The ledger is `/var/lib/os-ledger`;');
+ expect(check.fix).not.toContain('.objectstack/installed-packages');
+ });
+
+ it('drops the "under the project root" hedge — the resolved dir already says where it is', () => {
+ // The old literal was relative, so the row had to add a sentence locating
+ // it. `dir` is `cwd`-joined and absolute, which makes that sentence both
+ // redundant and (for a non-default `storageDir`) wrong.
+ const check = installedPackageLedgerFailureCheck(new Error('boom'), DIR);
+
+ expect(check.fix).not.toContain('project root');
  });
 });
 
@@ -430,6 +464,31 @@ describe('os doctor, end to end, against an unreadable installed-package ledger'
  expect(run.exitCode).toBeUndefined();
  }, 60_000);
 
+ it('the verbose fix names the directory doctor actually read, resolved from the authority (#6643)', async () => {
+ writeConfig();
+ fs.mkdirSync(path.dirname(ledgerPath()), { recursive: true });
+ fs.writeFileSync(ledgerPath(), 'not a directory\n');
+
+ // The expectation is COMPUTED FROM THE AUTHORITY, never hard-coded: the
+ // whole point of #6643 is that this row stops restating a value only
+ // `@objectstack/cloud-connection` decides. Writing `.objectstack/
+ // installed-packages` here would move the literal into the test and pin
+ // the row against the guess a second time — the defect, relocated.
+ // Re-imported rather than read off `LEDGER_REL` for the same reason.
+ const { DEFAULT_INSTALLED_PACKAGES_DIR } = await import('@objectstack/cloud-connection');
+ // `mkdtemp` makes this unique per run, which is what makes "the resolved
+ // dir, not the literal" assertable at all: no literal can contain it.
+ const resolved = path.join(tmp, DEFAULT_INSTALLED_PACKAGES_DIR);
+
+ const run = await runDoctor(['--verbose']);
+
+ expect(run.out).toContain(LEDGER_HEADLINE);
+ expect(run.out).toContain(`The ledger is \`${resolved}\`;`);
+ // And the relative literal it replaced is gone from the report entirely.
+ expect(run.out).not.toContain('The ledger is `.objectstack/installed-packages/` directory');
+ expect(run.out).not.toContain('under the\n project root');
+ }, 60_000);
+
  it('expands the detail under --verbose, and only under --verbose', async () => {
  writeConfig();
  fs.mkdirSync(path.dirname(ledgerPath()), { recursive: true });

packages/cli/src/commands/doctor.ts

@@ -1408,17 +1408,25 @@ const LEDGER_ROW_NAME = 'Installed packages';
  * • **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.
+ * • **`dir` is the directory doctor actually read** (#6643) — resolved from
+ * `DEFAULT_INSTALLED_PACKAGES_DIR` and carried on the reading, exactly as
+ * its `skipped` sibling takes it since #5996. It used to open with a
+ * re-hardcoded ``.objectstack/installed-packages/`` literal "under the
+ * project root": the consumer restating a value only the producer decides,
+ * and — since the resolved directory is `cwd`-joined — a vaguer answer than
+ * the one doctor was holding. A row reporting an unreadable directory owes
+ * the reader the directory it actually tried.
  */
-export function installedPackageLedgerFailureCheck(err: unknown): HealthCheckResult {
+export function installedPackageLedgerFailureCheck(err: unknown, dir: string): HealthCheckResult {
  const cause = describeThrown(err);
  return {
  name: LEDGER_ROW_NAME,
  status: 'warning',
  message: `Could not read the installed-package ledger (installed packages NOT checked) — ${reportRowHeadline(cause)}`,
  fix:
-'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". Every package it lists is one\n'
+`The ledger is \`${dir}\`; it exists here, which is why\n`
+ + ' this is reported rather than treated as "nothing was ever\n'
+ + ' 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'
@@ -1675,7 +1683,14 @@ export function installedPackageLedgerChecks(
  return [installedPackageLedgerDirAuthorityMissingCheck(reading.dirAuthorityMissing.received)];
  }
  const out: HealthCheckResult[] = [];
- if (reading.failure) out.push(installedPackageLedgerFailureCheck(reading.failure.cause));
+ // Same invariant as the `skipped` row below, one boundary out (#6643): a
+ // `failure` reading always carries the directory the failure was ABOUT.
+ // `failure` is set only in the `catch` of `readInstalledPackageEntries()`,
+ // and that `try` opens after `dir` is already resolved — the two are set on
+ // the same return. The `!` states that where the flat reading shape cannot,
+ // and the parameter stays required so this row can never quietly fall back
+ // to a guessed literal.
+ if (reading.failure) out.push(installedPackageLedgerFailureCheck(reading.failure.cause, reading.dir!));
  // 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

packages/cli/src/commands/package/install.ts

@@ -215,6 +215,41 @@ export default class PackageInstall extends Command {
  printKV(' Runtime', runtime);
  if (data.installedAt) printKV(' Installed', String(data.installedAt));
  console.log('');
+ // ⚠️ This path is a DESCRIPTION OF THE DEFAULT CONVENTION, deliberately
+ // left as a literal — not a consumer restating a value it could have read
+ // (#6643, the sibling half of #5996). Do not "fix" it into a reference to
+ // `DEFAULT_INSTALLED_PACKAGES_DIR`. Four measured reasons, in order:
+ //
+ // 1. **The directory is on the REMOTE host.** Everything above this
+ // line came back over HTTP from `runtime`; this command never
+ // touches the target's disk. A constant resolved HERE describes the
+ // machine typing the command, not the one that stored the manifest.
+ // 2. **The remote's directory is configurable, so the constant is only
+ // its default.** `MarketplaceInstallLocalPlugin` builds its ledger as
+ // `new LocalManifestSource(config.storageDir)` — the export is the
+ // fallback that ctor applies when the host configured nothing.
+ // Interpolating it would state a default as an observed fact.
+ // 3. **The response we just read does not carry the real answer.** The
+ // POST returns `{ manifestId, version, versionId, installedAt,
+ // hotLoaded, upgradedFrom, translationsLoaded, seeded, note }` — no
+ // `storageDir`. The GET listing endpoint does carry one; this one
+ // does not, and asking for it would be an extra round-trip (and a
+ // new failure mode) bolted onto a success hint.
+ // 4. **Importing it would cost this command its independence.** Every
+ // CLI reference to `@objectstack/cloud-connection` is a DYNAMIC load
+ // behind `loadOptionalPackage()` (doctor.ts) or a guarded `import()`
+ // (serve.ts), because the CLI must keep working where that package
+ // is absent or unbuilt. A static import for one hint line would make
+ // a pure-HTTP command fail at module load; a dynamic one needs a
+ // literal fallback — the exact `??` Prime Directive #12 forbids and
+ // #5996 deleted.
+ //
+ // So the divergence surface is accepted here, knowingly: if the constant
+ // ever changes value, this sentence goes stale and no gate will say so.
+ // The honest fix is upstream — the POST response carrying `storageDir`
+ // like its GET sibling — which would let this line quote the real remote
+ // directory. Tracked separately; `@objectstack/cloud-connection` is out
+ // of scope for #6643.
  console.log(' The manifest is cached under .objectstack/installed-packages/ on the');
  console.log(' runtime host and re-registers on every boot (survives restarts).');
  } catch (error) {