From a728593bbba77c215f9041e5b6a00c33e6f9cf80 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sat, 18 Jul 2026 00:25:44 +0200 Subject: [PATCH 1/4] =?UTF-8?q?fix(settings):=20reconcile=20memory-url=20p?= =?UTF-8?q?robe=20with=20#1904=20endpoints=20=E2=80=94=20fold=20reachabili?= =?UTF-8?q?ty=20logic=20into=20existing=20routes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fold _is_local_url, _probe_taosmd, and _MEMORY_URL_PROBE_CACHE into #1904's existing GET+PUT /api/settings/memory-url (no duplicate route registration). - GET returns {url, is_local, reachable} with 30s TTL probe cache. - PUT validates via Pydantic MemoryUrlUpdate model (no unguarded request.json()), persists via save_config_locked, and returns {url, is_local, reachable}. - Trailing-slash stripping uses explicit url.endswith('/') + url[:-1] instead of rstrip('/') character-set behavior. - Docstring explicitly states URL is always accepted and persisted; reachable field reports probe result but does not gate the save. - 13 tests: default URL, local/remote detection, PUT persistence, validation (empty URL, non-http scheme, missing hostname, malformed JSON), unreachable probe, trailing-slash strip, and _is_local_url unit coverage. - Closes #1911. Fixes Kilo 2W+2S on PR #1931. --- tests/test_routes_settings.py | 150 +++++++++++++++++++++++++++++++++ tinyagentos/routes/settings.py | 67 ++++++++++++++- 2 files changed, 213 insertions(+), 4 deletions(-) diff --git a/tests/test_routes_settings.py b/tests/test_routes_settings.py index 0bdd9c5bb..38f7b1412 100644 --- a/tests/test_routes_settings.py +++ b/tests/test_routes_settings.py @@ -330,3 +330,153 @@ async def test_result_fields_present(self, tmp_path): assert r.rebuilt is True assert r.success is False assert r.message == "x" + + +class TestMemoryUrlSettings: + """Contract: GET /api/settings/memory-url returns {url, is_local, reachable} + and PUT probes the new target before returning (#1911).""" + + @pytest.mark.asyncio + async def test_get_returns_default_url(self, client): + """Default URL is localhost:7900, is_local=True, reachable depends on env.""" + resp = await client.get("/api/settings/memory-url") + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "http://localhost:7900" + assert data["is_local"] is True + assert isinstance(data["reachable"], bool) + + @pytest.mark.asyncio + async def test_get_returns_is_local_true_for_localhost(self, client, app): + """Setting the config URL to a loopback address must report is_local=True.""" + app.state.config.memory_url = "http://127.0.0.1:7900" + resp = await client.get("/api/settings/memory-url") + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "http://127.0.0.1:7900" + assert data["is_local"] is True + + @pytest.mark.asyncio + async def test_get_returns_is_local_false_for_remote(self, client, app): + """A remote URL must report is_local=False.""" + app.state.config.memory_url = "https://memory.example.com:7900" + resp = await client.get("/api/settings/memory-url") + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "https://memory.example.com:7900" + assert data["is_local"] is False + + @pytest.mark.asyncio + async def test_put_sets_url_and_probes(self, client, app): + """PUT persists the URL to config and returns probe result.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http://192.168.1.100:7900"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "http://192.168.1.100:7900" + assert data["is_local"] is True # 192.168.x.x is private + assert isinstance(data["reachable"], bool) + # Verify it persisted on config + assert app.state.config.memory_url == "http://192.168.1.100:7900" + + @pytest.mark.asyncio + async def test_put_rejects_empty_url(self, client): + """Empty URL must return 400.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": ""}, + ) + assert resp.status_code == 400 + + @pytest.mark.asyncio + async def test_put_rejects_non_http_scheme(self, client): + """A non-http scheme must return 400.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "ftp://localhost:7900"}, + ) + assert resp.status_code == 400 + assert "http" in resp.json()["error"].lower() + + @pytest.mark.asyncio + async def test_get_reflects_put(self, client, app): + """After PUT, GET must return the new URL.""" + await client.put( + "/api/settings/memory-url", + json={"url": "https://taosmd.local:8443"}, + ) + resp = await client.get("/api/settings/memory-url") + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "https://taosmd.local:8443" + assert data["is_local"] is False + + @pytest.mark.asyncio + async def test_is_local_url_helper(self): + """Unit-test _is_local_url directly.""" + from tinyagentos.routes.settings import _is_local_url + + assert _is_local_url("http://localhost:7900") is True + assert _is_local_url("http://127.0.0.1:7900") is True + assert _is_local_url("http://[::1]:7900") is True + assert _is_local_url("http://10.0.0.1:7900") is True # private + assert _is_local_url("http://192.168.1.1:7900") is True # private + assert _is_local_url("https://example.com:7900") is False + assert _is_local_url("http://8.8.8.8:7900") is False # public IP + + @pytest.mark.asyncio + async def test_put_probes_and_reports_unreachable(self, client): + """When taosmd is not running, reachable must be False.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http://localhost:19999"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["reachable"] is False # no taosmd on that port + + @pytest.mark.asyncio + async def test_put_strips_trailing_slash(self, client, app): + """Trailing slash on the URL should be stripped.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http://localhost:7900/"}, + ) + assert resp.status_code == 200 + assert resp.json()["url"] == "http://localhost:7900" + assert app.state.config.memory_url == "http://localhost:7900" + + @pytest.mark.asyncio + async def test_put_rejects_url_without_hostname(self, client): + """A URL without a valid hostname must return 400 (netloc check).""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http:///path"}, + ) + assert resp.status_code == 400 + + @pytest.mark.asyncio + async def test_put_response_includes_is_local_and_reachable(self, client): + """PUT response must include is_local and reachable alongside url.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "https://remote.example.com:7900"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert "url" in data + assert "is_local" in data + assert "reachable" in data + assert data["is_local"] is False # remote hostname + + @pytest.mark.asyncio + async def test_put_rejects_malformed_json(self, client): + """Malformed JSON body must return 422 (Pydantic validation), not 500.""" + resp = await client.put( + "/api/settings/memory-url", + content=b"not json", + headers={"Content-Type": "application/json"}, + ) + assert resp.status_code == 422 diff --git a/tinyagentos/routes/settings.py b/tinyagentos/routes/settings.py index e4c611e15..53509360f 100644 --- a/tinyagentos/routes/settings.py +++ b/tinyagentos/routes/settings.py @@ -1,7 +1,9 @@ from __future__ import annotations import asyncio import datetime +import httpx import io +import ipaddress import logging import os import shutil @@ -1075,31 +1077,88 @@ async def set_update_channel(request: Request, body: UpdateChannel): # Memory URL (taOSmd) # --------------------------------------------------------------------------- +_MEMORY_URL_PROBE_CACHE: dict = {} # {url: (timestamp, bool)} + + +def _is_local_url(url: str) -> bool: + """Return True if *url* points to the local machine (loopback or private).""" + parsed = urlparse(url) + hostname = parsed.hostname + if not hostname: + return False + if hostname in ("localhost", "127.0.0.1", "::1"): + return True + try: + addr = ipaddress.ip_address(hostname) + return addr.is_loopback or addr.is_private + except ValueError: + return False + + +async def _probe_taosmd(request: Request, url: str) -> bool: + """Probe the taOSmd /health endpoint. Returns True if reachable.""" + try: + client = request.app.state.http_client + resp = await client.get( + f"{url}/health", + timeout=httpx.Timeout(3.0, connect=2.0), + ) + return resp.status_code == 200 + except Exception: + return False + + class MemoryUrlUpdate(BaseModel): url: str @router.get("/api/settings/memory-url") async def get_memory_url(request: Request): - """Return the current taOSmd memory URL.""" + """Return the current taOSmd memory URL with local/reachable probes.""" config = request.app.state.config - return {"url": config.memory_url} + url = config.memory_url + is_local = _is_local_url(url) + + # Return cached probe result if fresh (< 30 s), else re-probe. + cached = _MEMORY_URL_PROBE_CACHE.get(url) + now = time.monotonic() + if cached and (now - cached[0]) < 30: + reachable = cached[1] + else: + reachable = await _probe_taosmd(request, url) + _MEMORY_URL_PROBE_CACHE[url] = (now, reachable) + + return {"url": url, "is_local": is_local, "reachable": reachable} @router.put("/api/settings/memory-url") async def set_memory_url(request: Request, body: MemoryUrlUpdate): - """Update the taOSmd memory URL and persist to config.""" + """Update the taOSmd memory URL, persist to config, and probe reachability. + + The URL is always accepted and persisted. The *reachable* field reports + the probe result but does not gate the save.""" url = body.url.strip() if not url: return JSONResponse({"error": "URL must not be empty"}, status_code=400) + # Strip a single trailing slash (not a character set). + if url.endswith("/"): + url = url[:-1] parsed = urlparse(url) if parsed.scheme not in ("http", "https"): return JSONResponse({"error": "URL must use http:// or https:// scheme"}, status_code=400) if not parsed.netloc: return JSONResponse({"error": "URL must include a hostname"}, status_code=400) + + is_local = _is_local_url(url) + reachable = await _probe_taosmd(request, url) + config = request.app.state.config config.memory_url = url await save_config_locked(config, request.app.state.config_path) # Keep app.state.taosmd_url in sync for runtime access request.app.state.taosmd_url = url - return {"status": "saved", "url": url} + + # Cache the probe result so the next GET is instant. + _MEMORY_URL_PROBE_CACHE[url] = (time.monotonic(), reachable) + + return {"url": url, "is_local": is_local, "reachable": reachable} From 039adc8cb7cc5ac08ddec49aaa03bf2556588189 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sat, 18 Jul 2026 10:33:29 +0200 Subject: [PATCH 2/4] fix(settings): use parsed.hostname instead of parsed.netloc for memory-url hostname validation parsed.netloc returns ':7900' for http://:7900 (non-empty, bypasses validation). parsed.hostname correctly returns empty string, catching port-only URLs with no hostname. Added regression test for http://:7900 alongside existing http:///path case. --- tests/test_routes_settings.py | 10 +++++++++- tinyagentos/routes/settings.py | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_routes_settings.py b/tests/test_routes_settings.py index 38f7b1412..4471b916e 100644 --- a/tests/test_routes_settings.py +++ b/tests/test_routes_settings.py @@ -450,13 +450,21 @@ async def test_put_strips_trailing_slash(self, client, app): @pytest.mark.asyncio async def test_put_rejects_url_without_hostname(self, client): - """A URL without a valid hostname must return 400 (netloc check).""" + """A URL without a valid hostname must return 400 (hostname check).""" + # path-only: http:///path (hostname is empty string) resp = await client.put( "/api/settings/memory-url", json={"url": "http:///path"}, ) assert resp.status_code == 400 + # port-only: http://:7900 (hostname is empty, netloc non-empty — regression for netloc→hostname fix) + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http://:7900"}, + ) + assert resp.status_code == 400 + @pytest.mark.asyncio async def test_put_response_includes_is_local_and_reachable(self, client): """PUT response must include is_local and reachable alongside url.""" diff --git a/tinyagentos/routes/settings.py b/tinyagentos/routes/settings.py index 53509360f..f2561172b 100644 --- a/tinyagentos/routes/settings.py +++ b/tinyagentos/routes/settings.py @@ -1146,7 +1146,7 @@ async def set_memory_url(request: Request, body: MemoryUrlUpdate): parsed = urlparse(url) if parsed.scheme not in ("http", "https"): return JSONResponse({"error": "URL must use http:// or https:// scheme"}, status_code=400) - if not parsed.netloc: + if not parsed.hostname: return JSONResponse({"error": "URL must include a hostname"}, status_code=400) is_local = _is_local_url(url) From abdabfa6c12617f7b031b0f02931f3663b5a04c8 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sat, 18 Jul 2026 10:50:59 +0200 Subject: [PATCH 3/4] fix(settings): cap _MEMORY_URL_PROBE_CACHE at 32 entries with OrderedDict FIFO eviction Use collections.OrderedDict with popitem(last=False) to evict oldest entry when cache exceeds 32 entries, preventing unbounded growth. Closes review nit on #1931. --- tinyagentos/routes/settings.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tinyagentos/routes/settings.py b/tinyagentos/routes/settings.py index f2561172b..3b8f8086b 100644 --- a/tinyagentos/routes/settings.py +++ b/tinyagentos/routes/settings.py @@ -9,6 +9,7 @@ import shutil import tarfile import time +from collections import OrderedDict from pathlib import Path from urllib.parse import urlparse @@ -1077,7 +1078,7 @@ async def set_update_channel(request: Request, body: UpdateChannel): # Memory URL (taOSmd) # --------------------------------------------------------------------------- -_MEMORY_URL_PROBE_CACHE: dict = {} # {url: (timestamp, bool)} +_MEMORY_URL_PROBE_CACHE: OrderedDict = OrderedDict() # {url: (timestamp, bool)} — capped at 32 entries via FIFO eviction def _is_local_url(url: str) -> bool: @@ -1127,6 +1128,8 @@ async def get_memory_url(request: Request): else: reachable = await _probe_taosmd(request, url) _MEMORY_URL_PROBE_CACHE[url] = (now, reachable) + if len(_MEMORY_URL_PROBE_CACHE) > 32: + _MEMORY_URL_PROBE_CACHE.popitem(last=False) return {"url": url, "is_local": is_local, "reachable": reachable} @@ -1160,5 +1163,7 @@ async def set_memory_url(request: Request, body: MemoryUrlUpdate): # Cache the probe result so the next GET is instant. _MEMORY_URL_PROBE_CACHE[url] = (time.monotonic(), reachable) + if len(_MEMORY_URL_PROBE_CACHE) > 32: + _MEMORY_URL_PROBE_CACHE.popitem(last=False) return {"url": url, "is_local": is_local, "reachable": reachable} From 6736f24f0715c108f84511f8176b4a70233f2582 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sat, 18 Jul 2026 20:44:26 +0200 Subject: [PATCH 4/4] fix(settings): narrow _probe_taosmd exception catch to httpx errors Catch only (httpx.RequestError, httpx.InvalidURL) instead of the broad Exception, so unexpected failures (e.g. missing http_client) propagate rather than being silently swallowed as 'unreachable'. Closes CodeRabbit finding on PR #1931. --- tinyagentos/routes/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tinyagentos/routes/settings.py b/tinyagentos/routes/settings.py index 3b8f8086b..c904b4334 100644 --- a/tinyagentos/routes/settings.py +++ b/tinyagentos/routes/settings.py @@ -1105,7 +1105,7 @@ async def _probe_taosmd(request: Request, url: str) -> bool: timeout=httpx.Timeout(3.0, connect=2.0), ) return resp.status_code == 200 - except Exception: + except (httpx.RequestError, httpx.InvalidURL): return False