The use of the or operator for environment variable overrides prevents setting a value to an empty string if a default exists in the configuration. This is particularly problematic for username and password, where an empty string is a valid value indicating no authentication. Using os.environ.get(key, default) is a more robust pattern for these overrides.

Similar to the L2 adapter, using or for environment variable overrides prevents overriding a non-empty configuration value with an empty string (e.g., to disable authentication). It is better to use os.environ.get(key, default) to correctly handle cases where the environment variable is explicitly set to an empty string.

        username = os.environ.get("LMCACHE_RESP_USERNAME", str(extra_config.get("username", "")))
        password = os.environ.get("LMCACHE_RESP_PASSWORD", str(extra_config.get("password", "")))

Using os.environ.get(key, default) is cleaner and more consistent for handling environment variable overrides compared to the or pattern, which can have unexpected behavior with empty strings.

Typically, env var are set for long term usage, like a default value. I would recommend have the command line args override env var instead of the other way around.

agreed

Env var host/port fallback unreachable in MP mode

High Severity

The env var fallback for LMCACHE_RESP_HOST and LMCACHE_RESP_PORT in _create_resp_l2_adapter is dead code. from_dict() validates that host is a non-empty string and port is a positive integer, raising ValueError before the adapter factory ever runs. A user who omits host/port from the JSON and relies solely on env vars will get a crash at config parsing time, not a graceful fallback. The documentation, help() text, and tests all advertise this feature, but it cannot work.

 

^{Reviewed by Cursor Bugbot for commit 22a083a. Configure here.}

Tests duplicate logic instead of exercising production code

Medium Severity

Every test in this file copy-pastes the env var resolution logic inline (e.g. config.host or os.environ.get(...)) instead of calling _create_resp_l2_adapter or RESPConnectorAdapter.create_connector. The tests verify their own inlined logic, not the production code. They will always pass even if the real adapters break — and in fact they mask the from_dict validation bug that makes host/port env var fallback unreachable in MP mode.

 

^{Triggered by project rule: LMCache Code Review Style Guide}

^{Reviewed by Cursor Bugbot for commit 22a083a. Configure here.}

Env var host/port fallback unreachable in non-MP mode

High Severity

The env var fallback for LMCACHE_RESP_HOST and LMCACHE_RESP_PORT at lines 57–62 is dead code in non-MP mode. The parse_remote_url function (called just above on line 54) contains assert host is not None and assert port is not None, which will crash with an AssertionError for any URL missing a host or port — before the fallback logic ever runs. A user who omits host/port from remote_url expecting the env vars to fill them in will get an assertion crash instead.

 

^{Reviewed by Cursor Bugbot for commit 22a083a. Configure here.}