fix(app): only offer Remove for providers defined in config.json (#6568)

* fix(app): only offer Remove for providers defined in config.json

Plugin-registered providers (Antigravity, Muse Code) share source: "custom"
with config-defined ones, so Settings → Providers offered Remove on them.
The removeProviders patch only deletes config.json entries, so the action
succeeded silently while the plugin re-registered the provider.

Gate the Remove menu on the provider having an extends entry in config —
the actual definition custom providers live behind. Plugin providers keep
their enable/disable switch.

Fixes #6349

* Address review: named ownership check, E2E coverage for plugin providers

- Name the config-owned provider check instead of combining three
  conditions inline in canRemove.
- Replace the JSDOM component tests with a Playwright case in the
  existing provider-removal suite: a plugin-registered provider renders
  without the Remove action and survives a removeProviders patch.
This commit is contained in:
CtrlCreeper
2026-10-13 02:59:26 +02:00
committed by GitHub
parent 9eaf7ad0c1
commit e8108d9092
3 changed files with 70 additions and 3 deletions
+3
View File
@@ -93,3 +93,6 @@ app.json
# Heap snapshots dumped by --heapsnapshot-near-heap-limit; keep them for debugging. # Heap snapshots dumped by --heapsnapshot-near-heap-limit; keep them for debugging.
*.heapsnapshot *.heapsnapshot
# Project-local Serena configuration and memories.
.serena/
@@ -10,16 +10,29 @@ import {
openSettingsHost, openSettingsHost,
openSettingsHostSection, openSettingsHostSection,
} from "../support/helpers/settings"; } from "../support/helpers/settings";
import { copyPluginExample } from "../support/helpers/plugin-fixture";
const CUSTOM_PROVIDER = { const CUSTOM_PROVIDER = {
id: "junie", id: "junie",
name: "Junie", name: "Junie",
} as const; } as const;
const PLUGIN_PROVIDER = {
id: "direct-example",
name: "Direct provider example",
pluginId: "provider-direct-example",
} as const;
interface ProviderRemovalDaemonClient { interface ProviderRemovalDaemonClient {
connect(): Promise<void>; connect(): Promise<void>;
close(): Promise<void>; close(): Promise<void>;
patchDaemonConfig(config: { removeProviders?: string[] }): Promise<unknown>; getDaemonConfig(): Promise<{ config: { pluginsEnabled?: boolean } }>;
patchDaemonConfig(config: {
removeProviders?: string[];
pluginsEnabled?: boolean;
}): Promise<unknown>;
installDirectoryPlugin(path: string): Promise<unknown>;
removePlugin(pluginId: string): Promise<void>;
getProvidersSnapshot(): Promise<{ getProvidersSnapshot(): Promise<{
entries: Array<{ provider: string; source?: "builtin" | "custom" }>; entries: Array<{ provider: string; source?: "builtin" | "custom" }>;
}>; }>;
@@ -93,4 +106,45 @@ test.describe("provider removal", () => {
await client.close().catch(() => undefined); await client.close().catch(() => undefined);
} }
}); });
test("does not offer Remove for a plugin-registered provider", async ({ page }) => {
test.setTimeout(120_000);
const client = await connectDaemonClient<ProviderRemovalDaemonClient>({
clientIdPrefix: "provider-removal-e2e",
});
const plugin = await copyPluginExample("provider-direct");
try {
const previous = await client.getDaemonConfig();
try {
await client.patchDaemonConfig({ pluginsEnabled: true });
await client.installDirectoryPlugin(plugin.directory);
await gotoAppShell(page);
await openSettings(page);
await openSettingsHost(page, getServerId());
await openSettingsHostSection(page, getServerId(), "providers");
const pluginProviderRow = page.getByRole("button", {
name: `${PLUGIN_PROVIDER.name} provider details`,
exact: true,
});
await expect(pluginProviderRow).toBeVisible();
await expect(page.getByTestId(`provider-actions-${PLUGIN_PROVIDER.id}`)).toHaveCount(0);
// The provider survives a removeProviders patch pointed at it: the
// plugin, not config.json, owns the definition.
await client.patchDaemonConfig({ removeProviders: [PLUGIN_PROVIDER.id] });
await expect(pluginProviderRow).toBeVisible();
} finally {
await client.removePlugin(PLUGIN_PROVIDER.pluginId).catch(() => undefined);
await client
.patchDaemonConfig({ pluginsEnabled: previous.config.pluginsEnabled })
.catch(() => undefined);
}
} finally {
await client.close().catch(() => undefined);
await plugin.cleanup();
}
});
}); });
@@ -328,7 +328,7 @@ export function ProvidersSection({ serverId }: ProvidersSectionProps) {
const isConnected = useHostRuntimeIsConnected(serverId); const isConnected = useHostRuntimeIsConnected(serverId);
const supportsProviderRemoval = useHostFeature(serverId, "providerRemoval"); const supportsProviderRemoval = useHostFeature(serverId, "providerRemoval");
const { entries, isLoading, refresh } = useProvidersSnapshot(serverId); const { entries, isLoading, refresh } = useProvidersSnapshot(serverId);
const { patchConfig } = useDaemonConfig(serverId); const { config, patchConfig } = useDaemonConfig(serverId);
const openProviderSettings = useProviderSettingsStore((state) => state.open); const openProviderSettings = useProviderSettingsStore((state) => state.open);
const [pendingProviderId, setPendingProviderId] = useState<string | null>(null); const [pendingProviderId, setPendingProviderId] = useState<string | null>(null);
const [removingProviderId, setRemovingProviderId] = useState<string | null>(null); const [removingProviderId, setRemovingProviderId] = useState<string | null>(null);
@@ -338,6 +338,12 @@ export function ProvidersSection({ serverId }: ProvidersSectionProps) {
const providerDefinitions = useMemo(() => buildProviderDefinitions(entries), [entries]); const providerDefinitions = useMemo(() => buildProviderDefinitions(entries), [entries]);
const hasServer = serverId.length > 0; const hasServer = serverId.length > 0;
/** Custom providers are deletable only when config.json holds their `extends` definition. */
const isConfigOwnedProvider = useCallback(
(providerId: string) => Boolean(config?.providers?.[providerId]?.extends),
[config],
);
const handleOpenProviderSettings = useCallback( const handleOpenProviderSettings = useCallback(
(providerId: string) => { (providerId: string) => {
openProviderSettings({ serverId, provider: providerId }); openProviderSettings({ serverId, provider: providerId });
@@ -444,7 +450,11 @@ export function ProvidersSection({ serverId }: ProvidersSectionProps) {
enabled={entry.enabled ?? true} enabled={entry.enabled ?? true}
isToggling={pendingProviderId === def.id} isToggling={pendingProviderId === def.id}
isRemoving={removingProviderId === def.id} isRemoving={removingProviderId === def.id}
canRemove={supportsProviderRemoval && entry.source === "custom"} canRemove={
supportsProviderRemoval &&
entry.source === "custom" &&
isConfigOwnedProvider(def.id)
}
isFirst={index === 0} isFirst={index === 0}
onPress={handleOpenProviderSettings} onPress={handleOpenProviderSettings}
onToggleEnabled={handleToggleEnabled} onToggleEnabled={handleToggleEnabled}