You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
openviking, supermemory, hindsight, byterover, retaindb, holographic — is_available() only checks env vars / CLI presence / local file existence. None probe their backends. PR doctor: tighten generic memory-provider wording (#36) #37 softened doctor's wording from "X provider active" to "X provider configured" to stop the false-positive green check, but the underlying asymmetry remains.
This RFC proposes a path to symmetric coverage.
Proposed addition to agent/memory_provider.py
classMemoryProvider(ABC):
# ... existing methods ...defhealth_check(self) ->tuple[bool, str]:
"""Probe the backing service and report (healthy, reason). Default implementation delegates to ``is_available()`` — for provider implementations that haven't yet defined a real probe, the contract is preserved: the (False, "is_available returned False") case still fires when the basic config is missing. Implementations that CAN probe should override: * Return (True, "") when the backend responds successfully within a reasonable timeout (default ~5 seconds). * Return (False, "<one-line reason>") for any failure. Reason should distinguish auth vs network so the operator knows which fix to apply (rotate key vs wait / check connectivity). Strings like "auth: API key rejected", "unreachable: connection refused", "not_found: endpoint 404", "rate_limited: 429" are good prefixes. Must never raise — doctor invokes this in a loop and a raised exception would crash the section. """try:
return (self.is_available(), ""ifself.is_available()
else"is_available() returned False")
exceptExceptionasexc: # noqa: BLE001return (False, f"is_available() raised: {exc}")
The Honcho + Mem0 specific branches (lines ~2200-2310 in doctor.py today) collapse into this generic path once both plugins implement health_check() natively. That removes ~120 lines of provider-specific doctor logic.
Provider migration sketch
Provider
Probe call
Honcho
get_honcho_client(hcfg) (already validates)
Mem0
MemoryClient(api_key).get_all(user_id, limit=1) (already in #35)
openviking
httpx.get(endpoint + "/health") if endpoint has a health endpoint; otherwise httpx.head(endpoint)
supermemory
__import__("supermemory"); client.search("ping", limit=1) if a search endpoint exists
hindsight
depends on mode — for cloud, a small GET /health; for local, just check the runtime path
byterover
subprocess.run([brv_path, "status"], timeout=3)
retaindb
httpx.get(base_url + "/health") with bearer
holographic
Trivial — local SQLite, always (True, "")
Each provider owns its probe; doctor stays generic.
Out of scope for this RFC
Async health checks. The probe runs once when hermes doctor invokes it; sync semantics keep the call sites simple. Mem0 + Honcho already use sync clients.
Caching results. Doctor is a one-shot diagnostic; cache invalidation isn't worth the complexity.
Removing is_available(). Backward-compat: is_available() stays the cheap pre-flight check (no network calls); health_check() is the deeper probe. Many call sites in the codebase already use is_available() in hot paths.
Status
This is an RFC — design only, no PR. Comments / counter-proposals welcome. If the shape is accepted, the plan is:
Ship the ABC method with the default is_available() delegation (one commit, no behavior change).
Migrate Honcho + Mem0 to implement health_check() natively. Collapse the provider-specific doctor branches.
Migrate openviking + supermemory + hindsight + byterover + retaindb + holographic, one at a time. Each PR is small.
Background
After auditing every shipped memory provider's
is_available()implementation (see #36 + PR #37), the picture is:is_available()only checks env vars / CLI presence / local file existence. None probe their backends. PR doctor: tighten generic memory-provider wording (#36) #37 softened doctor's wording from"X provider active"to"X provider configured"to stop the false-positive green check, but the underlying asymmetry remains.This RFC proposes a path to symmetric coverage.
Proposed addition to
agent/memory_provider.pyDoctor consumption (in
hermes_cli/doctor.py)The generic-memory else-branch becomes:
The Honcho + Mem0 specific branches (lines ~2200-2310 in
doctor.pytoday) collapse into this generic path once both plugins implementhealth_check()natively. That removes ~120 lines of provider-specific doctor logic.Provider migration sketch
get_honcho_client(hcfg)(already validates)MemoryClient(api_key).get_all(user_id, limit=1)(already in #35)httpx.get(endpoint + "/health")if endpoint has a health endpoint; otherwisehttpx.head(endpoint)__import__("supermemory"); client.search("ping", limit=1)if a search endpoint existsmode— for cloud, a smallGET /health; for local, just check the runtime pathsubprocess.run([brv_path, "status"], timeout=3)httpx.get(base_url + "/health")with bearerEach provider owns its probe; doctor stays generic.
Out of scope for this RFC
hermes doctorinvokes it; sync semantics keep the call sites simple. Mem0 + Honcho already use sync clients.is_available(). Backward-compat:is_available()stays the cheap pre-flight check (no network calls);health_check()is the deeper probe. Many call sites in the codebase already useis_available()in hot paths.Status
This is an RFC — design only, no PR. Comments / counter-proposals welcome. If the shape is accepted, the plan is:
is_available()delegation (one commit, no behavior change).health_check()natively. Collapse the provider-specific doctor branches.Filed by hermes-maintainer (PowerCreek). Continues #36; does not close it.