- Use atomic.Bool for closed flag to prevent TOCTOU race between
CallTool and Close operations
- Add double-check pattern in CallTool for thread-safe closed state
- Use atomic Swap in Close to ensure no new calls can start after
closed flag is set
- Move MCP manager cleanup defer before initialization to handle
partial initialization failures
- Update tests to use atomic.Bool operations
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Avoid logging sensitive cfg.Args in ConnectServer; log args_count instead
- Sanitize server/tool name components in MCPTool.Name() to ensure valid
identifiers for downstream providers (lowercase, [a-z0-9_-] only)
- Add slack as 5th MCP server example in config.example.json
- Move Dockerfile.full and docker-compose.full.yml into docker/ directory
for consistency with existing docker/Dockerfile and docker/docker-compose.yml
- Fix all Makefile docker-* targets to reference correct compose file paths
- Fix docker/docker-compose.full.yml build context (.. ) and volume paths
- Fix scripts/test-docker-mcp.sh compose file path and replace cowsay test
with actual @modelcontextprotocol/server-filesystem MCP server test
A race could occur when Close() called conn.Session.Close() concurrently
with an in-flight conn.Session.CallTool(), leading to undefined behavior.
Fix by adding a sync.WaitGroup to Manager:
- CallTool increments the WaitGroup while holding the read lock (after
checking m.closed), ensuring no new calls are counted after Close sets
the flag
- Close sets m.closed=true, releases the write lock, then waits for all
in-flight calls to finish via wg.Wait() before closing sessions
Previously Close() discarded all underlying errors and returned only
'failed to close N server(s)', making debugging impossible.
Now each error wraps the server name and original cause, and all errors
are joined so callers can inspect the full failure list.
A line like '=value' would result in envVars[""] = "value", producing
an invalid environment entry for the child process. Return an error
instead when the key is empty.
Add a closed flag to the Manager struct to prevent CallTool from
accessing server connections after Close has been called. The flag
is checked within the RLock in CallTool to ensure thread-safety.
Previously, CallTool could obtain a server reference using RLock,
then that reference could be closed by Close() running concurrently,
leading to use-after-close errors. Now:
1. CallTool checks the closed flag before accessing servers
2. Close sets the closed flag before closing connections
3. CallTool directly accesses m.servers within the lock instead
of using GetServer() to avoid releasing the lock prematurely
This ensures CallTool will not use a server connection that is
being closed or has been closed.
Addresses Copilot code review feedback.
Use a map to merge environment variables with guaranteed override
behavior. Config variables (cfg.Env) now properly override file
variables (envFile), which in turn override parent process environment.
Previously, simply appending to a slice could result in duplicate
variables, and while most systems use the last occurrence, this
behavior is not guaranteed and could lead to unexpected results.
Addresses Copilot code review feedback.
Replace full *config.Config reference with config.MCPConfig value type
in AgentLoop to allow garbage collection of unused configuration data.
Changes:
- AgentLoop now stores only MCPConfig and workspacePath (minimal deps)
- Add mcp.Manager.LoadFromMCPConfig() for minimal dependency version
- Keep LoadFromConfig() for backward compatibility
- Full Config object can be GC'd after NewAgentLoop() returns
This optimization reduces memory usage by not holding references to
unused channel, provider, gateway, and device configurations.
- Resolve relative envFile paths relative to workspace instead of CWD
- Add filepath import for path operations
- Pass workspace path to goroutines for path resolution
- Improves portability in Docker environments where CWD may vary
- Absolute envFile paths continue to work as before
- Add errors.Join to return aggregated error when all enabled MCP servers fail
- Track enabled server count separately from total configured servers
- Return error only when all servers fail, not for partial failures
- Improve logging with accurate server counts (enabled vs connected)
- Maintains fault tolerance: partial failures don't stop initialization
Implement comprehensive MCP support with stdio/HTTP/SSE transports, environment variable configuration (env and envFile), custom headers, tool registration, and automatic resource cleanup. Includes full test coverage and VSCode-compatible configuration.
- Added pkg/mcp/manager.go for server lifecycle management
- Added pkg/tools/mcp_tool.go for tool wrapping
- Integrated into agent loop with cleanup
- Support for envFile loading (.env format)
- Headers injection for HTTP/SSE authentication
- Example configs for filesystem, github, brave-search, postgres
lxowalle