mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-06-12 18:08:54 +00:00
fix(agent): tie MCP connections to agent lifecycle context
- Defer MCP server initialization to Run() using agent's context - Add mcpConfig and mcpInitOnce fields to AgentLoop - Use sync.Once to ensure MCP loads exactly once with proper context - Prevents orphaned subprocesses and resource leaks on cancellation This fixes GitHub Copilot feedback that MCP connections with context.Background() won't terminate when the agent stops, causing potential resource leaks and orphaned stdio/SSE connections.
This commit is contained in:
+17
-9
@@ -42,7 +42,9 @@ type AgentLoop struct {
|
||||
state *state.Manager
|
||||
contextBuilder *ContextBuilder
|
||||
tools *tools.ToolRegistry
|
||||
mcpManager *mcp.Manager // MCP server manager for resource cleanup
|
||||
mcpManager *mcp.Manager // MCP server manager for resource cleanup
|
||||
mcpConfig *config.Config // Config for lazy MCP initialization
|
||||
mcpInitOnce sync.Once // Ensures MCP is initialized only once
|
||||
running atomic.Bool
|
||||
summarizing sync.Map // Tracks which sessions are currently being summarized
|
||||
channelManager *channels.Manager
|
||||
@@ -129,15 +131,9 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
|
||||
|
||||
restrict := cfg.Agents.Defaults.RestrictToWorkspace
|
||||
|
||||
// Initialize MCP Manager and load servers
|
||||
// Create MCP Manager (actual server loading deferred to Run())
|
||||
// This ensures MCP connections use the agent's lifecycle context
|
||||
mcpManager := mcp.NewManager()
|
||||
ctx := context.Background()
|
||||
if err := mcpManager.LoadFromConfig(ctx, cfg); err != nil {
|
||||
logger.WarnCF("agent", "Failed to load MCP servers, MCP tools will not be available",
|
||||
map[string]interface{}{
|
||||
"error": err.Error(),
|
||||
})
|
||||
}
|
||||
|
||||
// Create tool registry for main agent
|
||||
toolsRegistry := createToolRegistry(workspace, restrict, cfg, msgBus, mcpManager)
|
||||
@@ -177,6 +173,7 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
|
||||
contextBuilder: contextBuilder,
|
||||
tools: toolsRegistry,
|
||||
mcpManager: mcpManager,
|
||||
mcpConfig: cfg, // Store config for lazy initialization in Run()
|
||||
summarizing: sync.Map{},
|
||||
}
|
||||
}
|
||||
@@ -184,6 +181,17 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
|
||||
func (al *AgentLoop) Run(ctx context.Context) error {
|
||||
al.running.Store(true)
|
||||
|
||||
// Initialize MCP servers using the agent's lifecycle context
|
||||
// This ensures MCP connections are cancelled when the agent stops
|
||||
al.mcpInitOnce.Do(func() {
|
||||
if err := al.mcpManager.LoadFromConfig(ctx, al.mcpConfig); err != nil {
|
||||
logger.WarnCF("agent", "Failed to load MCP servers, MCP tools will not be available",
|
||||
map[string]interface{}{
|
||||
"error": err.Error(),
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
for al.running.Load() {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
|
||||
Reference in New Issue
Block a user