fix(cloud-connection): don't refuse upgrading an orphaned marketplace install (#2174)

xuyushun441-sys · hotlong · claude · web-flow · commit c213ed8fd659 · 2026-06-22T14:00:27.000+08:00
`findConflict` classified any manifest present in the engine registry but
absent from the on-disk ledger as `user-code`, and the install endpoint then
refused the upgrade with "manifest_id … is already defined by this runtime's
local code. Refusing to overwrite" (and told the user to edit
objectstack.config.ts — where it isn't).

But a marketplace package is hot-registered AND recorded in the ledger. If the
ledger entry goes missing or out of sync — a half-finished upgrade that left a
`.bak`, a manual cleanup, a corrupted file — the manifest stays registered while
`ledger.has()` returns false. The package is now an ORPHANED marketplace
install, not local code, yet every reinstall/upgrade was blocked with a
misleading error. (Hit live: a stale `app.objectstack.template.project.json.bak`
made the showcase refuse to install the fixed version.)

Disambiguate by source instead of by "is it registered":
- Snapshot the manifest ids already in the engine registry at `kernel:ready`,
  BEFORE rehydrating the ledger — those are genuine AppPlugin/config code.
- findConflict: ledger entry ⇒ 'marketplace'; in the boot snapshot ⇒ 'user-code'
  (still protected); registered but in neither ⇒ orphaned marketplace install ⇒
  'marketplace' (allow the upgrade to overwrite) rather than 'user-code'.

Adds `marketplace-install-local-conflict.test.ts` (3 cases: real user code
refused, orphaned install allowed, fresh install clean).

Co-authored-by: Jack Zhuang &lt;zhuangjianguo@gmail.com&gt;
Co-authored-by: Claude Opus 4.8 &lt;noreply@anthropic.com&gt;
diff --git a/packages/cloud-connection/src/marketplace-install-local-conflict.test.ts b/packages/cloud-connection/src/marketplace-install-local-conflict.test.ts
@@ -0,0 +1,125 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * findConflict: an ORPHANED marketplace install (registered in the engine but
+ * with no ledger entry on disk — e.g. a half-finished upgrade left the entry as
+ * a `.bak`) must NOT be mistaken for user/config code. Previously such a state
+ * returned 'user-code' and the install endpoint refused the upgrade with a
+ * misleading "defined by this runtime's local code" 409. It now resolves to
+ * 'marketplace' so the upgrade can overwrite it. Genuine user code (present in
+ * the registry at boot, before rehydrate) is still protected.
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { mkdtempSync, rmSync, readdirSync } from 'node:fs';
+import { join } from 'node:path';
+import { tmpdir } from 'node:os';
+import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.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),
+    };
+}
+
+/**
+ * ctx whose `objectql` exposes a registry backed by `registry` (an array). The
+ * `manifest.register()` mock pushes into it, so hot-registering a manifest makes
+ * the engine "know" it — exactly what findConflict reads via getAllPackages().
+ */
+function makeCtx(rawApp: any, registry: any[]) {
+    const hooks = new Map<string, any>();
+    const services: Record<string, any> = {
+        manifest: { register: (m: any) => registry.push({ manifest: m }) },
+        auth: { api: { getSession: async () => ({ user: { id: 'admin' } }) } },
+        objectql: { syncSchemas: async () => undefined, registry: { getAllPackages: () => registry } },
+        metadata: {},
+    };
+    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/x', raw: new Request('http://localhost:3000/x'), json: async () => body, param: () => undefined }, json };
+}
+
+let dir: string;
+beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'mil-conflict-')); });
+afterEach(() => { rmSync(dir, { recursive: true, force: true }); vi.restoreAllMocks(); });
+
+const MANIFEST = (id: string, version = '1.0.0') => ({ id, version, objects: [] });
+
+describe('findConflict — orphaned ledger vs real user code', () => {
+    it('refuses to overwrite genuine user/config code (registered at boot)', async () => {
+        const rawApp = makeRawApp();
+        // AppPlugin-style local code present in the registry BEFORE kernel:ready.
+        const registry: any[] = [{ manifest: { id: 'app.user.foo', objects: [] } }];
+        const { ctx, fire } = makeCtx(rawApp, registry);
+        const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir });
+        await plugin.start(ctx as any);
+        await fire(); // captures bootUserCodeIds = { app.user.foo }
+
+        const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local')!(
+            makeC({ manifest: MANIFEST('app.user.foo') }),
+        );
+        expect(res.status).toBe(409);
+        expect(res.payload?.error?.code).toBe('manifest_conflict');
+    });
+
+    it('allows upgrading an orphaned marketplace install (registered, ledger entry gone)', async () => {
+        const rawApp = makeRawApp();
+        const registry: any[] = []; // empty at boot → empty user-code snapshot
+        const { ctx, fire } = makeCtx(rawApp, registry);
+        const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir });
+        await plugin.start(ctx as any);
+        await fire();
+
+        const install = rawApp.routes.get('POST /api/v1/marketplace/install-local')!;
+
+        // First install: succeeds, writes a ledger entry, registers the manifest.
+        const first = await install(makeC({ manifest: MANIFEST('app.mk.bar', '1.0.0') }));
+        expect(first.payload?.success).toBe(true);
+        expect(registry.some((p) => p.manifest.id === 'app.mk.bar')).toBe(true);
+
+        // Orphan it: drop the ledger file but leave it registered in the engine.
+        for (const f of readdirSync(dir)) rmSync(join(dir, f));
+
+        // Re-install (upgrade): must NOT be refused as "user-code".
+        const second = await install(makeC({ manifest: MANIFEST('app.mk.bar', '1.0.1') }));
+        expect(second.status).not.toBe(409);
+        expect(second.payload?.success).toBe(true);
+        expect(second.payload?.error?.code).not.toBe('manifest_conflict');
+    });
+
+    it('still treats a fresh, never-seen manifest as a clean install', async () => {
+        const rawApp = makeRawApp();
+        const registry: any[] = [];
+        const { ctx, fire } = makeCtx(rawApp, registry);
+        const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir });
+        await plugin.start(ctx as any);
+        await fire();
+
+        const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local')!(
+            makeC({ manifest: MANIFEST('app.brand.new') }),
+        );
+        expect(res.payload?.success).toBe(true);
+    });
+});
diff --git a/packages/cloud-connection/src/marketplace-install-local-plugin.ts b/packages/cloud-connection/src/marketplace-install-local-plugin.ts
@@ -51,6 +51,11 @@ import { MARKETPLACE_INSTALLED_UI_BUNDLE } from './marketplace-ui.js';
 
 const ROUTE_BASE = '/api/v1/marketplace/install-local';
 
+/** Best-effort manifest id from a registry package entry (shape varies). */
+function manifestIdOf(p: any): string | undefined {
+    return p?.manifest?.id ?? p?.id ?? p?.manifest?.name ?? undefined;
+}
+
 export interface MarketplaceInstallLocalPluginConfig {
     /** Cloud control-plane base URL. When unset, falls back to OS_CLOUD_URL
      *  and then to the public ObjectStack cloud so a fresh `objectstack dev`
@@ -75,6 +80,14 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
     private readonly ledger: LocalManifestSource;
     private readonly storageDir: string;
     private readonly credentials: ConnectionCredentialStore;
+    /**
+     * Manifest ids already present in the engine registry at `kernel:ready`,
+     * BEFORE this plugin rehydrates its own ledger. These are genuine
+     * user/config-defined apps (AppPlugin from objectstack.config.ts). Used by
+     * findConflict to tell real local code apart from an orphaned marketplace
+     * install whose ledger entry went missing.
+     */
+    private readonly bootUserCodeIds = new Set<string>();
 
     constructor(config: MarketplaceInstallLocalPluginConfig = {}) {
         this.cloudUrl = resolveCloudUrl(config.controlPlaneUrl);
@@ -89,6 +102,13 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
 
     start = async (ctx: PluginContext): Promise<void> => {
         ctx.hook('kernel:ready', async () => {
+            // Snapshot the manifest ids the engine already knows about BEFORE
+            // we register anything or rehydrate the ledger — by now AppPlugin
+            // has loaded objectstack.config.ts, so whatever is registered here
+            // is genuine local/user code. findConflict uses this to avoid
+            // misreading an orphaned marketplace install as user code.
+            this.captureBootUserCodeIds(ctx);
+
             // Plugin-owned Setup nav (cloud ADR-0009): "Installed Apps"
             // ships WITH the local-install capability.
             try {
@@ -454,22 +474,47 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
      *                    (refuse to avoid silently overwriting authored code)
      */
     private findConflict = (ctx: PluginContext, manifestId: string): 'none' | 'marketplace' | 'user-code' => {
-        // First check: do we already have a marketplace install entry?
+        // 1. A live ledger entry is the authoritative "we installed this" record.
         if (this.ledger.has(manifestId)) {
             return 'marketplace';
         }
-        // Then check: is the manifest_id already in the engine's registry?
+        // 2. Present in the engine registry AND captured at boot before we
+        //    rehydrated — genuine user/config code. Refuse to overwrite.
+        if (this.bootUserCodeIds.has(manifestId)) {
+            return 'user-code';
+        }
+        // 3. Registered, but neither in the ledger nor in the boot snapshot.
+        //    This is an ORPHANED marketplace install — its ledger entry was
+        //    lost/renamed/corrupted (e.g. a half-finished upgrade left a
+        //    `.bak`). It is NOT user code, so treat it as a marketplace package
+        //    and let the upgrade overwrite it, rather than refusing with a
+        //    misleading "defined by this runtime's local code" error.
         try {
             const ql: any = ctx.getService('objectql');
             const packages: any[] = ql?.registry?.getAllPackages?.() ?? [];
-            const hit = packages.find((p: any) =>
-                (p?.manifest?.id ?? p?.id ?? p?.manifest?.name) === manifestId,
-            );
-            if (hit) return 'user-code';
+            if (packages.some((p: any) => manifestIdOf(p) === manifestId)) {
+                return 'marketplace';
+            }
         } catch { /* objectql not registered yet — treat as fresh */ }
         return 'none';
     };
 
+    /**
+     * Record the manifest ids the engine registry already holds, called once at
+     * `kernel:ready` before rehydrate. Best-effort: a missing/empty registry
+     * just yields an empty snapshot (every later install is treated as fresh).
+     */
+    private captureBootUserCodeIds = (ctx: PluginContext): void => {
+        try {
+            const ql: any = ctx.getService('objectql');
+            const packages: any[] = ql?.registry?.getAllPackages?.() ?? [];
+            for (const p of packages) {
+                const id = manifestIdOf(p);
+                if (id) this.bootUserCodeIds.add(id);
+            }
+        } catch { /* objectql not ready — leave snapshot empty */ }
+    };
+
     /**
      * Pull a userId out of the request's better-auth session, if any.
      * Returns null when there is no signed-in user. v1 does not check
