fix: we load extensions when agent starts so don't do it up front

[michaelneale]

Previously it would load the extensions on GUI start - and then when you start interacting, it would load them any way, this removes that un-necessary initial load.

[Copilot]

Pull request overview

This PR optimizes the desktop app startup by removing unnecessary extension loading that happened on GUI initialization. Extensions are now loaded only when the agent actually starts (i.e., when the user begins interacting), preventing duplicate loading work.

Key changes:

• Removed early extension/session loading from App.tsx on hub page load
• Added duplicate-prevention logic to ExtensionManager to avoid re-adding already loaded extensions

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
ui/desktop/src/App.tsx	Removed useEffect that pre-loaded chat session and extensions on hub page; removed unused imports and state setters; added explanatory comment
crates/goose/src/agents/extension_manager.rs	Added early return in add_extension() to skip re-adding extensions that are already loaded

[DOsinga]

nice. does need a bit of post LLM cleaning

[tlongwell-block]

/goose

[github-actions]

Summary: This PR improves startup performance by removing redundant extension loading on GUI start - extensions are now only loaded when the agent actually starts. The approach is sound, though there's some dead code cleanup that could be done.

🟡 Warnings

1. Dead state variables in App.tsx (lines 366, 369)

   The state variables agentWaitingMessage and isExtensionsLoading now have no setters:

   const [agentWaitingMessage] = useState<string | null>(null);
   const [isExtensionsLoading] = useState(false);

   These will always be null and false respectively, making them dead code. They're still passed to ChatProvider and Hub, but will never change. Consider either:

   • Removing them entirely and updating ChatProvider/Hub to not require these props
   • Hardcoding the values directly in the JSX props if the types require them

2. TOCTOU race condition in add_extension (extension_manager.rs:481-593)

   The early-return check acquires a lock, checks, and releases it:

   if self.extensions.lock().await.contains_key(&sanitized_name) {
       return Ok(());
   }
   // ... expensive client creation happens here without lock ...
   let mut extensions = self.extensions.lock().await;
   extensions.insert(final_name, ...);

   If two concurrent calls for the same extension occur (extensions are loaded in parallel via Promise.allSettled), both may pass the check, both create clients, and the second overwrites the first. This wastes work but doesn't cause data corruption - the end state is still correct. This is acceptable for the optimization goal of this PR, but worth noting for future improvement.

🟢 Suggestions

1. Remove unused import cleanup (App.tsx line 42)

   The blank line where NoProviderOrModelError, useAgent was imported now just shows:

   import { View, ViewOptions } from './utils/navigationUtils';
   
   import { useNavigation } from './hooks/useNavigation';

   This could be cleaned up to remove the extra blank line, though it's very minor.

✅ Highlights

• Good performance optimization - pre-loading extensions on startup was indeed redundant since they load when the agent starts anyway
• The comment explaining the change is helpful: // Don't pre-load session/extensions on Hub - wait until user actually starts chatting
• The idempotency check in add_extension is a reasonable safety measure

---

Review generated by goose

[michaelneale]

@DOsinga yeah this was a late night hack, I wasn't even sure I was reading things right, can tidy this up for sure.

[michaelneale]

thanks @DOsinga @tlongwell-block there was even more I could delete!

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[codefromthecrypt]

code looks best in red