fix(plugins): resolve marketplace aliases without recursive cycles (#117440)

Co-authored-by: Peter Steinberger <steipete@macos.shared>

src/plugins/marketplace.test.ts

@@ -3,6 +3,7 @@ import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
+import * as jsonFiles from "../infra/json-files.js";
 import { withEnvAsync } from "../test-utils/env.js";
 import { withTempDir } from "../test-utils/temp-dir.js";
 import {
@@ -102,6 +103,22 @@ async function writeLocalMarketplaceFixture(params: {
   return writeMarketplaceManifest(params.rootDir, params.manifest);
 }
 
+async function withKnownMarketplaceRegistry<T>(
+  marketplaces: Record<string, unknown>,
+  run: (homeDir: string) => Promise<T>,
+): Promise<T> {
+  return await withTempDir("openclaw-marketplace-known-", async (homeDir) => {
+    const openClawHome = path.join(homeDir, "openclaw-home");
+    const registryPath = path.join(homeDir, ".claude", "plugins", "known_marketplaces.json");
+    await fs.mkdir(path.dirname(registryPath), { recursive: true });
+    await fs.mkdir(openClawHome, { recursive: true });
+    await fs.writeFile(registryPath, JSON.stringify(marketplaces));
+    return await withEnvAsync({ HOME: homeDir, OPENCLAW_HOME: openClawHome }, async () =>
+      run(homeDir),
+    );
+  });
+}
+
 function mockRemoteMarketplaceClone(params: {
   manifest: unknown;
   pluginDir?: string;
@@ -533,6 +550,151 @@ describe("marketplace plugins", () => {
     });
   });
 
+  const cyclicKnownMarketplaces = [
+    {
+      label: "self-referential",
+      marketplace: "loop",
+      marketplaces: {
+        loop: { source: { source: "path", path: "loop" } },
+      },
+      cycle: "loop -> loop",
+    },
+    {
+      label: "two-marketplace",
+      marketplace: "alpha",
+      marketplaces: {
+        alpha: { source: { source: "path", path: "beta" } },
+        beta: { source: { source: "path", path: "alpha" } },
+      },
+      cycle: "alpha -> beta -> alpha",
+    },
+  ] as const;
+
+  it.each(cyclicKnownMarketplaces)(
+    "rejects a $label known marketplace alias cycle while listing",
+    async ({ marketplace, marketplaces, cycle }) => {
+      await withKnownMarketplaceRegistry(marketplaces, async () => {
+        const registryRead = vi.spyOn(jsonFiles, "tryReadJson");
+        for (let read = 0; read <= Object.keys(marketplaces).length; read += 1) {
+          registryRead.mockResolvedValueOnce(marketplaces);
+        }
+        // Bound the unfixed recursive loader so the regression never leaves a runaway task.
+        registryRead.mockResolvedValueOnce({});
+
+        try {
+          const result = await listMarketplacePlugins({ marketplace });
+
+          expect(result).toEqual({
+            ok: false,
+            error: `known marketplace source cycle: ${cycle}`,
+          });
+          expect(registryRead).toHaveBeenCalledTimes(1);
+          expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
+          expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled();
+        } finally {
+          registryRead.mockRestore();
+        }
+      });
+    },
+  );
+
+  it.each(cyclicKnownMarketplaces)(
+    "rejects a $label known marketplace alias cycle before installing",
+    async ({ marketplace, marketplaces, cycle }) => {
+      await withKnownMarketplaceRegistry(marketplaces, async () => {
+        const registryRead = vi.spyOn(jsonFiles, "tryReadJson");
+        for (let read = 0; read <= Object.keys(marketplaces).length; read += 1) {
+          registryRead.mockResolvedValueOnce(marketplaces);
+        }
+        // Bound the unfixed recursive loader so the lifecycle-owning call always settles.
+        registryRead.mockResolvedValueOnce({});
+
+        try {
+          const result = await installPluginFromMarketplace({
+            marketplace,
+            plugin: "frontend-design",
+          });
+
+          expect(result).toEqual({
+            ok: false,
+            error: `known marketplace source cycle: ${cycle}`,
+          });
+          expect(registryRead).toHaveBeenCalledTimes(1);
+          expect(installPluginFromPathMock).not.toHaveBeenCalled();
+          expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
+          expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled();
+        } finally {
+          registryRead.mockRestore();
+        }
+      });
+    },
+  );
+
+  it("resolves independent known marketplace alias calls from one registry snapshot each", async () => {
+    await withKnownMarketplaceRegistry({}, async (homeDir) => {
+      const marketplaceRoot = path.join(homeDir, "known-marketplace");
+      const pluginDir = path.join(marketplaceRoot, "plugins", "frontend-design");
+      await writeLocalMarketplaceFixture({
+        rootDir: marketplaceRoot,
+        pluginDir,
+        manifest: {
+          plugins: [{ name: "frontend-design", source: "./plugins/frontend-design" }],
+        },
+      });
+      await fs.writeFile(
+        path.join(homeDir, ".claude", "plugins", "known_marketplaces.json"),
+        JSON.stringify({
+          alpha: { source: { source: "path", path: "beta" } },
+          beta: {
+            installLocation: marketplaceRoot,
+            source: { source: "path", path: "alpha" },
+          },
+        }),
+      );
+      installPluginFromPathMock.mockResolvedValue({
+        ok: true,
+        pluginId: "frontend-design",
+        targetDir: "/tmp/frontend-design",
+        version: "0.1.0",
+        extensions: ["index.ts"],
+      });
+      const registryRead = vi.spyOn(jsonFiles, "tryReadJson");
+
+      try {
+        const listed = await listMarketplacePlugins({ marketplace: "alpha" });
+        const installed = await installPluginFromMarketplace({
+          marketplace: "alpha",
+          plugin: "frontend-design",
+        });
+
+        expect(listed).toMatchObject({ ok: true, sourceLabel: "beta" });
+        expectLocalMarketplaceInstallResult({
+          result: installed,
+          pluginDir,
+          marketplaceSource: "alpha",
+        });
+        expect(registryRead).toHaveBeenCalledTimes(2);
+        expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
+        expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled();
+      } finally {
+        registryRead.mockRestore();
+      }
+    });
+  });
+
+  it("preserves ordinary source fallback for an invalid known marketplace alias", async () => {
+    await withKnownMarketplaceRegistry({ broken: { source: { source: "path" } } }, async () => {
+      const result = await listMarketplacePlugins({ marketplace: "broken" });
+
+      expect(result).toEqual({
+        ok: false,
+        error: "unsupported marketplace source: broken",
+      });
+      expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
+      expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled();
+    });
+  });
+
   it("installs remote marketplace plugins from relative paths inside the cloned repo", async () => {
     mockRemoteMarketplaceClone({
       pluginDir: path.join("plugins", "frontend-design"),

src/plugins/marketplace.ts

@@ -672,27 +672,39 @@ async function loadMarketplace(params: {
     return undefined;
   };
 
+  // Resolve aliases against one snapshot so a cycle cannot retain a plugin lifecycle lease.
   const knownMarketplaces = await readClaudeKnownMarketplaces();
-  const known = knownMarketplaces[params.source];
-  if (known) {
+  const visitedKnownMarketplaces = new Set<string>();
+  let source = params.source;
+
+  while (true) {
+    const known = knownMarketplaces[source];
+    if (!known) {
+      break;
+    }
+    if (visitedKnownMarketplaces.has(source)) {
+      return {
+        ok: false,
+        error: `known marketplace source cycle: ${[...visitedKnownMarketplaces, source].join(" -> ")}`,
+      };
+    }
+    visitedKnownMarketplaces.add(source);
+
     if (known.installLocation) {
       const local = await resolveLocalMarketplaceSource(known.installLocation);
       if (local?.ok) {
-        return await loadResolvedLocalMarketplace(local, params.source);
+        return await loadResolvedLocalMarketplace(local, source);
       }
     }
 
     const normalizedSource = normalizeEntrySource(known.source);
-    if (normalizedSource.ok) {
-      return await loadMarketplace({
-        source: marketplaceEntrySourceToInput(normalizedSource.source),
-        logger: params.logger,
-        timeoutMs: params.timeoutMs,
-      });
+    if (!normalizedSource.ok) {
+      break;
     }
+    source = marketplaceEntrySourceToInput(normalizedSource.source);
   }
 
-  const local = await resolveLocalMarketplaceSource(params.source);
+  const local = await resolveLocalMarketplaceSource(source);
   if (local?.ok === false) {
     return local;
   }
@@ -702,7 +714,7 @@ async function loadMarketplace(params: {
   }
 
   const cloned = await cloneMarketplaceRepo({
-    source: params.source,
+    source,
     timeoutMs: params.timeoutMs,
     logger: params.logger,
   });