diff --git a/integrations/mem0-agent/docs/CONTRACT.md b/integrations/mem0-agent/docs/CONTRACT.md index b80c84e41..0b0e0981e 100644 --- a/integrations/mem0-agent/docs/CONTRACT.md +++ b/integrations/mem0-agent/docs/CONTRACT.md @@ -6,19 +6,27 @@ each disagreement is marked and was reproduced. ## Four rules that apply to every call -### 1. Pin the project in the request BODY +### 1. Scope is the API key's job; body ids are only an override + +An API key is bound to exactly one `(org_id, project_id)` pair server-side. Send nothing and +every call resolves correctly — verified live: `add`, `get_all` and `feedback` all return 200 +with no ids anywhere. **Do not require them in client config**; that is redundant state that +can go stale. + +They exist for one purpose: pointing a key at a *different* project in the same org. When you +do that, both ids must travel in the **body** — as query params they are silently ignored: ```python -body = {..., "project_id": "proj_...", "org_id": "org_..."} # routes correctly +body = {..., "project_id": "proj_...", "org_id": "org_..."} # overrides the key params = {"project_id": "proj_..."} # SILENTLY IGNORED ``` -The API key carries a default project. Passing `project_id` as a query param does not -override it — the call lands in the key's default project with no error. Two identical -probe writes, one routed each way, landed in two different projects. +Two probe writes, one routed each way, landed in two different projects. Half an override +(one id without the other) is treated as no override. -This is almost certainly how v1's benchmark corpora (39% of the production project) got -there. `Api._pin()` applies this automatically; `strict=True` raises if it cannot. +The only endpoints that genuinely need the ids are the project-configuration ones, which +carry both in the URL path. They resolve them from `GET /v1/ping/` and cache the result, so +nothing has to be configured locally. ### 2. Every read passes `latest_only=True` @@ -107,7 +115,7 @@ Identity comes from `GET /v1/ping/` → `user_email`, `org_id`, `project_id`. |---|---| | `DELETE /v1/memories/` | Takes **query params**; a body returns 400 "at least one filter required" | | `GET .../projects//` | `fields` must be **repeated** params (`?fields=a&fields=b`), not comma-joined | -| `POST /v1/feedback/` | **404s without the project pin** in the body | +| `POST /v1/feedback/` | 404s when the memory is in a project the key is not bound to and no override is sent; fine unpinned otherwise | | `POST /v3/memories/` | This is `get_all`; `page`/`page_size` are query params, filters go in the body | ## Metadata schema diff --git a/integrations/mem0-agent/src/mem0_agent/api.py b/integrations/mem0-agent/src/mem0_agent/api.py index c1a718ba9..1ca98ad69 100644 --- a/integrations/mem0-agent/src/mem0_agent/api.py +++ b/integrations/mem0-agent/src/mem0_agent/api.py @@ -45,17 +45,39 @@ class Api: self.strict = strict self._opener = opener or urllib.request.urlopen self.last_error: str | None = None + self._scope: tuple[str | None, str | None] | None = None # ---------- plumbing ---------- def _pin(self, project_id: str | None = None) -> dict[str, str]: + """Body-level scope OVERRIDE, deliberately empty by default. + + An API key is already bound to one (org, project) server-side, so sending nothing + is the normal case and the backend resolves it. These ids are only for pointing a + key at a different project in the same org, and they must travel in the BODY -- + as query params they are silently ignored. + """ pid = project_id or self.project_id oid = self.org_id if not pid or not oid: - if self.strict: - raise ContractError("project_id and org_id must be set; unpinned calls leak into the key's default project") return {} return {"project_id": pid, "org_id": oid} + def resolve_scope(self) -> tuple[str | None, str | None]: + """The (org_id, project_id) this key acts on, asked of the API and cached. + + Needed only by the project-configuration endpoints, which carry both ids in the + URL path. Memory operations never need this. + """ + if self._scope is None: + status, body = self.ping() + if status == 200 and isinstance(body, dict): + self._scope = (body.get("org_id"), body.get("project_id")) + else: + self._scope = (None, None) + org = self.org_id or self._scope[0] + project = self.project_id or self._scope[1] + return org, project + def call(self, method: str, path: str, body: dict | None = None, params: dict | None = None, timeout: float = READ_TIMEOUT) -> tuple[int, Any]: """Returns (status, parsed_body). Never raises on HTTP or network error.""" @@ -146,18 +168,27 @@ class Api: # ---------- project config ---------- def project_get(self, *, project_id: str | None = None, fields: list[str] | None = None) -> tuple[int, Any]: - pid = project_id or self.project_id - params = {"fields": fields} if fields else None # rule 4: repeated params - return self.call("GET", f"/api/v1/orgs/organizations/{self.org_id}/projects/{pid}/", + org, pid = self.resolve_scope() + pid = project_id or pid + if not org or not pid: + return 0, {"error": "could not resolve org/project from the API key"} + params = {"fields": fields} if fields else None # repeated params, not comma-joined + return self.call("GET", f"/api/v1/orgs/organizations/{org}/projects/{pid}/", None, params=params) def project_update(self, *, project_id: str | None = None, **kw) -> tuple[int, Any]: - pid = project_id or self.project_id - return self.call("PATCH", f"/api/v1/orgs/organizations/{self.org_id}/projects/{pid}/", + org, pid = self.resolve_scope() + pid = project_id or pid + if not org or not pid: + return 0, {"error": "could not resolve org/project from the API key"} + return self.call("PATCH", f"/api/v1/orgs/organizations/{org}/projects/{pid}/", kw, timeout=WRITE_TIMEOUT) def project_create(self, name: str, description: str = "") -> tuple[int, Any]: - return self.call("POST", f"/api/v1/orgs/organizations/{self.org_id}/projects/", + org, _ = self.resolve_scope() + if not org: + return 0, {"error": "could not resolve org from the API key"} + return self.call("POST", f"/api/v1/orgs/organizations/{org}/projects/", {"name": name, "description": description}, timeout=WRITE_TIMEOUT) diff --git a/integrations/mem0-agent/src/mem0_agent/ctx.py b/integrations/mem0-agent/src/mem0_agent/ctx.py index 85dc8eb3a..4c63ca495 100644 --- a/integrations/mem0-agent/src/mem0_agent/ctx.py +++ b/integrations/mem0-agent/src/mem0_agent/ctx.py @@ -62,24 +62,22 @@ def build(session_id: str | None = None, cwd: str | None = None, *, strict: bool if not key: return Ctx(None, settings, state, "", "", session_id, None, False, "no API key") - org = settings.get("org_id") or os.environ.get("MEM0_ORG_ID") - project = settings.get("memory_project_id") or settings.get("default_project_id") \ - or os.environ.get("MEM0_PLATFORM_PROJECT_ID") + # An API key is already bound to one (org, project) on the backend, so neither id is + # required here. They are sent only to OVERRIDE that binding -- i.e. to point one key at + # a different project in the same org. Left unset, every call resolves server-side. + org = settings.get("org_id") or os.environ.get("MEM0_AGENT_ORG_ID") + project = settings.get("memory_project_id") or os.environ.get("MEM0_AGENT_PROJECT_ID") api = Api(key, org_id=org, project_id=project, breaker=Breaker(state.breaker_path), strict=strict) - # First run: learn identity and the key's home project from the API itself. - if not org or not project: - status, body = api.ping() - if status == 200 and isinstance(body, dict): - api.org_id = org = body.get("org_id") - api.project_id = project = settings.get("memory_project_id") or body.get("project_id") - settings.set("org_id", org) - settings.set("default_project_id", body.get("project_id")) - else: - return Ctx(None, settings, state, "", "", session_id, None, False, "identity unavailable") + # An override needs both halves; one alone would be ignored and quietly mislead. + if bool(org) != bool(project): + api.org_id = api.project_id = None + settings_warning = "project override ignored: set both org_id and memory_project_id" + else: + settings_warning = "" user_id = resolve_user_id(api, settings) app_id = resolve_app_id(cwd, settings) branch = resolve_branch(cwd) - return Ctx(api, settings, state, user_id, app_id, session_id, branch, True) + return Ctx(api, settings, state, user_id, app_id, session_id, branch, True, settings_warning) diff --git a/integrations/mem0-agent/src/mem0_agent/onboard.py b/integrations/mem0-agent/src/mem0_agent/onboard.py index dbc5371bb..e7030bd42 100644 --- a/integrations/mem0-agent/src/mem0_agent/onboard.py +++ b/integrations/mem0-agent/src/mem0_agent/onboard.py @@ -191,9 +191,11 @@ def run_onboard(interactive: bool = True, **overrides: Any) -> dict: detail = f"{size} memories" if size is not None else "size unknown" if busy or size is None: report["warnings"].append( - f"Using the API key's default project {name or ctx.api.project_id} ({detail}). " - "If it also serves a production app, create a dedicated coding-memory project and " - "set `memory_project_id` in " + str(settings.path) + "." + f"Using the API key's project {name or 'unknown'} ({detail}). " + "If it also serves a production app, the cleanest fix is a separate API key " + "issued for a dedicated coding-memory project -- the key carries the scope, so " + "nothing needs configuring here. As an escape hatch you can override with " + "org_id + memory_project_id in " + str(settings.path) + "." ) # 4. The memory-mode decision. Default follows what the repo already does. diff --git a/integrations/mem0-agent/src/mem0_agent/settings.py b/integrations/mem0-agent/src/mem0_agent/settings.py index d49021ecf..26f38a9dd 100644 --- a/integrations/mem0-agent/src/mem0_agent/settings.py +++ b/integrations/mem0-agent/src/mem0_agent/settings.py @@ -134,11 +134,9 @@ def resolve_user_id(api, settings: Settings | None = None) -> str: email = body.get("user_email") or "" uid = email.split("@")[0] if email else "" if uid: + # Cache the identity only. Scope is deliberately NOT persisted: the API key + # already carries it, and storing a copy here would be state that can go stale. settings.set("user_id", uid) - if body.get("org_id"): - settings.set("org_id", body["org_id"]) - if body.get("project_id"): - settings.set("default_project_id", body["project_id"]) return uid return os.environ.get("USER", "default") diff --git a/integrations/mem0-agent/tests/test_contract.py b/integrations/mem0-agent/tests/test_contract.py index 253501d5d..d67b2b422 100644 --- a/integrations/mem0-agent/tests/test_contract.py +++ b/integrations/mem0-agent/tests/test_contract.py @@ -54,8 +54,9 @@ def make_api(**kw): return api, rec -# --- Rule 1: the project pin travels in the body, never as a query param --- -def test_writes_pin_project_in_body(): +# --- Rule 1: a scope OVERRIDE travels in the body, never as a query param. +# Omitting it is normal: the API key is already bound to one (org, project). --- +def test_writes_send_the_override_in_the_body(): api, rec = make_api() api.add([{"role": "user", "content": "hi"}], user_id="u", infer=True) assert rec.last["body"]["project_id"] == "proj_Y" @@ -63,24 +64,61 @@ def test_writes_pin_project_in_body(): assert "project_id=" not in rec.last["url"] -def test_reads_pin_project_in_body(): +def test_reads_send_the_override_in_the_body(): api, rec = make_api() api.get_all(F.all_in_scope("u", "app")) assert rec.last["body"]["project_id"] == "proj_Y" assert "project_id=" not in rec.last["url"] -def test_feedback_carries_the_pin_or_it_404s_in_production(): +def test_feedback_carries_the_override_when_one_is_configured(): + """Feedback 404s only when the memory lives in a project the key is not bound to and + no override is sent -- not, as first assumed, on every unpinned call.""" api, rec = make_api() api.feedback("mem-1", "POSITIVE", "referenced in session") assert rec.last["body"]["project_id"] == "proj_Y" assert rec.last["body"]["org_id"] == "org_X" -def test_strict_mode_refuses_unpinned_calls(): - api = Api("key", strict=True) - with pytest.raises(ContractError): - api._pin() +def test_unpinned_calls_send_no_scope_and_let_the_key_decide(): + """An API key is already bound to one (org, project) server-side, so the ids are an + override, not a requirement. Verified live: add, get_all and feedback all return 200 + with nothing in the body.""" + api, rec = make_api() + api.org_id = api.project_id = None + api.add([{"role": "user", "content": "hi"}], user_id="u", infer=True) + body = rec.last["body"] + assert "project_id" not in body and "org_id" not in body + + api.get_all(F.all_in_scope("u", "app")) + assert "project_id" not in rec.last["body"] + + api.feedback("mem-1", "POSITIVE") + assert "project_id" not in rec.last["body"] + + +def test_override_travels_in_the_body_when_both_ids_are_set(): + api, rec = make_api() + api.get_all(F.all_in_scope("u", "app")) + assert rec.last["body"]["project_id"] == "proj_Y" + assert rec.last["body"]["org_id"] == "org_X" + assert "project_id=" not in rec.last["url"], "query params are silently ignored" + + +def test_half_an_override_is_no_override(): + """One id alone would be ignored by the backend; sending it would only mislead.""" + api, _ = make_api() + api.org_id = None + assert api._pin() == {} + + +def test_project_endpoints_resolve_scope_from_the_key(): + """Only the project-config endpoints need the ids, and they ask the API for them.""" + api, rec = make_api(recorder_kw={"body": {"org_id": "org_ping", "project_id": "proj_ping"}}) + api.org_id = api.project_id = None + api.project_get(fields=["decay"]) + assert "/organizations/org_ping/projects/proj_ping/" in rec.last["url"] + assert rec.calls[0]["url"].endswith("/v1/ping/"), "scope comes from ping, not from config" # --- Rule 2: latest_only on every read, or superseded facts resurface ---