From bd6d170ebbbffd31083559f96bf137e79b6cad9c Mon Sep 17 00:00:00 2001 From: Mgeeeek Date: Thu, 14 May 2026 19:39:05 +0530 Subject: [PATCH] fix(cli): pingKey distinguishes network failures from invalid keys + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- cli/node/src/agent-detect.ts | 13 +- cli/node/src/commands/agent-mode.ts | 46 +++- cli/node/src/commands/init.ts | 55 ++++- cli/node/src/plugin-sync.ts | 9 +- cli/node/tests/init-internals.test.ts | 168 ++++++++++++++ cli/python/src/mem0_cli/agent_detect.py | 17 +- .../src/mem0_cli/commands/agent_mode_cmd.py | 52 +++-- cli/python/src/mem0_cli/commands/init_cmd.py | 43 +++- cli/python/tests/test_init_internals.py | 211 ++++++++++++++++++ 9 files changed, 573 insertions(+), 41 deletions(-) create mode 100644 cli/node/tests/init-internals.test.ts create mode 100644 cli/python/tests/test_init_internals.py diff --git a/cli/node/src/agent-detect.ts b/cli/node/src/agent-detect.ts index 495f99a5a..197b0c824 100644 --- a/cli/node/src/agent-detect.ts +++ b/cli/node/src/agent-detect.ts @@ -1,11 +1,14 @@ /** - * 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 ` (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 null when no agent is detected. + * Returns a short name or null. Honest reporting depends on `--agent-caller`; + * this list is just enough to enable the zero-friction auto-bootstrap UX. */ const AGENT_CALLER_ENV: ReadonlyArray = [ diff --git a/cli/node/src/commands/agent-mode.ts b/cli/node/src/commands/agent-mode.ts index 581be997b..d606e046d 100644 --- a/cli/node/src/commands/agent-mode.ts +++ b/cli/node/src/commands/agent-mode.ts @@ -24,6 +24,28 @@ export interface BootstrapEnvelope { mem0_notice?: string; } +function isValidEnvelope(v: unknown): v is BootstrapEnvelope { + return ( + !!v && + typeof v === "object" && + typeof (v as BootstrapEnvelope).api_key === "string" && + (v as BootstrapEnvelope).api_key.length > 0 && + typeof (v as BootstrapEnvelope).default_user_id === "string" && + (v as BootstrapEnvelope).default_user_id.length > 0 + ); +} + +/** + * POST /api/v1/auth/agent_mode/ and mutate config in place. + * + * @param config - Mem0Config mutated in place with the new platform values. + * @param source - `--source` flag passthrough (analytics tag, free-form). + * @param agentCaller - Self-declared agent identity passed via `--agent-caller` + * (e.g. `claude-code`, `cursor`). May be null when the caller omitted the + * flag; the agent can backfill later via `mem0 identify `. Sent to the + * backend in the request body and saved into `platform.agentCaller` for + * local introspection. + */ export async function bootstrapViaBackend( config: Mem0Config, { @@ -68,8 +90,11 @@ export async function bootstrapViaBackend( if (!resp.ok) { let detail: string = resp.statusText; try { - const body = (await resp.json()) as { error?: string; detail?: string }; - detail = body.error ?? body.detail ?? resp.statusText; + const errBody = (await resp.json()) as { + error?: string; + detail?: string; + }; + detail = errBody.error ?? errBody.detail ?? resp.statusText; } catch { /* leave detail as statusText */ } @@ -88,6 +113,15 @@ export async function bootstrapViaBackend( } const envelope = (await resp.json()) as BootstrapEnvelope; + if (!isValidEnvelope(envelope)) { + // Defend against partial/malformed backend responses (e.g. {api_key: null}). + // Without this guard, the typed `string` field is silently set to + // undefined/null and persisted, producing confusing downstream errors. + printError( + "Bootstrap response missing required fields — please update the CLI.", + ); + process.exit(1); + } config.platform.apiKey = envelope.api_key; config.platform.baseUrl = baseUrl; @@ -219,17 +253,17 @@ export async function claimViaOtp( process.exit(1); } - const body = (await verifyResp.json()) as { + const claimBody = (await verifyResp.json()) as { claimed?: boolean; claimed_at?: string; }; - if (!body.claimed) { - printError(`Unexpected verify response: ${JSON.stringify(body)}`); + if (!claimBody.claimed) { + printError(`Unexpected verify response: ${JSON.stringify(claimBody)}`); process.exit(1); } config.platform.agentMode = false; - config.platform.claimedAt = body.claimed_at ?? new Date().toISOString(); + config.platform.claimedAt = claimBody.claimed_at ?? new Date().toISOString(); config.platform.userEmail = email; config.platform.createdVia = "email"; saveConfig(config); diff --git a/cli/node/src/commands/init.ts b/cli/node/src/commands/init.ts index ceec3774d..a6e427deb 100644 --- a/cli/node/src/commands/init.ts +++ b/cli/node/src/commands/init.ts @@ -35,19 +35,62 @@ function validateEmail(email: string): void { } } -async function pingKey( +/** @internal — exported for unit tests. */ +export async function pingKey( apiKey: string, baseUrl: string, timeoutMs = 5000, ): Promise { + // 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 { const resp = await fetch(`${baseUrl.replace(/\/+$/, "")}/v1/ping/`, { headers: { Authorization: `Token ${apiKey}` }, signal: AbortSignal.timeout(timeoutMs), }); - return resp.status === 200; + return resp.status !== 401 && resp.status !== 403; } catch { - return false; + return true; // unknown — prefer reuse + } +} + +async function maybeIdentify( + key: string, + baseUrl: string, + agentCaller: string | undefined, +): Promise { + // 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 (!agentCaller) return; + try { + const resp = await fetch( + `${baseUrl.replace(/\/+$/, "")}/api/v1/auth/agent_mode/caller/`, + { + method: "PATCH", + headers: { + Authorization: `Token ${key}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ agent_caller: agentCaller }), + signal: AbortSignal.timeout(10_000), + }, + ); + if (resp.ok) { + try { + const body = (await resp.json()) as { agent_caller?: string }; + if (fs.existsSync(CONFIG_FILE)) { + const cfg = loadConfig(); + cfg.platform.agentCaller = body.agent_caller ?? agentCaller; + saveConfig(cfg); + } + } catch { + /* swallow — best effort */ + } + } + } catch { + /* swallow — best effort */ } } @@ -354,6 +397,7 @@ export async function runInit( // Rule 1: env MEM0_API_KEY valid → reuse, no new key. const envKey = (process.env.MEM0_API_KEY || "").trim(); if (envKey && (await pingKey(envKey, baseUrl))) { + await maybeIdentify(envKey, baseUrl, opts.agentCaller); emitReuseEnvelope("env"); fireInit("existing_key"); return; @@ -363,6 +407,11 @@ export async function runInit( savedConfig.platform.apiKey && (await pingKey(savedConfig.platform.apiKey, baseUrl)) ) { + await maybeIdentify( + savedConfig.platform.apiKey, + baseUrl, + opts.agentCaller, + ); emitReuseEnvelope("config"); fireInit("existing_key"); return; diff --git a/cli/node/src/plugin-sync.ts b/cli/node/src/plugin-sync.ts index 4c98eabfd..0dcf8f71b 100644 --- a/cli/node/src/plugin-sync.ts +++ b/cli/node/src/plugin-sync.ts @@ -49,7 +49,11 @@ export function syncApiKey(apiKey: string): string[] { return updated; } -function updateClaudeSettings(filePath: string, apiKey: string): boolean { +/** @internal — exported for unit tests; consumers should use {@link syncApiKey}. */ +export function updateClaudeSettings( + filePath: string, + apiKey: string, +): boolean { if (!fs.existsSync(filePath)) return false; let raw: string; let data: Record; @@ -70,7 +74,8 @@ function updateClaudeSettings(filePath: string, apiKey: string): boolean { return true; } -function updateShellRc(filePath: string, apiKey: string): boolean { +/** @internal — exported for unit tests; consumers should use {@link syncApiKey}. */ +export function updateShellRc(filePath: string, apiKey: string): boolean { if (!fs.existsSync(filePath)) return false; let text: string; try { diff --git a/cli/node/tests/init-internals.test.ts b/cli/node/tests/init-internals.test.ts new file mode 100644 index 000000000..7abe2d9bf --- /dev/null +++ b/cli/node/tests/init-internals.test.ts @@ -0,0 +1,168 @@ +/** + * Unit tests for init internals — decision tree primitives + plugin sync. + * + * Mirror of `cli/python/tests/test_init_internals.py`. Both files MUST stay + * in sync — if you add a behavioral assertion here, mirror it on the Python + * side and vice versa. + * + * - `pingKey` 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. + */ + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { pingKey } from "../src/commands/init.js"; +import { updateClaudeSettings, updateShellRc } from "../src/plugin-sync.js"; + +// ── pingKey ────────────────────────────────────────────────────────────── + +describe("pingKey — network vs auth distinction", () => { + const origFetch = globalThis.fetch; + afterEach(() => { + globalThis.fetch = origFetch; + vi.restoreAllMocks(); + }); + + it("returns true for 200", async () => { + globalThis.fetch = vi.fn().mockResolvedValue({ status: 200 } as Response); + await expect(pingKey("k", "http://x")).resolves.toBe(true); + }); + + it("returns false for 401 (definitively invalid)", async () => { + globalThis.fetch = vi.fn().mockResolvedValue({ status: 401 } as Response); + await expect(pingKey("k", "http://x")).resolves.toBe(false); + }); + + it("returns false for 403 (definitively invalid)", async () => { + globalThis.fetch = vi.fn().mockResolvedValue({ status: 403 } as Response); + await expect(pingKey("k", "http://x")).resolves.toBe(false); + }); + + it("returns true for 5xx (transient upstream — prefer reuse)", async () => { + globalThis.fetch = vi.fn().mockResolvedValue({ status: 503 } as Response); + await expect(pingKey("k", "http://x")).resolves.toBe(true); + }); + + it("returns true on network error (prefer reuse over re-mint)", async () => { + globalThis.fetch = vi.fn().mockRejectedValue(new Error("ECONNREFUSED")); + await expect(pingKey("k", "http://x")).resolves.toBe(true); + }); + + it("returns true on timeout (prefer reuse)", async () => { + globalThis.fetch = vi.fn().mockRejectedValue(new Error("aborted")); + await expect(pingKey("k", "http://x")).resolves.toBe(true); + }); +}); + +// ── updateShellRc ──────────────────────────────────────────────────────── + +describe("updateShellRc — exists-only contract", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "mem0-test-")); + }); + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it("updates existing export and preserves trailing newline", () => { + const rc = path.join(tmpDir, ".zshrc"); + fs.writeFileSync(rc, 'export MEM0_API_KEY="old"\n'); + expect(updateShellRc(rc, "newkey")).toBe(true); + expect(fs.readFileSync(rc, "utf-8")).toBe('export MEM0_API_KEY="newkey"\n'); + }); + + it("does NOT create a new export when none exists", () => { + const rc = path.join(tmpDir, ".zshrc"); + fs.writeFileSync(rc, "alias ll='ls -la'\n"); + expect(updateShellRc(rc, "newkey")).toBe(false); + expect(fs.readFileSync(rc, "utf-8")).toBe("alias ll='ls -la'\n"); + }); + + it("preserves surrounding content", () => { + const rc = path.join(tmpDir, ".zshrc"); + const original = + "# my zshrc\n" + + "alias ll='ls -la'\n" + + "export MEM0_API_KEY='old'\n" + + "export OTHER=keepme\n"; + fs.writeFileSync(rc, original); + updateShellRc(rc, "newkey"); + const after = fs.readFileSync(rc, "utf-8"); + expect(after).toContain("alias ll='ls -la'\n"); + expect(after).toContain("export OTHER=keepme\n"); + expect(after).toContain("# my zshrc\n"); + expect(after).toContain('export MEM0_API_KEY="newkey"\n'); + }); + + it("is idempotent when value already matches", () => { + const rc = path.join(tmpDir, ".zshrc"); + fs.writeFileSync(rc, 'export MEM0_API_KEY="same"\n'); + expect(updateShellRc(rc, "same")).toBe(false); + }); + + it("is a no-op for missing files", () => { + const rc = path.join(tmpDir, ".zshrc"); // does not exist + expect(updateShellRc(rc, "x")).toBe(false); + }); +}); + +// ── updateClaudeSettings ───────────────────────────────────────────────── + +describe("updateClaudeSettings — never creates entries", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "mem0-test-")); + }); + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it("does not create env block when none exists", () => { + const settings = path.join(tmpDir, "settings.json"); + fs.writeFileSync(settings, JSON.stringify({ otherKey: 1 })); + expect(updateClaudeSettings(settings, "newkey")).toBe(false); + expect(JSON.parse(fs.readFileSync(settings, "utf-8"))).toEqual({ + otherKey: 1, + }); + }); + + it("does not create MEM0_API_KEY entry in existing env block", () => { + const settings = path.join(tmpDir, "settings.json"); + fs.writeFileSync(settings, JSON.stringify({ env: { OTHER_KEY: "x" } })); + expect(updateClaudeSettings(settings, "newkey")).toBe(false); + }); + + it("updates existing entry and preserves siblings", () => { + const settings = path.join(tmpDir, "settings.json"); + fs.writeFileSync( + settings, + JSON.stringify({ env: { MEM0_API_KEY: "old", OTHER: "y" } }, null, 2), + ); + expect(updateClaudeSettings(settings, "fresh")).toBe(true); + const data = JSON.parse(fs.readFileSync(settings, "utf-8")); + expect(data.env.MEM0_API_KEY).toBe("fresh"); + expect(data.env.OTHER).toBe("y"); + }); + + it("is idempotent when value already matches", () => { + const settings = path.join(tmpDir, "settings.json"); + fs.writeFileSync( + settings, + JSON.stringify({ env: { MEM0_API_KEY: "same" } }), + ); + expect(updateClaudeSettings(settings, "same")).toBe(false); + }); + + it("is a no-op for malformed JSON", () => { + const settings = path.join(tmpDir, "settings.json"); + fs.writeFileSync(settings, "{ this is not json"); + expect(updateClaudeSettings(settings, "x")).toBe(false); + }); +}); diff --git a/cli/python/src/mem0_cli/agent_detect.py b/cli/python/src/mem0_cli/agent_detect.py index 8cdbbf137..3223567a2 100644 --- a/cli/python/src/mem0_cli/agent_detect.py +++ b/cli/python/src/mem0_cli/agent_detect.py @@ -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 `` (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 diff --git a/cli/python/src/mem0_cli/commands/agent_mode_cmd.py b/cli/python/src/mem0_cli/commands/agent_mode_cmd.py index e88aeb9ce..275cef488 100644 --- a/cli/python/src/mem0_cli/commands/agent_mode_cmd.py +++ b/cli/python/src/mem0_cli/commands/agent_mode_cmd.py @@ -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 ``. 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) diff --git a/cli/python/src/mem0_cli/commands/init_cmd.py b/cli/python/src/mem0_cli/commands/init_cmd.py index 5bc32feac..f491ec9d4 100644 --- a/cli/python/src/mem0_cli/commands/init_cmd.py +++ b/cli/python/src/mem0_cli/commands/init_cmd.py @@ -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 diff --git a/cli/python/tests/test_init_internals.py b/cli/python/tests/test_init_internals.py new file mode 100644 index 000000000..20346c597 --- /dev/null +++ b/cli/python/tests/test_init_internals.py @@ -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