Skip to content

Add LSP multiplexer to support multiple LSP toolsets#1970

Merged
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/multiple-lsp-7f7e9871
Mar 9, 2026
Merged

Add LSP multiplexer to support multiple LSP toolsets#1970
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/multiple-lsp-7f7e9871

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented Mar 6, 2026

When multiple LSP toolsets are configured (e.g., gopls for Go and pyright for Python), each produces tools with identical names (lsp_hover, lsp_definition, etc.), causing LLM APIs to reject the request because tool names must be unique.

Changes

  • pkg/tools/builtin/lsp_multiplexer.go — New LSPMultiplexer that combines multiple LSP backends into a single toolset:

    • File-based tools (hover, definition, references, etc.) are routed to the correct backend by matching the file extension via HandlesFile()
    • Non-file tools (workspace, workspace_symbols) are broadcast to all backends with results merged
    • Implements ToolSet, Startable, and Instructable interfaces
    • Deduplicates identical instructions across backends
  • pkg/teamloader/teamloader.go — Modified getToolsForAgent to detect multiple LSP toolsets and combine them into a single multiplexer. Per-toolset config (tool filters, instructions, toon, defer) is preserved.

  • Tests — Comprehensive test coverage for routing, broadcasting, error handling, deduplication, and teamloader integration.

Fixes #1969

@dgageot dgageot requested a review from a team as a code owner March 6, 2026 15:00
Copy link
Copy Markdown

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Assessment: 🔴 CRITICAL

This PR adds LSP multiplexing functionality to combine multiple LSP toolsets. The core routing logic is sound, but there are 1 high-severity resource leak and 4 medium-severity issues that should be addressed before merging.

Critical Issues

HIGH: Resource leak in Start method - if initialization fails partway through, previously started LSP backends are left running.

Notable Issues

  • Empty file arguments may route to unintended backends
  • Type assertion failures are silently ignored in teamloader
  • Broadcast operations abort on first error instead of aggregating results
  • Constructor panics on invalid input (could return error instead)

Details

See inline comments below.

@dgageot dgageot force-pushed the board/multiple-lsp-7f7e9871 branch 3 times, most recently from aa55338 to 9221d03 Compare March 6, 2026 17:15
@dgageot dgageot force-pushed the board/multiple-lsp-7f7e9871 branch 7 times, most recently from 4870c9a to 47ff48c Compare March 9, 2026 12:27
Also simplifies the aqua registry from which lsp commands are downloaded from

Fixes docker#1969

Assisted-By: cagent
Signed-off-by: David Gageot <david.gageot@docker.com>
@dgageot dgageot force-pushed the board/multiple-lsp-7f7e9871 branch from 47ff48c to dc49baf Compare March 9, 2026 13:40
@dgageot dgageot merged commit fe313c3 into docker:main Mar 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple LSP toolsets in a single agent cause 'Tool names must be unique' error

2 participants