From 59c5b2df51722569d62482c24d4ee3803d37f45b Mon Sep 17 00:00:00 2001 From: Shanu Date: Sat, 22 Aug 2026 20:22:40 +0530 Subject: [PATCH 1/3] Add a live contract lane for the remote engines Every adapter test in this crate runs against a double written from the same documentation the adapter was. That agreement is worth having, but it cannot catch a service whose behaviour differs from its documentation: when that happens the adapter and the double are wrong together and the suite stays green. Add an env-gated target that runs the full provider contract against a real hosted endpoint. It skips unless both the URL and the key are set, so the default `cargo test` stays offline, deterministic, and independent of a vendor's uptime. --- .../tests/live_remote_engines.rs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 crates/tinymemory-remote/tests/live_remote_engines.rs diff --git a/crates/tinymemory-remote/tests/live_remote_engines.rs b/crates/tinymemory-remote/tests/live_remote_engines.rs new file mode 100644 index 0000000..e9d0ba5 --- /dev/null +++ b/crates/tinymemory-remote/tests/live_remote_engines.rs @@ -0,0 +1,51 @@ +//! The contract suite against a real hosted service, not a double. +//! +//! Every other test in this crate runs an adapter over a double written from +//! the same documentation the adapter was. That agreement is worth having, but +//! it cannot catch a service that behaves differently from its documentation — +//! and when it does, the adapter and the double are wrong together and the +//! suite stays green. Issue #80 is exactly that: Supermemory strips two +//! characters from stored content server-side, and nothing here could see it +//! because nothing here ever spoke to Supermemory. +//! +//! So this target exists to be pointed at the real thing. It is skipped unless +//! the credentials are present, which keeps `cargo test` offline, +//! deterministic, and independent of a vendor's uptime by default. +//! +//! ```sh +//! TINYMEMORY_TEST_SUPERMEMORY_URL=https://api.supermemory.ai \ +//! TINYMEMORY_TEST_SUPERMEMORY_KEY=sm_... \ +//! cargo test -p tinymemory-remote --test live_remote_engines +//! ``` +//! +//! The suite writes and deletes records under its own namespaces in whatever +//! account the key belongs to. Point it at a scratch account rather than one +//! holding anything you would miss. + +#![allow(clippy::expect_used)] + +use std::sync::Arc; + +use tinymemory_remote::{supermemory_provider, SupermemoryMemory}; + +/// Reads one engine's endpoint and key, or `None` when either is unset. +/// +/// Both are required rather than defaulting the URL: a live test that invents +/// its own endpoint can end up silently exercising the wrong service. +fn credentials(engine: &str) -> Option<(String, String)> { + let url = std::env::var(format!("TINYMEMORY_TEST_{engine}_URL")).ok()?; + let key = std::env::var(format!("TINYMEMORY_TEST_{engine}_KEY")).ok()?; + (!url.is_empty() && !key.is_empty()).then_some((url, key)) +} + +/// Runs the full provider contract against the live Supermemory API. +/// +/// Skipped without `TINYMEMORY_TEST_SUPERMEMORY_URL` and `..._KEY`. +#[tokio::test] +async fn live_supermemory_upholds_the_provider_contract() { + let Some((url, key)) = credentials("SUPERMEMORY") else { + return; + }; + let provider = supermemory_provider(SupermemoryMemory::api(&url, &key).expect("client")); + tinymemory_conformance::assert_provider(Arc::new(provider)).await; +} From 259f70994a5b3f84100ea72fd78b24f1c807f173 Mon Sep 17 00:00:00 2001 From: Shanu Date: Sat, 22 Aug 2026 20:22:49 +0530 Subject: [PATCH 2/3] Refuse content Supermemory would alter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Supermemory removes two characters from stored content server-side, so a record containing either reads back shorter than it was written. Measured against the live API rather than inferred: POST /v4/memories echoes the stored value in its own 201, and the echo is missing them. The conformance suite's round-trip case fails there, and it is right to. MemoryCore::store promises that what is read back equals what was stored. A driver may refuse a shape outright, but may not accept one and hand back another, so refuse with Invalid instead of storing a value the service will rewrite. The check precedes the request because the service accepts and alters in the same breath, leaving no later point at which the adapter could object. The refusal is no wider than the defect. Every other C0 control, plus DEL, NEL, ZWSP, BOM and U+2028, survives the live service unchanged, and a test pins that so the predicate cannot quietly grow. It is scoped to content because identity travels in metadata, which is not sanitised — keys and namespaces round-trip both characters intact, and a test pins that too, since widening the refusal there should have to re-measure first. Fixes #80 --- README.md | 23 +++ crates/tinymemory-remote/src/supermemory.rs | 60 +++++++ .../tinymemory-remote/src/supermemory_test.rs | 165 ++++++++++++++++++ 3 files changed, 248 insertions(+) diff --git a/README.md b/README.md index 4769102..c446719 100644 --- a/README.md +++ b/README.md @@ -281,6 +281,29 @@ All three advertise the mandatory Core, Recall, and Portability families. The live Docker harness and conformance command are documented in [`integration/remote-engines/`](integration/remote-engines/README.md). +One of them restricts what it will store. Supermemory removes `U+0000` and +`U+FFFD` from content server-side, so the adapter refuses such content with +`MemoryError::Invalid` rather than storing a value the service would quietly +rewrite: `MemoryCore::store` promises that what is read back equals what was +stored, and a driver may refuse a shape but may not accept one and hand back +another. The restriction is no wider than the defect — every other C0 control, +plus DEL, NEL, ZWSP, BOM and U+2028, survives — and identity is untouched, +because keys and namespaces travel in metadata, which the service does not +sanitise. Callers that might hold either character should strip or replace it +first; `U+FFFD` in particular arrives in any text that has been through a lossy +decode (issue #80). + +Behaviour like that is visible only against the real service, so +`tinymemory-remote` carries a live target that runs the full contract suite +against a hosted endpoint when credentials are present and skips when they are +not. Point it at a scratch account: the suite writes and deletes records. + +```bash +TINYMEMORY_TEST_SUPERMEMORY_URL=https://api.supermemory.ai \ +TINYMEMORY_TEST_SUPERMEMORY_KEY=sm_... \ + cargo test -p tinymemory-remote --test live_remote_engines +``` + ## Development ```bash diff --git a/crates/tinymemory-remote/src/supermemory.rs b/crates/tinymemory-remote/src/supermemory.rs index c439ae7..456a58e 100644 --- a/crates/tinymemory-remote/src/supermemory.rs +++ b/crates/tinymemory-remote/src/supermemory.rs @@ -3,6 +3,7 @@ use async_trait::async_trait; use reqwest::Method; use serde_json::{json, Value}; +use tinymemory_api::error::MemoryError; use tinymemory_api::recall::RecallOpts; use tinymemory_api::traits::Memory; use tinymemory_api::types::MemoryTaint; @@ -180,6 +181,40 @@ struct SupermemoryDialect { client: HttpClient, } +/// The characters Supermemory removes from stored content (issue #80). +/// +/// Measured against the live API rather than inferred from documentation: +/// `POST /v4/memories` echoes the stored value back in its own 201, and for +/// these two the echo is shorter than what was sent. Everything else offered +/// to it survives unchanged — every other C0 control, DEL, NEL, ZWSP, BOM and +/// U+2028 — so the refusal below stays as narrow as the defect. +/// +/// Only `content` is affected. Identity rides in `metadata`, which the service +/// does not sanitise: `tinymemory_key` and `tinymemory_namespace` round-trip +/// both characters intact, so a key is never quietly rewritten into another +/// key's. That is why [`Dialect::upsert`] inspects the content alone. +const CONTENT_CHARACTERS_SUPERMEMORY_DROPS: [char; 2] = ['\u{0}', '\u{FFFD}']; + +/// Returns the first character Supermemory would drop, and where it sits. +fn dropped_content_character(content: &str) -> Option<(usize, char)> { + content + .char_indices() + .find(|(_, character)| CONTENT_CHARACTERS_SUPERMEMORY_DROPS.contains(character)) +} + +/// Names a character without reproducing it. +/// +/// The name goes into an error message, and a raw NUL travels from there into +/// logs, terminals, and shells that render it as nothing — turning a precise +/// refusal into a message that appears to name no character at all. +fn character_name(character: char) -> String { + match character { + '\u{0}' => "U+0000 (NUL)".to_string(), + '\u{FFFD}' => "U+FFFD (the replacement character)".to_string(), + other => format!("U+{:04X}", other as u32), + } +} + impl SupermemoryDialect { /// Maps an arbitrary TinyMemory namespace into Supermemory's bounded tag grammar. fn container_tag(namespace: &str) -> String { @@ -357,7 +392,32 @@ impl Dialect for SupermemoryDialect { } /// Replaces an existing exact record or creates a direct v4 memory. + /// + /// Refuses content Supermemory would alter. `MemoryCore::store` promises + /// that what is read back equals what was stored, and this service strips + /// [`CONTENT_CHARACTERS_SUPERMEMORY_DROPS`] server-side; storing anyway + /// would break that promise silently, which is the one outcome the + /// contract rules out — a driver may refuse a shape, but not accept one + /// and hand back something else. The check precedes the request because + /// the service answers `201` and alters the value in the same breath, so + /// there is no later point at which the adapter could still object. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] — the refusal class for caller input a driver + /// rejects — carried as the anyhow payload every other typed error here + /// uses, so a caller can match on it after the usual downcast. async fn upsert(&self, entry: StoredEntry) -> anyhow::Result<()> { + if let Some((at, character)) = dropped_content_character(&entry.content) { + return Err(anyhow::Error::new(MemoryError::Invalid(format!( + "supermemory removes {} from stored content, so `{}`/`{}` would read back \ + changed (first occurrence at byte {at}); remove the character, or store \ + this record through a driver that preserves it", + character_name(character), + entry.namespace, + entry.key + )))); + } let existing = self.find_entry(&entry.namespace, &entry.key).await?; let metadata = Self::metadata(&entry); if let Some(existing) = existing { diff --git a/crates/tinymemory-remote/src/supermemory_test.rs b/crates/tinymemory-remote/src/supermemory_test.rs index 284b765..79413f4 100644 --- a/crates/tinymemory-remote/src/supermemory_test.rs +++ b/crates/tinymemory-remote/src/supermemory_test.rs @@ -12,6 +12,7 @@ use axum::{ }; use serde_json::{json, Value}; use tinymemory_api::{ + error::MemoryError, provider::{MemoryCore, MemoryProvider, MemoryRecall}, recall::OwnedRecallOpts, traits::Memory, @@ -323,3 +324,167 @@ async fn keyed_reads_scope_to_one_container_tag() { "the request names exactly the namespace's tag" ); } + +/// Spawns the Supermemory double and returns its fixture beside a driver. +/// +/// The refusal tests below assert on what the double *did not* receive, so +/// they need the fixture as much as the driver. +async fn spawn_double() -> (AppState, impl MemoryProvider) { + let state = AppState::default(); + let app = Router::new() + .route("/v3/container-tags/list", get(tags)) + .route("/v4/memories/list", post(list)) + .route("/v4/memories", post(add).patch(update).delete(remove)) + .route("/v4/search", post(search)) + .route("/", get(|| async { StatusCode::OK })) + .with_state(state.clone()); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let endpoint = format!("http://{}", listener.local_addr().expect("address")); + tokio::spawn(async move { + axum::serve(listener, app).await.expect("serve"); + }); + let driver = crate::supermemory_provider( + super::SupermemoryMemory::self_hosted(&endpoint, "secret").expect("client"), + ); + (state, driver) +} + +/// Issue #80: Supermemory removes NUL and U+FFFD from `content` server-side. +/// +/// `MemoryCore::store` promises the content read back equals the content +/// stored, so the adapter must refuse rather than store a value the service +/// will quietly rewrite. `Invalid` is the documented refusal class — what is +/// being rejected is the caller's input. +#[tokio::test] +async fn content_supermemory_would_alter_is_refused() { + let (state, driver) = spawn_double().await; + for (label, ch) in [("NUL", '\u{0}'), ("replacement char", '\u{FFFD}')] { + let content = format!("a{ch}b"); + let result = driver + .store( + "project", + "decision", + &content, + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await; + assert!( + matches!(result, Err(MemoryError::Invalid(_))), + "{label}: content the service would alter must be refused as Invalid, \ + got {result:?}" + ); + } + // Refused before the request, not after a round trip: the service accepts + // this content with a 201 and alters it, so an adapter that asked first + // would have already lost the character by the time it could object. + assert!( + state.0.lock().expect("state lock").records.is_empty(), + "the refusal must short-circuit before any write reaches the service" + ); +} + +/// The refusal has to be actionable without re-emitting the character: a raw +/// NUL in an error string propagates into logs, terminals, and shells that +/// hide it, turning a clear refusal into a confusing one. +#[tokio::test] +async fn the_refusal_names_the_character_without_emitting_it() { + let (_state, driver) = spawn_double().await; + let result = driver + .store( + "project", + "decision", + "a\u{0}b", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await; + assert!(result.is_err(), "content carrying a NUL must be refused"); + let message = result + .err() + .map(|error| error.to_string()) + .unwrap_or_default(); + assert!( + message.contains("U+0000"), + "the message names the character: {message}" + ); + assert!( + !message.contains('\u{0}'), + "the message must not carry the character itself: {message:?}" + ); +} + +/// The predicate stays as narrow as the defect. +/// +/// Only these two characters were measured as dropped; every other C0 control +/// — plus DEL, NEL, ZWSP, BOM and U+2028 — survives the live service intact. +/// A refusal that widened to "control characters" would reject content +/// Supermemory stores perfectly well, and `assert_awkward_content_round_trips` +/// would still pass while the driver quietly became less useful. +#[tokio::test] +async fn content_supermemory_preserves_is_still_stored() { + let (state, driver) = spawn_double().await; + for (label, content) in [ + ("C0 control", "a\u{1}b".to_string()), + ("tab and newlines", "a\tb\nc\r\nd".to_string()), + ("unicode", "héllo — 👋 まいど".to_string()), + ( + "zero-width space and BOM", + "a\u{200B}b\u{FEFF}c".to_string(), + ), + ] { + let result = driver + .store( + "project", + label, + &content, + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await; + assert!(result.is_ok(), "{label} must still store: {result:?}"); + } + assert_eq!( + state.0.lock().expect("state lock").records.len(), + 4, + "every preserved shape reached the service" + ); +} + +/// The refusal is scoped to `content`, because the defect is. +/// +/// Identity rides in `metadata`, which the live service does not sanitise: +/// `tinymemory_key` and `tinymemory_namespace` round-trip both characters +/// unchanged, so a key is never silently rewritten into another key's. Were +/// that untrue the failure would be worse than mangled content — a re-store +/// would stop matching its own record and duplicate it instead. This pins the +/// scoping to the measurement, so widening it later has to re-measure first. +#[tokio::test] +async fn identity_carrying_the_same_characters_is_not_refused() { + let (_state, driver) = spawn_double().await; + driver + .store( + "project\u{0}alpha", + "decision\u{0}beta", + "ordinary content", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect("metadata is not sanitised, so identity is not refused"); + let stored = driver + .get("project\u{0}alpha", "decision\u{0}beta") + .await + .expect("get"); + assert_eq!( + stored.map(|entry| entry.content), + Some("ordinary content".to_string()), + "the record is reachable under the key it was stored with" + ); +} From 7a11357a845f064a973a299fbf3c4c4847f9e623 Mon Sep 17 00:00:00 2001 From: Shanu Date: Sat, 22 Aug 2026 20:58:32 +0530 Subject: [PATCH 3/3] Keep the refusal and the live target clean under review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings, all of them right. The refusal interpolated the namespace and key as raw text. Metadata is not sanitised, so an identity may itself hold a NUL and be stored quite happily — which means a refusal could emit the exact character the naming exists to keep out of logs. Debug-escape both, and pin it with a test that combines content the service would alter with an identity that carries a control character. The live target opened with a blanket clippy::expect_used allowance covering everything in it. Return a Result and propagate the client construction instead, so the guardrail stays where it was. The identity test exercised only U+0000, while the claim it pins covers both characters. Run it over both. --- crates/tinymemory-remote/src/supermemory.rs | 6 +- .../tinymemory-remote/src/supermemory_test.rs | 77 ++++++++++++++----- .../tests/live_remote_engines.rs | 9 +-- 3 files changed, 66 insertions(+), 26 deletions(-) diff --git a/crates/tinymemory-remote/src/supermemory.rs b/crates/tinymemory-remote/src/supermemory.rs index 456a58e..0d867f0 100644 --- a/crates/tinymemory-remote/src/supermemory.rs +++ b/crates/tinymemory-remote/src/supermemory.rs @@ -410,7 +410,11 @@ impl Dialect for SupermemoryDialect { async fn upsert(&self, entry: StoredEntry) -> anyhow::Result<()> { if let Some((at, character)) = dropped_content_character(&entry.content) { return Err(anyhow::Error::new(MemoryError::Invalid(format!( - "supermemory removes {} from stored content, so `{}`/`{}` would read back \ + // Debug-escaped, not raw: metadata is not sanitised, so an + // identity may itself hold a NUL — and a refusal that emits + // one lands in the same logs and terminals that render it as + // nothing, which is the failure this message exists to avoid. + "supermemory removes {} from stored content, so {:?}/{:?} would read back \ changed (first occurrence at byte {at}); remove the character, or store \ this record through a driver that preserves it", character_name(character), diff --git a/crates/tinymemory-remote/src/supermemory_test.rs b/crates/tinymemory-remote/src/supermemory_test.rs index 79413f4..7719828 100644 --- a/crates/tinymemory-remote/src/supermemory_test.rs +++ b/crates/tinymemory-remote/src/supermemory_test.rs @@ -418,6 +418,41 @@ async fn the_refusal_names_the_character_without_emitting_it() { ); } +/// A refusal must stay clean even when the identity is not. +/// +/// Metadata is not sanitised, so a namespace or key may itself carry a NUL and +/// be stored perfectly happily. That makes the two halves meet here: content +/// the service would alter, under an identity that holds a control character. +/// The message interpolates the identity, so this is where a refusal would +/// leak the very character the naming exists to keep out of logs. +#[tokio::test] +async fn a_refusal_emits_no_control_character_from_the_identity_either() { + let (_state, driver) = spawn_double().await; + let result = driver + .store( + "project\u{0}alpha", + "decision\u{0}beta", + "a\u{0}b", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await; + assert!(result.is_err(), "the content must still be refused"); + let message = result + .err() + .map(|error| error.to_string()) + .unwrap_or_default(); + assert!( + !message.chars().any(|character| character.is_control()), + "the refusal must carry no control character at all: {message:?}" + ); + assert!( + message.contains("project") && message.contains("decision"), + "the identity is still named, just escaped: {message}" + ); +} + /// The predicate stays as narrow as the defect. /// /// Only these two characters were measured as dropped; every other C0 control @@ -467,24 +502,26 @@ async fn content_supermemory_preserves_is_still_stored() { #[tokio::test] async fn identity_carrying_the_same_characters_is_not_refused() { let (_state, driver) = spawn_double().await; - driver - .store( - "project\u{0}alpha", - "decision\u{0}beta", - "ordinary content", - MemoryCategory::Core, - None, - MemoryTaint::Internal, - ) - .await - .expect("metadata is not sanitised, so identity is not refused"); - let stored = driver - .get("project\u{0}alpha", "decision\u{0}beta") - .await - .expect("get"); - assert_eq!( - stored.map(|entry| entry.content), - Some("ordinary content".to_string()), - "the record is reachable under the key it was stored with" - ); + for (label, character) in [("nul", '\u{0}'), ("replacement", '\u{FFFD}')] { + let namespace = format!("project{character}alpha"); + let key = format!("decision{character}beta"); + let content = format!("ordinary content for {label}"); + driver + .store( + &namespace, + &key, + &content, + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect("metadata is not sanitised, so identity is not refused"); + let stored = driver.get(&namespace, &key).await.expect("get"); + assert_eq!( + stored.map(|entry| entry.content), + Some(content), + "{label}: the record is reachable under the identity it was stored with" + ); + } } diff --git a/crates/tinymemory-remote/tests/live_remote_engines.rs b/crates/tinymemory-remote/tests/live_remote_engines.rs index e9d0ba5..6aed17b 100644 --- a/crates/tinymemory-remote/tests/live_remote_engines.rs +++ b/crates/tinymemory-remote/tests/live_remote_engines.rs @@ -22,8 +22,6 @@ //! account the key belongs to. Point it at a scratch account rather than one //! holding anything you would miss. -#![allow(clippy::expect_used)] - use std::sync::Arc; use tinymemory_remote::{supermemory_provider, SupermemoryMemory}; @@ -42,10 +40,11 @@ fn credentials(engine: &str) -> Option<(String, String)> { /// /// Skipped without `TINYMEMORY_TEST_SUPERMEMORY_URL` and `..._KEY`. #[tokio::test] -async fn live_supermemory_upholds_the_provider_contract() { +async fn live_supermemory_upholds_the_provider_contract() -> anyhow::Result<()> { let Some((url, key)) = credentials("SUPERMEMORY") else { - return; + return Ok(()); }; - let provider = supermemory_provider(SupermemoryMemory::api(&url, &key).expect("client")); + let provider = supermemory_provider(SupermemoryMemory::api(&url, &key)?); tinymemory_conformance::assert_provider(Arc::new(provider)).await; + Ok(()) }