feat(memory): show taOSmd running mode, reachability, tier + switch-to-remote control#1959
feat(memory): show taOSmd running mode, reachability, tier + switch-to-remote control#1959hognek wants to merge 4 commits into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThe change adds backend APIs for configuring and probing the taOSmd memory endpoint, typed desktop client helpers, and a new Memory Settings card for viewing endpoint status and switching between local and remote URLs. ChangestaOSmd memory endpoint
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MemorySettings
participant memory.ts
participant SettingsAPI
participant taOSmd
MemorySettings->>memory.ts: Update endpoint URL
memory.ts->>SettingsAPI: PUT /api/settings/memory-url
SettingsAPI->>taOSmd: Probe /health
SettingsAPI-->>memory.ts: Return endpoint status
memory.ts-->>MemorySettings: Refresh displayed status
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
|
|
||
| def _taosmd_default_url(request: Request) -> str: | ||
| """Return the configured taOSmd URL, falling back to the local default.""" | ||
| return getattr(request.app.state, "taosmd_url", None) or "http://localhost:7900" |
There was a problem hiding this comment.
WARNING: _taosmd_default_url ignores config.memory_url, so GET /api/settings/memory-url always reports http://localhost:7900 on a fresh boot even when a remote memory_url is persisted in config.
This is inconsistent with _taosmd_base in user_memory.py:30-33, which does consult config.memory_url. app.state.taosmd_url is only set by a PUT (and deleted on config save/restore), so after a restart with a remote URL configured in config.yaml/AppConfig, this endpoint returns localhost while the actual memory backend uses the remote URL. The new TaOSmdEndpointCard reads this endpoint, so it would falsely show LOCAL mode. Consider falling back to request.app.state.config.memory_url before the hardcoded default:
| return getattr(request.app.state, "taosmd_url", None) or "http://localhost:7900" | |
| url = getattr(request.app.state, "taosmd_url", None) | |
| if url: | |
| return url | |
| config = getattr(request.app.state, "config", None) | |
| if config and getattr(config, "memory_url", None): | |
| return config.memory_url | |
| return "http://localhost:7900" |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| async def set_memory_url(request: Request): | ||
| """Set the taOSmd memory URL, probing the new target before accepting it.""" | ||
| body = await request.json() | ||
| url = (body.get("url") or "").strip().rstrip("/") |
There was a problem hiding this comment.
SUGGESTION: The PUT no longer rejects URLs that fail the reachability probe, despite the PR description stating it "probes the new target before accepting it." set_memory_url (lines 618-624) always persists memory_url to config and sets app.state.taosmd_url even when reachable is False.
If persisting an unreachable URL is intentional (e.g. connecting to an offline server), that's fine, but the description/old behavior implied validation. At minimum, consider surfacing reachable: false prominently and/or guarding against persisting clearly broken configs. As written, a typo'd URL is silently saved to disk.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| url = (body.get("url") or "").strip().rstrip("/") | ||
| if not url: | ||
| return JSONResponse({"error": "URL required"}, status_code=400) | ||
| parsed = urlparse(url) |
There was a problem hiding this comment.
WARNING: URL validation only checks the scheme, not that a host is present. A URL like http:// (no netloc) passes the parsed.scheme in ("http", "https") check. _is_local_url then returns False (empty hostname), and _probe_taosmd issues a request to http:///health, which is malformed. The removed prior implementation validated parsed.netloc. Add an explicit hostname/netloc check, e.g.:
| parsed = urlparse(url) | |
| parsed = urlparse(url) | |
| if parsed.scheme not in ("http", "https"): | |
| return JSONResponse( | |
| {"error": "URL must start with http:// or https://"}, | |
| status_code=400, | |
| ) | |
| if not parsed.hostname: | |
| return JSONResponse( | |
| {"error": "URL must include a hostname"}, | |
| status_code=400, | |
| ) |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No New Issues in Incremental Diff | Recommendation: Merge (prior findings resolved) Overview
Incremental Review (583ca98..caf829d)The incremental diff (
No new issues were introduced by the incremental changes. The frontend files ( Files Reviewed (1 file in incremental diff)
Previous Review Summaries (2 snapshots, latest commit 583ca98)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 583ca98)Status: No New Issues in Incremental Diff | Recommendation: Merge (prior findings resolved) Overview
Incremental Review (f5b041a..583ca98)The incremental diff touches only
No new issues were introduced by the incremental changes. The prior SUGGESTION about persisting Files Reviewed (1 file in incremental diff)
Previous review (commit f5b041a)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (9 files)
Reviewed by hy3:free · Input: 72.4K · Output: 4.5K · Cached: 262.1K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/src/apps/SettingsApp.tsx`:
- Line 421: Update the new control’s styling around the input className and the
related lines 435-438 to replace hardcoded white, sky, and amber palette
utilities with the existing shell/accent/error semantic theme tokens. Preserve
the current layout and focus/error states while ensuring the control adapts to
alternate themes.
In `@desktop/src/components/memory/MemorySettings.tsx`:
- Around line 94-99: Replace the fixed mode/status and remaining new card color
classes in MemorySettings.tsx with the project’s semantic theme tokens,
preserving the local/remote state distinction. In
desktop/src/apps/SettingsApp.tsx lines 421-438, replace the new input, focus,
and error color classes with the corresponding semantic theme tokens; update
both sites as part of the same endpoint-controls styling change.
In `@desktop/src/lib/memory.ts`:
- Around line 77-90: Update fetchMemoryEndpoint and updateMemoryEndpoint to stop
passing fabricated fallback endpoint objects to fetchJson; use the
failure-propagating request behavior so HTTP, parsing, and network errors are
thrown to callers. Preserve successful response handling while ensuring failed
GET and PUT requests do not return synthetic local endpoint state.
In `@tinyagentos/routes/settings.py`:
- Around line 545-555: Update _is_local_url in tinyagentos/routes/settings.py
(lines 545-555) to return true only for localhost and loopback addresses,
removing private-address classification. Update the private-IP endpoint
expectation in tests/test_routes_settings.py (lines 370-382) and private IPv4
helper expectations at lines 421-425 to assert false; the loopback expectations
should remain true.
- Around line 618-624: The endpoint update flow around _is_local_url,
_probe_taosmd, and save_config_locked must reject unreachable remote URLs before
mutating or persisting configuration; preserve local endpoint behavior and leave
the previous configured/runtime URL unchanged on rejection. Update
tests/test_routes_settings.py lines 430-438 to assert the rejection and verify
both prior configuration and runtime URLs remain unchanged.
- Around line 607-617: Strengthen URL validation in the endpoint handling flow
after urlparse(url): require an HTTP(S) URL with a non-empty hostname, reject
credentials, and reject any path, query, or fragment so only a clean origin is
persisted. Return the existing 400-style validation response for malformed
values, while preserving valid http and https origins.
- Around line 560-568: Update _probe_taosmd and the related settings flow around
_taosmd_tier() to obtain the active tier from the remote taOSmd health/status
response, rather than reading the local taosmd_default.json. Preserve
reachability handling while returning or propagating the remote tier so the API
reports the selected remote instance’s tier.
- Around line 573-575: Update _taosmd_default_url to fall back to the persisted
config.memory_url when request.app.state.taosmd_url is absent, and only use the
local localhost:7900 default when neither configured value exists. Keep GET URL
resolution consistent with user_memory._taosmd_base().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f07ebba6-068d-465c-a4ef-ba8294de985a
📒 Files selected for processing (9)
desktop/src/apps/SettingsApp.tsxdesktop/src/components/memory/MemorySettings.tsxdesktop/src/lib/memory.tstests/test_config.pytests/test_routes_settings.pytests/test_user_memory.pytinyagentos/config.pytinyagentos/routes/settings.pytinyagentos/routes/user_memory.py
| type="text" | ||
| value={memoryUrl} | ||
| onChange={(e) => { setMemoryUrl(e.target.value); setMemoryUrlDirty(true); }} | ||
| className="flex-1 px-3 py-2 text-sm rounded bg-white/5 border border-white/10 text-shell-text focus:outline-none focus:border-sky-500/50" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use theme tokens instead of fixed palette utilities.
The new control hardcodes white, sky, and amber colors, so it cannot reliably adapt to alternate themes. Replace these classes with the existing shell/accent/error semantic tokens.
Also applies to: 435-438
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/SettingsApp.tsx` at line 421, Update the new control’s
styling around the input className and the related lines 435-438 to replace
hardcoded white, sky, and amber palette utilities with the existing
shell/accent/error semantic theme tokens. Preserve the current layout and
focus/error states while ensuring the control adapts to alternate themes.
| const modeClasses = isLocal | ||
| ? "bg-green-500/15 text-green-400 border-green-500/30" | ||
| : "bg-blue-500/15 text-blue-400 border-blue-500/30"; | ||
|
|
||
| return ( | ||
| <Card className="bg-white/[0.02] border-white/8"> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use semantic theme colors throughout the new endpoint controls.
desktop/src/components/memory/MemorySettings.tsx#L94-L99: replace fixed mode/status palettes and apply the same correction to the card’s remaining new color classes.desktop/src/apps/SettingsApp.tsx#L421-L438: replace fixed input, focus, and error colors with theme tokens.
📍 Affects 2 files
desktop/src/components/memory/MemorySettings.tsx#L94-L99(this comment)desktop/src/apps/SettingsApp.tsx#L421-L438
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/components/memory/MemorySettings.tsx` around lines 94 - 99,
Replace the fixed mode/status and remaining new card color classes in
MemorySettings.tsx with the project’s semantic theme tokens, preserving the
local/remote state distinction. In desktop/src/apps/SettingsApp.tsx lines
421-438, replace the new input, focus, and error color classes with the
corresponding semantic theme tokens; update both sites as part of the same
endpoint-controls styling change.
| export async function fetchMemoryEndpoint(): Promise<TaOSmdEndpoint> { | ||
| const result = await fetchJson<TaOSmdEndpoint>( | ||
| `${API}/settings/memory-url`, | ||
| { url: "", is_local: true, reachable: false }, | ||
| ); | ||
| return result; | ||
| } | ||
|
|
||
| export async function updateMemoryEndpoint(url: string): Promise<TaOSmdEndpoint> { | ||
| return fetchJson<TaOSmdEndpoint>(`${API}/settings/memory-url`, { url, is_local: true, reachable: false }, { | ||
| method: "PUT", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ url }), | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not fabricate endpoint state when requests fail.
fetchJson() converts HTTP errors, invalid JSON, and network failures into fallback objects. Consequently, a rejected PUT appears successful as {url, is_local: true}, and the card can display a fake local endpoint after a failed GET. These helpers must expose failure—preferably by throwing—so callers can retain current state and show an error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/lib/memory.ts` around lines 77 - 90, Update fetchMemoryEndpoint
and updateMemoryEndpoint to stop passing fabricated fallback endpoint objects to
fetchJson; use the failure-propagating request behavior so HTTP, parsing, and
network errors are thrown to callers. Preserve successful response handling
while ensuring failed GET and PUT requests do not return synthetic local
endpoint state.
| 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 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Private-LAN does not mean local-machine. Classifying LAN hosts as local makes the desktop show LOCAL and removes its revert control after connecting remotely.
tinyagentos/routes/settings.py#L545-L555: return true only for localhost and loopback addresses.tests/test_routes_settings.py#L370-L382: expect the private-IP endpoint to reportis_local=False.tests/test_routes_settings.py#L421-L425: change private IPv4 helper expectations to false.
📍 Affects 2 files
tinyagentos/routes/settings.py#L545-L555(this comment)tests/test_routes_settings.py#L370-L382tests/test_routes_settings.py#L421-L425
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/routes/settings.py` around lines 545 - 555, Update _is_local_url
in tinyagentos/routes/settings.py (lines 545-555) to return true only for
localhost and loopback addresses, removing private-address classification.
Update the private-IP endpoint expectation in tests/test_routes_settings.py
(lines 370-382) and private IPv4 helper expectations at lines 421-425 to assert
false; the loopback expectations should remain true.
| 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 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Report the tier from the probed endpoint.
_taosmd_tier() reads this machine’s taosmd_default.json, so after switching to a remote instance the API can display the local tier as the remote server’s active tier. Preserve tier information from the remote health/status response instead.
Also applies to: 578-600
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/routes/settings.py` around lines 560 - 568, Update _probe_taosmd
and the related settings flow around _taosmd_tier() to obtain the active tier
from the remote taOSmd health/status response, rather than reading the local
taosmd_default.json. Preserve reachability handling while returning or
propagating the remote tier so the API reports the selected remote instance’s
tier.
| body = await request.json() | ||
| url = (body.get("url") or "").strip().rstrip("/") | ||
| if not url: | ||
| return JSONResponse({"error": "URL required"}, status_code=400) | ||
| parsed = urlparse(url) | ||
| if parsed.scheme not in ("http", "https"): | ||
| return JSONResponse( | ||
| {"error": "URL must start with http:// or https://"}, | ||
| status_code=400, | ||
| ) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject malformed endpoint URLs before persistence.
Values such as http://, URLs with credentials, or URLs containing paths/queries pass the scheme check and are then persisted, producing invalid /health, /search, and /ingest targets. Require a hostname and a clean HTTP(S) origin.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/routes/settings.py` around lines 607 - 617, Strengthen URL
validation in the endpoint handling flow after urlparse(url): require an HTTP(S)
URL with a non-empty hostname, reject credentials, and reject any path, query,
or fragment so only a clean origin is persisted. Return the existing 400-style
validation response for malformed values, while preserving valid http and https
origins.
| 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) | ||
| request.app.state.taosmd_url = url |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
The endpoint probe must gate remote activation. The current implementation persists known-unreachable targets, and the test requires that behavior.
tinyagentos/routes/settings.py#L618-L624: reject unreachable remote endpoints before mutating or saving configuration.tests/test_routes_settings.py#L430-L438: assert rejection and verify the previous configured/runtime URL remains unchanged.
📍 Affects 2 files
tinyagentos/routes/settings.py#L618-L624(this comment)tests/test_routes_settings.py#L430-L438
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/routes/settings.py` around lines 618 - 624, The endpoint update
flow around _is_local_url, _probe_taosmd, and save_config_locked must reject
unreachable remote URLs before mutating or persisting configuration; preserve
local endpoint behavior and leave the previous configured/runtime URL unchanged
on rejection. Update tests/test_routes_settings.py lines 430-438 to assert the
rejection and verify both prior configuration and runtime URLs remain unchanged.
…dd netloc validation Addresses Kilo bot findings on PR jaylfc#1959: - _taosmd_default_url now checks config.memory_url as fallback before defaulting to localhost, so a persisted remote URL survives restart - PUT /api/settings/memory-url now rejects URLs without a hostname (e.g. http:// alone) with a clear 400 error
…d memory URL (jaylfc#1911) Add GET /api/settings/memory-url returning {url, is_local, reachable} with health-probe against the configured taOSmd. Add PUT endpoint that validates the URL, probes the new target, and persists it to app.state. Add comprehensive tests: default URL, local/remote detection, PUT validation (empty/invalid scheme), trailing-slash stripping, URL persistence, and _is_local_url unit coverage.
…o-remote control - Add _taosmd_tier helper reading taosmd_default.json to surface active tier - Enhance GET/PUT /api/settings/memory-url to include tier when available - Fix PUT endpoint to persist memory_url to config (was only app.state) - Remove old duplicate memory-url endpoints superseded by PR jaylfc#1931 - Add TaOSmdEndpointCard component: LOCAL/REMOTE badge, reachability, backend capabilities, tier badge, switch-to-remote URL input, connection test, revert-to-local button, ARIA labels - Add fetchMemoryEndpoint/updateMemoryEndpoint to memory.ts client - Closes jaylfc#1892
…dd netloc validation Addresses Kilo bot findings on PR jaylfc#1959: - _taosmd_default_url now checks config.memory_url as fallback before defaulting to localhost, so a persisted remote URL survives restart - PUT /api/settings/memory-url now rejects URLs without a hostname (e.g. http:// alone) with a clear 400 error
583ca98 to
c22f774
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tinyagentos/routes/settings.py`:
- Around line 570-571: Update the exception handling around the settings request
to catch the specific network failure type, such as httpx.RequestError, instead
of a blanket Exception. Preserve returning False for expected request failures
while allowing initialization, attribute, and other programming errors to
propagate.
- Around line 595-596: Validate the request payload in the settings endpoint
before accessing url: handle JSON decoding failures and reject non-object JSON
values, while requiring url to be a valid string. Prefer a Pydantic
MemoryUrlPayload parameter if consistent with the endpoint’s existing FastAPI
patterns; otherwise add explicit validation around await request.json() and
body.get("url") so invalid payloads return a client error instead of raising an
unhandled exception.
- Around line 561-572: Update _probe_taosmd to return the discovered tier
alongside reachability, then thread that tier through get_memory_url and
set_memory_url responses. Ensure both endpoints include tier with url, is_local,
and reachable, preserving the existing behavior when probing fails or no tier is
available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 293d5b21-4a29-4222-b622-09d089ba1b8e
📒 Files selected for processing (3)
desktop/src/components/memory/MemorySettings.tsxdesktop/src/lib/memory.tstinyagentos/routes/settings.py
🚧 Files skipped from review as they are similar to previous changes (2)
- desktop/src/lib/memory.ts
- desktop/src/components/memory/MemorySettings.tsx
| 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 | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -A 5 -i 'def health' tinyagentos/Repository: jaylfc/taOS
Length of output: 11718
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== settings.py relevant ranges =="
sed -n '520,660p' tinyagentos/routes/settings.py
echo
echo "== tier mentions in repo =="
rg -n '\btier\b' tinyagentos/Repository: jaylfc/taOS
Length of output: 22964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend_adapters.py outline =="
ast-grep outline tinyagentos/backend_adapters.py --view expanded
echo
echo "== taOSmd health implementation =="
rg -n -A 40 -B 8 'class .*TaOS|def health\(self, client: httpx\.AsyncClient, url: str\) -> dict' tinyagentos/backend_adapters.py
echo
echo "== taOSmd route definitions =="
ast-grep outline tinyagentos/routes/taosmd.py --view expandedRepository: jaylfc/taOS
Length of output: 11066
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== routes/taosmd.py relevant slices =="
sed -n '1,260p' tinyagentos/routes/taosmd.py
echo
echo "== TaOSmdEndpoint references =="
rg -n 'TaOSmdEndpoint|memory-url|reachable|is_local|tier' tinyagentos/ -g '!**/__pycache__/**'Repository: jaylfc/taOS
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== settings.py later memory-url block =="
sed -n '1220,1325p' tinyagentos/routes/settings.py
echo
echo "== TaOSmdEndpoint type/interface =="
rg -n -A 20 -B 10 'TaOSmdEndpoint' . --glob '!**/__pycache__/**'
echo
echo "== any taosmd /health response shape =="
rg -n -A 20 -B 10 '"/health"|/health|health\(' tinyagentos/routes tinyagentos/backend_adapters.py tinyagentos/qmd_client.py tinyagentos/health.pyRepository: jaylfc/taOS
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== taosmd routes and server files =="
git ls-files 'tinyagentos/**' | rg 'taosmd|memory|health|settings|backend|qmd'
echo
echo "== explicit /health routes in tinyagentos =="
rg -n '(`@router`\.(get|post|put)\("/health"|`@app`\.(get|post|put)\("/health"|async def health\()' tinyagentos/Repository: jaylfc/taOS
Length of output: 4209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== memory type and endpoint consumers =="
sed -n '1,140p' desktop/src/lib/memory.ts
echo
rg -n -A 6 -B 6 '\btier\b|TaOSmdEndpoint|fetchMemoryEndpoint|updateMemoryEndpoint' desktop/src/components/memory desktop/src/lib/memory.ts desktop/src/lib/memory-api.ts desktop/src/components -g '!**/__pycache__/**'Repository: jaylfc/taOS
Length of output: 50367
Return tier from the memory URL endpoints. TaOSmdEndpoint already includes tier?, and MemorySettings.tsx renders it, but _probe_taosmd drops the value and both get_memory_url / set_memory_url return only url, is_local, and reachable. Thread the probe’s tier through both responses.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 570-570: Do not catch blind exception: Exception
(BLE001)
📍 Affects 1 file
tinyagentos/routes/settings.py#L561-L572(this comment)tinyagentos/routes/settings.py#L588-L589tinyagentos/routes/settings.py#L612-L616
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/routes/settings.py` around lines 561 - 572, Update _probe_taosmd
to return the discovered tier alongside reachability, then thread that tier
through get_memory_url and set_memory_url responses. Ensure both endpoints
include tier with url, is_local, and reachable, preserving the existing behavior
when probing fails or no tier is available.
| except Exception: | ||
| return False |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Avoid catching blind exceptions.
Catching a blanket Exception masks internal programming errors, such as an AttributeError if request.app.state.http_client is not initialized or misspelled. Catch specific network exceptions like httpx.RequestError instead.
🛠️ Proposed fix
- except Exception:
+ except httpx.RequestError:
return False📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except Exception: | |
| return False | |
| except httpx.RequestError: | |
| return False |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 570-570: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/routes/settings.py` around lines 570 - 571, Update the exception
handling around the settings request to catch the specific network failure type,
such as httpx.RequestError, instead of a blanket Exception. Preserve returning
False for expected request failures while allowing initialization, attribute,
and other programming errors to propagate.
Source: Linters/SAST tools
| body = await request.json() | ||
| url = (body.get("url") or "").strip().rstrip("/") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the JSON payload to prevent unhandled 500 errors.
await request.json() raises an unhandled exception if the payload isn't valid JSON, and body.get("url") raises an AttributeError if the payload is a JSON array instead of an object. Consider using a Pydantic model (body: MemoryUrlPayload) for idiomatic FastAPI validation, or manually validate the payload type.
🛠️ Proposed manual validation fix
- body = await request.json()
+ try:
+ body = await request.json()
+ if not isinstance(body, dict):
+ return JSONResponse({"error": "Payload must be a JSON object"}, status_code=400)
+ except ValueError:
+ return JSONResponse({"error": "Invalid JSON payload"}, status_code=400)
+
url = (body.get("url") or "").strip().rstrip("/")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| body = await request.json() | |
| url = (body.get("url") or "").strip().rstrip("/") | |
| try: | |
| body = await request.json() | |
| if not isinstance(body, dict): | |
| return JSONResponse({"error": "Payload must be a JSON object"}, status_code=400) | |
| except ValueError: | |
| return JSONResponse({"error": "Invalid JSON payload"}, status_code=400) | |
| url = (body.get("url") or "").strip().rstrip("/") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/routes/settings.py` around lines 595 - 596, Validate the request
payload in the settings endpoint before accessing url: handle JSON decoding
failures and reject non-object JSON values, while requiring url to be a valid
string. Prefer a Pydantic MemoryUrlPayload parameter if consistent with the
endpoint’s existing FastAPI patterns; otherwise add explicit validation around
await request.json() and body.get("url") so invalid payloads return a client
error instead of raising an unhandled exception.
The rebase onto origin/dev left two copies of the taOSmd memory URL routes. The stale copy (lines 542-616) used raw request.json(), blanket Exception, and no Pydantic validation. The refined copy at the end of the file already had MemoryUrlUpdate, specific httpx.RequestError handling, probe caching with FIFO eviction, and save_config_locked. Remove the stale duplicate; the canonical version is the one already present at the end of settings.py. Fixes CodeRabbit findings: specific exception catch (httpx.RequestError instead of Exception) and request payload validation (Pydantic model).
1 participant
Closes #1892.
Summary
Adds a status card in the Memory Settings page showing taOSmd running mode (LOCAL/REMOTE), reachability, backend capabilities, and active memory tier, plus a switch-to-remote control.
Changes
Backend (tinyagentos/routes/settings.py)
_taosmd_tierhelper readingtaosmd_default.jsonto surface the active memory tierGET /api/settings/memory-urlto includetierwhen availablePUT /api/settings/memory-urlto persistmemory_urlto config viasave_config_locked(was only settingapp.state.taosmd_url)PUT /api/settings/memory-urlto includetierin responseFrontend (desktop/src/components/memory/MemorySettings.tsx)
TaOSmdEndpointCardcomponent with:TaOSmdEndpointCardintoMemorySettingslayoutClient (desktop/src/lib/memory.ts)
TaOSmdEndpointinterface ({url, is_local, reachable, tier?})fetchMemoryEndpoint()functionupdateMemoryEndpoint(url)functionTesting
npx tsc --noEmitpasses (0 errors)test_get_returns_default_url✅,test_is_local_url_helper✅ (7 cases),TestMemoryUrlconfig tests ✅ (5/5)Depends on PR #1931 (merged — adds
GET/PUT /api/settings/memory-urlwith reachability probing).Summary by CodeRabbit