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
This commit is contained in:
@@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string> = 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<AuthStorage["login"]>[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<string, string>();
|
||||
|
||||
|
||||
@@ -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<string>>(),
|
||||
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<string, unknown>;
|
||||
dependencies?: string[];
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
}
|
||||
|
||||
function makePlugin(overrides?: Partial<PluginLike>): 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<string, PluginLike>();
|
||||
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<typeof makeManifest>;
|
||||
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<string, unknown>) => {
|
||||
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<string> {
|
||||
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");
|
||||
});
|
||||
});
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user