diff --git a/docs/plans/2026-06-14-observer-filter-cache-key-collision/plan.md b/docs/plans/2026-06-14-observer-filter-cache-key-collision/plan.md index fddd4be..e2a954a 100644 --- a/docs/plans/2026-06-14-observer-filter-cache-key-collision/plan.md +++ b/docs/plans/2026-06-14-observer-filter-cache-key-collision/plan.md @@ -18,7 +18,22 @@ Investigation findings: - The frontend correctly sends repeated params: `?observed_by=A&observed_by=B` (`web/static/js/spa/api.js` appends each array item; `pages/messages.js:263`). -**Root cause — cache key collision.** The cached-response key is built by +**Primary root cause — the web proxy collapses repeated params.** The SPA calls +the backend through the web API proxy (`src/meshcore_hub/web/app.py:696` +`api_proxy`). It forwarded query params with: + +```python +params = dict(request.query_params) # collapses repeated keys to LAST value +``` + +`dict(QueryParams)` keeps only the last value of a repeated key, so +`?observed_by=A&observed_by=B` was forwarded to the backend as `observed_by=B` +only. The backend then filtered to B's events, dropping the A-only message — the +exact reported symptom, and independent of caching. Fix: forward +`request.query_params.multi_items()` (a list of `(key, value)` tuples; httpx +preserves them as repeated params). + +**Secondary root cause — cache key collision.** The cached-response key is built by `sorted_query_string` (`src/meshcore_hub/api/cache.py:15`): ```python diff --git a/src/meshcore_hub/web/app.py b/src/meshcore_hub/web/app.py index e9d24b1..b653b3c 100644 --- a/src/meshcore_hub/web/app.py +++ b/src/meshcore_hub/web/app.py @@ -719,8 +719,11 @@ def create_app( client: httpx.AsyncClient = request.app.state.http_client url = f"/api/{path}" - # Forward query parameters - params = dict(request.query_params) + # Forward query parameters. Use multi_items() (a list of (key, value) + # tuples) rather than dict(...), which would collapse repeated keys to + # the last value and drop all but one value of multi-valued filters such + # as observed_by (?observed_by=A&observed_by=B). + params = request.query_params.multi_items() # Forward body for write methods body = None diff --git a/tests/test_web/conftest.py b/tests/test_web/conftest.py index ff6f468..a1bdc52 100644 --- a/tests/test_web/conftest.py +++ b/tests/test_web/conftest.py @@ -28,6 +28,9 @@ class MockHttpClient: def __init__(self) -> None: """Initialize mock client with default responses.""" self._responses: dict[str, dict[str, Any]] = {} + # Records the params forwarded by the most recent request() call so + # tests can assert how the proxy forwards query parameters. + self.last_request_params: Any = None self._default_responses() def _default_responses(self) -> None: @@ -265,6 +268,7 @@ class MockHttpClient: headers: dict | None = None, ) -> Response: """Mock generic request (used by API proxy).""" + self.last_request_params = params key = f"{method.upper()}:{url}" if key in self._responses: return self._create_response(key) diff --git a/tests/test_web/test_app.py b/tests/test_web/test_app.py index a42530e..5076e54 100644 --- a/tests/test_web/test_app.py +++ b/tests/test_web/test_app.py @@ -175,6 +175,32 @@ class TestConfigJsonXssEscaping: assert parsed["role_names"]["test"] == "test" +class TestApiProxyQueryParams: + """The proxy must forward repeated query params without collapsing them.""" + + def test_repeated_query_params_all_forwarded( + self, client: TestClient, mock_http_client: MockHttpClient + ) -> None: + # Multi-valued observer filter: the backend must receive BOTH values. + # dict(request.query_params) would drop "A" and only forward "B", + # making OR-filtered messages disappear when a second observer is added. + client.get("/api/v1/messages?observed_by=A&observed_by=B") + + forwarded = mock_http_client.last_request_params + pairs = list(forwarded) # list of (key, value) tuples + assert ("observed_by", "A") in pairs + assert ("observed_by", "B") in pairs + + def test_single_query_param_forwarded( + self, client: TestClient, mock_http_client: MockHttpClient + ) -> None: + client.get("/api/v1/messages?observed_by=A&limit=10") + + pairs = list(mock_http_client.last_request_params) + assert ("observed_by", "A") in pairs + assert ("limit", "10") in pairs + + class TestCheckApiAccess: """Unit tests for check_api_access with _OPEN, _AUTHENTICATED, and role-based levels."""