From 3c3481f25e4a6f0f135271840295e61778050d5e Mon Sep 17 00:00:00 2001 From: Fusion Date: Sun, 3 May 2026 14:31:23 -0700 Subject: [PATCH] feat(FN-3318): harden bundled-plugin-install to detect stale path/version Adds hardening logic to the bundled-plugin-install command to detect stale path or version conditions, with comprehensive tests covering those edge cases. Also includes a minor update and test coverage for the provider-auth command. Fusion-Task-Id: FN-3318 --- .../commands/__tests__/provider-auth.test.ts | 63 ++++ packages/cli/src/commands/provider-auth.ts | 19 +- .../__tests__/bundled-plugin-install.test.ts | 325 ++++++++++++++++++ .../cli/src/plugins/bundled-plugin-install.ts | 33 +- 4 files changed, 435 insertions(+), 5 deletions(-) create mode 100644 packages/cli/src/plugins/__tests__/bundled-plugin-install.test.ts diff --git a/packages/cli/src/commands/__tests__/provider-auth.test.ts b/packages/cli/src/commands/__tests__/provider-auth.test.ts index 8e89724a1..a53eadcb1 100644 --- a/packages/cli/src/commands/__tests__/provider-auth.test.ts +++ b/packages/cli/src/commands/__tests__/provider-auth.test.ts @@ -147,4 +147,67 @@ describe("wrapAuthStorageWithApiKeyProviders", () => { expect(await storage.getApiKey("openai-codex")).toBe("legacy-access-token"); }); + + describe("Anthropic reclassification from OAuth to API key", () => { + it("filters anthropic out of getOAuthProviders even when upstream reports it as OAuth", () => { + const fusionAuth = makeAuthStorage(); + fusionAuth.getOAuthProviders = vi.fn(() => [ + { id: "anthropic", name: "Anthropic" }, + { id: "github-copilot", name: "GitHub Copilot" }, + ]); + const modelRegistry = { getAll: vi.fn(() => []) } as any; + + const wrapped = wrapAuthStorageWithApiKeyProviders(fusionAuth, modelRegistry); + const oauthProviders = wrapped.getOAuthProviders(); + + const oauthIds = oauthProviders.map((p) => p.id); + expect(oauthIds).not.toContain("anthropic"); + expect(oauthIds).toContain("github-copilot"); + }); + + it("includes anthropic in getApiKeyProviders with correct display name", () => { + const fusionAuth = makeAuthStorage(); + fusionAuth.getOAuthProviders = vi.fn(() => [ + { id: "anthropic", name: "Anthropic" }, + ]); + const modelRegistry = { getAll: vi.fn(() => []) } as any; + + const wrapped = wrapAuthStorageWithApiKeyProviders(fusionAuth, modelRegistry); + const apiKeyProviders = wrapped.getApiKeyProviders(); + + const anthropic = apiKeyProviders.find((p) => p.id === "anthropic"); + expect(anthropic).toBeDefined(); + expect(anthropic!.name).toBe("Anthropic"); + }); + + it("stores anthropic credentials as api_key type", () => { + const fusionAuth = makeAuthStorage(); + fusionAuth.getOAuthProviders = vi.fn(() => [ + { id: "anthropic", name: "Anthropic" }, + ]); + const modelRegistry = { getAll: vi.fn(() => []) } as any; + + const wrapped = wrapAuthStorageWithApiKeyProviders(fusionAuth, modelRegistry); + wrapped.setApiKey("anthropic", "sk-ant-api03-test-key"); + + expect(fusionAuth.set).toHaveBeenCalledWith("anthropic", { + type: "api_key", + key: "sk-ant-api03-test-key", + }); + }); + + it("detects anthropic as authenticated via hasApiKey after storing API key", () => { + const fusionAuth = makeAuthStorage({ + anthropic: { type: "api_key", key: "sk-ant-api03-test" }, + }); + fusionAuth.getOAuthProviders = vi.fn(() => [ + { id: "anthropic", name: "Anthropic" }, + ]); + const modelRegistry = { getAll: vi.fn(() => []) } as any; + + const wrapped = wrapAuthStorageWithApiKeyProviders(fusionAuth, modelRegistry); + + expect(wrapped.hasApiKey("anthropic")).toBe(true); + }); + }); }); diff --git a/packages/cli/src/commands/provider-auth.ts b/packages/cli/src/commands/provider-auth.ts index 0dd767369..2a940933d 100644 --- a/packages/cli/src/commands/provider-auth.ts +++ b/packages/cli/src/commands/provider-auth.ts @@ -40,7 +40,18 @@ type StoredCredential = { [key: string]: unknown; }; +/** + * Provider IDs that should be treated as OAuth-backed by the upstream + * pi-coding-agent AuthStorage but which Fusion reclassifies as API-key + * providers. These IDs are stripped from getOAuthProviders() results so + * the dashboard never offers a browser-based OAuth login for them. + */ +const OAUTH_TO_API_KEY_RECLASSIFICATIONS: ReadonlySet = new Set([ + "anthropic", +]); + const BUILT_IN_API_KEY_PROVIDERS: Array<{ id: string; name: string }> = [ + { id: "anthropic", name: "Anthropic" }, { id: "brave", name: "Brave Search" }, { id: "kimi-coding", name: "Kimi" }, { id: "minimax", name: "Minimax" }, @@ -78,14 +89,20 @@ export function wrapAuthStorageWithApiKeyProviders( getOAuthProviders: () => mergedAuthStorage .getOAuthProviders() + .filter((provider) => !OAUTH_TO_API_KEY_RECLASSIFICATIONS.has(provider.id)) .map((provider) => ({ id: provider.id, name: provider.name })), hasAuth: (provider) => mergedAuthStorage.hasAuth(provider), login: (providerId, callbacks) => mergedAuthStorage.login(providerId as Parameters[0], callbacks), logout: (provider) => mergedAuthStorage.logout(provider), getApiKeyProviders: () => { + // Use the reclassified (filtered) OAuth provider list so that providers + // moved to API-key (e.g. anthropic) are not skipped by the OAuth dedup. const oauthProviderIds = new Set( - mergedAuthStorage.getOAuthProviders().map((provider) => provider.id), + mergedAuthStorage + .getOAuthProviders() + .filter((provider) => !OAUTH_TO_API_KEY_RECLASSIFICATIONS.has(provider.id)) + .map((provider) => provider.id), ); const providers = new Map(); diff --git a/packages/cli/src/plugins/__tests__/bundled-plugin-install.test.ts b/packages/cli/src/plugins/__tests__/bundled-plugin-install.test.ts new file mode 100644 index 000000000..b4f1ae171 --- /dev/null +++ b/packages/cli/src/plugins/__tests__/bundled-plugin-install.test.ts @@ -0,0 +1,325 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// ── Mocks ──────────────────────────────────────────────────────────── +// vi.mock factories are hoisted, so we use vi.hoisted() for mock references. + +const { mockExistsSync, mockReadFile, mockValidatePluginManifest } = vi.hoisted(() => ({ + mockExistsSync: vi.fn<(path: string) => boolean>(), + mockReadFile: vi.fn<(path: string, encoding: string) => Promise>(), + mockValidatePluginManifest: vi.fn<(manifest: unknown) => { valid: boolean; errors: string[] }>(), +})); + +vi.mock("node:fs", () => ({ + existsSync: mockExistsSync, +})); + +vi.mock("node:fs/promises", () => ({ + readFile: mockReadFile, +})); + +vi.mock("@fusion/core", () => ({ + validatePluginManifest: mockValidatePluginManifest, +})); + +// Import SUT after mocks are in place +import { ensureBundledDependencyGraphPluginInstalled } from "../bundled-plugin-install.js"; + +// ── Helpers ────────────────────────────────────────────────────────── + +const BUNDLED_PLUGIN_ID = "fusion-plugin-dependency-graph"; + +function makeManifest(overrides?: Partial<{ id: string; version: string }>) { + return { + id: BUNDLED_PLUGIN_ID, + name: "Dependency Graph", + version: "0.1.0", + description: "Top-level dependency graph dashboard view", + dashboardViews: [ + { + viewId: "graph", + label: "Graph", + componentPath: "./src/DependencyGraphView.tsx", + icon: "Network", + placement: "more", + order: 40, + }, + ], + ...overrides, + }; +} + +interface PluginLike { + id: string; + name: string; + version: string; + description?: string; + path: string; + enabled: boolean; + state: string; + settings: Record; + dependencies?: string[]; + createdAt: string; + updatedAt: string; +} + +function makePlugin(overrides?: Partial): PluginLike { + return { + id: BUNDLED_PLUGIN_ID, + name: "Dependency Graph", + version: "0.1.0", + description: "Top-level dependency graph dashboard view", + path: "", // callers should set this + enabled: true, + state: "installed", + settings: {}, + dependencies: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + ...overrides, + }; +} + +function makePluginStore() { + const plugins = new Map(); + return { + getPlugin: vi.fn(async (id: string) => { + const plugin = plugins.get(id); + if (!plugin) + throw Object.assign(new Error(`Plugin "${id}" not found`), { code: "ENOENT" }); + return { ...plugin }; + }), + registerPlugin: vi.fn(async (input: { manifest: unknown; path: string }) => { + const manifest = input.manifest as ReturnType; + const plugin = makePlugin({ + id: manifest.id, + name: manifest.name, + version: manifest.version, + description: manifest.description, + path: input.path, + }); + plugins.set(manifest.id, plugin); + return plugin; + }), + updatePlugin: vi.fn(async (id: string, updates: Record) => { + const plugin = plugins.get(id); + if (!plugin) throw new Error(`Plugin "${id}" not found`); + const updated = { ...plugin, ...updates, updatedAt: new Date().toISOString() }; + plugins.set(id, updated); + return updated; + }), + /** Directly inject a plugin record for test setup */ + _inject(plugin: PluginLike) { + plugins.set(plugin.id, { ...plugin }); + }, + }; +} + +function makePluginLoader() { + return { + loadPlugin: vi.fn(async () => {}), + unloadPlugin: vi.fn(async () => {}), + getLoadedPlugins: vi.fn(() => new Map()), + isPluginLoaded: vi.fn(() => false), + }; +} + +/** + * Setup: bundled manifest exists at the first candidate path and is valid. + * The resolver's first candidate includes "dist/plugins/..." when running from source. + */ +function setupBundleExists(manifestOverrides?: Partial<{ id: string; version: string }>) { + const manifest = makeManifest(manifestOverrides); + mockExistsSync.mockImplementation((p: string) => { + if (typeof p === "string" && p.endsWith("manifest.json") && p.includes("dist")) return true; + return false; + }); + mockReadFile.mockResolvedValue(JSON.stringify(manifest)); + mockValidatePluginManifest.mockReturnValue({ valid: true, errors: [] }); + return manifest; +} + +/** Setup: no bundled manifest found on any candidate path. */ +function setupBundleMissing() { + mockExistsSync.mockReturnValue(false); +} + +/** Setup: bundled manifest found but invalid. */ +function setupBundleInvalid() { + mockExistsSync.mockImplementation((p: string) => { + if (typeof p === "string" && p.endsWith("manifest.json") && p.includes("dist")) return true; + return false; + }); + const badManifest = { id: "bad" }; + mockReadFile.mockResolvedValue(JSON.stringify(badManifest)); + mockValidatePluginManifest.mockReturnValue({ + valid: false, + errors: ["Missing required field: name"], + }); +} + +/** + * Probe the resolver to determine the actual resolved bundled path. + * Registers the plugin and captures the path from the registerPlugin call. + */ +async function getResolvedBundledPath(): Promise { + setupBundleExists(); + const probeStore = makePluginStore(); + const probeLoader = makePluginLoader(); + await ensureBundledDependencyGraphPluginInstalled( + probeStore as unknown as import("@fusion/core").PluginStore, + probeLoader as unknown as import("@fusion/core").PluginLoader, + ); + const call = probeStore.registerPlugin.mock.calls[0]; + return (call?.[0] as { path: string })?.path ?? ""; +} + +// ── Tests ──────────────────────────────────────────────────────────── + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("ensureBundledDependencyGraphPluginInstalled", () => { + it("fresh install: registers and loads the plugin when not in DB", async () => { + setupBundleExists(); + const store = makePluginStore(); + const loader = makePluginLoader(); + + const result = await ensureBundledDependencyGraphPluginInstalled( + store as unknown as import("@fusion/core").PluginStore, + loader as unknown as import("@fusion/core").PluginLoader, + ); + + expect(result).toBe("installed"); + expect(store.registerPlugin).toHaveBeenCalledOnce(); + expect(store.registerPlugin).toHaveBeenCalledWith( + expect.objectContaining({ + manifest: expect.objectContaining({ id: BUNDLED_PLUGIN_ID }), + }), + ); + // Fresh install → enabled by default → should be loaded + expect(loader.loadPlugin).toHaveBeenCalledWith(BUNDLED_PLUGIN_ID); + }); + + it("already installed with matching path/version → returns already-installed without DB writes", async () => { + // First probe to get the actual resolved path + const bundledPath = await getResolvedBundledPath(); + + vi.clearAllMocks(); + const manifest = setupBundleExists(); + const store = makePluginStore(); + const loader = makePluginLoader(); + + // Inject a plugin that matches the current bundle path and version + store._inject(makePlugin({ path: bundledPath, version: manifest.version })); + + const result = await ensureBundledDependencyGraphPluginInstalled( + store as unknown as import("@fusion/core").PluginStore, + loader as unknown as import("@fusion/core").PluginLoader, + ); + + expect(result).toBe("already-installed"); + expect(store.updatePlugin).not.toHaveBeenCalled(); + expect(store.registerPlugin).not.toHaveBeenCalled(); + expect(loader.loadPlugin).not.toHaveBeenCalled(); + }); + + it("already installed with stale path → updates path to current bundled path", async () => { + const bundledPath = await getResolvedBundledPath(); + const OLD_PATH = "/old/cli/dist/plugins/fusion-plugin-dependency-graph"; + + vi.clearAllMocks(); + const manifest = setupBundleExists(); + const store = makePluginStore(); + const loader = makePluginLoader(); + + // Plugin registered with the OLD path, but current version + store._inject(makePlugin({ path: OLD_PATH, version: manifest.version })); + + const result = await ensureBundledDependencyGraphPluginInstalled( + store as unknown as import("@fusion/core").PluginStore, + loader as unknown as import("@fusion/core").PluginLoader, + ); + + expect(result).toBe("updated"); + expect(store.updatePlugin).toHaveBeenCalledWith( + BUNDLED_PLUGIN_ID, + expect.objectContaining({ path: bundledPath }), + ); + // Plugin was enabled → should be loaded + expect(loader.loadPlugin).toHaveBeenCalledWith(BUNDLED_PLUGIN_ID); + }); + + it("already installed with stale version → updates version to current manifest version", async () => { + const bundledPath = await getResolvedBundledPath(); + + vi.clearAllMocks(); + const manifest = setupBundleExists({ version: "0.2.0" }); + const store = makePluginStore(); + const loader = makePluginLoader(); + + // Plugin registered with old version but same path + store._inject(makePlugin({ path: bundledPath, version: "0.1.0" })); + + const result = await ensureBundledDependencyGraphPluginInstalled( + store as unknown as import("@fusion/core").PluginStore, + loader as unknown as import("@fusion/core").PluginLoader, + ); + + expect(result).toBe("updated"); + expect(store.updatePlugin).toHaveBeenCalledWith( + BUNDLED_PLUGIN_ID, + expect.objectContaining({ version: "0.2.0" }), + ); + expect(loader.loadPlugin).toHaveBeenCalledWith(BUNDLED_PLUGIN_ID); + }); + + it("disabled plugin → path/version updated but plugin NOT loaded (user choice respected)", async () => { + setupBundleExists({ version: "0.2.0" }); + const store = makePluginStore(); + const loader = makePluginLoader(); + + // Plugin explicitly disabled by user with stale version + // Use a path that definitely won't match the resolved path + store._inject(makePlugin({ path: "/stale/path/plugin", version: "0.1.0", enabled: false })); + + const result = await ensureBundledDependencyGraphPluginInstalled( + store as unknown as import("@fusion/core").PluginStore, + loader as unknown as import("@fusion/core").PluginLoader, + ); + + expect(result).toBe("updated"); + expect(store.updatePlugin).toHaveBeenCalled(); + // User disabled the plugin → should NOT be loaded + expect(loader.loadPlugin).not.toHaveBeenCalled(); + }); + + it("missing bundle (no bundled manifest found) → returns missing-bundle without error", async () => { + setupBundleMissing(); + const store = makePluginStore(); + const loader = makePluginLoader(); + + const result = await ensureBundledDependencyGraphPluginInstalled( + store as unknown as import("@fusion/core").PluginStore, + loader as unknown as import("@fusion/core").PluginLoader, + ); + + expect(result).toBe("missing-bundle"); + expect(store.registerPlugin).not.toHaveBeenCalled(); + expect(store.updatePlugin).not.toHaveBeenCalled(); + expect(loader.loadPlugin).not.toHaveBeenCalled(); + }); + + it("invalid bundled manifest → throws descriptive error", async () => { + setupBundleInvalid(); + const store = makePluginStore(); + const loader = makePluginLoader(); + + await expect( + ensureBundledDependencyGraphPluginInstalled( + store as unknown as import("@fusion/core").PluginStore, + loader as unknown as import("@fusion/core").PluginLoader, + ), + ).rejects.toThrow("Invalid plugin manifest"); + }); +}); diff --git a/packages/cli/src/plugins/bundled-plugin-install.ts b/packages/cli/src/plugins/bundled-plugin-install.ts index a532c6e3d..bfc0d2b04 100644 --- a/packages/cli/src/plugins/bundled-plugin-install.ts +++ b/packages/cli/src/plugins/bundled-plugin-install.ts @@ -2,7 +2,7 @@ import { existsSync } from "node:fs"; import { readFile } from "node:fs/promises"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; -import { validatePluginManifest, type PluginLoader, type PluginManifest, type PluginStore } from "@fusion/core"; +import { validatePluginManifest, type PluginInstallation, type PluginLoader, type PluginManifest, type PluginStore } from "@fusion/core"; const DEPENDENCY_GRAPH_PLUGIN_ID = "fusion-plugin-dependency-graph"; @@ -40,10 +40,10 @@ function resolveBundledDependencyGraphPath(): string | null { export async function ensureBundledDependencyGraphPluginInstalled( pluginStore: PluginStore, pluginLoader: PluginLoader, -): Promise<"installed" | "already-installed" | "missing-bundle"> { +): Promise<"installed" | "updated" | "already-installed" | "missing-bundle"> { + let existingPlugin: PluginInstallation | null = null; try { - await pluginStore.getPlugin(DEPENDENCY_GRAPH_PLUGIN_ID); - return "already-installed"; + existingPlugin = await pluginStore.getPlugin(DEPENDENCY_GRAPH_PLUGIN_ID); } catch { // Continue; plugin not installed yet. } @@ -54,6 +54,31 @@ export async function ensureBundledDependencyGraphPluginInstalled( } const manifest = await loadManifest(bundledPath); + + if (existingPlugin) { + // Check if stored path or version is stale compared to the bundled copy + const pathChanged = existingPlugin.path !== bundledPath; + const versionChanged = existingPlugin.version !== manifest.version; + + if (!pathChanged && !versionChanged) { + return "already-installed"; + } + + // Update the stored record to match the current bundled copy + await pluginStore.updatePlugin(DEPENDENCY_GRAPH_PLUGIN_ID, { + ...(pathChanged ? { path: bundledPath } : {}), + ...(versionChanged ? { version: manifest.version } : {}), + }); + + // If the plugin is enabled, load it so it picks up the new path/version + if (existingPlugin.enabled) { + await pluginLoader.loadPlugin(existingPlugin.id); + } + + return "updated"; + } + + // Fresh install const plugin = await pluginStore.registerPlugin({ manifest, path: bundledPath,