mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
69aea2fc4f
## Description A stray or typo'd `HEADROOM_QDRANT_PORT` can crash proxy startup during `ProxyConfig()` construction, even when memory is disabled. `memory_qdrant_port` is resolved through a dataclass field `default_factory`. That factory runs on every `ProxyConfig()` construction, regardless of whether memory or the `qdrant-neo4j` backend is enabled. The strict qdrant env parser raises on non-integer or out-of-range ports, so inherited values such as `HEADROOM_QDRANT_PORT=0`, `70000`, or `not-a-port` could prevent the proxy from starting for an off-by-default subsystem. ## Fix Use a fail-soft wrapper for the `ProxyConfig.memory_qdrant_port` default factory. Bad env values log a warning and fall back to the default port `6333`; valid env values are still honored. The strict `qdrant_env.qdrant_env_port()` parser remains unchanged for direct memory APIs and explicit tests, and the CLI `--memory-qdrant-port` option still validates `1..65535` before constructing `ProxyConfig`. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/proxy/models.py`: add `_qdrant_env_port_or_default()` and use it for the proxy config qdrant port default. - `tests/test_proxy_config_qdrant_port.py`: cover bad env values falling back, valid env values passing through, and `ProxyConfig()` construction surviving bad env. - `CHANGELOG.md`: add a bug-fix entry. - Merged current `main` to pick up the repository-wide memory factory type annotation fix that was breaking the PR lint job. ## Testing - [x] Unit tests pass (`pytest` focused locally; broader CI passed on the pre-merge head and fresh CI is running on the main-merged head) - [x] Linting passes (`ruff check` focused locally) - [x] Type checking passes for the prior mypy blocker after merging current `main` - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text uvx ruff@0.15.17 check headroom/proxy/models.py tests/test_proxy_config_qdrant_port.py headroom/memory/factory.py All checks passed! uvx ruff@0.15.17 format --check headroom/proxy/models.py tests/test_proxy_config_qdrant_port.py headroom/memory/factory.py 3 files already formatted git diff --check headroomlabs/main...HEAD # no output uv run --extra dev python -m pytest tests/test_proxy_config_qdrant_port.py -q 5 passed ``` ## Real Behavior Proof - Environment: Windows 11 review worktree, Python 3.13.3. - Exact command / steps: ran focused qdrant proxy config tests and targeted lint/format checks. - Observed result: invalid `HEADROOM_QDRANT_PORT` values fall back to `6333`; valid `6444` is honored; `ProxyConfig()` no longer raises during construction when env is bad. - Not tested: full suite; fresh CI is queued after the main merge. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes This intentionally changes only the unconditional proxy config default path. Direct qdrant env parsing remains strict, and explicit CLI qdrant port input is still range-validated by Click. Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
33 lines
1.2 KiB
Python
33 lines
1.2 KiB
Python
"""ProxyConfig construction must survive a bad HEADROOM_QDRANT_PORT.
|
|
|
|
The port is resolved by a field default_factory that runs on every ProxyConfig()
|
|
construction, so a stray/typo'd value must not crash proxy startup for an
|
|
off-by-default subsystem."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
|
|
from headroom.memory import qdrant_env
|
|
from headroom.proxy.models import ProxyConfig, _qdrant_env_port_or_default
|
|
|
|
|
|
@pytest.mark.parametrize("bad", ["not-a-port", "70000", "0"])
|
|
def test_bad_qdrant_port_falls_back_to_default(monkeypatch, bad):
|
|
monkeypatch.setenv("HEADROOM_QDRANT_PORT", bad)
|
|
assert _qdrant_env_port_or_default() == qdrant_env.DEFAULT_QDRANT_PORT
|
|
|
|
|
|
def test_valid_qdrant_port_is_honored(monkeypatch):
|
|
monkeypatch.setenv("HEADROOM_QDRANT_PORT", "6444")
|
|
assert _qdrant_env_port_or_default() == 6444
|
|
|
|
|
|
def test_proxyconfig_construction_survives_bad_qdrant_port(monkeypatch):
|
|
monkeypatch.setenv("HEADROOM_QDRANT_PORT", "not-a-port")
|
|
|
|
# Must not raise even though the port is unparseable (memory is off by
|
|
# default and unrelated to core proxying).
|
|
config = ProxyConfig()
|
|
|
|
assert config.memory_qdrant_port == qdrant_env.DEFAULT_QDRANT_PORT
|