fix(cli): pingKey distinguishes network failures from invalid keys + tests
PR review surfaced a data-loss path: pingKey returned False on ANY exception,
so a VPN flap or DNS hiccup made Rules 1/2 (reuse existing valid key) fall
through to Rule 3 (mint new shadow), silently rotating the user's API key
and rewriting plugin-sync targets (~/.claude/settings.json, .zshrc).
Now pingKey returns False ONLY on a definitive auth failure (HTTP 401/403).
Network errors, timeouts, and 5xx responses return True so the existing key
is preferred over re-minting. Mirror change in both Python and Node.
Additional fixes from the same review pass:
- --agent-caller is now PATCHed to the backend when supplied on a Rule 1
or Rule 2 reuse path (previously silently dropped). Best-effort —
failures don't break reuse.
- bootstrap_via_backend / bootstrapViaBackend renames the `body` local on
the error path to `err_body` (no longer shadows the request payload).
Same rename in the claim flow.
- Bootstrap envelope is now validated for non-empty api_key +
default_user_id before mutation — defends against partial backend
responses silently persisting null/undefined into typed string fields.
- Stale docstrings in agent_detect.{py,ts} and bootstrap_via_backend no
longer claim env-var sniffing fills `agent_caller`; the field is
self-declared via --agent-caller only.
Tests (new file mirrored across runtimes):
- test_init_internals.py / init-internals.test.ts
- pingKey: 200/401/403/5xx/connect-error/timeout matrix
- plugin_sync.updateShellRc: trailing-newline preservation, no-create,
surrounding-content preservation, idempotency, missing-file no-op
- plugin_sync.updateClaudeSettings: no env-block creation, no
MEM0_API_KEY insertion into existing env, idempotency, malformed JSON
no-op
- Python only: bootstrap 403 "permission" → daily-limit translation
Python: 161 tests pass. Node: 112 tests pass. ruff + biome clean.
This commit is contained in:
@@ -1,12 +1,15 @@
|
||||
"""Detect which AI agent is invoking the CLI via environment variables.
|
||||
"""Detect whether the CLI is being invoked from inside an AI-agent context.
|
||||
|
||||
Used by `mem0 init` to:
|
||||
1. Decide whether to auto-bootstrap an Agent Mode key (positive agent signal).
|
||||
2. Tag the `agent_caller` PostHog property on the cli.init event.
|
||||
Used by `mem0 init` to auto-enter Agent Mode (Rule 3 bootstrap) when an
|
||||
agent runtime env var is present. The return value is a context **trigger
|
||||
only** — the canonical agent identity is self-declared by the agent via
|
||||
``--agent-caller <name>`` (Proof Editor-style) and never sniffed from env
|
||||
vars to fill the ``agent_caller`` field on the APIKey row.
|
||||
|
||||
Returns a canonical short name or None when no agent is detected. The list
|
||||
is curated, not exhaustive — agents we don't recognise fall through to None,
|
||||
which groups into the "unknown" bucket on dashboards.
|
||||
Returns a short name or None. The list is curated, not exhaustive — env
|
||||
vars we don't recognise fall through to None (caller treated as
|
||||
non-agent). Honest reporting depends on ``--agent-caller``; this list is
|
||||
just enough to enable the zero-friction auto-bootstrap UX.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import sys
|
||||
from datetime import datetime, timezone
|
||||
from typing import Any
|
||||
@@ -28,6 +29,26 @@ _SOURCE_HEADERS = {
|
||||
}
|
||||
|
||||
|
||||
def _validate_envelope(envelope: Any) -> None:
|
||||
"""Defend against partial/malformed backend responses.
|
||||
|
||||
A backend regression that returns ``{"api_key": null}`` would otherwise be
|
||||
silently persisted, producing confusing downstream errors far from the
|
||||
source. Fail fast with a clear message if the required fields are missing.
|
||||
"""
|
||||
if not isinstance(envelope, dict):
|
||||
print_error(err_console, "Bootstrap response was not a JSON object.")
|
||||
raise typer.Exit(1)
|
||||
for field in ("api_key", "default_user_id"):
|
||||
value = envelope.get(field)
|
||||
if not isinstance(value, str) or not value:
|
||||
print_error(
|
||||
err_console,
|
||||
f"Bootstrap response missing required field {field!r} — please update the CLI.",
|
||||
)
|
||||
raise typer.Exit(1)
|
||||
|
||||
|
||||
def bootstrap_via_backend(
|
||||
config: Mem0Config,
|
||||
*,
|
||||
@@ -39,9 +60,11 @@ def bootstrap_via_backend(
|
||||
Args:
|
||||
config: Mem0Config mutated in place with the new platform values.
|
||||
source: ``--source`` flag passthrough (analytics tag, free-form).
|
||||
agent_caller: Canonical agent name detected from env vars
|
||||
(claude-code, cursor, ...). Persisted on the backend APIKey and
|
||||
saved into ``platform.agent_caller`` for local introspection.
|
||||
agent_caller: Self-declared agent identity passed via ``--agent-caller``
|
||||
(e.g. ``claude-code``, ``cursor``). May be None when the caller
|
||||
omitted the flag; the agent can backfill later via
|
||||
``mem0 identify <name>``. Sent to the backend in the request body
|
||||
and saved into ``platform.agent_caller`` for local introspection.
|
||||
|
||||
Raises typer.Exit(1) on failure.
|
||||
"""
|
||||
@@ -72,9 +95,9 @@ def bootstrap_via_backend(
|
||||
if resp.status_code != 200:
|
||||
detail = resp.text
|
||||
try:
|
||||
body = resp.json()
|
||||
detail = body.get("error") or body.get("detail") or resp.text
|
||||
except Exception:
|
||||
err_body = resp.json()
|
||||
detail = err_body.get("error") or err_body.get("detail") or resp.text
|
||||
except (json.JSONDecodeError, ValueError, AttributeError):
|
||||
pass
|
||||
# Backend's @ratelimit decorator raises PermissionDenied, which DRF
|
||||
# translates to a generic 403 "You do not have permission to perform
|
||||
@@ -89,6 +112,7 @@ def bootstrap_via_backend(
|
||||
raise typer.Exit(1)
|
||||
|
||||
envelope = resp.json()
|
||||
_validate_envelope(envelope)
|
||||
config.platform.api_key = envelope["api_key"]
|
||||
config.platform.base_url = base_url
|
||||
config.platform.agent_mode = True
|
||||
@@ -185,10 +209,10 @@ def claim_via_otp(config: Mem0Config, *, email: str, code: str | None = None) ->
|
||||
|
||||
if verify.status_code != 200:
|
||||
try:
|
||||
body = verify.json()
|
||||
detail = body.get("error", verify.text)
|
||||
code_str = body.get("code", "")
|
||||
except Exception:
|
||||
err_body = verify.json()
|
||||
detail = err_body.get("error", verify.text)
|
||||
code_str = err_body.get("code", "")
|
||||
except (json.JSONDecodeError, ValueError, AttributeError):
|
||||
detail = verify.text
|
||||
code_str = ""
|
||||
print_error(err_console, f"Claim failed: {detail}")
|
||||
@@ -198,13 +222,13 @@ def claim_via_otp(config: Mem0Config, *, email: str, code: str | None = None) ->
|
||||
)
|
||||
raise typer.Exit(1)
|
||||
|
||||
body = verify.json()
|
||||
if not body.get("claimed"):
|
||||
print_error(err_console, f"Unexpected verify response: {body}")
|
||||
claim_body = verify.json()
|
||||
if not claim_body.get("claimed"):
|
||||
print_error(err_console, f"Unexpected verify response: {claim_body}")
|
||||
raise typer.Exit(1)
|
||||
|
||||
config.platform.agent_mode = False
|
||||
config.platform.claimed_at = body.get("claimed_at") or _utcnow_iso()
|
||||
config.platform.claimed_at = claim_body.get("claimed_at") or _utcnow_iso()
|
||||
config.platform.user_email = email
|
||||
config.platform.created_via = "email"
|
||||
save_config(config)
|
||||
|
||||
@@ -104,16 +104,22 @@ def _validate_email(email: str) -> None:
|
||||
|
||||
|
||||
def _ping_key(api_key: str, base_url: str, timeout: float = 5.0) -> bool:
|
||||
"""True if api_key passes /v1/ping/ against base_url within timeout."""
|
||||
"""Validate api_key against /v1/ping/.
|
||||
|
||||
Returns False ONLY on a definitive "invalid key" signal (HTTP 401 / 403).
|
||||
Network errors, timeouts, and 5xx responses return True so we prefer
|
||||
reusing an existing key over silently minting a new shadow on a transient
|
||||
blip (which would also clobber config + plugin-sync targets).
|
||||
"""
|
||||
try:
|
||||
resp = httpx.get(
|
||||
f"{base_url.rstrip('/')}/v1/ping/",
|
||||
headers={"Authorization": f"Token {api_key}"},
|
||||
timeout=timeout,
|
||||
)
|
||||
return resp.status_code == 200
|
||||
except Exception:
|
||||
return False
|
||||
except httpx.HTTPError:
|
||||
return True # unknown — prefer reuse
|
||||
return resp.status_code not in (401, 403)
|
||||
|
||||
|
||||
def _email_login(
|
||||
@@ -282,9 +288,37 @@ def run_init(
|
||||
)
|
||||
print_success(console, msg)
|
||||
|
||||
def _maybe_identify(key: str) -> None:
|
||||
"""Best-effort PATCH agent_caller when --agent-caller is supplied on a
|
||||
reused key. Silent no-op on any failure — reuse must not break.
|
||||
"""
|
||||
if not agent_caller:
|
||||
return
|
||||
try:
|
||||
resp = httpx.patch(
|
||||
f"{base_url.rstrip('/')}/api/v1/auth/agent_mode/caller/",
|
||||
headers={
|
||||
"Authorization": f"Token {key}",
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
json={"agent_caller": agent_caller},
|
||||
timeout=10.0,
|
||||
)
|
||||
# Also reflect in local config so introspection matches backend.
|
||||
if resp.status_code == 200 and CONFIG_FILE.exists():
|
||||
try:
|
||||
cfg = load_config()
|
||||
cfg.platform.agent_caller = resp.json().get("agent_caller", agent_caller)
|
||||
save_config(cfg)
|
||||
except Exception:
|
||||
pass
|
||||
except httpx.HTTPError:
|
||||
pass
|
||||
|
||||
# Rule 1: env MEM0_API_KEY valid → reuse, no new key.
|
||||
_env_key = (os.environ.get("MEM0_API_KEY") or "").strip()
|
||||
if _env_key and _ping_key(_env_key, base_url):
|
||||
_maybe_identify(_env_key)
|
||||
_emit_reuse("env")
|
||||
_fire_init("existing_key")
|
||||
return
|
||||
@@ -292,6 +326,7 @@ def run_init(
|
||||
if CONFIG_FILE.exists():
|
||||
_existing = load_config()
|
||||
if _existing.platform.api_key and _ping_key(_existing.platform.api_key, base_url):
|
||||
_maybe_identify(_existing.platform.api_key)
|
||||
_emit_reuse("config")
|
||||
_fire_init("existing_key")
|
||||
return
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
"""Unit tests for init internals — decision tree primitives + plugin sync.
|
||||
|
||||
These tests exercise the units that the high-level subprocess parity tests in
|
||||
``test_agent_mode.py`` deliberately can't reach:
|
||||
|
||||
- ``_ping_key`` must NOT treat network errors as "invalid key" (else a VPN
|
||||
flap silently mints a new shadow over a working key).
|
||||
- ``plugin_sync`` must only update entries that already exist, preserve
|
||||
trailing newlines, and never mangle other lines.
|
||||
- The 403→ratelimit translation in ``bootstrap_via_backend`` surfaces the
|
||||
real cause instead of DRF's opaque "You do not have permission" string.
|
||||
|
||||
Mirror surface lives in ``cli/node/tests/agent-mode.test.ts``; if you add a
|
||||
behavioral assertion here, mirror it on the Node side and vice versa.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from mem0_cli.commands.init_cmd import _ping_key
|
||||
from mem0_cli.plugin_sync import _update_claude_settings, _update_shell_rc
|
||||
|
||||
# ── _ping_key ──────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class _Resp:
|
||||
def __init__(self, status_code: int) -> None:
|
||||
self.status_code = status_code
|
||||
|
||||
|
||||
def test_ping_key_200_is_valid(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(httpx, "get", lambda *a, **kw: _Resp(200))
|
||||
assert _ping_key("k", "http://x") is True
|
||||
|
||||
|
||||
def test_ping_key_401_is_invalid(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(httpx, "get", lambda *a, **kw: _Resp(401))
|
||||
assert _ping_key("k", "http://x") is False
|
||||
|
||||
|
||||
def test_ping_key_403_is_invalid(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(httpx, "get", lambda *a, **kw: _Resp(403))
|
||||
assert _ping_key("k", "http://x") is False
|
||||
|
||||
|
||||
def test_ping_key_5xx_is_not_definitively_invalid(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
# Transient upstream failure must NOT cause a shadow to be minted.
|
||||
monkeypatch.setattr(httpx, "get", lambda *a, **kw: _Resp(503))
|
||||
assert _ping_key("k", "http://x") is True
|
||||
|
||||
|
||||
def test_ping_key_connect_error_prefers_reuse(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
# Network blip (DNS, captive portal, etc.) — must NOT trigger a re-mint.
|
||||
def boom(*a, **kw):
|
||||
raise httpx.ConnectError("nope")
|
||||
|
||||
monkeypatch.setattr(httpx, "get", boom)
|
||||
assert _ping_key("k", "http://x") is True
|
||||
|
||||
|
||||
def test_ping_key_timeout_prefers_reuse(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def boom(*a, **kw):
|
||||
raise httpx.ReadTimeout("slow")
|
||||
|
||||
monkeypatch.setattr(httpx, "get", boom)
|
||||
assert _ping_key("k", "http://x") is True
|
||||
|
||||
|
||||
# ── plugin_sync._update_shell_rc ──────────────────────────────────────────
|
||||
|
||||
|
||||
def test_shell_rc_updates_existing_export_preserves_trailing_newline(tmp_path) -> None:
|
||||
rc = tmp_path / ".zshrc"
|
||||
rc.write_text('export MEM0_API_KEY="old"\n', encoding="utf-8")
|
||||
changed = _update_shell_rc(rc, "newkey")
|
||||
assert changed is True
|
||||
assert rc.read_text(encoding="utf-8") == 'export MEM0_API_KEY="newkey"\n'
|
||||
|
||||
|
||||
def test_shell_rc_does_not_create_new_export(tmp_path) -> None:
|
||||
rc = tmp_path / ".zshrc"
|
||||
rc.write_text("alias ll='ls -la'\n", encoding="utf-8")
|
||||
changed = _update_shell_rc(rc, "newkey")
|
||||
assert changed is False
|
||||
assert rc.read_text(encoding="utf-8") == "alias ll='ls -la'\n"
|
||||
|
||||
|
||||
def test_shell_rc_preserves_surrounding_content(tmp_path) -> None:
|
||||
rc = tmp_path / ".zshrc"
|
||||
original = (
|
||||
"# my zshrc\n"
|
||||
"alias ll='ls -la'\n"
|
||||
"export MEM0_API_KEY='old'\n"
|
||||
"export OTHER=keepme\n"
|
||||
)
|
||||
rc.write_text(original, encoding="utf-8")
|
||||
_update_shell_rc(rc, "newkey")
|
||||
after = rc.read_text(encoding="utf-8")
|
||||
assert "alias ll='ls -la'\n" in after
|
||||
assert "export OTHER=keepme\n" in after
|
||||
assert "# my zshrc\n" in after
|
||||
assert 'export MEM0_API_KEY="newkey"\n' in after
|
||||
|
||||
|
||||
def test_shell_rc_idempotent_when_already_matching(tmp_path) -> None:
|
||||
rc = tmp_path / ".zshrc"
|
||||
rc.write_text('export MEM0_API_KEY="same"\n', encoding="utf-8")
|
||||
assert _update_shell_rc(rc, "same") is False
|
||||
|
||||
|
||||
def test_shell_rc_missing_file_is_noop(tmp_path) -> None:
|
||||
rc = tmp_path / ".zshrc" # does not exist
|
||||
assert _update_shell_rc(rc, "x") is False
|
||||
|
||||
|
||||
# ── plugin_sync._update_claude_settings ────────────────────────────────────
|
||||
|
||||
|
||||
def test_claude_settings_does_not_create_env_block(tmp_path) -> None:
|
||||
import json
|
||||
|
||||
settings = tmp_path / "settings.json"
|
||||
settings.write_text(json.dumps({"otherKey": 1}), encoding="utf-8")
|
||||
changed = _update_claude_settings(settings, "newkey")
|
||||
assert changed is False
|
||||
# Original content unchanged.
|
||||
assert json.loads(settings.read_text(encoding="utf-8")) == {"otherKey": 1}
|
||||
|
||||
|
||||
def test_claude_settings_does_not_create_mem0_entry_in_existing_env(tmp_path) -> None:
|
||||
import json
|
||||
|
||||
settings = tmp_path / "settings.json"
|
||||
settings.write_text(json.dumps({"env": {"OTHER_KEY": "x"}}), encoding="utf-8")
|
||||
changed = _update_claude_settings(settings, "newkey")
|
||||
assert changed is False
|
||||
|
||||
|
||||
def test_claude_settings_updates_existing_entry(tmp_path) -> None:
|
||||
import json
|
||||
|
||||
settings = tmp_path / "settings.json"
|
||||
settings.write_text(
|
||||
json.dumps({"env": {"MEM0_API_KEY": "old", "OTHER": "y"}}, indent=2),
|
||||
encoding="utf-8",
|
||||
)
|
||||
changed = _update_claude_settings(settings, "fresh")
|
||||
assert changed is True
|
||||
data = json.loads(settings.read_text(encoding="utf-8"))
|
||||
assert data["env"]["MEM0_API_KEY"] == "fresh"
|
||||
assert data["env"]["OTHER"] == "y" # other keys preserved
|
||||
|
||||
|
||||
def test_claude_settings_idempotent(tmp_path) -> None:
|
||||
import json
|
||||
|
||||
settings = tmp_path / "settings.json"
|
||||
settings.write_text(json.dumps({"env": {"MEM0_API_KEY": "same"}}), encoding="utf-8")
|
||||
assert _update_claude_settings(settings, "same") is False
|
||||
|
||||
|
||||
def test_claude_settings_malformed_json_is_noop(tmp_path) -> None:
|
||||
settings = tmp_path / "settings.json"
|
||||
settings.write_text("{ this is not json", encoding="utf-8")
|
||||
assert _update_claude_settings(settings, "x") is False
|
||||
|
||||
|
||||
# ── bootstrap rate-limit translation ──────────────────────────────────────
|
||||
|
||||
|
||||
def test_bootstrap_403_permission_surfaces_ratelimit(monkeypatch, capsys) -> None:
|
||||
"""DRF 403 'You do not have permission' must be translated to the daily limit message."""
|
||||
from mem0_cli.commands.agent_mode_cmd import bootstrap_via_backend
|
||||
from mem0_cli.config import Mem0Config
|
||||
|
||||
fake_resp = MagicMock()
|
||||
fake_resp.status_code = 403
|
||||
fake_resp.text = '{"detail": "You do not have permission to perform this action."}'
|
||||
fake_resp.json = MagicMock(
|
||||
return_value={"detail": "You do not have permission to perform this action."}
|
||||
)
|
||||
|
||||
class _Client:
|
||||
def __init__(self, *a, **kw):
|
||||
pass
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *a):
|
||||
return False
|
||||
|
||||
def post(self, *a, **kw):
|
||||
return fake_resp
|
||||
|
||||
monkeypatch.setattr(httpx, "Client", _Client)
|
||||
cfg = Mem0Config()
|
||||
cfg.platform.base_url = "https://api.mem0.ai"
|
||||
import typer
|
||||
|
||||
with pytest.raises(typer.Exit):
|
||||
bootstrap_via_backend(cfg)
|
||||
|
||||
captured = capsys.readouterr()
|
||||
combined = captured.out + captured.err
|
||||
assert "Daily Agent Mode signup limit reached" in combined
|
||||
assert "permission to perform this action" not in combined
|
||||
Reference in New Issue
Block a user