fix(memgw): catch ValueError on malformed URL in is_available (Codex PR#31 review)

[dizhaky]

Summary

This fixes a P2 issue identified by Codex during review of PR #31 ("fix(memgw): cancel timed-out MCP calls + strict loopback host parsing").

Problem

In plugins/memory/memgw/__init__.py, the is_available() method accesses urlparse(url).hostname to validate loopback URLs in keyless mode. However, urlparse().hostname raises ValueError for malformed URLs — for example:

• Typoed IPv6 URLs: http://[::1:8081/mcp (missing closing ])
• Invalid NFKC hostnames that fail Unicode normalization

Some status/setup paths call p.is_available() directly without a try/except, so this ValueError would propagate as an uncaught exception rather than gracefully returning False (unavailable).

Fix

Wrapped the urlparse(url).hostname call in a try/except ValueError block in is_available(). A malformed URL now returns False (treating the provider as unavailable) instead of raising.

Change in plugins/memory/memgw/__init__.py (around line 177):

# Before
host = (urlparse(url).hostname or '').lower()
return host in ('localhost', '127.0.0.1', '::1')

# After
try:
    host = (urlparse(url).hostname or '').lower()
except ValueError:
    # Malformed URL (e.g. typoed IPv6 like "http://[::1:8081/mcp") --
    # treat as unavailable rather than propagating the exception.
    return False
return host in ('localhost', '127.0.0.1', '::1')

References

• Codex P2 finding on PR fix(memgw): cancel timed-out MCP calls + strict loopback host parsing — Codex PR #30 review #31
• Related: PR feat(memory): memgw provider — Memory Gateway hybrid recall as default #30 fix for loopback host parsing

---

Generated by Claude Code

[github-actions]

🔎 Lint report: fix/codex-pr31-malformed-url-handling vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8649 on HEAD, 8649 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4570 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

[dizhaky]

Codex Review Coverage Summary

This PR closes one of the two P2 items from the PR #31 Codex review (ValueError on malformed URL). Here is the current state of all Codex findings from the PR #30 review chain:

Fixed ✅

• P2 Cancel timed-out MCP calls (PR fix(memgw): cancel timed-out MCP calls + strict loopback host parsing — Codex PR #30 review #31)
• P2 Parse host before allowing keyless local mode (PR fix(memgw): cancel timed-out MCP calls + strict loopback host parsing — Codex PR #30 review #31)
• P2 Handle malformed URL ValueError in is_available() (this PR)

Deferred — tracking needed

P1 items (higher risk):

• P1 Scope memgw calls by gateway user — recall/write/reflect payloads carry no user/chat scope; with a shared MEMGW_API_KEY all gateway users share the same namespace. This was called out as needing an interface change in the PR fix(memgw): cancel timed-out MCP calls + strict loopback host parsing — Codex PR #30 review #31 description.
• P1 Refresh memgw user scope on shared gateway turns — user_id is stored only at initialize(), so in shared thread sessions the second participant's calls use the first user's ID.
• P1 Keep memgw out of bundled providers — AGENTS.md says built-in plugins/memory/ is closed; new backends must ship as standalone plugins.
• P1 Leave default memory provider unset — hard-coding memgw in DEFAULT_CONFIG causes doctor/status to report an unavailable provider on fresh installs.

P2 items (still open):

• P2 Check MCP client dependency before activating (is_available() should verify mcp package is importable)
• P2 Pin MCP dependency range in plugin.yaml (bounded spec e.g. mcp>=1.26.0,<2)
• P2 Track every sync writer before shutdown (current code drops references to in-flight writes)
• P2 Avoid blocking turn path on old syncs (joining old writer threads blocks response)
• P2 Scope cached prefetch results by session (stale results can leak across /resume//new)
• P2 Preserve delegation writes during shutdown (untracked daemon threads can be killed on exit)
• P2 Handle MCP isError before unwrapping (tool-level errors not distinguished from success)
• P2 Serialize event-loop initialization (race condition in _ensure_loop() under concurrent calls)
• P2 Ignore stale prefetch workers (older query can overwrite newer prefetch cache)

Automated Codex review audit by Claude Code.

---

Generated by Claude Code

[dizhaky]

Corrected Codex Review Status (post-merge audit)

The prior summary overstated the number of open items. After reading the merged code in main, here is the accurate status:

All P2 items — confirmed FIXED in main ✅

Finding	How fixed
Check MCP client dep before activating	is_available() returns False when importlib.util.find_spec("mcp") is None
Pin MCP dependency range	plugin.yaml: mcp>=1.26.0,<2
Track every sync writer before shutdown	_sync_threads list; shutdown() joins all live entries
Avoid blocking turn path on old syncs	sync_turn() starts a daemon thread and returns immediately — no join on turn path
Scope cached prefetch results by session	on_session_switch() clears _prefetch_result + bumps _prefetch_gen
Ignore stale prefetch workers	_prefetch_gen monotonic guard in queue_prefetch()
Preserve delegation writes during shutdown	_delegation_threads list; shutdown() joins all live entries
Handle MCP isError before unwrapping	client._unwrap() raises RuntimeError on isError=True
Serialize event-loop initialization	_loop_lock wraps _ensure_loop() in client.py
Leave default memory provider unset	DEFAULT_CONFIG["memory"]["provider"] = "" in hermes_cli/config.py

P1 items — status

Finding	Status
Keep memgw out of bundled providers	Deliberate exception (documented in README + PR comments); inert without mcp/MEMGW_API_KEY
Leave default memory provider unset	✅ Fixed — DEFAULT_CONFIG uses "provider": ""
Scope/Refresh memgw user_id on shared gateway turns	⚠️ OPEN — genuine bug

Remaining open: 1 P1 bug

Source: Codex comment on PR #30 at __init__.py:345 (2026-06-25T19:01:16Z)

self._user_id is stored once in initialize() and never refreshed. When thread_sessions_per_user=False (default), multiple gateway users share a cached AIAgent. User B's turns will call _user_scope() with user A's ID — routing B's memories into A's namespace.

Fix needed: Pass user_id as a per-call parameter through queue_prefetch(), prefetch(), and sync_turn(), or enforce thread_sessions_per_user=True whenever memgw is active.

Automated Codex review audit by Claude Code.

---

Generated by Claude Code

[dizhaky]

Automated follow-up — P1 user-scope bug fixed

PR #33 (fix/memgw-user-id-per-call-scoping) implements the fix for the last remaining open item from the Codex review chain:

Finding: _user_id stored at initialize() was reused unchanged for every subsequent sync_turn/queue_prefetch call. In shared gateway sessions (thread_sessions_per_user=False), User B's turns called _user_scope() with User A's ID, routing B's memories into A's namespace.

Fix: user_id is now propagated as an explicit kwarg through run_agent._sync_external_memory_for_turn → MemoryManager.sync_all/queue_prefetch_all → MemoryProvider base interface → MemGatewayProvider._user_scope(user_id). The per-call override wins over the cached instance value; single-user CLI sessions are unaffected (both are "").

All 28 tests pass.

Automated Codex review follow-up by Claude Code.

---

Generated by Claude Code