feat(cloud-connection,cli): install-local POST reports storageDir, CLI quotes it (#6721)

The install POST's `data` block now carries `storageDir: this.storageDir` —
the same resolved ledger directory (`LocalManifestSource.dir`) the GET
listing sibling already served. `os package install` runs on a different
machine and never touches the runtime's disk, so before this the CLI could
only describe the cache location by literal, and that literal was the
plugin's default — wrong for every host that configures `storageDir`.

The CLI now quotes the reported value, and prints NO directory sentence when
the response lacks the field (an older host). No literal fallback: a `??`
there would reinstate the defect PD #12 forbids and #5996 deleted.

Fixes #6721

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0158ZQo7LiHSxGWpYKuPq1wu

Author: claude

.changeset/install-local-post-storage-dir.md

@@ -0,0 +1,46 @@
+---
+"@objectstack/cloud-connection": minor
+"@objectstack/cli": patch
+---
+
+feat(cloud-connection,cli): the install-local POST reports where it cached the manifest, and `os package install` quotes it (#6721)
+
+The two endpoints of `MarketplaceInstallLocalPlugin` disagreed about one fact.
+`GET /api/v1/marketplace/install-local` — the console's Installed Apps list —
+served `storageDir: this.storageDir`, the ledger directory as resolved by
+`LocalManifestSource` (`config.storageDir` when the host set one, the
+`.objectstack/installed-packages` default when it did not). The `POST` that
+performs the install did not, so its `data` block described everything about
+the install except **where the install went**.
+
+That gap was load-bearing for the one consumer of that response. `os package
+install` runs on a different machine from the runtime it installs into: it
+speaks HTTP and never touches the target's disk. With no directory in the
+response it could only describe the cache location by literal, and the literal
+it printed was the plugin's *default* — wrong for every host that configures
+`storageDir`, and wrong today rather than eventually. No consumer-side fix
+existed: a locally-resolved constant names the wrong machine, and importing it
+from `@objectstack/cloud-connection` would make a pure-HTTP command fail at
+module load wherever that package is absent. The producer is the contract, so
+the fix is there.
+
+**`@objectstack/cloud-connection` (additive, no migration).** The install POST
+response's `data` now carries `storageDir`, read from the same
+`this.storageDir` field the GET listing already returns — one field, two
+endpoints, so they cannot drift apart again. No existing key changed, and
+nothing needs to read the new one.
+
+**`@objectstack/cli`.** The post-install hint now quotes the directory the
+runtime reported:
+
+```
+  The manifest is cached on the runtime host and re-registers on every
+  boot (survives restarts):
+    /srv/objectstack/state/ledger-packages
+```
+
+Against a runtime older than this release — one whose response has no
+`storageDir` — the CLI prints **no** directory sentence at all. It does not
+fall back to the old literal: a consumer stating a value the producer declined
+to state is the defect Prime Directive #12 forbids, and saying less is correct
+where guessing is not. Everything else the command prints is unchanged.

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

@@ -215,43 +215,32 @@ 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:
+      // #6721: the cache directory is quoted from the RESPONSE, never from a
+      // literal or a locally-resolved constant. The directory lives on the
+      // REMOTE host — everything above this line came back over HTTP from
+      // `runtime`, and this command never touches the target's disk — so the
+      // only truthful source is the host itself. #6643 documented at length why
+      // no consumer-side answer works (a local constant describes the machine
+      // typing the command; it is only the *default*, wrong the moment a host
+      // configures `storageDir`; a static import of
+      // `@objectstack/cloud-connection` would make a pure-HTTP command fail at
+      // module load). The fix was upstream, and it landed: the POST response now
+      // carries `storageDir: this.storageDir` — the same resolved value
+      // (`ledger.dir`) its GET listing sibling already served.
       //
-      //   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).');
+      // ⛔ No fallback. When the field is missing — an older host that predates
+      // the producer half — this block prints NOTHING. A `??
+      // '.objectstack/installed-packages/'` would re-introduce exactly the
+      // defect Prime Directive #12 forbids and #5996 deleted: a consumer
+      // inventing a value the producer declined to state. Saying less is
+      // correct; guessing is not. An empty or non-string value is the same
+      // "host did not state it" case, not a reason to print a blank path.
+      const storageDir = typeof data.storageDir === 'string' ? data.storageDir.trim() : '';
+      if (storageDir) {
+        console.log('  The manifest is cached on the runtime host and re-registers on every');
+        console.log('  boot (survives restarts):');
+        console.log(`    ${storageDir}`);
+      }
     } catch (error) {
       printError((error as Error).message);
       this.exit(1);

packages/cli/test/package-install-storage-dir.test.ts

@@ -0,0 +1,108 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #6721 — `os package install`'s post-install directory hint quotes the
+ * RESPONSE, and prints nothing when the response has no directory to quote.
+ *
+ * The directory lives on the runtime host, which is a different machine: this
+ * command speaks only HTTP and never touches the target's disk. #6643 measured
+ * that no consumer-side answer exists — a locally-resolved constant describes
+ * the wrong machine and is in any case only the host's *default*, wrong for
+ * every host that configures `storageDir` — and left the literal in place with
+ * a comment saying the honest fix was upstream. It landed (POST now carries
+ * `storageDir`), so the literal is gone.
+ *
+ * The absent case is the one that keeps it gone. A `?? '.objectstack/…'` bolted
+ * onto the new reference would read as a harmless kindness to older hosts and
+ * would reinstate precisely the defect Prime Directive #12 forbids and #5996
+ * deleted: a consumer stating a value the producer declined to state. Only a
+ * case that FAILS when the sentence appears anyway can hold that door shut —
+ * a test covering just the happy path leaves it open.
+ */
+
+import { describe, it, expect, afterEach, vi } from 'vitest';
+import PackageInstall from '../src/commands/package/install.js';
+
+/** The host configured its ledger somewhere no default would ever guess. */
+const REMOTE_DIR = '/srv/objectstack/state/ledger-packages';
+
+/** Stub the runtime's install POST with the given `data` block. */
+function stubRuntime(data: Record<string, unknown>): void {
+  vi.stubGlobal('fetch', vi.fn(async () => ({
+    ok: true,
+    status: 200,
+    statusText: 'OK',
+    headers: { get: () => null },
+    json: async () => ({ success: true, data }),
+  }) as any));
+}
+
+/** Run the command in catalog mode and return everything it printed. */
+async function runInstall(): Promise<string> {
+  const lines: string[] = [];
+  const capture = (...args: unknown[]) => { lines.push(args.map(String).join(' ')); };
+  vi.spyOn(console, 'log').mockImplementation(capture);
+  vi.spyOn(console, 'error').mockImplementation(capture);
+  await PackageInstall.run(['com.acme.crm', '--runtime', 'http://runtime.test']);
+  return lines.join('\n');
+}
+
+const INSTALLED = {
+  manifestId: 'com.acme.crm',
+  version: '1.0.0',
+  versionId: 'ver_1',
+  installedAt: '2026-08-10T00:00:00.000Z',
+  hotLoaded: true,
+};
+
+describe('os package install — post-install directory hint', () => {
+  afterEach(() => {
+    vi.unstubAllGlobals();
+    vi.restoreAllMocks();
+  });
+
+  it('quotes the storageDir the runtime reported, verbatim', async () => {
+    stubRuntime({ ...INSTALLED, storageDir: REMOTE_DIR });
+
+    const out = await runInstall();
+
+    expect(out).toContain('Package installed into the running kernel');
+    expect(out).toContain(REMOTE_DIR);
+    expect(out).toContain('re-registers on every');
+    // The old literal is not printed alongside the real value — the point is
+    // that the sentence has ONE source, and it is the response.
+    expect(out).not.toContain('.objectstack/installed-packages');
+  });
+
+  // The delivery criterion of this card: no fallback. An older host that
+  // predates the producer half says nothing about its ledger, so neither do we.
+  it('prints NO directory sentence when the response omits storageDir', async () => {
+    stubRuntime({ ...INSTALLED });
+
+    const out = await runInstall();
+
+    // The install itself is still reported — only the directory claim is gone.
+    expect(out).toContain('Package installed into the running kernel');
+    expect(out).toContain('com.acme.crm');
+    expect(out).not.toContain('cached');
+    expect(out).not.toContain('survives restarts');
+    expect(out).not.toContain('.objectstack/installed-packages');
+    expect(out).not.toContain('undefined');
+  });
+
+  // A host that answers with an empty/blank value has told us nothing either;
+  // that is the same case, not a licence to print a blank path.
+  it.each([
+    ['an empty string', ''],
+    ['whitespace', '   '],
+    ['null', null],
+  ])('prints no directory sentence when storageDir is %s', async (_label, value) => {
+    stubRuntime({ ...INSTALLED, storageDir: value });
+
+    const out = await runInstall();
+
+    expect(out).toContain('Package installed into the running kernel');
+    expect(out).not.toContain('cached');
+    expect(out).not.toContain('survives restarts');
+  });
+});

packages/cloud-connection/src/marketplace-install-local-plugin.ts

@@ -731,6 +731,15 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
                 upgradedFrom: conflict === 'marketplace' ? 'previous-marketplace-version' : null,
                 translationsLoaded: seededSummary.translationsLoaded,
                 seeded: seededSummary.seeded,
+                // #6721: the RESOLVED ledger directory on THIS host — the same
+                // value the GET listing serves (`handleList`), from the same
+                // field. It is here because the installer is remote: `os package
+                // install` never touches this machine's disk, so without it the
+                // CLI can only describe the directory by literal, and that
+                // literal is wrong the moment a host configures `storageDir`.
+                // Keep the two endpoints reading `this.storageDir` — a second
+                // derivation is how they diverged in the first place.
+                storageDir: this.storageDir,
                 note: 'App is now available in this runtime. Refresh the console to see it in the app switcher.',
             },
         }, 200);

packages/cloud-connection/src/marketplace-install-local-storage-dir.test.ts

@@ -0,0 +1,143 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #6721 — the install POST reports WHERE it cached the manifest.
+ *
+ * The two endpoints of this plugin had diverged on one fact. `GET
+ * /api/v1/marketplace/install-local` (the console's Installed Apps list) served
+ * `storageDir: this.storageDir`; the `POST` that performs the install did not,
+ * so its only consumer — `os package install`, which runs on a DIFFERENT machine
+ * and never touches this host's disk — had no resolved value to quote and could
+ * only describe the ledger directory by literal. That literal is the ctor's
+ * default, i.e. wrong for every host that configures `storageDir`.
+ *
+ * The non-default-`storageDir` case below is therefore the point of this file,
+ * not a variation on it: it is exactly the configuration the old literal
+ * misreported, and the only one that can tell "quotes the resolved value" apart
+ * from "restates the default".
+ *
+ * `this.storageDir` is `this.ledger.dir` — already resolved by
+ * `LocalManifestSource`'s ctor — so both endpoints must keep reading that one
+ * field. A second derivation is how they diverged in the first place, which is
+ * why the parity case asserts POST and GET are the same string rather than
+ * asserting each against a separately-computed expectation.
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { mkdtempSync, rmSync } from 'node:fs';
+import { join, resolve } from 'node:path';
+import { tmpdir } from 'node:os';
+import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js';
+import { DEFAULT_INSTALLED_PACKAGES_DIR } from './local-manifest-source.js';
+
+type Handler = (c: any) => Promise<any>;
+
+function makeRawApp() {
+    const routes = new Map<string, Handler>();
+    return {
+        routes,
+        get: (p: string, h: Handler) => routes.set(`GET ${p}`, h),
+        post: (p: string, h: Handler) => routes.set(`POST ${p}`, h),
+        delete: (p: string, h: Handler) => routes.set(`DELETE ${p}`, h),
+    };
+}
+
+function makeCtx(rawApp: any) {
+    const hooks = new Map<string, any>();
+    const services: Record<string, any> = {
+        manifest: { register: vi.fn() },
+        auth: { api: { getSession: async () => ({ user: { id: 'admin' } }) } },
+        objectql: { syncSchemas: async () => undefined },
+    };
+    return {
+        ctx: {
+            hook: (e: string, h: any) => hooks.set(e, h),
+            getService: (name: string) => {
+                if (name === 'http-server') return { getRawApp: () => rawApp };
+                const svc = services[name];
+                if (svc === undefined) throw new Error(`no ${name}`);
+                return svc;
+            },
+            logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
+        },
+        fire: async () => { await hooks.get('kernel:ready')?.(); },
+    };
+}
+
+function makeC(body: any) {
+    const json = vi.fn((payload: any, status?: number) => ({ payload, status: status ?? 200 }));
+    return {
+        req: {
+            url: 'http://localhost:3000/api/v1/marketplace/install-local',
+            raw: new Request('http://localhost:3000/x'),
+            json: async () => body,
+            param: () => undefined,
+        },
+        json,
+    };
+}
+
+/** Boot the plugin far enough that its routes are mounted. */
+async function mount(storageDir?: string) {
+    const rawApp = makeRawApp();
+    const { ctx, fire } = makeCtx(rawApp);
+    const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir });
+    await plugin.start(ctx as any);
+    await fire();
+    return {
+        install: async (manifest: any) =>
+            rawApp.routes.get('POST /api/v1/marketplace/install-local')!(makeC({ manifest })),
+        list: async () =>
+            rawApp.routes.get('GET /api/v1/marketplace/install-local')!(makeC({})),
+    };
+}
+
+const MANIFEST = { id: 'com.acme.crm', name: 'Acme CRM', version: '1.0.0', objects: [] };
+
+let dir: string;
+beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'mil-storage-dir-')); });
+afterEach(() => { rmSync(dir, { recursive: true, force: true }); vi.restoreAllMocks(); });
+
+describe('install-local POST response — storageDir', () => {
+    it('carries the RESOLVED ledger directory of a host that configured a non-default storageDir', async () => {
+        const { install } = await mount(dir);
+
+        const res = await install(MANIFEST);
+
+        expect(res.payload?.success).toBe(true);
+        // The configured directory, resolved — not the ctor default. Both
+        // halves matter: the first is the value the CLI must be able to quote,
+        // the second is the assertion the old literal would have failed.
+        expect(res.payload.data.storageDir).toBe(resolve(dir));
+        expect(res.payload.data.storageDir).not.toContain(DEFAULT_INSTALLED_PACKAGES_DIR);
+    });
+
+    it('carries the ctor-default directory when the host configured nothing', async () => {
+        // chdir into the temp dir first: with no `storageDir` the ledger
+        // resolves against `process.cwd()`, and the install genuinely writes a
+        // file there. Without this the case would litter the repo checkout.
+        const prevCwd = process.cwd();
+        process.chdir(dir);
+        try {
+            const { install } = await mount(undefined);
+
+            const res = await install(MANIFEST);
+
+            expect(res.payload?.success).toBe(true);
+            expect(res.payload.data.storageDir).toBe(resolve(process.cwd(), DEFAULT_INSTALLED_PACKAGES_DIR));
+        } finally {
+            process.chdir(prevCwd);
+        }
+    });
+
+    it('reports the same directory as the GET listing — one field, two endpoints', async () => {
+        const { install, list } = await mount(dir);
+
+        const installed = await install(MANIFEST);
+        const listed = await list();
+
+        expect(installed.payload.data.storageDir).toBe(listed.payload.data.storageDir);
+        expect(typeof installed.payload.data.storageDir).toBe('string');
+        expect(installed.payload.data.storageDir.length).toBeGreaterThan(0);
+    });
+});