Skip to content

Make capability registration resilient to unresponsive clients#250

Closed
yasmewad wants to merge 1 commit into
smithy-lang:mainfrom
yasmewad:fix/resilient-capability-registration
Closed

Make capability registration resilient to unresponsive clients#250
yasmewad wants to merge 1 commit into
smithy-lang:mainfrom
yasmewad:fix/resilient-capability-registration

Conversation

@yasmewad
Copy link
Copy Markdown
Contributor

Some LSP clients (such as Kiro CLI) don't respond to client/registerCapability or client/unregisterCapability
requests, which blocks lsp4j's message loop and hangs the server. This is especially problematic
because the unregister→register chain runs on every file change, workspace folder change, and
build file save (since clients don't de-duplicate file watchers).

Per the LSP 3.17 spec, dynamic registration is optional — "Not all clients need to support
dynamic capability registration"
— and didChangeWatchedFiles has no static fallback
("the current protocol doesn't support static configuration for file changes from the server
side"
). The server must degrade gracefully when registration isn't available.

Changes

  • Best-effort capability registration: Added registerBestEffort() and
    unregisterBestEffort() helper methods that wrap all 6 registerCapability/
    unregisterCapability call sites with .exceptionally() handlers. Each call site
    logs a distinct warning describing which registration failed.

  • Fixed unregister→register chains: Changed thenRun to thenCompose in the 3
    places that chain unregister then register (didChangeWatchedFiles,
    didChangeWorkspaceFolders, didSave). thenRun silently skips the re-registration
    if the upstream future completes exceptionally; thenCompose properly composes the
    two async operations so re-registration always proceeds.

  • Removed unused version.properties: The createProperties Gradle task and startup
    banner in connect() were dead code. Removed along with their now-unused imports.

  • Made StubClient extensible: Removed final modifier to allow test-specific
    subclasses that simulate client failures.

Testing

  • initializedHandlesRegisterCapabilityFailure — verifies the server continues when
    registerCapability returns a failed future during startup
  • unregisterFailureDoesNotBlockReRegistration — verifies the unregister→register
    chain in didChangeWatchedFiles still works when unregisterCapability fails
  • After using the binary generated, it works well with agentic coding tools like Kiro CLI

Some LSP clients don't respond to client/registerCapability or
client/unregisterCapability requests, which blocks lsp4j's message
loop and hangs the server.

This change adds .exceptionally() handlers with warning-level logging
to all 6 registerCapability/unregisterCapability call sites, treating
capability registration as best-effort. The server continues to
function without dynamic registrations.

Also removes the unused version.properties build task and startup
banner from connect(), along with their now-dead imports.
@yasmewad yasmewad requested a review from a team as a code owner April 18, 2026 14:57
@yasmewad yasmewad requested a review from JordonPhillips April 18, 2026 14:57
@yasmewad yasmewad closed this Apr 19, 2026
@yasmewad
Copy link
Copy Markdown
Contributor Author

Closing as I realized the existing binary pattern works with Kiro CLI. It was just that I was missing some setup instructions in correct order.

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.

1 participant