From 376a363615e17cbec8e0aab377dd0da2c6eef37a Mon Sep 17 00:00:00 2001 From: Lily Delalande <119957291+lily-de@users.noreply.github.com> Date: Sun, 30 Mar 2025 11:01:48 -0400 Subject: [PATCH] ui: clean up extensions (#1914) --- .../settings_v2/extensions/ExtensionsSection.tsx | 1 - .../settings_v2/extensions/agent-api.ts | 15 ++++++++++----- .../settings_v2/extensions/extension-manager.ts | 2 +- .../extensions/modal/ExtensionConfigFields.tsx | 3 ++- .../extensions/subcomponents/ExtensionList.tsx | 5 +++-- .../components/settings_v2/extensions/utils.ts | 8 ++++++++ ui/desktop/src/toasts.tsx | 14 -------------- 7 files changed, 24 insertions(+), 24 deletions(-) diff --git a/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx b/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx index 534fdd69..3eda7950 100644 --- a/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx +++ b/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx @@ -60,7 +60,6 @@ export default function ExtensionsSection() { await fetchExtensions(); // Refresh the list after successful toggle return true; // Indicate success } catch (error) { - console.error('Toggle extension failed:', error); // Don't refresh the extension list on failure - this allows our visual state rollback to work // The actual state in the config hasn't changed anyway throw error; // Re-throw to let the ExtensionItem component know it failed diff --git a/ui/desktop/src/components/settings_v2/extensions/agent-api.ts b/ui/desktop/src/components/settings_v2/extensions/agent-api.ts index 17a08524..799b0dbc 100644 --- a/ui/desktop/src/components/settings_v2/extensions/agent-api.ts +++ b/ui/desktop/src/components/settings_v2/extensions/agent-api.ts @@ -20,14 +20,15 @@ export async function extensionApiCall( type: isActivating ? 'activating' : 'removing', verb: isActivating ? 'Activating' : 'Removing', pastTense: isActivating ? 'activated' : 'removed', + presentTense: isActivating ? 'activate' : 'remove', }; // for adding the payload is an extensionConfig, for removing payload is just the name const extensionName = isActivating ? payload.name : payload; let toastId; - // Step 1: Show loading toast (only for activation) - if (isActivating) { + // Step 1: Show loading toast (only for activation of stdio) + if (isActivating && (payload as ExtensionConfig) && payload.type == 'stdio') { toastId = toastService.loading({ title: extensionName, msg: `${action.verb} ${extensionName} extension...`, @@ -75,6 +76,12 @@ export async function extensionApiCall( } catch (error) { // Final catch-all error handler toastService.dismiss(toastId); + const msg = error.length < 100 ? error : `Failed to ${action.presentTense} extension`; + toastService.error({ + title: extensionName, + msg: msg, + traceback: error, + }); console.error(`Error in extensionApiCall for ${extensionName}:`, error); throw error; } @@ -142,13 +149,11 @@ export async function addToAgent( // Check if this is a 428 error and make the message more descriptive if (error.message && error.message.includes('428')) { const enhancedError = new Error( - 'Agent is not initialized. Please initialize the agent first. (428 Precondition Required)' + 'Failed to add extension. Goose Agent was still starting up. Please try again.' ); console.error(`Failed to add extension ${extension.name} to agent: ${enhancedError.message}`); throw enhancedError; } - - console.error(`Failed to add extension ${extension.name} to agent:`, error); throw error; } } diff --git a/ui/desktop/src/components/settings_v2/extensions/extension-manager.ts b/ui/desktop/src/components/settings_v2/extensions/extension-manager.ts index 29ba7e56..2a7730a6 100644 --- a/ui/desktop/src/components/settings_v2/extensions/extension-manager.ts +++ b/ui/desktop/src/components/settings_v2/extensions/extension-manager.ts @@ -176,7 +176,7 @@ export async function toggleExtension({ showEscMessage: false, }); } catch (error) { - console.error('Error adding extension to agent. Will try to toggle back off. Error:', error); + console.error('Error adding extension to agent. Will try to toggle back off.'); try { await toggleExtension({ toggle: 'toggleOff', diff --git a/ui/desktop/src/components/settings_v2/extensions/modal/ExtensionConfigFields.tsx b/ui/desktop/src/components/settings_v2/extensions/modal/ExtensionConfigFields.tsx index 4f9bd7f4..0f814bcf 100644 --- a/ui/desktop/src/components/settings_v2/extensions/modal/ExtensionConfigFields.tsx +++ b/ui/desktop/src/components/settings_v2/extensions/modal/ExtensionConfigFields.tsx @@ -1,5 +1,6 @@ import { Input } from '../../../ui/input'; import React from 'react'; +import { removeShims } from '../utils'; interface ExtensionConfigFieldsProps { type: 'stdio' | 'sse' | 'builtin'; @@ -25,7 +26,7 @@ export default function ExtensionConfigFields({
onChange('cmd', e.target.value)} placeholder="e.g. npx -y @modelcontextprotocol/my-extension " className={`w-full ${!submitAttempted || isValid ? 'border-borderSubtle' : 'border-red-500'} text-textStandard`} diff --git a/ui/desktop/src/components/settings_v2/extensions/subcomponents/ExtensionList.tsx b/ui/desktop/src/components/settings_v2/extensions/subcomponents/ExtensionList.tsx index c92d8bcf..ebc473a8 100644 --- a/ui/desktop/src/components/settings_v2/extensions/subcomponents/ExtensionList.tsx +++ b/ui/desktop/src/components/settings_v2/extensions/subcomponents/ExtensionList.tsx @@ -3,7 +3,7 @@ import { FixedExtensionEntry } from '../../../ConfigContext'; import { ExtensionConfig } from '../../../../api/types.gen'; import ExtensionItem from './ExtensionItem'; import builtInExtensionsData from '../../../../built-in-extensions.json'; -import { combineCmdAndArgs } from '../utils'; +import { combineCmdAndArgs, removeShims } from '../utils'; interface ExtensionListProps { extensions: FixedExtensionEntry[]; @@ -57,7 +57,8 @@ export function getSubtitle(config: ExtensionConfig): string { return 'Built-in extension'; } if (config.type === 'stdio') { - const full_command = combineCmdAndArgs(config.cmd, config.args); + // remove shims for displaying + const full_command = combineCmdAndArgs(removeShims(config.cmd), config.args); return `STDIO extension${config.description ? `: ${config.description}` : ''}${full_command ? `\n${full_command}` : ''}`; } if (config.type === 'sse') { diff --git a/ui/desktop/src/components/settings_v2/extensions/utils.ts b/ui/desktop/src/components/settings_v2/extensions/utils.ts index 596cada2..abc3006a 100644 --- a/ui/desktop/src/components/settings_v2/extensions/utils.ts +++ b/ui/desktop/src/components/settings_v2/extensions/utils.ts @@ -143,3 +143,11 @@ export async function replaceWithShims(cmd: string) { return cmd; } + +export function removeShims(cmd: string) { + const segments = cmd.split('/'); + // Filter out any empty segments (which can happen with trailing slashes) + const nonEmptySegments = segments.filter((segment) => segment.length > 0); + // Return the last segment or empty string if there are no segments + return nonEmptySegments.length > 0 ? nonEmptySegments[nonEmptySegments.length - 1] : ''; +} diff --git a/ui/desktop/src/toasts.tsx b/ui/desktop/src/toasts.tsx index 1075a736..73d3dd13 100644 --- a/ui/desktop/src/toasts.tsx +++ b/ui/desktop/src/toasts.tsx @@ -23,31 +23,21 @@ export default class ToastService { } configure(options: ToastServiceOptions = {}): void { - console.log('ToastService.configure called with options:', options); if (options.silent !== undefined) { - console.log(`Setting silent from ${this.silent} to ${options.silent}`); this.silent = options.silent; } if (options.showEscMessage !== undefined) { - console.log( - `Setting showEscMessage from ${this.showEscMessage} to ${options.showEscMessage}` - ); this.showEscMessage = options.showEscMessage; } if (options.shouldThrow !== undefined) { - console.log(`Setting shouldThrow from ${this.shouldThrow} to ${options.shouldThrow}`); this.shouldThrow = options.shouldThrow; } } error({ title, msg, traceback }: { title: string; msg: string; traceback: string }): void { - console.log(`ToastService.error called - silent=${this.silent}`, { title, msg }); if (!this.silent) { toastError({ title, msg, traceback }); - } else { - console.log('Toast suppressed because silent=true'); } - console.error(msg, traceback); if (this.shouldThrow) { throw new Error(msg); @@ -55,9 +45,7 @@ export default class ToastService { } loading({ title, msg }: { title: string; msg: string }): string | number | undefined { - console.log(`ToastService.loading called - silent=${this.silent}`, { title, msg }); if (this.silent) { - console.log('Toast suppressed because silent=true'); return undefined; } @@ -72,9 +60,7 @@ export default class ToastService { } success({ title, msg }: { title: string; msg: string }): void { - console.log(`ToastService.success called - silent=${this.silent}`, { title, msg }); if (this.silent) { - console.log('Toast suppressed because silent=true'); return; } toastSuccess({ title, msg });