UAT: LspRegistry.register() raises ValueError on duplicate — no update/replace semantics for re-registration #4676

New issue

Open

opened 2026-04-08 17:59:46 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-08 17:59:46 +00:00

Owner

Bug Report

Feature Area: LSP Registry — server registration
Severity: Medium — re-registering an LSP server (e.g., after config reload) raises an error instead of updating
Source: src/cleveragents/lsp/registry.py

What Was Tested

Code-level analysis of LspRegistry.register() to verify that the spec-described registration behavior is correctly implemented, including update semantics.

Expected Behavior (from spec)

The spec describes the LSP Registry as supporting server registration with the ability to update existing registrations. When a server with the same name is re-registered (e.g., after a config file reload), the registry should update the existing entry rather than raising an error.

The spec also describes a register_or_update() or equivalent upsert operation for config reload scenarios.

Actual Behavior

LspRegistry.register() raises ValueError if a server with the same name is already registered:

def register(self, config: LspServerConfig) -> None:
    if config is None:
        raise ValueError("config must not be None")
    with self._lock:
        if config.name in self._servers:
            raise ValueError(f"LSP server '{config.name}' is already registered")
        self._servers[config.name] = config
        logger.info("Registered LSP server: %s", config.name)

There is no update(), register_or_update(), or upsert() method. The only way to update a server config is to call remove() first, then register() — but this is not atomic and creates a window where the server appears unregistered.

Code Location

src/cleveragents/lsp/registry.py — register() method

Impact

Config reload scenarios (e.g., user edits their LSP server config file) will fail with ValueError instead of updating the registration.
Test setup code that calls register() twice (e.g., in parametrized tests) will fail unexpectedly.
There is no atomic update path — callers must remove() then register() with a race condition window in between.

Fix

Add a register_or_update() method (or add an update: bool = False parameter to register()):

def register(self, config: LspServerConfig, *, update: bool = False) -> None:
    if config is None:
        raise ValueError("config must not be None")
    with self._lock:
        if config.name in self._servers and not update:
            raise ValueError(f"LSP server '{config.name}' is already registered")
        self._servers[config.name] = config
        action = "Updated" if config.name in self._servers else "Registered"
        logger.info("%s LSP server: %s", action, config.name)

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: uat-tester

## Bug Report **Feature Area:** LSP Registry — server registration **Severity:** Medium — re-registering an LSP server (e.g., after config reload) raises an error instead of updating **Source:** `src/cleveragents/lsp/registry.py` --- ## What Was Tested Code-level analysis of `LspRegistry.register()` to verify that the spec-described registration behavior is correctly implemented, including update semantics. ## Expected Behavior (from spec) The spec describes the LSP Registry as supporting server registration with the ability to update existing registrations. When a server with the same name is re-registered (e.g., after a config file reload), the registry should update the existing entry rather than raising an error. The spec also describes a `register_or_update()` or equivalent upsert operation for config reload scenarios. ## Actual Behavior `LspRegistry.register()` raises `ValueError` if a server with the same name is already registered: ```python def register(self, config: LspServerConfig) -> None: if config is None: raise ValueError("config must not be None") with self._lock: if config.name in self._servers: raise ValueError(f"LSP server '{config.name}' is already registered") self._servers[config.name] = config logger.info("Registered LSP server: %s", config.name) ``` There is no `update()`, `register_or_update()`, or `upsert()` method. The only way to update a server config is to call `remove()` first, then `register()` — but this is not atomic and creates a window where the server appears unregistered. ## Code Location - `src/cleveragents/lsp/registry.py` — `register()` method ## Impact 1. Config reload scenarios (e.g., user edits their LSP server config file) will fail with `ValueError` instead of updating the registration. 2. Test setup code that calls `register()` twice (e.g., in parametrized tests) will fail unexpectedly. 3. There is no atomic update path — callers must `remove()` then `register()` with a race condition window in between. ## Fix Add a `register_or_update()` method (or add an `update: bool = False` parameter to `register()`): ```python def register(self, config: LspServerConfig, *, update: bool = False) -> None: if config is None: raise ValueError("config must not be None") with self._lock: if config.name in self._servers and not update: raise ValueError(f"LSP server '{config.name}' is already registered") self._servers[config.name] = config action = "Updated" if config.name in self._servers else "Registered" logger.info("%s LSP server: %s", action, config.name) ``` --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 added this to the v3.6.0 milestone

2026-04-08 18:05:29 +00:00

HAL9000 referenced this issue

2026-04-08 20:11:34 +00:00

[Automated] CleverAgents Build Session - 2026-04-08 #4799