diff --git a/src/features/connections/ui/ConnectionsSettings.pane.test.tsx b/src/features/connections/ui/ConnectionsSettings.pane.test.tsx index 0938354d6..713a3c43c 100644 --- a/src/features/connections/ui/ConnectionsSettings.pane.test.tsx +++ b/src/features/connections/ui/ConnectionsSettings.pane.test.tsx @@ -136,6 +136,9 @@ describe("ConnectionsSettings", () => { it("renders passive local inventory without managed connections", async () => { renderConnectionsSettings(); expect(await screen.findByText("GitHub")).toBeInTheDocument(); + expect( + screen.queryByText("connections.skillsHint"), + ).not.toBeInTheDocument(); expect( screen.getByText("connections.worksWith:Goose, Codex"), ).toBeInTheDocument(); @@ -165,6 +168,7 @@ describe("ConnectionsSettings", () => { await screen.findByText("connections.sections.managed"), ).toBeInTheDocument(); expect(screen.getByText("connections.sections.local")).toBeInTheDocument(); + expect(screen.getByText("connections.skillsHint")).toBeInTheDocument(); expect( screen.queryByText("connections.sections.installed"), ).not.toBeInTheDocument(); diff --git a/src/features/connections/ui/ConnectionsSettings.tsx b/src/features/connections/ui/ConnectionsSettings.tsx index e64a85bb5..c5fe309aa 100644 --- a/src/features/connections/ui/ConnectionsSettings.tsx +++ b/src/features/connections/ui/ConnectionsSettings.tsx @@ -149,6 +149,12 @@ export function ConnectionsSettings({ } /> + {showManagedConnections ? ( +

+ {t("connections.skillsHint")} +

+ ) : null} + { - const extensions = await listExtensions(); - for (const extension of extensions) { - if (!KEEP_ENABLED.has(extension.config_key)) { - continue; - } - if (extension.enabled) { - continue; - } - try { - await toggleExtension(extension.config_key, true); - } catch (error) { - console.warn( - `Failed to re-enable always-on extension '${extension.config_key}':`, - error, - ); - } - } -} diff --git a/src/features/extensions/lib/reconcileExtensions.test.ts b/src/features/extensions/lib/reconcileExtensions.test.ts new file mode 100644 index 000000000..3c5beeedd --- /dev/null +++ b/src/features/extensions/lib/reconcileExtensions.test.ts @@ -0,0 +1,142 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { ExtensionEntry } from "../types"; +import { reconcileExtensions } from "./reconcileExtensions"; + +const { listExtensions, toggleExtension, removeExtension, backupGooseConfig } = + vi.hoisted(() => ({ + listExtensions: vi.fn(), + toggleExtension: vi.fn(), + removeExtension: vi.fn(), + backupGooseConfig: vi.fn(), + })); +vi.mock("@/features/extensions/api/extensions", () => ({ + listExtensions, + toggleExtension, + removeExtension, +})); +vi.mock("@/features/migration/api/migration", () => ({ backupGooseConfig })); + +const legacy: ExtensionEntry = { + type: "stdio", + config_key: "salesforce-sq", + name: "Salesforce (Square)", + description: "", + cmd: "uvx", + args: ["mcp_salesforce_sq@0.2.4"], + bundled: true, + enabled: true, +}; + +describe("startup extension reconciliation", () => { + beforeEach(() => { + vi.resetAllMocks(); + backupGooseConfig.mockResolvedValue({ + backedUp: true, + backupPath: "/config.yaml.backup", + }); + removeExtension.mockResolvedValue(undefined); + toggleExtension.mockResolvedValue(undefined); + }); + + it.each([ + true, + false, + ])("retires the bundled stdio Salesforce MCP when enabled=%s, including after onboarding", async (enabled) => { + listExtensions.mockResolvedValue([{ ...legacy, enabled }]); + await reconcileExtensions(); + expect(backupGooseConfig).toHaveBeenCalledOnce(); + expect(removeExtension).toHaveBeenCalledWith("salesforce-sq"); + expect(listExtensions).toHaveBeenCalledTimes(2); + expect(backupGooseConfig.mock.invocationCallOrder[0]).toBeLessThan( + listExtensions.mock.invocationCallOrder[1], + ); + }); + + it("recognizes uvx paths and normalized package names", async () => { + listExtensions.mockResolvedValue([ + { ...legacy, cmd: "/usr/local/bin/uvx", args: ["mcp-salesforce-sq"] }, + ]); + await reconcileExtensions(); + expect(removeExtension).toHaveBeenCalledWith("salesforce-sq"); + }); + + it("preserves user-owned, remote, credential-configured, and unrelated servers", async () => { + listExtensions.mockResolvedValue([ + { ...legacy, bundled: false }, + { ...legacy, bundled: undefined }, + { ...legacy, type: "streamable_http", uri: "https://example.com/mcp" }, + { ...legacy, envs: { SALESFORCE_TOKEN: "configured" } }, + { ...legacy, env_keys: ["SALESFORCE_TOKEN"] }, + { ...legacy, cmd: "custom-wrapper" }, + { ...legacy, args: ["mcp_salesforce_sq_custom"] }, + { ...legacy, args: ["mcp_salesforce_sq@git+https://example.com/custom"] }, + { ...legacy, args: ["mcp_salesforce_sq", "--custom"] }, + ]); + await reconcileExtensions(); + expect(backupGooseConfig).not.toHaveBeenCalled(); + expect(removeExtension).not.toHaveBeenCalled(); + }); + + it.each([ + { current: [{ ...legacy, envs: { SALESFORCE_TOKEN: "configured" } }] }, + { current: [{ ...legacy, env_keys: ["SALESFORCE_TOKEN"] }] }, + { current: [{ ...legacy, args: ["mcp_salesforce_sq", "--custom"] }] }, + { current: [{ ...legacy, name: "My Salesforce" }] }, + { current: [{ ...legacy, config_key: "another-salesforce" }] }, + { current: [] }, + ])("preserves an entry customized or removed during backup: $current", async ({ + current, + }) => { + listExtensions.mockResolvedValueOnce([legacy]).mockResolvedValue(current); + await reconcileExtensions(); + expect(backupGooseConfig).toHaveBeenCalledOnce(); + expect(removeExtension).not.toHaveBeenCalled(); + }); + + it("revalidates each candidate after the preceding removal", async () => { + const second = { ...legacy, config_key: "other-salesforce" }; + listExtensions + .mockResolvedValueOnce([legacy, second]) + .mockResolvedValueOnce([legacy, second]) + .mockResolvedValueOnce([ + { ...second, envs: { SALESFORCE_TOKEN: "configured" } }, + ]); + await reconcileExtensions(); + expect(removeExtension).toHaveBeenCalledExactlyOnceWith("salesforce-sq"); + }); + + it("enables core tools while retiring Salesforce and becomes a no-op on the next boot", async () => { + listExtensions + .mockResolvedValueOnce([ + legacy, + { + type: "builtin", + name: "skills", + description: "", + config_key: "skills", + enabled: false, + }, + { + type: "builtin", + name: "developer", + description: "", + config_key: "developer", + enabled: true, + }, + ]) + .mockResolvedValueOnce([legacy]) + .mockResolvedValueOnce([]); + await reconcileExtensions(); + await reconcileExtensions(); + expect(toggleExtension).toHaveBeenCalledExactlyOnceWith("skills", true); + expect(removeExtension).toHaveBeenCalledOnce(); + expect(backupGooseConfig).toHaveBeenCalledOnce(); + }); + + it("does not remove anything if the backup fails", async () => { + listExtensions.mockResolvedValue([legacy]); + backupGooseConfig.mockRejectedValue(new Error("backup failed")); + await expect(reconcileExtensions()).rejects.toThrow("backup failed"); + expect(removeExtension).not.toHaveBeenCalled(); + }); +}); diff --git a/src/features/extensions/lib/reconcileExtensions.ts b/src/features/extensions/lib/reconcileExtensions.ts new file mode 100644 index 000000000..6d959279b --- /dev/null +++ b/src/features/extensions/lib/reconcileExtensions.ts @@ -0,0 +1,61 @@ +import { + listExtensions, + removeExtension, + toggleExtension, +} from "@/features/extensions/api/extensions"; +import { backupGooseConfig } from "@/features/migration/api/migration"; +import type { ExtensionEntry } from "../types"; +import { KEEP_ENABLED } from "./keepEnabled"; + +// This bundled stdio server expects an HTTP request for hosted authentication. +// Keep user-owned servers and configurations that supply their own token. +function isLegacySalesforceMcp(extension: ExtensionEntry): boolean { + return ( + extension.type === "stdio" && + extension.bundled === true && + extension.cmd.split(/[\\/]/).at(-1) === "uvx" && + extension.args.length === 1 && + /^mcp[-_]salesforce[-_]sq(?:@\d+(?:\.\d+){1,2}(?:[a-zA-Z0-9.+-]*))?$/.test( + extension.args[0], + ) && + !Object.hasOwn(extension.envs ?? {}, "SALESFORCE_TOKEN") && + !extension.env_keys?.includes("SALESFORCE_TOKEN") + ); +} + +/** + * Reconcile extension policy on every boot, including already-migrated installs. + * Core tools stay enabled. Retired bundled Salesforce MCPs are backed up and + * removed so the extension manager cannot load them on demand. + * Startup callers log failures and continue, allowing a retry on the next boot. + */ +export async function reconcileExtensions(): Promise { + const extensions = await listExtensions(); + for (const extension of extensions) { + if (!KEEP_ENABLED.has(extension.config_key) || extension.enabled) continue; + try { + await toggleExtension(extension.config_key, true); + } catch (error) { + console.warn( + `Failed to re-enable always-on extension '${extension.config_key}':`, + error, + ); + } + } + + const retired = extensions.filter(isLegacySalesforceMcp); + if (retired.length === 0) return; + await backupGooseConfig(); + for (const extension of retired) { + const current = (await listExtensions()).find( + (entry) => entry.config_key === extension.config_key, + ); + if ( + current && + isLegacySalesforceMcp(current) && + JSON.stringify(current) === JSON.stringify(extension) + ) { + await removeExtension(current.config_key); + } + } +} diff --git a/src/features/migration/hooks/useMigrationGate.ts b/src/features/migration/hooks/useMigrationGate.ts index 1612ca820..0d8b72379 100644 --- a/src/features/migration/hooks/useMigrationGate.ts +++ b/src/features/migration/hooks/useMigrationGate.ts @@ -1,5 +1,5 @@ import { useCallback, useEffect, useRef, useState } from "react"; -import { reconcileAlwaysOnExtensions } from "@/features/extensions/lib/reconcileAlwaysOn"; +import { reconcileExtensions } from "@/features/extensions/lib/reconcileExtensions"; import { cleanupLegacyBundledExtensions } from "../cleanupLegacyBundledExtensions"; import { getMigrationStatus, @@ -81,16 +81,12 @@ export function useMigrationGate(startupReady: boolean): MigrationGate { } } - // Heal extensions whose desired state has changed since the user's - // migration ran (e.g. a newly-added always-on entry). Best-effort — - // failures don't block startup. + // Apply current extension policy even after onboarding has completed. + // Best-effort: failures do not block startup. try { - await reconcileAlwaysOnExtensions(); + await reconcileExtensions(); } catch (reconcileError) { - console.warn( - "Failed to reconcile always-on extensions:", - reconcileError, - ); + console.warn("Failed to reconcile extensions:", reconcileError); } if (cancelled) return; setStoreStatus(latestStatus); @@ -129,10 +125,10 @@ export function useMigrationGate(startupReady: boolean): MigrationGate { if (cancelled) return; try { - await reconcileAlwaysOnExtensions(); + await reconcileExtensions(); } catch (reconcileError) { console.warn( - "Failed to reconcile always-on extensions after migration:", + "Failed to reconcile extensions after migration:", reconcileError, ); } diff --git a/src/shared/i18n/locales/en/settings.json b/src/shared/i18n/locales/en/settings.json index 8306209ac..3eb3fb99b 100644 --- a/src/shared/i18n/locales/en/settings.json +++ b/src/shared/i18n/locales/en/settings.json @@ -131,6 +131,7 @@ "extendAccess": "Extend access", "noResults": "No connections match your search.", "reconnect": "Reconnect", + "skillsHint": "Connect an app here to authorize access. Find and install its skill in the skill marketplace so agents know how to use it.", "search": "Search connections", "sections": { "managed": "Company managed", diff --git a/src/shared/i18n/locales/es/settings.json b/src/shared/i18n/locales/es/settings.json index a48741d33..1e16a03d2 100644 --- a/src/shared/i18n/locales/es/settings.json +++ b/src/shared/i18n/locales/es/settings.json @@ -131,6 +131,7 @@ "extendAccess": "Ampliar acceso", "noResults": "Ninguna conexión coincide con tu búsqueda.", "reconnect": "Reconectar", + "skillsHint": "Conecta una aplicación aquí para autorizar el acceso. Busca e instala su habilidad en el catálogo de habilidades para que los agentes sepan cómo usarla.", "search": "Buscar conexiones", "sections": { "managed": "Administradas por la empresa",