From 4518f8da89d65bbf969c538cccad3954cbea22be Mon Sep 17 00:00:00 2001
From: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com>
Date: Fri, 17 Jul 2026 14:18:01 +0530
Subject: [PATCH] fix(memory): use stable document_id as sync upsert key (fixes
 #4947 Bug 2 secret-guard sync failure) (#4953)

---
 src/openhuman/memory_store/client.rs       | 26 +++++++-
 src/openhuman/memory_store/client_tests.rs | 71 ++++++++++++++++++++++
 2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/src/openhuman/memory_store/client.rs b/src/openhuman/memory_store/client.rs
index 9cc318f88..8fa0139f1 100644
--- a/src/openhuman/memory_store/client.rs
+++ b/src/openhuman/memory_store/client.rs
@@ -254,9 +254,33 @@ impl MemoryClient {
         document_id: Option<String>,
     ) -> Result<(), String> {
         let namespace = format!("skill-{}", skill_id.trim());
+        // The upsert dedup key must be a stable, opaque identifier — never
+        // free-form human text. Sync providers pass a `{toolkit}:{id}`
+        // `document_id` (e.g. `gmail:<message_id>`); use it as the key so the
+        // provider's human-readable title (an email subject can legitimately
+        // contain verification codes, token-rotation notices, or other
+        // secret-/PII-looking strings) is never run through the
+        // `upsert_document` secret/PII namespace/key guard. A single such
+        // subject would otherwise abort the whole non-tolerant provider sync
+        // with `document namespace/key cannot contain secrets` (see #4947).
+        // Callers without a stable id (e.g. LinkedIn enrichment) keep the
+        // title as the key, unchanged.
+        let stable_id = document_id
+            .as_deref()
+            .map(str::trim)
+            .filter(|id| !id.is_empty());
+        let key = match stable_id {
+            Some(id) => id.to_string(),
+            None => title.to_string(),
+        };
+        tracing::debug!(
+            namespace = %namespace,
+            key_from_document_id = stable_id.is_some(),
+            "[memory_store] store_skill_sync: upserting synchronized document"
+        );
         let input = NamespaceDocumentInput {
             namespace,
-            key: title.to_string(),
+            key,
             title: title.to_string(),
             content: content.to_string(),
             source_type: source_type.unwrap_or_else(|| "doc".to_string()),
diff --git a/src/openhuman/memory_store/client_tests.rs b/src/openhuman/memory_store/client_tests.rs
index cce95553f..32c9855e6 100644
--- a/src/openhuman/memory_store/client_tests.rs
+++ b/src/openhuman/memory_store/client_tests.rs
@@ -109,6 +109,77 @@ async fn clear_namespace_removes_all_docs_in_namespace() {
     assert!(docs_arr.is_empty());
 }
 
+#[tokio::test]
+async fn store_skill_sync_with_secret_like_title_uses_stable_document_id_as_key() {
+    // Regression for #4947 (Bug 2): a Composio provider sync passes the
+    // provider's human-readable title as the document title, but the upsert
+    // *key* must be the stable opaque `document_id` (`gmail:<message_id>`),
+    // NOT the title. Some email subjects legitimately look secret-like
+    // (verification codes, token-rotation notices), and `upsert_document`
+    // rejects any secret-like namespace/key. Because gmail's tinycortex
+    // pipeline does not tolerate scope errors, one such subject would abort
+    // the entire scheduled sync with `document namespace/key cannot contain
+    // secrets`, leaving the source stale ("Last synced 17d ago").
+    let (_tmp, client) = make_client();
+
+    // A subject that trips the secret guard. Assert the precondition so the
+    // test cannot silently pass if the detector's patterns change.
+    let secret_like_title = "Security alert: token glpat-aaaaaaaaaaaaaaaaaaaa was created";
+    assert!(
+        crate::openhuman::memory_store::safety::has_likely_secret(secret_like_title),
+        "test title must trip the secret detector for this regression to be meaningful"
+    );
+
+    // With a stable document_id provided, the write must succeed — the key is
+    // the opaque id, not the secret-like subject. Before the fix the key was
+    // the title and this returned the secrets error.
+    client
+        .store_skill_sync(
+            "gmail",
+            "conn-1",
+            secret_like_title,
+            "email body",
+            Some("composio-provider-incremental".into()),
+            None,
+            Some("medium".into()),
+            None,
+            None,
+            Some("gmail:19af23bc00112233".into()),
+        )
+        .await
+        .expect("secret-like subject must not block a stable-id-keyed sync write");
+
+    // The document is persisted and keyed by the stable id, so a second sync
+    // of the same message dedups (updates in place) rather than duplicating.
+    client
+        .store_skill_sync(
+            "gmail",
+            "conn-1",
+            secret_like_title,
+            "email body v2",
+            Some("composio-provider-incremental".into()),
+            None,
+            Some("medium".into()),
+            None,
+            None,
+            Some("gmail:19af23bc00112233".into()),
+        )
+        .await
+        .expect("re-sync of same message id must succeed");
+
+    let docs = client.list_documents(Some("skill-gmail")).await.unwrap();
+    let arr = docs
+        .get("documents")
+        .and_then(|v| v.as_array())
+        .cloned()
+        .unwrap_or_default();
+    assert_eq!(
+        arr.len(),
+        1,
+        "stable-id key must dedupe both syncs into a single document"
+    );
+}
+
 #[tokio::test]
 async fn clear_skill_memory_targets_prefixed_namespace() {
     let (_tmp, client) = make_client();