Skip to content

Remote + embedded audit at v1.1.0: Supermemory re-store 400s (PATCH drops containerTag), keyed-seam gaps, and verified adapter defects #75

Description

@YellowSnnowmann

Summary

An eight-lane production audit of the memory stack (run after #69/#71 merged) verified one blocker and a set of majors in the remote adapters and the embedded engine. Every finding below was traced in source; engine-API claims rest on vendor OpenAPI/SDK definitions and engine server source, not live calls. All paths are the post-#73 crates/ layout, verified present at v1.1.0 (1d501fb).

Blocker

  • Supermemory upsert's update branch omits containerTag, which PATCH /v4/memories requirescrates/tinymemory-remote/src/supermemory.rs:355-366. The PATCH body is {id, newContent, metadata}; Supermemory's OpenAPI marks containerTag required on PATCH ("Required to scope the operation", 400 on missing fields) and both official SDKs enforce it as non-optional. The adapter's POST and DELETE both send the tag — PATCH alone doesn't. First store of a key works; every re-store of the same key returns 400 → MemoryError::Invalid, permanently for that key. The suite stays green because neither the supermemory double nor the conformance double enforces the field — fix should also make both doubles reject a tagless PATCH, the way sm_create polices POST.

Majors

Mem0

  • delete never moved to the U2: keyed CRUD for the Mem0/Cognee adapters — investigation + design (get drops from O(N) to ≤3 requests) #69 keyed seamcrates/tinymemory-remote/src/mem0.rs:678. Resolves via entries() (whole-account walk) instead of the entry() seam upsert already uses. Self-hosted: forget dies at the ≥1000-record account ceiling even for a tiny namespace. Hosted: up to 500×200-row pages to delete one record. Fix is one line: resolve through self.entry(namespace, key). (Found independently by two audit lanes.)
  • Current OSS server admin-gates the unscoped listing — the whole-store walk (GET /memories with no entity id) requires the admin role on today's mem0 server, so count()/namespace_summaries()/list(None)/forget() fail 403 with a regular key, and the adapter's "check the API key" hint sends the operator down the wrong runbook.
  • Namespace-less recall always 400s against current OSS — the SelfHosted search arm sends "filters": {} when there is no namespace; the server requires at least one of user_id/agent_id/run_id. Fix direction for both: stamp agent_id on OSS writes and use it as the unscoped scope (mirrors the Cloud arm), plus an actionable 403 message.

Supermemory

  • Account-wide ops page every foreign container tagmemories() lists all tags and pages each with no filter for the adapter-owned tinymemory: prefix; on a shared account, count()/list(None)/namespace_summaries()/export walk thousands of foreign pages and discard everything at decode.
  • Concurrent same-key stores create permanent duplicates; delete removes only the newest and the stale twin resurrects — upsert is find-then-write with a page-walk-long race; nothing heals duplicates afterward. Fix: sweep all (namespace, key) matches on delete and on the PATCH branch so duplicates self-heal.
  • The per-tag pager has no page ceiling — it trusts server-supplied totalPages unboundedly; mem0 got CLOUD_MAX_PAGES = 500 for exactly this failure class. Same ensure! shape needed.

Cognee

  • First recall in a fresh namespace errors instead of answering emptysearch() sends the dataset name with no find_dataset guard; real Cognee 404s ("No datasets found") when the name resolves to nothing. Every sibling op treats a missing dataset as empty; only recall diverges.
  • "updatedAt": null halts the timestamp fallback chain — both backfill sites (crates/tinymemory-remote/src/cognee.rs:269 and the keyed-path copy near :311). Real Cognee serializes updatedAt: null for never-updated records (the common case); Value::get on a present-but-null key returns Some(Null), so the or_else chain never reaches createdAt and essentially every record answers an empty timestamp. Fix: find_map(|k| data.get(k).and_then(Value::as_str)) over the four candidates, plus a double row with a null updatedAt.
  • Records larger than one chunk are silently invisible to recall — recall keeps only chunks whose text parses as a complete envelope; Cognee chunks uploads at the embedding model's token cap and the adapter sends no chunk_size, so an oversized record comes back as fragments that fail both parses and vanish without a warning (stored fine, visible to get/list, unfindable via recall; ~2 KB threshold on 512-token embedders).
  • The multipart upload leg bypasses the typed error taxonomy — raw .send() + hand-rolled anyhow!: 400/413 isn't Invalid, 401 isn't Unauthorized, 429/503 isn't Unavailable — all flatten to Other, violating §A4. The failure suite misses it because its double fails the preceding typed GET.

Shared plumbing

  • Non-2xx bodies are buffered with no size cap — the 64 MiB read_capped guard applies only to success bodies; all four error paths (crates/tinymemory-remote/src/common.rs, the response.text() sites) buffer unboundedly, of which 300 chars are ever used. A 64 KiB cap suffices.

Embedded engine (tinycortex)

  • KV family: put→get misses, put→delete workscrates/tinymemory-tinycortex/src/engine/mod.rs:547 compares the RAW caller key against canonicalized stored keys over a full-namespace listing (kv_list same for prefixes), while writes canonicalize and kv_delete goes through the symmetric shim — an incoherent split. Machine-verified twice as drift, not design (the #5164 comment's own "invisible" class); trigger class is canonicalizer-rewritten keys (card-shaped digit runs — emails are deliberately excluded from the PII gate). Zero KV conformance coverage. Fix: canonicalize in kv_get/kv_list (the symmetric kv_get_namespace shim already exists), add a keyed fetch to stop the namespace walk, and add KV conformance.
  • The mandatory path runs blocking SQLite inline on the async executor — and the repo documents the oppositecrates/tinymemory-core/src/store/memory_trait.rs (zero spawn_blocking under core/store; machine-verified twice). Every Memory-trait method locks the process-wide connection mutex and runs synchronous SQL inside an async fn, while the same adapter's Cargo.toml claims "every family method runs synchronous engine work off the async executor" — and the profile/episodic families that DO offload hold the same mutex from the blocking pool. Fix: move the Memory-impl bodies onto spawn_blocking like the profile family.

Minors (verified, lower blast radius)

  • Taint decode is inconsistent at the fail line: absent marker → Internal (trusted, via #[default]) while malformed → ExternalSync (crates/tinymemory-api/src/types.rs:113-118); absent should fail closed too.
  • Cognee: one corrupt/0-byte envelope poisons every enumeration op; count()/namespace_summaries() still fan out the 1+D+N raw walk U2: keyed CRUD for the Mem0/Cognee adapters — investigation + design (get drops from O(N) to ≤3 requests) #69 removed elsewhere; store() blocks on the full synchronous cognify pipeline inside the 60s budget (a timeout still leaves the record ingested); duplicate-name resolution assumes insertion order but the listing sorts by data_size.
  • Supermemory: recall forwards limits >100 to an API whose documented max is 100 and category/session post-filters under-return silently; records are created isStatic: false, opting exact records into dynamic rewriting; 1..10,000-char content limits unenforced.
  • Mem0: OSS update merges metadata so a cleared session_id stamp survives; Cloud sends "run_id": null against its own omit-don't-null rule; health probes are unauthenticated (revoked key reports Ready).
  • Shared: 429 retries ignore Retry-After; valid-JSON envelope drift decodes as an empty store (absence laundering) on listing paths.
  • Embedded: recall fabricates entry identity (positional id, Utc::now() timestamp, dropped session) while get/list return the truth for the same row; export re-materializes the namespace per page under the global mutex; health_check is pure path-existence against a contract that promises "able to serve" (both machine-verified).

Suggested order

  1. The blocker + the two one-liners (mem0 delete-through-seam, pager ceiling) + the cognee trio (recall guard, find_map timestamps, chunk-size/keyed-fallback) — one adapter-correctness PR.
  2. Error-body caps, taint fail-closed, multipart typing.
  3. Embedded spawn_blocking + KV canonicalization + KV conformance.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions