Skip to content

Allow add-ons to register and execute commands - #4201

Open
pvcresin wants to merge 4 commits into
Shopify:mainfrom
pvcresin:allow-addon-commands
Open

pvcresin wants to merge 4 commits into
Shopify:mainfrom
pvcresin:allow-addon-commands

Conversation

@pvcresin

@pvcresin pvcresin commented Sep 6, 2026 •

Copy link
Copy Markdown

Motivation

Ruby LSP add-ons can contribute Code Lenses and other editor features, but there is currently no generic way for an add-on to register and handle the commands referenced by those features. This forces add-ons that need server-side command handling to provide additional editor integration.

Implementation

  • Add Addon#commands and Addon#execute_command hooks.
  • Add Addon#command_id, which appends a UUID generated for each add-on instance to logical command identifiers.
  • Register the scoped add-on command IDs through the standard client/registerCapability request after add-ons are loaded.
  • Handle the standard workspace/executeCommand request by resolving scoped IDs back to logical commands and routing them to the owning add-on.
  • Support multiple Ruby LSP server instances in multi-root workspaces without command name collisions.
  • Respect the workspace.executeCommand.dynamicRegistration capability reported by the client and ignore errored add-ons.
  • Document the API and add protocol/server coverage.

Add-ons are loaded after the initialize response, so dynamic registration keeps the existing add-on lifecycle unchanged. The VS Code extension does not require changes because its existing vscode-languageclient dependency handles standard dynamic execute-command registration.

Automated Tests

  • test/requests/execute_command_test.rb (registration, command ID uniqueness, and routing)
  • test/global_state_test.rb
  • Related Server, Addon, and Code Lens tests
  • RuboCop
  • Sorbet

Manual Tests

Not run in a full VS Code session. The new server tests verify the dynamic registration payload and command dispatch. An end-to-end test can be performed with an add-on that returns a command from commands, implements execute_command, and exposes a Code Lens using command_id.

@pvcresin
pvcresin requested a review from a team as a code owner September 6, 2026 12:16
@pvcresin

pvcresin commented Sep 6, 2026

Copy link
Copy Markdown
Author

I have signed the CLA!

@soutaro soutaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working on this! There’s a multi-root workspace case we need to handle. Ruby LSP runs a separate server for each workspace folder, so two folders loading the same add-on will register the same command names, which results in a registration error in VS Code.

Could we make the command names unique per server instance? One option is a UUID suffix, with a helper that add-ons can use when building Code Lenses and other command references. Ruby LSP could map those names back to the original commands when dispatching to the add-on. I tested this approach in VS Code and confirmed that commands reached the correct server, including after restarting one of them.

@soutaro soutaro added enhancement New feature or request server This pull request should be included in the server gem's release notes labels Sep 17, 2026
@pvcresin

Copy link
Copy Markdown
Author

@soutaro
Thanks for pointing this out! I updated the implementation in 41dda40.
Each add-on instance now gets a UUID-based command ID through Addon#command_id. Ruby LSP registers these scoped IDs and resolves them back to the original logical command before dispatching to Addon#execute_command. This prevents command collisions between Ruby LSP server instances in multi-root workspaces.
I also added tests covering command ID uniqueness and routing commands to the correct add-on instance, and updated the add-on documentation. The targeted tests, RuboCop, and Sorbet checks all pass.

@pvcresin
pvcresin requested a review from soutaro September 17, 2026 09:43

@soutaro soutaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! There are a few minor things, but the PR looks good overall.

Comment thread lib/ruby_lsp/addon.rb Outdated
# typed: strict
# frozen_string_literal: true

require "securerandom"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this require can be deleted, since we usually load ruby_lsp library to use addon.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed the explicit require "securerandom" in c0bf20a.

Comment thread test/requests/execute_command_test.rb Outdated
addon = @addon_class.new
Addon.addons << addon

server = Server.new(test_mode: true)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any reason not to use with_server(load_addons: false) here?

@pvcresin pvcresin Sep 24, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you for the suggestion.
I updated the test in bc946d4 to use with_server(load_addons: false).
The existing capability setup and stubs remain in place, while the shared helper now handles server cleanup, so the manual ensure and run_shutdown are no longer necessary.

@pvcresin

Copy link
Copy Markdown
Author

@soutaro All done with the handling work.

@pvcresin
pvcresin requested a review from soutaro September 24, 2026 03:57

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request server This pull request should be included in the server gem's release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants