vipune 0.20.1

A minimal memory layer for AI agents
Documentation
//! Supersede-path tests for the MCP surface — the `store_memory` tool an agent
//! calls, which is the frame that decides which supersede wrapper a `supersedes`
//! call reaches.
//!
//! Split out of `tests.rs` for issue #271 so both files stay inside the
//! per-file size cap. The gate must keep the `feature = "mcp"` half: the tool
//! handler these tests drive does not exist in the `--no-default-features` lane
//! CI runs. The shared fixtures (`seed_row`, `store_memory_args`,
//! `tool_result_id`) come from `tests.rs` behind `pub(super)`.
#[cfg(all(test, feature = "mcp"))]
mod mcp_supersede {
    use crate::config::Config;
    use crate::mcp::store_wrapper::StoreWrapper;
    use crate::mcp::tests::{seed_row, store_memory_args, tool_result_id};
    use crate::mcp::tools::ToolHandler;
    use crate::memory::lifecycle::MemoryImportance;
    use crate::memory::{MemoryStore, NewMemory};
    use std::sync::{Arc, Mutex};

    /// Issue #258: `store_memory`'s `supersedes` arm parsed and validated
    /// `status` and then called the status-less `StoreWrapper::supersede`, so an
    /// agent passing `status: "candidate"` together with `supersedes` got an
    /// `active` replacement row with no warning — the MCP twin of the CLI drop.
    ///
    /// Drives the `store_memory` TOOL HANDLER itself (the frame where the status
    /// was dropped, and the frame that decides which wrapper the supersedes arm
    /// reaches), not the wrapper below it, and reads the stored column back via
    /// `StoreWrapper::get`, the read path that applies no status filter.
    #[tokio::test]
    async fn store_memory_supersedes_honours_the_requested_status() {
        use crate::memory::lifecycle::{MemoryStatus, MemoryType};

        let (_dir, store) = MemoryStore::test_store();
        let store = Arc::new(Mutex::new(store));
        let wrapper = StoreWrapper::new(store.clone());
        let handler = ToolHandler::new(store, "status-proj".to_string(), Config::default());

        let seeded = seed_row(&wrapper, "status-proj", "original content");
        let old_id = seeded.id.clone();

        // The MCP equivalent of `add --supersedes <id> --status candidate`,
        // called the way an agent calls it: through the tool router, so the
        // wiring inside `store_memory` is what is under test.
        let result = handler
            .call_tool_for_test(
                "store_memory",
                store_memory_args("replacement content", Some("candidate"), Some(&old_id)),
            )
            .await
            .expect("store_memory with supersedes must succeed");
        let new_id = tool_result_id(&result);

        let replacement = wrapper
            .get(&new_id, "status-proj")
            .expect("get")
            .expect("replacement row stored");
        assert_eq!(
            replacement.status,
            MemoryStatus::Candidate.as_str(),
            "the tool handler must hand its validated status to the store, not the wrapper's default"
        );
        assert_eq!(replacement.content, "replacement content");

        // The old row keeps the lifecycle the supersede transaction owns.
        let replaced_row = wrapper
            .get(&old_id, "status-proj")
            .expect("get")
            .expect("superseded row still readable");
        assert_eq!(replaced_row.status, "superseded");
        assert_eq!(replaced_row.superseded_by.as_deref(), Some(new_id.as_str()));

        // Identical outcome to the CLI: a candidate replacement is invisible to
        // the active-only default list on this surface too. A second, unrelated
        // active row is seeded as the control — the superseded original is not
        // listed either — so the guard cannot be satisfied by a list call that
        // returned nothing.
        let control = wrapper
            .ingest_with_lifecycle(
                &NewMemory {
                    project_id: "status-proj",
                    content: "unrelated active control",
                    metadata: None,
                    memory_type: MemoryType::Fact,
                    status: MemoryStatus::Active,
                },
                true,
                MemoryImportance::Medium,
            )
            .expect("seed control");
        let control_id = control["id"].as_str().expect("control id").to_string();

        let listed: Vec<serde_json::Value> =
            serde_json::from_value(wrapper.list("status-proj", 20, None, None).expect("list"))
                .expect("list payload is an array");
        let listed_ids: Vec<&str> = listed
            .iter()
            .map(|m| m["id"].as_str().expect("listed id"))
            .collect();
        assert!(
            listed_ids.contains(&control_id.as_str()),
            "the default list itself works, so its silence about the candidate is meaningful: {listed_ids:?}"
        );
        assert!(
            !listed_ids.contains(&new_id.as_str()),
            "a candidate replacement must stay out of the default list: {listed_ids:?}"
        );

        // And the status-less entry point (`supersede_memory`, which has no
        // `status` parameter at all) still writes `active` by name — the same
        // column value the `status: "active"` case above pins from the tool.
        let plain = wrapper
            .supersede(
                "status-proj",
                "plain replacement",
                None,
                MemoryType::Fact,
                &new_id,
            )
            .expect("level-less supersede");
        let plain_id = plain["id"].as_str().expect("plain id").to_string();
        assert_eq!(
            wrapper
                .get(&plain_id, "status-proj")
                .expect("get")
                .expect("plain row stored")
                .status,
            MemoryStatus::Active.as_str(),
            "the status-less wrapper keeps writing active explicitly"
        );
    }

    /// The `status` an agent names on a `supersedes` call must reach the new
    /// row for **both** insertable spellings. `store_memory_supersedes_honours_the
    /// _requested_status` pins `candidate`; without an `active` case a handler
    /// that hardcoded `active` — the #258 defect — would still pass, since
    /// `active` is also the wrapper's own default. Driven through the router, so
    /// the frame under test is the one that chooses the wrapper.
    #[tokio::test]
    async fn store_memory_supersedes_honours_an_explicit_active_status() {
        use crate::memory::lifecycle::{MemoryStatus, MemoryType};

        let (_dir, store) = MemoryStore::test_store();
        let store = Arc::new(Mutex::new(store));
        let wrapper = StoreWrapper::new(store.clone());
        let handler = ToolHandler::new(store, "active-proj".to_string(), Config::default());

        let old = seed_row(&wrapper, "active-proj", "original content");
        let result = handler
            .call_tool_for_test(
                "store_memory",
                store_memory_args("replacement content", Some("active"), Some(&old.id)),
            )
            .await
            .expect("store_memory with supersedes must succeed");
        let new_id = tool_result_id(&result);

        let replacement = wrapper
            .get(&new_id, "active-proj")
            .expect("get")
            .expect("replacement row stored");
        assert_eq!(replacement.status, MemoryStatus::Active.as_str());
        assert_eq!(replacement.content, "replacement content");
        assert_eq!(replacement.memory_type, MemoryType::Fact.as_str());

        let replaced_row = wrapper
            .get(&old.id, "active-proj")
            .expect("get")
            .expect("superseded row still readable");
        assert_eq!(replaced_row.status, "superseded");
        assert_eq!(replaced_row.superseded_by.as_deref(), Some(new_id.as_str()));
    }

    /// Issue #305: `supersede_memory` serialised an absent/`null` metadata
    /// argument to the 4-character string `"null"` and handed it to the store as
    /// a `&str`, so the replacement row's metadata column held that literal while
    /// `store_memory` stored SQL NULL for the same input. Every read path renders
    /// both spellings as JSON `null` (`parse_metadata`, and the `trim() != "null"`
    /// guards in the wrapper's search/list), so the difference is invisible in
    /// tool output — this test reads the `Option<String>` **column** back through
    /// `StoreWrapper::get`.
    ///
    /// Drives the `supersede_memory` TOOL (nothing else does) through the router
    /// for every shape its handler can receive — including the non-object scalars
    /// and strings, which a "simplify" to an object-only match would silently null
    /// out — and pins parity with a `store_memory` row whose `metadata` key was
    /// likewise omitted.
    #[tokio::test]
    async fn supersede_memory_stores_no_metadata_for_omitted_or_null_metadata() {
        use serde_json::json;

        let (_dir, store) = MemoryStore::test_store();
        let store = Arc::new(Mutex::new(store));
        let wrapper = StoreWrapper::new(store.clone());
        let handler = ToolHandler::new(store.clone(), "meta-proj".to_string(), Config::default());

        // Parity control: a `store_memory` row with the metadata key omitted.
        // Its column must equal every no-metadata row the tool loop stores below,
        // so one read of it replaces a separate equality assertion at the end.
        let stored = handler
            .call_tool_for_test("store_memory", json!({ "text": "stored control" }))
            .await
            .expect("store_memory must succeed");
        let stored_row = wrapper
            .get(&tool_result_id(&stored), "meta-proj")
            .expect("get")
            .expect("stored row");
        assert_eq!(
            stored_row.metadata, None,
            "the parity control must itself store no metadata"
        );

        // (arguments sent to the tool, metadata column expected on the new row).
        let cases = [
            ("omitted", json!({}), None),
            ("explicit null", json!({ "metadata": null }), None),
            (
                "json-null string",
                json!({ "metadata": "null" }),
                Some("\"null\""),
            ),
            ("a number", json!({ "metadata": 0 }), Some("0")),
            ("a boolean", json!({ "metadata": false }), Some("false")),
            ("an empty string", json!({ "metadata": "" }), Some(r#""""#)),
            (
                "a real document",
                json!({ "metadata": { "topic": "auth" } }),
                Some(r#"{"topic":"auth"}"#),
            ),
        ];

        for (label, extra, expected) in cases {
            let old = seed_row(&wrapper, "meta-proj", "original content");
            let mut args = json!({
                "new_text": "replacement content",
                "old_memory_id": old.id,
            });
            for (key, value) in extra.as_object().expect("extra args are an object") {
                args[key] = value.clone();
            }

            let result = handler
                .call_tool_for_test("supersede_memory", args)
                .await
                .unwrap_or_else(|err| panic!("{label}: supersede_memory must succeed: {err:?}"));
            let new_id = tool_result_id(&result);

            let row = wrapper
                .get(&new_id, "meta-proj")
                .expect("get")
                .unwrap_or_else(|| panic!("{label}: replacement row stored"));
            assert_eq!(
                row.metadata.as_deref(),
                expected,
                "{label}: the metadata column must hold what the handler was given"
            );
            assert_eq!(row.content, "replacement content");
            assert_eq!(row.status, "active");

            // The retirement half of the same transaction: a handler that
            // inserted the replacement without marking the old row would still
            // pass every metadata assertion above.
            let retired = wrapper
                .get(&old.id, "meta-proj")
                .expect("get")
                .unwrap_or_else(|| panic!("{label}: superseded row still readable"));
            assert_eq!(
                retired.status, "superseded",
                "{label}: the supersede call must retire the old row"
            );
            assert_eq!(
                retired.superseded_by.as_deref(),
                Some(new_id.as_str()),
                "{label}: the old row must link to the row that replaced it"
            );
        }
    }

    /// Issue #271: the re-supersede guard has to answer an agent in the tool's
    /// own error shape, not as a bare database message. Superseding a row the
    /// supersede transaction already retired would fork the chain, so the tool
    /// refuses and writes nothing.
    #[tokio::test]
    async fn store_memory_refuses_to_supersede_a_superseded_row() {
        let (_dir, store) = MemoryStore::test_store();
        let store = Arc::new(Mutex::new(store));
        let wrapper = StoreWrapper::new(store.clone());
        let handler = ToolHandler::new(store, "chain-proj".to_string(), Config::default());

        let old = seed_row(&wrapper, "chain-proj", "original content");
        let first = handler
            .call_tool_for_test(
                "store_memory",
                store_memory_args("first replacement", None, Some(&old.id)),
            )
            .await
            .expect("first supersede must succeed");
        let first_id = tool_result_id(&first);

        let err = handler
            .call_tool_for_test(
                "store_memory",
                store_memory_args("second replacement", None, Some(&old.id)),
            )
            .await
            .expect_err("re-superseding a superseded row must be refused");
        assert!(
            err.message.contains("superseded") && err.message.contains(&first_id),
            "the refusal must name the current status and its superseded_by: {err:?}"
        );

        // Nothing written: the chain still points at the first replacement and
        // the project holds exactly those two rows.
        let replaced = wrapper
            .get(&old.id, "chain-proj")
            .expect("get")
            .expect("superseded row still readable");
        assert_eq!(replaced.superseded_by.as_deref(), Some(first_id.as_str()));
        let listed: Vec<serde_json::Value> =
            serde_json::from_value(wrapper.list("chain-proj", 20, None, None).expect("list"))
                .expect("list payload is an array");
        assert_eq!(
            listed.len(),
            1,
            "only the live replacement is listed: {listed:?}"
        );
        let all: Vec<serde_json::Value> = serde_json::from_value(
            wrapper
                .list(
                    "chain-proj",
                    20,
                    None,
                    Some(["active", "candidate", "superseded", "deprecated"].as_slice()),
                )
                .expect("list"),
        )
        .expect("list payload is an array");
        assert_eq!(all.len(), 2, "the refused call inserted nothing: {all:?}");
    }
}