From 24d3cf0395d2132761620666e896ce2ce2218c52 Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Wed, 11 Nov 2026 09:43:47 +1000 Subject: [PATCH] =?UTF-8?q?feat:=20memory=20lifecycle=20=E2=80=94=20status?= =?UTF-8?q?,=20expiry,=20history=20and=20conditional=20saves=20(#33)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A save no longer overwrites blind or loses what it replaces: - It reads the current version, copies it verbatim (still signed by its own author) to `archive///v`, links it as `supersedes`, and writes conditionally (put/0.3 `expectedVersion`) on the version it read. A writer that got in first wins; the losing save comes back as SaveOutcome::Conflict carrying the checked winner. Callers that read earlier pass that `version` as `expectedVersion` (0 = must be new). - History keys sit outside every `/` prefix, so recall never sees them; at most five versions are kept per memory. `memory_get` with `history: true` returns them, each verified against its original key. Records gain a signed lifecycle: `status` (active | deprecated | disputed | retracted) with a required `statusReason`, `validUntil` and `reviewAfter`. Recall leaves out retracted and expired memories, ranks deprecated and disputed ones at half weight, and flags `stale` ones past their review date. `memory_list` still shows everything, and `stale: true` narrows it to what needs review. New tools: memory_dispute, memory_deprecate, memory_retract. Retract leaves a content-free tombstone; it is the only status change allowed on a refused record. `memory_forget` stays the hard delete and now removes the history too. `/memories --stale` walks the user through review. Bumps the plugin to 0.6.0. Closes #33 Signed-off-by: Glenn Gore --- .claude-plugin/plugin.json | 2 +- CLAUDE.md | 13 +- Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 17 +- commands/forget.md | 4 +- commands/memories.md | 10 + skills/agent-memory/SKILL.md | 45 ++- src/lazy.rs | 2 +- src/lib.rs | 1 + src/lifecycle.rs | 319 ++++++++++++++++++ src/main.rs | 14 +- src/record.rs | 129 +++++++- src/server.rs | 224 ++++++++++++- src/store.rs | 523 ++++++++++++++++++++++++------ tests/mcp_starts_without_a_vta.rs | 3 + tests/memory_roundtrip.rs | 460 +++++++++++++++++++++++++- 17 files changed, 1604 insertions(+), 166 deletions(-) create mode 100644 src/lifecycle.rs diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index f1c38ab..30c3f70 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "vta-agent-memory", "description": "Durable agent memory stored in your own Verifiable Trust Agent, not in the tool. Save and recall facts across sessions, scoped to a VTA trust context you control and can revoke.", - "version": "0.5.0", + "version": "0.6.0", "keywords": [ "memory", "vta", diff --git a/CLAUDE.md b/CLAUDE.md index d2c11a3..43e5ba7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,8 @@ src/ record.rs the record shape, key encoding, and recall ranking authorship.rs signing records on save, verifying author + location on read provenance.rs basis / confidence / evidence: vocabulary, checks, rank weight - server.rs the six MCP tools + lifecycle.rs status, validUntil/reviewAfter, history keys, standing + server.rs the nine MCP tools tests/ memory_roundtrip.rs Store end-to-end against an in-process fake VTA mcp_starts_without_a_vta.rs the real binary, over stdio, with no VTA @@ -162,6 +163,16 @@ unattributed; and the resolver is `did:key`-only, so a stored record can never make recall fetch a URL. The signer is loaded from the session store **after** connecting — the first connect of an enrolled identity rotates its key. +**A save never overwrites blind.** `Store::save_versioned` reads, archives the +current version to `archive//v`, then writes with `expectedVersion` = +the version it read; a `versionConflict` is a `SaveOutcome::Conflict` carrying the +(checked) winner, never an error to retry. Do not "fix" a conflict by re-saving +unconditionally — that is the lost update this exists to prevent. History keys +must stay outside every `/` prefix, and `forget` must delete them: "forget" +has to mean forget. A status change is a new version signed by whoever made it; +only `retract` is allowed on a refused record, because anything else would +re-sign content nobody vouches for. + ## The retrieval constraint `vta/memory/list/0.2` pages a context in ascending key order and can narrow by diff --git a/Cargo.lock b/Cargo.lock index 14db339..69efd24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5437,7 +5437,7 @@ checksum = "5c3082ca00d5a5ef149bb8b555a72ae84c9c59f7250f013ac822ac2e49b19c64" [[package]] name = "vta-agent-memory" -version = "0.5.0" +version = "0.6.0" dependencies = [ "anyhow", "chrono", diff --git a/Cargo.toml b/Cargo.toml index c12dee1..689bed4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "vta-agent-memory" -version = "0.5.0" +version = "0.6.0" edition = "2024" rust-version = "1.95.0" description = "Agentic memory for Claude Code, stored in a Verifiable Trust Agent" diff --git a/README.md b/README.md index ceefb9c..bce1c66 100644 --- a/README.md +++ b/README.md @@ -250,6 +250,19 @@ downgraded. Verification is `did:key`-only and does no network I/O, so a stored record cannot make recall fetch anything. `doctor` shows the DID this machine signs as. +**Nothing a save replaces is lost, and two writers cannot silently overwrite +each other.** A save reads the current version, copies it verbatim — still signed +by its own author — to `archive///v` (outside every type prefix, so +recall never sees it; at most five per memory), and then writes **conditionally** +on the version it read. A writer that got in first wins, and the losing save +comes back as `saved: false` with the winner, not as an overwrite. A caller that +read a memory earlier passes that `version` as `expectedVersion`. Memories also +have a lifecycle — `active`, `deprecated`, `disputed` (both recalled lower, with a +reason) or `retracted` (a tombstone that never recalls) — plus `validUntil` +(recall drops it after) and `reviewAfter` (recall flags it `stale`). Retract is +the "this was wrong" path; `forget` is still the hard delete, and takes the +history with it. + **Memory is not application state.** "Forget everything" has to stay a safe thing for a user to ask, which it stops being the moment account state lives here. The VTA's `vta/app-state/*/1.0` family — versioned, namespaced, with a change feed — @@ -319,7 +332,7 @@ An entry created before this release grants `any`, under which The plugin does not disappear. The MCP server starts without contacting the VTA and connects on the first memory call, so: -- The six tools are always present, and a failure comes back as a tool error the +- The nine tools are always present, and a failure comes back as a tool error the model can relay — "run `vta-agent-memory setup`" rather than silence. - `memory_context` answers *without* connecting, so you can still ask which VTA and context you are pointed at while it is down. @@ -337,7 +350,7 @@ cargo test ``` `tests/mcp_starts_without_a_vta.rs` drives the real binary over stdio with no -config at all, asserting it completes an MCP handshake, offers all six tools, and +config at all, asserting it completes an MCP handshake, offers all nine tools, and returns an actionable error from a tool call. That is a property about process lifetime, so it can only be observed by running the thing. diff --git a/commands/forget.md b/commands/forget.md index 19cbf38..43a40b0 100644 --- a/commands/forget.md +++ b/commands/forget.md @@ -5,7 +5,9 @@ argument-hint: [memory key or description] Forget this memory: $ARGUMENTS -This is permanent — there is no undo and no grace window. +This is permanent — the memory and its history are deleted, with no undo and no +grace window. If the user wants it marked wrong rather than gone, offer +`memory_retract` instead: it stops the memory recalling and keeps a record of why. If the argument is already a key (`type/name`), confirm what it is with `memory_get` and show the user before deleting. If it is a description, use diff --git a/commands/memories.md b/commands/memories.md index bdaf420..6baccb4 100644 --- a/commands/memories.md +++ b/commands/memories.md @@ -1,5 +1,6 @@ --- description: Show everything in your VTA-backed memory +argument-hint: [--stale] --- Show the user what is in their VTA-backed memory. @@ -12,5 +13,14 @@ bodies. set there are more: say how many in all, and fetch further pages with `offset` only if the user wants to see them. +Show each memory's `status` when it is not `active`, and mark ones that are +`stale` or `expired`. + +If the arguments include `--stale`, call `memory_list` with `stale: true` +instead, and for each memory it returns — due for review, or past its +`validUntil` — ask the user whether it still holds. Then, per their answer: save +it again (optionally with a new `reviewAfter`), `memory_deprecate` it, or +`memory_retract` it. Do not change any of them without asking. + If the list is empty, say which context you looked in, since an empty result usually means the wrong context rather than no memories. diff --git a/skills/agent-memory/SKILL.md b/skills/agent-memory/SKILL.md index 0302f08..b995e05 100644 --- a/skills/agent-memory/SKILL.md +++ b/skills/agent-memory/SKILL.md @@ -6,9 +6,10 @@ description: How to use the user's VTA-backed memory — what is worth saving, w # Agent memory (VTA-backed) The user's durable memory lives in their own Verifiable Trust Agent, not in this -tool and not in a file in the repo. You reach it through six MCP tools: -`memory_recall`, `memory_get`, `memory_save`, `memory_forget`, `memory_list`, -`memory_context`. +tool and not in a file in the repo. You reach it through nine MCP tools: +`memory_recall`, `memory_get`, `memory_save`, `memory_list`, `memory_context`, +the status changes `memory_dispute`, `memory_deprecate` and `memory_retract`, and +the hard delete `memory_forget`. The whole store is scoped to one **VTA trust context**. That context is the isolation boundary — an agent scoped to a different one cannot read, write, or @@ -179,16 +180,46 @@ Recall first. If a memory already covers it, update that one — `memory_save` with the same name replaces it — rather than creating a near-duplicate. Two memories that disagree are worse than neither. +When you update a memory you read, pass its `version` as `expectedVersion`. If +another device or agent changed it since, the save is **not** written and you get +`saved: false` with what is there now (`current`). Do not blindly save again: +read it, merge if your change still makes sense, and save with the new +`currentVersion` — or tell the user it changed under you. Nothing a save +replaces is lost; the previous version goes to the memory's history +(`memory_get` with `history: true`). + +If a fact has a shelf life — "on leave until the 1st", "using the staging +cluster this sprint" — give it a `validUntil` date and recall will stop returning +it after that. If it is a preference that may drift, give it a `reviewAfter` +date; recall will then flag it `stale`, and you should confirm it with the user +before relying on it. + Write the description as the answer to: *would I want this loaded at the start of a session?* If the honest answer is no, do not save it. +## When a memory is wrong or out of date + +Prefer marking it to deleting it. Each takes a one-line `reason`, which is +shown with the memory from then on: + +| Tool | When | Effect | +|---|---|---| +| `memory_dispute` | Somebody says it is wrong, but it is not settled. | Still recalls, ranked lower, shown as `disputed`. | +| `memory_deprecate` | It was true, it is not any more, or another memory replaced it. | Still recalls, ranked lower, shown as `deprecated`. | +| `memory_retract` | It is wrong, or should never have been saved. | Never recalls again. A tombstone says it existed and why it was withdrawn. | + +Saving the memory again settles a dispute: the new version is active. A status +change is a new version signed by you, so the memory's `author` becomes you; the +original author's version is in its history. + ## Forgetting -`memory_forget` is permanent. There is no undo and no grace window. +`memory_forget` is permanent: the memory *and its history* are deleted. There is +no undo and no grace window. -Only forget when the user asks, or when you have just saved a memory that -supersedes the old one. If a memory turns out to be wrong, forgetting it is -right — a wrong memory is worse than a missing one. +Only forget when the user asks for something to be gone — not merely marked +wrong. If a memory turns out to be wrong, `memory_retract` it; that keeps the +fact that it was once believed, and why it was withdrawn. If the user asks you to forget *everything*, confirm the scope first, list what is there with `memory_list`, and only then delete. `memory_list` returns one page diff --git a/src/lazy.rs b/src/lazy.rs index e7df30a..a935d61 100644 --- a/src/lazy.rs +++ b/src/lazy.rs @@ -2,7 +2,7 @@ //! //! Claude Code starts an MCP server when a session starts and takes what it //! gets. A server that exits before speaking MCP does not appear as a broken -//! memory service — it does not appear at all: the six tools are simply absent, +//! memory service — it does not appear at all: the memory tools are simply absent, //! and the model has no way to tell anyone why, because the thing it would use //! to say so is the thing that is missing. //! diff --git a/src/lib.rs b/src/lib.rs index f700fbc..aa11be6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,6 +15,7 @@ pub mod config; pub mod enrol; pub mod fence; pub mod lazy; +pub mod lifecycle; pub mod pnm; pub mod provenance; pub mod record; diff --git a/src/lifecycle.rs b/src/lifecycle.rs new file mode 100644 index 0000000..fafa8ca --- /dev/null +++ b/src/lifecycle.rs @@ -0,0 +1,319 @@ +//! A memory's standing over time (#33): status, expiry, review, and what it +//! replaced. +//! +//! Before this, a memory had two states — there, or hard-deleted — and a save +//! silently overwrote whatever it replaced. So a preference from last year had +//! the same standing as one from today, a wrong memory could only be erased +//! (not marked wrong), and two writers saving at once lost one of the saves +//! without anybody knowing. Data Rooms already solved the same problems for +//! shared records (`status`, `expectedVersion`, tombstones); this is the +//! personal-memory counterpart. +//! +//! - **`status`** — `active` (the default), `deprecated` (superseded or out of +//! date, kept for reference), `disputed` (somebody says it is wrong), or +//! `retracted` (withdrawn — a tombstone). Recall leaves retracted memories +//! out and ranks deprecated and disputed ones lower, saying why. +//! - **`validUntil`** — after it, the memory has expired and recall leaves it +//! out. For facts with a shelf life: "on leave until 2026-12-01". +//! - **`reviewAfter`** — after it, the memory still recalls, but is flagged +//! `stale` so somebody confirms it. For preferences that drift. +//! - **`supersedes`** — every save that replaces a memory first copies the +//! version it replaces, verbatim and still signed by its own author, to a +//! history key, and links to it. Nothing a save replaces is lost. +//! +//! All of it is inside the record's signature. Changing a status is a new +//! version, signed by whoever changed it — so a disputed memory's `author` is +//! the disputer, and the original author's signed text is one `supersedes` +//! hop away in the history. +//! +//! # History keys +//! +//! `archive///v` — deliberately *not* `/…`, so a +//! typed recall's prefix never reaches history, and an untyped list skips it as +//! a key shape it does not own. At most [`MAX_HISTORY`] versions are kept per +//! memory; `memory_forget` removes the history with the memory, because +//! "forget" has to mean forget. + +use chrono::{DateTime, NaiveDate, Utc}; +use serde_json::Value; + +use crate::record::{MemoryKey, MemoryRecord}; + +/// Versions of one memory kept in its history, newest first. +pub const MAX_HISTORY: usize = 5; +/// Longest status reason, in characters. +pub const MAX_REASON_CHARS: usize = 300; + +/// Where a memory stands. +#[derive( + Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize, schemars::JsonSchema, +)] +#[serde(rename_all = "lowercase")] +pub enum Status { + /// Current. What a memory is unless it says otherwise. + Active, + /// Out of date or replaced, kept for reference. + Deprecated, + /// Somebody says it is wrong; the reason says why. + Disputed, + /// Withdrawn. Its content is gone; that it existed is not. + Retracted, +} + +impl Status { + /// The wire form. + pub fn as_str(&self) -> &'static str { + match self { + Status::Active => "active", + Status::Deprecated => "deprecated", + Status::Disputed => "disputed", + Status::Retracted => "retracted", + } + } + + /// Parse the wire form. An absent status is `active`; one this version + /// does not know is `None`, and is treated as disputed — standing nobody + /// has vouched for. + pub fn parse(s: Option<&str>) -> Option { + match s.map(|s| s.trim().to_ascii_lowercase()).as_deref() { + None | Some("active") => Some(Status::Active), + Some("deprecated") => Some(Status::Deprecated), + Some("disputed") => Some(Status::Disputed), + Some("retracted") => Some(Status::Retracted), + Some(_) => None, + } + } +} + +/// How much a memory's status counts in ranking, out of [`FULL_WEIGHT`]. +pub fn weight(status: Option) -> u32 { + match status { + Some(Status::Active) => FULL_WEIGHT, + Some(Status::Deprecated) | Some(Status::Disputed) | None => 5, + Some(Status::Retracted) => 0, + } +} + +/// The weight of an active memory. +pub const FULL_WEIGHT: u32 = 10; + +/// The history key for `key` at `version`. +pub fn archive_key(key: &MemoryKey, version: u64) -> String { + format!("{}v{version}", archive_prefix(key)) +} + +/// The prefix every history key of `key` starts with. +pub fn archive_prefix(key: &MemoryKey) -> String { + format!("archive/{key}/") +} + +/// The version a history key names, if it is one of `key`'s. +pub fn archived_version(key: &MemoryKey, raw: &str) -> Option { + raw.strip_prefix(&archive_prefix(key))? + .strip_prefix('v')? + .parse() + .ok() +} + +/// Parse a date a caller gave: RFC 3339, or a bare `YYYY-MM-DD` meaning the +/// start of that day in UTC. Returned in RFC 3339, so stored dates compare. +pub fn normalise_date(field: &str, raw: &str) -> Result { + let raw = raw.trim(); + if let Ok(t) = DateTime::parse_from_rfc3339(raw) { + return Ok(t.with_timezone(&Utc).to_rfc3339()); + } + if let Ok(d) = NaiveDate::parse_from_str(raw, "%Y-%m-%d") { + let t = d.and_hms_opt(0, 0, 0).expect("midnight exists").and_utc(); + return Ok(t.to_rfc3339()); + } + Err(format!( + "`{field}` is `{raw}`; expected a date (`2026-12-01`) or an RFC 3339 time" + )) +} + +/// `when` has passed at `now`. An unparseable date has not: a malformed +/// expiry must not make a memory vanish. +fn passed(when: Option<&str>, now: DateTime) -> bool { + when.and_then(|w| DateTime::parse_from_rfc3339(w).ok()) + .is_some_and(|t| t <= now) +} + +/// What recall needs to know about a record's standing right now. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Standing { + /// `None` for a status this version does not know. + pub status: Option, + /// Past `validUntil`. + pub expired: bool, + /// Past `reviewAfter`. + pub stale: bool, +} + +impl Standing { + /// The standing of `record` at `now`. + pub fn of(record: &MemoryRecord, now: DateTime) -> Self { + Self { + status: Status::parse(record.status.as_deref()), + expired: passed(record.valid_until.as_deref(), now), + stale: passed(record.review_after.as_deref(), now), + } + } + + /// Whether recall returns it at all. + pub fn recallable(&self) -> bool { + !self.expired && self.status != Some(Status::Retracted) + } + + /// Ranking weight, out of [`FULL_WEIGHT`]. + pub fn weight(&self) -> u32 { + weight(self.status) + } +} + +/// The lifecycle fields recall, get and list return. +pub fn to_json(record: &MemoryRecord, standing: Standing) -> Value { + let mut v = serde_json::json!({ + "status": standing.status.map(|s| s.as_str()).unwrap_or("unknown"), + }); + if let Some(r) = &record.status_reason { + v["statusReason"] = Value::String(crate::fence::Fence::sanitize(r)); + } + if let Some(d) = &record.valid_until { + v["validUntil"] = Value::String(d.clone()); + } + if let Some(d) = &record.review_after { + v["reviewAfter"] = Value::String(d.clone()); + } + if standing.expired { + v["expired"] = Value::Bool(true); + } + if standing.stale { + v["stale"] = Value::Bool(true); + } + if let Some(s) = &record.supersedes { + v["supersedes"] = s.clone(); + } + v +} + +/// A short note for a rendered listing, or `None` for an active, current memory. +pub fn note(record: &MemoryRecord, standing: Standing) -> Option { + let mut parts = Vec::new(); + match standing.status { + Some(Status::Active) => {} + Some(s) => parts.push(match &record.status_reason { + Some(r) => format!("{}: {r}", s.as_str()), + None => s.as_str().to_string(), + }), + None => parts.push("status unknown".to_string()), + } + if standing.expired { + parts.push("expired".to_string()); + } + if standing.stale { + parts.push("due for review".to_string()); + } + (!parts.is_empty()).then(|| parts.join(", ")) +} + +/// Check a status reason a caller gave. +pub fn check_reason(reason: &str) -> Result<(), String> { + if reason.trim().is_empty() { + return Err("a reason is required".to_string()); + } + let n = reason.chars().count(); + if n > MAX_REASON_CHARS { + return Err(format!( + "the reason is {n} characters; the limit is {MAX_REASON_CHARS} — it should be one line" + )); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::record::MemoryType; + + fn at(s: &str) -> DateTime { + DateTime::parse_from_rfc3339(s).unwrap().with_timezone(&Utc) + } + + fn record() -> MemoryRecord { + MemoryRecord::new(MemoryType::User, "n", "d", "b", vec![]) + } + + #[test] + fn dates_accept_a_day_or_a_time_and_refuse_anything_else() { + assert_eq!( + normalise_date("validUntil", "2026-12-01").unwrap(), + "2026-12-01T00:00:00+00:00" + ); + assert_eq!( + normalise_date("validUntil", "2026-12-01T10:00:00+02:00").unwrap(), + "2026-12-01T08:00:00+00:00" + ); + let err = normalise_date("reviewAfter", "next tuesday").unwrap_err(); + assert!(err.contains("reviewAfter"), "{err}"); + } + + #[test] + fn expiry_and_review_are_judged_against_now() { + let mut r = record(); + r.valid_until = Some("2026-12-01T00:00:00+00:00".into()); + r.review_after = Some("2026-06-01T00:00:00+00:00".into()); + let before = Standing::of(&r, at("2026-05-01T00:00:00Z")); + assert!(!before.expired && !before.stale && before.recallable()); + let between = Standing::of(&r, at("2026-07-01T00:00:00Z")); + assert!(!between.expired && between.stale && between.recallable()); + let after = Standing::of(&r, at("2027-01-01T00:00:00Z")); + assert!(after.expired && !after.recallable()); + } + + #[test] + fn a_malformed_expiry_does_not_make_a_memory_vanish() { + let mut r = record(); + r.valid_until = Some("whenever".into()); + assert!(Standing::of(&r, Utc::now()).recallable()); + } + + #[test] + fn retracted_is_not_recalled_and_an_unknown_status_is_demoted() { + let mut r = record(); + r.status = Some("retracted".into()); + assert!(!Standing::of(&r, Utc::now()).recallable()); + r.status = Some("quarantined".into()); + let s = Standing::of(&r, Utc::now()); + assert!(s.recallable()); + assert!(s.weight() < FULL_WEIGHT); + assert_eq!(to_json(&r, s)["status"], "unknown"); + } + + #[test] + fn notes_say_what_is_unusual_and_nothing_otherwise() { + let mut r = record(); + let now = at("2027-01-01T00:00:00Z"); + assert_eq!(note(&r, Standing::of(&r, now)), None); + r.status = Some("disputed".into()); + r.status_reason = Some("user says spaces".into()); + r.review_after = Some("2026-06-01T00:00:00+00:00".into()); + assert_eq!( + note(&r, Standing::of(&r, now)).as_deref(), + Some("disputed: user says spaces, due for review") + ); + } + + #[test] + fn history_keys_are_outside_every_type_prefix() { + let key = MemoryKey::parse("user/name").unwrap(); + let archived = archive_key(&key, 7); + assert_eq!(archived, "archive/user/name/v7"); + assert!(MemoryKey::parse(&archived).is_err(), "list must skip it"); + for kind in crate::record::MemoryType::ALL { + assert!(!MemoryKey::raw_key_has_type(&archived, kind)); + } + assert_eq!(archived_version(&key, &archived), Some(7)); + let other = MemoryKey::parse("user/name-two").unwrap(); + assert_eq!(archived_version(&other, &archived), None); + } +} diff --git a/src/main.rs b/src/main.rs index 9fb751b..bea3c4e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -541,10 +541,7 @@ fn render_memories( "\n- **{}** (`{}`) — {}", e.record.name, e.key, e.record.description ); - let notes: Vec = [e.record.basis_note(), e.authorship.note()] - .into_iter() - .flatten() - .collect(); + let notes = e.notes(); if !notes.is_empty() { item.push_str(&format!(" _[{}]_", notes.join("; "))); } @@ -659,15 +656,14 @@ mod tests { use record::{Entry, MemoryRecord}; fn entry(kind: MemoryType, name: &str, desc: &str, body: &str) -> Entry { - Entry { - key: MemoryKey::new(kind, name).unwrap(), - record: MemoryRecord::new(kind, name, desc, body, vec![]).with_provenance( + Entry::local( + MemoryKey::new(kind, name).unwrap(), + MemoryRecord::new(kind, name, desc, body, vec![]).with_provenance( vta_agent_memory::provenance::Basis::Declared, None, vec![], ), - authorship: vta_agent_memory::authorship::Authorship::Unattributed, - } + ) } #[test] diff --git a/src/record.rs b/src/record.rs index 45174c1..933c34a 100644 --- a/src/record.rs +++ b/src/record.rs @@ -31,6 +31,7 @@ use crate::authorship::Authorship; use crate::fence::Fence; +use crate::lifecycle::Standing; use crate::provenance::{self, Basis, Evidence}; use serde::{Deserialize, Serialize}; use std::fmt; @@ -267,6 +268,24 @@ pub struct MemoryRecord { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub evidence: Vec, + // ---- lifecycle (#33): the record's standing. See `crate::lifecycle`. + /// `active` (absent means active), `deprecated`, `disputed`, `retracted`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub status: Option, + /// Why the status is what it is — required for anything but active. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub status_reason: Option, + /// RFC 3339. After it the memory has expired and recall leaves it out. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub valid_until: Option, + /// RFC 3339. After it the memory is flagged for review. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub review_after: Option, + /// `{ "key": , "version": n }` — the version this one + /// replaced, kept verbatim under its history key. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub supersedes: Option, + // ---- v2: authorship. Set by `authorship::Signer::sign`, never by a caller, // and only believed once `authorship::verify` has checked them against the // proof and the place the record was found. @@ -308,6 +327,11 @@ impl MemoryRecord { basis: None, confidence: None, evidence: Vec::new(), + status: None, + status_reason: None, + valid_until: None, + review_after: None, + supersedes: None, author: None, author_kind: None, context_id: None, @@ -363,6 +387,11 @@ impl MemoryRecord { basis: None, confidence: None, evidence: Vec::new(), + status: None, + status_reason: None, + valid_until: None, + review_after: None, + supersedes: None, author: None, author_kind: None, context_id: None, @@ -439,6 +468,29 @@ impl MemoryRecord { .map_err(|e| format!("malformed `evidence`: {e}"))?; provenance::check(basis, self.confidence, &evidence)?; } + // Lifecycle: a status other than active must say why, and dates must + // be dates (stored normalised, so they compare). + let status = crate::lifecycle::Status::parse(self.status.as_deref()).ok_or_else(|| { + format!( + "unknown `status` `{}`", + self.status.as_deref().unwrap_or("") + ) + })?; + match (&self.status_reason, status) { + (Some(r), _) => crate::lifecycle::check_reason(r)?, + (None, crate::lifecycle::Status::Active) => {} + (None, s) => return Err(format!("a `{}` memory needs a reason", s.as_str())), + } + for (field, value) in [ + ("validUntil", &self.valid_until), + ("reviewAfter", &self.review_after), + ] { + if let Some(v) = value + && crate::lifecycle::normalise_date(field, v)? != *v + { + return Err(format!("`{field}` is not normalised RFC 3339: `{v}`")); + } + } Ok(()) } @@ -512,22 +564,68 @@ pub struct Entry { pub key: MemoryKey, pub record: MemoryRecord, pub authorship: Authorship, + /// The version the VTA holds it at — what an update passes back as + /// `expectedVersion`. 0 for an entry that did not come from a VTA. + pub version: u64, + /// Its status, expiry and review state, judged when it was read. + pub standing: Standing, } impl Entry { - /// [`MemoryRecord::summary`] plus who wrote it — what recall returns. + /// An entry that did not come from a VTA: unattributed, version 0, judged + /// now. For tests and local ranking. + pub fn local(key: MemoryKey, record: MemoryRecord) -> Self { + let standing = Standing::of(&record, chrono::Utc::now()); + Self { + key, + record, + authorship: Authorship::Unattributed, + version: 0, + standing, + } + } + + /// [`MemoryRecord::summary`] plus who wrote it, its version and its + /// standing — what recall returns. pub fn summary(&self) -> serde_json::Value { let mut v = self.record.summary(&self.key); - v["author"] = self.authorship.to_json(); + self.annotate(&mut v); v } - /// [`MemoryRecord::full`] plus who wrote it — what get returns. + /// [`MemoryRecord::full`] plus the same — what get returns. pub fn full(&self) -> serde_json::Value { let mut v = self.record.full(&self.key); - v["author"] = self.authorship.to_json(); + self.annotate(&mut v); v } + + fn annotate(&self, v: &mut serde_json::Value) { + v["author"] = self.authorship.to_json(); + v["version"] = serde_json::json!(self.version); + let life = crate::lifecycle::to_json(&self.record, self.standing); + if let (Some(v), Some(l)) = (v.as_object_mut(), life.as_object()) { + v.extend(l.clone()); + } + } + + /// Notes for a rendered listing: status, basis, authorship. + pub fn notes(&self) -> Vec { + [ + crate::lifecycle::note(&self.record, self.standing), + self.record.basis_note(), + self.authorship.note(), + ] + .into_iter() + .flatten() + .collect() + } + + /// How much this entry counts in ranking beyond its relevance: basis + /// weight times standing weight. + pub fn weight(&self) -> u32 { + self.record.basis_weight() * self.standing.weight() + } } /// A ranked recall hit. @@ -550,7 +648,10 @@ pub struct Hit { /// ([`provenance::weight`]): a stated preference outranks an inference that /// matches the query equally well, but a strong match on an inference still /// beats a passing mention of a stated fact. `confidence` is never ranked on — -/// two authors' estimates are not on one scale. +/// two authors' estimates are not on one scale. It is multiplied again by the +/// entry's standing ([`crate::lifecycle::weight`]): a deprecated or disputed +/// memory counts half. (Retracted and expired memories never reach ranking — +/// `Store::recall` leaves them out.) /// /// An empty query scores everything 0, which makes recall degrade into list — /// the right behaviour for a session-start hook that just wants what is there. @@ -589,7 +690,7 @@ pub fn rank(entries: Vec, query: &str) -> Vec { s }) .sum(); - relevance * entry.record.basis_weight() + relevance * entry.weight() }; Hit { entry, score } }) @@ -598,12 +699,7 @@ pub fn rank(entries: Vec, query: &str) -> Vec { hits.sort_by(|a, b| { b.score .cmp(&a.score) - .then_with(|| { - b.entry - .record - .basis_weight() - .cmp(&a.entry.record.basis_weight()) - }) + .then_with(|| b.entry.weight().cmp(&a.entry.weight())) .then_with(|| b.entry.record.updated_at.cmp(&a.entry.record.updated_at)) .then_with(|| a.entry.key.to_string().cmp(&b.entry.key.to_string())) }); @@ -615,11 +711,10 @@ mod tests { use super::*; fn entry(kind: MemoryType, name: &str, desc: &str, body: &str) -> Entry { - Entry { - key: MemoryKey::new(kind, name).unwrap(), - record: MemoryRecord::new(kind, name, desc, body, vec![]), - authorship: Authorship::Unattributed, - } + Entry::local( + MemoryKey::new(kind, name).unwrap(), + MemoryRecord::new(kind, name, desc, body, vec![]), + ) } #[test] diff --git a/src/server.rs b/src/server.rs index c6063be..d244c0d 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1,4 +1,4 @@ -//! The MCP surface: five memory tools over stdio. +//! The MCP surface: nine memory tools over stdio. //! //! Narrow on purpose. `vta-mcp` already exposes the VTA's whole management //! console — including `vta_call`, which can reach `vta/memory/*` today. This @@ -22,8 +22,10 @@ use serde::Deserialize; use crate::fence::{Fence, Provenance}; use crate::lazy::LazyStore; +use crate::lifecycle::{Status, normalise_date}; use crate::provenance::{Basis, Evidence}; use crate::record::{MemoryKey, MemoryRecord, MemoryType}; +use crate::store::SaveOutcome; use crate::store::Store; /// `memory_list` page size when the caller does not ask for one. @@ -69,6 +71,43 @@ fn fenced_json(value: impl serde::Serialize) -> Result ])) } +/// A save's outcome as tool output. A conflict is not an error: it carries +/// what is stored now, which the caller needs to merge — fenced, since it is +/// stored text somebody else wrote. +fn outcome_json(store: &Store, outcome: SaveOutcome) -> Result { + match outcome { + SaveOutcome::Saved { + key, + version, + superseded, + } => ok_json(serde_json::json!({ + "key": key.to_string(), + "contextId": store.context_id(), + "saved": true, + "version": version, + "supersededVersion": superseded, + })), + SaveOutcome::Conflict { key, conflict } => { + let current = match &conflict.current { + Some(Ok(entry)) => entry.full(), + Some(Err(refused)) => serde_json::json!({ "refused": refused.to_json() }), + None => serde_json::Value::Null, + }; + fenced_json(serde_json::json!({ + "key": key.to_string(), + "contextId": store.context_id(), + "saved": false, + "conflict": conflict.reason, + "currentVersion": conflict.current_version, + "current": current, + "next": "Nothing was overwritten. Read `current`, merge if it still makes sense, \ + and save again with `expectedVersion` set to `currentVersion` — or tell \ + the user the memory changed under you.", + })) + } + } +} + /// Surface an anyhow chain to the model with its context intact — the VTA's /// refusals (`permissionDenied`, `not found`) are the useful part. fn to_mcp(e: anyhow::Error) -> McpError { @@ -124,6 +163,31 @@ pub struct SaveParams { /// by key, or a data-room record. At most 8. #[serde(default)] pub evidence: Vec, + /// When updating a memory you read, the `version` recall or get returned + /// for it — the save is refused, and you are shown what is there instead, + /// if anybody changed it since. `0` means "this must be new". Omit to + /// replace whatever is there now. + #[serde(default, rename = "expectedVersion")] + pub expected_version: Option, + /// Optional date (`2026-12-01`) after which this is no longer true, and + /// recall stops returning it. For facts with a shelf life. + #[serde(default, rename = "validUntil")] + pub valid_until: Option, + /// Optional date after which this should be confirmed with the user; + /// recall flags it `stale`. For preferences that drift. + #[serde(default, rename = "reviewAfter")] + pub review_after: Option, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct StatusParams { + /// The memory's key, as returned by `memory_recall`. + pub key: String, + /// Why, in one line. Shown with the memory from now on. + pub reason: String, + /// The `version` you read, so a change made since is not overridden. + #[serde(default, rename = "expectedVersion")] + pub expected_version: Option, } #[derive(Debug, Deserialize, JsonSchema)] @@ -146,6 +210,10 @@ pub struct GetParams { /// The memory's key, as returned by `memory_recall` (e.g. /// `feedback/no-pr-attribution`). pub key: String, + /// Also return the earlier versions this one superseded (at most 5), + /// newest first. + #[serde(default)] + pub history: bool, } #[derive(Debug, Deserialize, JsonSchema)] @@ -167,6 +235,10 @@ pub struct ListParams { /// `nextOffset` to get the next page. #[serde(default)] pub offset: Option, + /// Only memories due for review or expired — for confirming or clearing + /// out what may no longer be true. + #[serde(default)] + pub stale: bool, } /// The MCP server. @@ -190,6 +262,23 @@ impl MemoryMcp { self.lazy.store().await.map_err(to_mcp) } + /// Change a memory's status and report the outcome. + async fn set_status( + &self, + p: StatusParams, + status: Status, + ) -> Result { + let key = + MemoryKey::parse(&p.key).map_err(|e| McpError::invalid_params(e.to_string(), None))?; + crate::lifecycle::check_reason(&p.reason).map_err(|e| McpError::invalid_params(e, None))?; + let store = self.store().await?; + let outcome = store + .set_status(&key, status, &p.reason, p.expected_version) + .await + .map_err(to_mcp)?; + outcome_json(store, outcome) + } + /// Default `limit` for recall. Falls back rather than failing: a caller who /// asked for a specific limit should not be refused because the config is /// unreadable — the store call underneath will report that properly. @@ -234,19 +323,79 @@ impl MemoryMcp { None, ) })?; - let record = MemoryRecord::new(kind, &p.name, &p.description, &p.body, p.links) + let mut record = MemoryRecord::new(kind, &p.name, &p.description, &p.body, p.links) .with_provenance(basis, p.confidence, p.evidence); + let date = |field: &str, raw: Option| { + raw.map(|d| normalise_date(field, &d)) + .transpose() + .map_err(|e| McpError::invalid_params(e, None)) + }; + record.valid_until = date("validUntil", p.valid_until)?; + record.review_after = date("reviewAfter", p.review_after)?; // Before connecting: an oversized save is the caller's mistake, and // saying so must not depend on the VTA being reachable. record .check_limits() .map_err(|e| McpError::invalid_params(e, None))?; - let key = self.store().await?.save(&record).await.map_err(to_mcp)?; - ok_json(serde_json::json!({ - "key": key.to_string(), - "contextId": self.store().await?.context_id(), - "saved": true, - })) + let store = self.store().await?; + let outcome = store + .save_versioned(&record, p.expected_version) + .await + .map_err(to_mcp)?; + outcome_json(store, outcome) + } + + #[tool( + description = "Withdraw a memory that is wrong or should no longer be used, keeping a \ + record that it existed and why it was withdrawn. Its content no longer \ + recalls (the previous version stays in its history). Prefer this to \ + `memory_forget` when a memory turns out to be wrong; forget is for when the \ + user wants it gone entirely.", + annotations( + read_only_hint = false, + destructive_hint = false, + open_world_hint = false + ) + )] + async fn memory_retract( + &self, + Parameters(p): Parameters, + ) -> Result { + self.set_status(p, Status::Retracted).await + } + + #[tool( + description = "Mark a memory as disputed — somebody (the user, or evidence you found) says \ + it is wrong, but it is not settled. It still recalls, ranked lower and \ + shown as disputed with your reason. Save it again to settle it.", + annotations( + read_only_hint = false, + destructive_hint = false, + open_world_hint = false + ) + )] + async fn memory_dispute( + &self, + Parameters(p): Parameters, + ) -> Result { + self.set_status(p, Status::Disputed).await + } + + #[tool( + description = "Mark a memory as deprecated — out of date or replaced by another, but worth \ + keeping for reference. It still recalls, ranked lower and shown as \ + deprecated with your reason.", + annotations( + read_only_hint = false, + destructive_hint = false, + open_world_hint = false + ) + )] + async fn memory_deprecate( + &self, + Parameters(p): Parameters, + ) -> Result { + self.set_status(p, Status::Deprecated).await } #[tool( @@ -259,7 +408,11 @@ impl MemoryMcp { or `unattributed` (written before records were signed — nothing vouches \ for who wrote it). `refused` lists stored records that failed their \ signature check — altered, or copied from elsewhere; they are not \ - returned as memory, and the user should be told about them.", + returned as memory, and the user should be told about them. Retracted and \ + expired memories are left out; `status` says when one is `deprecated` or \ + `disputed` (ranked lower, with `statusReason`), and `stale: true` means it \ + is past its review date — confirm it before relying on it. Pass a \ + result's `version` as `expectedVersion` when you update it.", annotations(read_only_hint = true, open_world_hint = false) )] async fn memory_recall( @@ -287,7 +440,8 @@ impl MemoryMcp { #[tool( description = "Read one memory in full, by the key `memory_recall` returned. Use after \ - recall when a summary is not enough.", + recall when a summary is not enough. `history: true` adds the earlier \ + versions it superseded, each with its own author.", annotations(read_only_hint = true, open_world_hint = false) )] async fn memory_get( @@ -296,8 +450,17 @@ impl MemoryMcp { ) -> Result { let key = MemoryKey::parse(&p.key).map_err(|e| McpError::invalid_params(e.to_string(), None))?; - match self.store().await?.get(&key).await.map_err(to_mcp)? { - Some(entry) => fenced_json(entry.full()), + let store = self.store().await?; + match store.get(&key).await.map_err(to_mcp)? { + Some(entry) => { + let mut v = entry.full(); + if p.history { + let history = store.history(&key).await.map_err(to_mcp)?; + v["history"] = + serde_json::Value::Array(history.iter().map(|e| e.full()).collect()); + } + fenced_json(v) + } None => Err(McpError::invalid_params( format!( "no memory `{key}` in context `{}`", @@ -338,7 +501,9 @@ impl MemoryMcp { `total` for how many exist and `nextOffset` for the next page. Use when the \ user asks what you remember; use `memory_recall` when you are looking for \ something specific. Summaries carry `author` as in `memory_recall`, and \ - `refused` lists records that failed their signature check.", + `refused` lists records that failed their signature check. Unlike recall, \ + it includes retracted and expired memories, with their `status`; \ + `stale: true` lists only those due for review or expired.", annotations(read_only_hint = true, open_world_hint = false) )] async fn memory_list( @@ -354,6 +519,9 @@ impl MemoryMcp { let store = self.store().await?; let listing = store.list_of_type(kind).await.map_err(to_mcp)?; let mut entries = listing.entries; + if p.stale { + entries.retain(|e| e.standing.stale || e.standing.expired); + } entries.sort_by_key(|e| e.key.to_string()); let total = entries.len(); let items: Vec<_> = entries @@ -469,6 +637,9 @@ mod tests { "memory_forget", "memory_list", "memory_context", + "memory_retract", + "memory_dispute", + "memory_deprecate", ]; let have: Vec = router .list_all() @@ -502,6 +673,14 @@ mod tests { let save = annotations("memory_save"); assert_eq!(save.read_only_hint, Some(false)); + // Status changes keep everything (the old version goes to history), + // so they are writes but not destructive ones. + for name in ["memory_retract", "memory_dispute", "memory_deprecate"] { + let a = annotations(name); + assert_eq!(a.read_only_hint, Some(false), "{name}"); + assert_eq!(a.destructive_hint, Some(false), "{name}"); + } + for name in [ "memory_recall", "memory_get", @@ -647,6 +826,7 @@ mod tests { kind: None, limit, offset, + stale: false, }; unfence(&mcp.memory_list(Parameters(params)).await.unwrap()) } @@ -688,6 +868,7 @@ mod tests { let result = mcp .memory_get(Parameters(GetParams { key: "project/release-notes".to_string(), + history: false, })) .await .unwrap(); @@ -730,6 +911,9 @@ mod tests { basis: "declared".to_string(), confidence: None, evidence: vec![], + expected_version: None, + valid_until: None, + review_after: None, } } @@ -747,6 +931,20 @@ mod tests { ("unknown basis", with("hearsay", None)), ("must cite", with("verified", None)), ("between 0 and 1", with("inferred", Some(1.5))), + ( + "validUntil", + SaveParams { + valid_until: Some("next tuesday".into()), + ..with("declared", None) + }, + ), + ( + "reviewAfter", + SaveParams { + review_after: Some("soon".into()), + ..with("declared", None) + }, + ), ] { let err = mcp.memory_save(Parameters(params)).await.unwrap_err(); assert_eq!(err.code, ErrorCode::INVALID_PARAMS, "{what}: {err:?}"); diff --git a/src/store.rs b/src/store.rs index 99bf346..6b9014b 100644 --- a/src/store.rs +++ b/src/store.rs @@ -72,6 +72,7 @@ use vta_sdk::specs::vta::memory::{ }; use crate::authorship::{Refused, Signer, verify}; +use crate::lifecycle::{self, Standing, Status}; use crate::record::{Entry, Hit, MemoryKey, MemoryRecord, MemoryType, rank}; /// Items per `list/0.3` page. @@ -138,14 +139,19 @@ impl Store { } /// Check one stored value found at `key`. - async fn check(&self, key: MemoryKey, value: &str) -> Result { + async fn check(&self, key: MemoryKey, value: &str, version: u64) -> Result { let key_str = key.to_string(); match verify(value, &self.context_id, &key_str, self.own_did()).await { - Ok(authorship) => Ok(Entry { - record: MemoryRecord::decode(&key, value), - key, - authorship, - }), + Ok(authorship) => { + let record = MemoryRecord::decode(&key, value); + Ok(Entry { + standing: Standing::of(&record, chrono::Utc::now()), + record, + key, + authorship, + version, + }) + } Err(reason) => Err(Refused { key: key_str, reason, @@ -190,18 +196,43 @@ impl Store { self.client.shutdown().await; } - /// Upsert one memory. A second save of the same `(type, name)` replaces it. + /// Save one memory, replacing any earlier version under the same + /// `(type, name)`. Shorthand for [`save_versioned`](Self::save_versioned) + /// with no expected version, where a lost race is an error. + pub async fn save(&self, record: &MemoryRecord) -> anyhow::Result { + match self.save_versioned(record, None).await? { + SaveOutcome::Saved { key, .. } => Ok(key), + SaveOutcome::Conflict { key, conflict } => anyhow::bail!( + "memory `{key}` was changed by another writer while saving ({}); nothing was \ + overwritten", + conflict.reason + ), + } + } + + /// Save one memory without losing what it replaces or what somebody else + /// wrote meanwhile. /// - /// The stored value is `record` signed by this store's [`Signer`], naming it - /// as author and binding it to this context and key. Without a signer the - /// save is refused before anything is sent. + /// 1. The current version, if any, is read. + /// 2. If the caller says which version it last read (`expected`; 0 for "I + /// expect none") and that is not what is there now, nothing is written: + /// the outcome is a [`Conflict`] carrying what is there. + /// 3. The current version is copied, verbatim and still signed by its own + /// author, to its history key, and the new record names it in + /// `supersedes`. + /// 4. The new record is signed and written **conditionally** on the version + /// read in step 1 — so a writer that got in between steps 1 and 4 wins, + /// and this save comes back as a [`Conflict`] naming that winner rather + /// than silently overwriting it. + /// 5. History beyond [`lifecycle::MAX_HISTORY`] versions is pruned. /// - /// Unconditional (`put/0.3` without `expectedVersion`): a save is the user - /// asking for this to be remembered now, whatever was there. Runs under - /// [`VtaClient::idempotent`], which retries transient faults with one stable - /// idempotency key, so a websocket blip mid-session neither loses the save - /// nor applies it twice. - pub async fn save(&self, record: &MemoryRecord) -> anyhow::Result { + /// Every write runs under [`VtaClient::idempotent`], so a transient fault + /// retries with one stable idempotency key and cannot apply twice. + pub async fn save_versioned( + &self, + record: &MemoryRecord, + expected: Option, + ) -> anyhow::Result { let key = MemoryKey::new(record.kind, &record.name)?; let key_str = key.to_string(); let signer = self.signer.as_ref().ok_or_else(|| { @@ -210,39 +241,253 @@ impl Store { are signed by their writer" ) })?; - let value = signer.sign(record, &self.context_id, &key).await?; + let current = self.get_raw(&key_str).await?; + let current_version = current.as_ref().map(|(_, v)| *v).unwrap_or(0); + if let Some(exp) = expected + && exp != current_version + { + let current = match current { + Some((value, version)) => Some(self.check(key.clone(), &value, version).await), + None => None, + }; + return Ok(SaveOutcome::Conflict { + key, + conflict: Box::new(Conflict { + reason: format!( + "expected version {exp}, but it is at {}", + if current_version == 0 { + "no version (it does not exist)".to_string() + } else { + current_version.to_string() + } + ), + current_version: (current_version > 0).then_some(current_version), + current, + }), + }); + } + + let mut record = record.clone(); + if let Some((value, version)) = ¤t { + let archived = lifecycle::archive_key(&key, *version); + // Create-only: a history version is immutable, and one already + // there (an earlier attempt of this save got this far) is the same + // bytes. + match self.put(&archived, value, Some(0)).await? { + Ok(_) => {} + Err(c) if c.reason == "recordExists" => {} + Err(c) => anyhow::bail!("archiving `{key}` v{version}: {}", c.reason), + } + record.supersedes = Some(json!({ "key": archived, "version": version })); + } + let value = signer.sign(&record, &self.context_id, &key).await?; + + match self.put(&key_str, &value, Some(current_version)).await? { + Ok(version) => { + if current.is_some() + && let Err(e) = self.prune_history(&key).await + { + // The save landed; old history outliving its bound is not + // worth failing it over. + tracing::warn!(key = %key, error = %format!("{e:#}"), "could not prune memory history"); + } + Ok(SaveOutcome::Saved { + key, + version, + superseded: current.map(|(_, v)| v), + }) + } + Err(raw) => { + let current = match (raw.current_value, raw.current_version) { + (Some(value), Some(version)) => { + Some(self.check(key.clone(), &value, version).await) + } + _ => None, + }; + Ok(SaveOutcome::Conflict { + key, + conflict: Box::new(Conflict { + reason: format!("another writer saved it first ({})", raw.reason), + current_version: raw.current_version, + current, + }), + }) + } + } + } + + /// Change a memory's status, as a new version signed by this store. + /// + /// `deprecated` and `disputed` keep the memory's content and say why. + /// `retracted` is a tombstone: the content is withdrawn — the new version + /// keeps the name and type and says why, and the content stays only in the + /// history the save writes — so it never recalls again. A refused record + /// (one whose signature does not hold) can be retracted but nothing else, + /// since anything else would re-sign content nobody vouches for. Like a + /// save, conditional on `expected` if given. + pub async fn set_status( + &self, + key: &MemoryKey, + status: Status, + reason: &str, + expected: Option, + ) -> anyhow::Result { + lifecycle::check_reason(reason).map_err(|e| anyhow::anyhow!(e))?; + let key_str = key.to_string(); + let Some((value, version)) = self.get_raw(&key_str).await? else { + anyhow::bail!("memory `{key}` not found in context `{}`", self.context_id); + }; + let checked = self.check(key.clone(), &value, version).await; + let mut record = match (status, checked) { + (Status::Retracted, current) => { + let name = current + .as_ref() + .map(|e| e.record.name.clone()) + .unwrap_or_else(|_| key.slug.clone()); + MemoryRecord::new(key.kind, name, format!("Retracted: {reason}"), "", vec![]) + } + (_, Ok(entry)) => { + let mut r = entry.record; + r.updated_at = chrono::Utc::now().to_rfc3339(); + r + } + (_, Err(refused)) => anyhow::bail!( + "memory `{key}` is refused ({}), so it can only be retracted or forgotten", + refused.reason + ), + }; + // The record's own name must address the same key, or the save would + // land elsewhere. A foreign value's name is its slug, which does. + if MemoryKey::new(record.kind, &record.name).ok().as_ref() != Some(key) { + record.name = key.slug.clone(); + } + record.status = (status != Status::Active).then(|| status.as_str().to_string()); + record.status_reason = Some(reason.to_string()); + self.save_versioned(&record, Some(expected.unwrap_or(version))) + .await + } + + /// Earlier versions of `key`, newest first, each checked against the key + /// it was written to. + pub async fn history(&self, key: &MemoryKey) -> anyhow::Result> { + let key_str = key.to_string(); + let mut out = Vec::new(); + for (raw, value, _) in self.raw_list(&lifecycle::archive_prefix(key)).await? { + let Some(version) = lifecycle::archived_version(key, &raw) else { + continue; + }; + // Verified against the key it was *written* to: a history copy is + // the original bytes, signature and all. + match verify(&value, &self.context_id, &key_str, self.own_did()).await { + Ok(authorship) => { + let record = MemoryRecord::decode(key, &value); + out.push(Entry { + standing: Standing::of(&record, chrono::Utc::now()), + key: key.clone(), + record, + authorship, + version, + }) + } + Err(reason) => { + tracing::warn!(key = %raw, reason = %reason, "refusing archived memory version") + } + } + } + out.sort_by_key(|e| std::cmp::Reverse(e.version)); + Ok(out) + } + + /// Delete history beyond [`lifecycle::MAX_HISTORY`] versions. + async fn prune_history(&self, key: &MemoryKey) -> anyhow::Result<()> { + let mut versions: Vec<(u64, String)> = self + .raw_list(&lifecycle::archive_prefix(key)) + .await? + .into_iter() + .filter_map(|(raw, _, _)| lifecycle::archived_version(key, &raw).map(|v| (v, raw))) + .collect(); + versions.sort_by_key(|v| std::cmp::Reverse(v.0)); + for (_, raw) in versions.into_iter().skip(lifecycle::MAX_HISTORY) { + self.delete(&raw).await?; + } + Ok(()) + } + + /// One `put/0.3`: `Ok(Ok(version))` written, `Ok(Err(conflict))` the + /// precondition failed, `Err` anything else. + async fn put( + &self, + key: &str, + value: &str, + expected: Option, + ) -> anyhow::Result> { let resp = self .client .idempotent(|| { self.client.memory_put_versioned_for( &self.context_id, - &key_str, - &value, - None, + key, + value, + expected, Some(&self.purpose), ) }) - .await - .map_err(|e| { - self.failed( + .await; + let resp = match resp { + Ok(r) => r, + Err(e) => { + if let Some(c) = RawConflict::from_error(&e) { + return Ok(Err(c)); + } + return Err(self.failed( e, format!("saving memory `{key}` in context `{}`", self.context_id), - ) - })?; - + )); + } + }; let decoded: put_spec::Response = serde_json::from_value(resp) .with_context(|| format!("decoding the response to saving `{key}`"))?; // The VTA echoes the key it stored. A mismatch means the value landed // somewhere this store cannot address, which is worse than a failure — // recall would never find it and the user would think it saved. - if decoded.key.as_str() != key_str { + if decoded.key.as_str() != key { anyhow::bail!( - "the VTA stored memory `{}` when `{key_str}` was requested", + "the VTA stored memory `{}` when `{key}` was requested", decoded.key.as_str() ); } - Ok(key) + Ok(Ok(decoded.version.0.get())) + } + + /// One unconditional `delete/0.3`, returning whether anything was there. + async fn delete(&self, key: &str) -> anyhow::Result { + let resp = self + .client + .idempotent(|| { + self.client.memory_delete_versioned_for( + &self.context_id, + key, + None, + Some(&self.purpose), + ) + }) + .await + .map_err(|e| { + self.failed( + e, + format!("forgetting memory `{key}` in context `{}`", self.context_id), + ) + })?; + let decoded: delete_spec::Response = serde_json::from_value(resp) + .with_context(|| format!("decoding the response to forgetting `{key}`"))?; + if decoded.key.as_str() != key { + anyhow::bail!( + "the VTA deleted memory `{}` when `{key}` was requested", + decoded.key.as_str() + ); + } + Ok(decoded.existed) } /// Every memory in the context, a page at a time. @@ -254,15 +499,40 @@ impl Store { /// asked only for keys under `/`. /// /// Entries whose keys this tool did not write are skipped rather than - /// erroring, so a context shared with another writer stays usable. + /// erroring, so a context shared with another writer stays usable. That + /// includes history keys (`archive/…`), which are reached through + /// [`history`](Self::history). pub async fn list_of_type(&self, kind: Option) -> anyhow::Result { + let prefix = kind.map(MemoryKey::prefix).unwrap_or_default(); let mut listing = Listing::default(); + for (raw, value, version) in self.raw_list(&prefix).await? { + // One level down from the envelope, tolerance is right: a key that + // is not `/` belongs to something else writing into the + // same context, and skipping it keeps `list` total. + let Ok(key) = MemoryKey::parse(&raw) else { + tracing::debug!(key = %raw, "skipping entry with an unrecognised key shape"); + continue; + }; + match self.check(key, &value, version).await { + Ok(entry) => listing.entries.push(entry), + Err(refused) => { + tracing::warn!(key = %refused.key, reason = %refused.reason, "refusing stored memory"); + listing.refused.push(refused); + } + } + } + Ok(listing) + } + + /// Every live `(key, value, version)` under `prefix`, a page at a time. + async fn raw_list(&self, prefix: &str) -> anyhow::Result> { + let mut out = Vec::new(); let mut cursor: Option = None; loop { let mut query = Map::new(); query.insert("limit".into(), json!(PAGE)); - if let Some(k) = kind { - query.insert("prefix".into(), json!(MemoryKey::prefix(k))); + if !prefix.is_empty() { + query.insert("prefix".into(), json!(prefix)); } if let Some(c) = &cursor { query.insert("cursor".into(), json!(c)); @@ -284,25 +554,17 @@ impl Store { self.context_id ) })?; - for (key, value) in page { - match self.check(key, &value).await { - Ok(entry) => listing.entries.push(entry), - Err(refused) => { - tracing::warn!(key = %refused.key, reason = %refused.reason, "refusing stored memory"); - listing.refused.push(refused); - } - } - } + out.extend(page); match next { Some(n) => cursor = Some(n), None => break, } } - Ok(listing) + Ok(out) } - /// Ranked recall: list, optionally narrow by type, rank against `query`, - /// take `limit`. + /// Ranked recall: list, optionally narrow by type, leave out what is + /// retracted or expired, rank against `query`, take `limit`. pub async fn recall( &self, query: &str, @@ -310,6 +572,10 @@ impl Store { limit: usize, ) -> anyhow::Result { let Listing { entries, refused } = self.list_of_type(kind).await?; + let entries = entries + .into_iter() + .filter(|e| e.standing.recallable()) + .collect(); let mut hits = rank(entries, query); // A non-empty query that matched nothing should say so rather than // hand back the `limit` least-irrelevant memories in the context — @@ -324,28 +590,13 @@ impl Store { /// One memory by key, or `None` if absent — one `vta/memory/get/0.2`. /// /// A record that fails its authorship check is an error naming why, not - /// the record: it is not returned as memory. + /// the record: it is not returned as memory. A retracted or expired memory + /// *is* returned — asked for by key, it says what it is. pub async fn get(&self, key: &MemoryKey) -> anyhow::Result> { - let key_str = key.to_string(); - let resp = self - .client - .memory_get_for(&self.context_id, &[key_str.as_str()], Some(&self.purpose)) - .await - .map_err(|e| { - self.failed( - e, - format!("reading memory `{key}` in context `{}`", self.context_id), - ) - })?; - let got: get_spec::Response = serde_json::from_value(resp) - .with_context(|| format!("decoding the response to reading `{key}`"))?; - let Some(value) = got.items.into_iter().find_map(|item| { - let value = item.value.filter(|_| !item.deleted)?; - (item.key.as_str() == key_str).then_some(value) - }) else { + let Some((value, version)) = self.get_raw(&key.to_string()).await? else { return Ok(None); }; - match self.check(key.clone(), value.as_str()).await { + match self.check(key.clone(), &value, version).await { Ok(entry) => Ok(Some(entry)), Err(refused) => anyhow::bail!( "memory `{key}` is stored but refused: {}. It is not returned as memory; \ @@ -355,44 +606,44 @@ impl Store { } } - /// Delete one memory. An absent key is an error, as it always has been. - /// - /// `delete/0.3` leaves a tombstone, so the user's other devices learn of the - /// deletion, and answers a key holding nothing with `existed: false` rather - /// than an error. This store keeps refusing that case: "forget X" for an X - /// that is not there is more likely a mistyped name than a success. Under - /// [`VtaClient::idempotent`], whose idempotency key stays the same across - /// attempts, so a retry of a delete that did land is answered with that - /// delete's own response (`existed: true`), not a spurious "not found". - pub async fn forget(&self, key: &MemoryKey) -> anyhow::Result<()> { - let key_str = key.to_string(); + /// The live value and version at `key`, unchecked. + async fn get_raw(&self, key: &str) -> anyhow::Result> { let resp = self .client - .idempotent(|| { - self.client.memory_delete_versioned_for( - &self.context_id, - &key_str, - None, - Some(&self.purpose), - ) - }) + .memory_get_for(&self.context_id, &[key], Some(&self.purpose)) .await .map_err(|e| { self.failed( e, - format!("forgetting memory `{key}` in context `{}`", self.context_id), + format!("reading memory `{key}` in context `{}`", self.context_id), ) })?; + let got: get_spec::Response = serde_json::from_value(resp) + .with_context(|| format!("decoding the response to reading `{key}`"))?; + Ok(got.items.into_iter().find_map(|item| { + let value = item.value.filter(|_| !item.deleted)?; + (item.key.as_str() == key).then(|| (value.as_str().to_string(), item.version.0.get())) + })) + } - let decoded: delete_spec::Response = serde_json::from_value(resp) - .with_context(|| format!("decoding the response to forgetting `{key}`"))?; - if decoded.key.as_str() != key_str { - anyhow::bail!( - "the VTA deleted memory `{}` when `{key_str}` was requested", - decoded.key.as_str() - ); + /// Delete one memory and its history. An absent key is an error, as it + /// always has been. + /// + /// The hard-delete path: unlike retracting, nothing is kept — the history + /// a save leaves behind goes too, because "forget" has to mean forget. + /// `delete/0.3` leaves a VTA-level tombstone (no content), so the user's + /// other devices learn of the deletion. It answers a key holding nothing + /// with `existed: false`; this store refuses that case, since "forget X" + /// for an X that is not there is more likely a mistyped name than a + /// success. Under [`VtaClient::idempotent`], a retry of a delete that did + /// land is answered with that delete's own response. + pub async fn forget(&self, key: &MemoryKey) -> anyhow::Result<()> { + let existed = self.delete(&key.to_string()).await?; + let history = self.raw_list(&lifecycle::archive_prefix(key)).await?; + for (raw, _, _) in &history { + self.delete(raw).await?; } - if !decoded.existed { + if !existed && history.is_empty() { anyhow::bail!( "memory `{key}` not found in context `{}` — nothing to forget", self.context_id @@ -402,6 +653,68 @@ impl Store { } } +/// What a save did. +#[derive(Debug)] +pub enum SaveOutcome { + /// Written. + Saved { + key: MemoryKey, + /// The version it took — what the next update passes as expected. + version: u64, + /// The version it replaced, now in history; `None` for a new memory. + superseded: Option, + }, + /// Not written: somebody else's version is there. + Conflict { + key: MemoryKey, + conflict: Box, + }, +} + +/// A save that lost, and what won. +#[derive(Debug)] +pub struct Conflict { + /// Why, in words. + pub reason: String, + /// The version there now, if any. + pub current_version: Option, + /// What is there now — checked like any read, so a forged winner is a + /// [`Refused`], not an [`Entry`]. + pub current: Option>, +} + +/// A `vta/memory/put:versionConflict`, as the VTA sent it. +#[derive(Debug)] +struct RawConflict { + reason: String, + current_version: Option, + current_value: Option, +} + +impl RawConflict { + fn from_error(e: &VtaError) -> Option { + let VtaError::Rejected { code, details, .. } = e else { + return None; + }; + if !(code.starts_with("vta/memory/put") && code.ends_with(":versionConflict")) { + return None; + } + let d = details.clone().unwrap_or_default(); + let deleted = d["currentDeleted"].as_bool().unwrap_or(false); + Some(Self { + reason: d["reason"] + .as_str() + .unwrap_or("versionConflict") + .to_string(), + current_version: d["currentVersion"].as_u64(), + current_value: d["currentValue"] + .as_str() + .filter(|_| !deleted) + .map(str::to_string), + }) + } +} + /// The explanation for a refusal of `purpose` (`vta/memory/*:purposeNotGranted`), /// naming the `pnm` command that grants it; `None` for any other error. /// @@ -425,9 +738,9 @@ pub fn purpose_refused(e: &VtaError, purpose: &str) -> Option { )) } -/// One decoded `list/0.3` page: the live `(key, value)` pairs this tool can -/// read, still to be checked, and the cursor for the next page. -type Page = (Vec<(MemoryKey, String)>, Option); +/// One decoded `list/0.3` page: the live `(key, value, version)` triples, +/// unparsed and unchecked, and the cursor for the next page. +type Page = (Vec<(String, String, u64)>, Option); /// Decode one `list/0.3` response into a [`Page`]. fn decode_page(resp: Value) -> anyhow::Result { @@ -445,14 +758,11 @@ fn decode_page(resp: Value) -> anyhow::Result { item.key.as_str() ); }; - // One level down from the envelope, tolerance is right: a key that is - // not `/` belongs to something else writing into the same - // context, and skipping it keeps `list` total. - let Ok(key) = MemoryKey::parse(item.key.as_str()) else { - tracing::debug!(key = %item.key.as_str(), "skipping entry with an unrecognised key shape"); - continue; - }; - entries.push((key, value.as_str().to_string())); + entries.push(( + item.key.as_str().to_string(), + value.as_str().to_string(), + item.version.0.get(), + )); } Ok((entries, page.next_cursor)) } @@ -463,11 +773,10 @@ mod tests { use crate::record::MemoryRecord; fn entry(kind: MemoryType, name: &str, desc: &str) -> Entry { - Entry { - key: MemoryKey::new(kind, name).unwrap(), - record: MemoryRecord::new(kind, name, desc, "body", vec![]), - authorship: crate::authorship::Authorship::Unattributed, - } + Entry::local( + MemoryKey::new(kind, name).unwrap(), + MemoryRecord::new(kind, name, desc, "body", vec![]), + ) } /// The recall pipeline without a VTA: the filter → rank → drop-zeros → diff --git a/tests/mcp_starts_without_a_vta.rs b/tests/mcp_starts_without_a_vta.rs index 2173d5c..694079e 100644 --- a/tests/mcp_starts_without_a_vta.rs +++ b/tests/mcp_starts_without_a_vta.rs @@ -94,6 +94,9 @@ fn every_memory_tool_is_offered_with_no_vta_reachable() { "memory_forget", "memory_list", "memory_context", + "memory_retract", + "memory_dispute", + "memory_deprecate", ] { assert!( names.contains(&expected), diff --git a/tests/memory_roundtrip.rs b/tests/memory_roundtrip.rs index ee3c2ed..edc6bc0 100644 --- a/tests/memory_roundtrip.rs +++ b/tests/memory_roundtrip.rs @@ -47,6 +47,10 @@ struct FakeMemoryKeyspace { seen: Mutex>, /// The purposes the caller's entry lists; `None` is `any`. granted: Mutex>>, + /// A write another writer makes right after the next `get` is answered: + /// `(context, key, value)`. Lets a test land a save in the window between a + /// store's read and its conditional write. + race: Mutex>, } impl FakeMemoryKeyspace { @@ -54,8 +58,25 @@ impl FakeMemoryKeyspace { self.seen.lock().unwrap().clone() } + /// Live memories, not counting history copies. fn len(&self) -> usize { - self.entries.lock().unwrap().len() + self.entries + .lock() + .unwrap() + .keys() + .filter(|(_, k)| !k.starts_with("archive/")) + .count() + } + + /// History copies held for `key` in `context`. + fn history_len(&self, context: &str, key: &str) -> usize { + let prefix = format!("archive/{key}/"); + self.entries + .lock() + .unwrap() + .keys() + .filter(|(c, k)| c == context && k.starts_with(&prefix)) + .count() } fn record(context_id: &str, key: &str, value: &str, version: u64) -> Value { @@ -102,6 +123,36 @@ impl LoopbackSink for FakeMemoryKeyspace { u if u.ends_with("/vta/memory/put/0.3") => { let key = field(payload, "key")?; let value = field(payload, "value")?; + // `expectedVersion`: 0 is create-only, n requires version n. + // A failed precondition is `versionConflict` with the current + // version and value, as the VTA sends it. + if let Some(expected) = payload.get("expectedVersion").and_then(Value::as_u64) { + let current = self + .entries + .lock() + .unwrap() + .get(&(context_id.to_string(), key.to_string())) + .cloned(); + let reason = match (¤t, expected) { + (None, 0) => None, + (Some(_), 0) => Some("recordExists"), + (None, _) => Some("recordAbsent"), + (Some((_, v)), e) if *v != e => Some("versionMismatch"), + _ => None, + }; + if let Some(reason) = reason { + let mut details = json!({ "reason": reason }); + if let Some((v, ver)) = current { + details["currentVersion"] = json!(ver); + details["currentValue"] = json!(v); + } + return Err(VtaError::Rejected { + code: "vta/memory/put:versionConflict".into(), + message: "precondition failed".into(), + details: Some(details), + }); + } + } let mut counter = self.counter.lock().unwrap(); *counter += 1; let created = self @@ -132,6 +183,7 @@ impl LoopbackSink for FakeMemoryKeyspace { Ok(json!({ "items": items, "highWatermark": *self.counter.lock().unwrap() })) } u if u.ends_with("/vta/memory/get/0.2") => { + let race = self.race.lock().unwrap().take(); let entries = self.entries.lock().unwrap(); let (mut items, mut missing) = (Vec::new(), Vec::new()); for k in payload["keys"].as_array().into_iter().flatten() { @@ -141,6 +193,10 @@ impl LoopbackSink for FakeMemoryKeyspace { None => missing.push(json!(k)), } } + drop(entries); + if let Some((c, k, v)) = race { + self.plant(&c, &k, v); + } Ok(json!({ "items": items, "missing": missing })) } u if u.ends_with("/vta/memory/delete/0.3") => { @@ -422,7 +478,10 @@ async fn the_payloads_on_the_wire_are_camel_case_and_context_scoped() { store.forget(&key).await.unwrap(); let seen = vta.recorded(); - let (put_uri, put) = &seen[0]; + let (put_uri, put) = seen + .iter() + .find(|(u, _)| u.contains("/memory/put/")) + .expect("a put"); assert!(put_uri.ends_with("/vta/memory/put/0.3"), "{put_uri}"); assert_eq!(put.get("contextId").unwrap(), "proj"); assert_eq!(put.get("key").unwrap(), "user/name"); @@ -438,7 +497,10 @@ async fn the_payloads_on_the_wire_are_camel_case_and_context_scoped() { assert_eq!(stored.kind, MemoryType::User); assert!(!stored.updated_at.is_empty(), "every write is stamped"); - let (del_uri, del) = &seen[seen.len() - 1]; + let (del_uri, del) = seen + .iter() + .find(|(u, _)| u.contains("/memory/delete/")) + .expect("a delete"); assert!(del_uri.ends_with("/vta/memory/delete/0.3"), "{del_uri}"); assert_eq!(del.get("key").unwrap(), "user/name"); } @@ -504,7 +566,10 @@ async fn a_put_that_echoes_a_different_key_is_refused() { // it and the user would believe the save succeeded. struct WrongKey; impl LoopbackSink for WrongKey { - fn dispatch(&self, _uri: &str, _payload: &Value) -> Result { + fn dispatch(&self, uri: &str, _payload: &Value) -> Result { + if uri.contains("/memory/get/") { + return Ok(json!({ "items": [], "missing": ["user/name"] })); + } Ok(json!({ "contextId": "proj", "key": "user/something-else", "version": 1, "wasCreated": true, "updatedAt": "2026-11-09T00:00:00Z", @@ -648,7 +713,13 @@ async fn every_operation_names_its_purpose() { store.forget(&key).await.unwrap(); let seen = vta.recorded(); - assert_eq!(seen.len(), 4, "{seen:?}"); + for op in ["get", "put", "list", "delete"] { + assert!( + seen.iter() + .any(|(u, _)| u.contains(&format!("/memory/{op}/"))), + "{op} was exercised: {seen:?}" + ); + } for (uri, payload) in &seen { assert_eq!(payload["purpose"], "personalisation", "{uri}: {payload}"); } @@ -1104,3 +1175,382 @@ async fn recall_prefers_what_the_user_said_over_what_was_inferred() { assert_eq!(hits[0].entry.record.name, "Tabs said"); assert_eq!(hits[1].entry.record.name, "Tabs guess"); } + +// ---- #33: lifecycle --------------------------------------------------------- + +/// A save that replaces a memory keeps the replaced version, verbatim and still +/// attributed to its own author, and links to it. +#[tokio::test] +async fn a_replacing_save_keeps_the_previous_version_in_history() { + let vta = Arc::new(FakeMemoryKeyspace::default()); + let laptop = store_on(&vta, "proj"); + let phone = store_on(&vta, "proj"); + let first_author = laptop.signer().unwrap().did().to_string(); + + let key = laptop + .save(&record(MemoryType::Project, "Gateway", "first", "one")) + .await + .unwrap(); + let v1 = laptop.get(&key).await.unwrap().unwrap().version; + phone + .save(&record(MemoryType::Project, "Gateway", "second", "two")) + .await + .unwrap(); + + let current = laptop.get(&key).await.unwrap().unwrap(); + assert_eq!(current.record.description, "second"); + let supersedes = current.summary()["supersedes"].clone(); + assert_eq!(supersedes["version"], v1); + assert_eq!(supersedes["key"], format!("archive/project/gateway/v{v1}")); + + let history = laptop.history(&key).await.unwrap(); + assert_eq!(history.len(), 1); + assert_eq!(history[0].record.description, "first"); + assert_eq!(history[0].version, v1); + assert_eq!( + history[0].authorship, + Authorship::Verified { + author: first_author, + kind: Some(AuthorKind::Agent), + by_this_agent: true, + }, + "the old version keeps its own author's signature" + ); + + // History never leaks into a listing or a recall. + assert_eq!(laptop.list().await.unwrap().entries.len(), 1); + assert_eq!(laptop.recall("", None, 50).await.unwrap().hits.len(), 1); +} + +/// History is bounded, oldest first out. +#[tokio::test] +async fn history_keeps_only_the_most_recent_versions() { + use vta_agent_memory::lifecycle::MAX_HISTORY; + + let (store, vta) = store_with("proj"); + for i in 0..MAX_HISTORY + 3 { + store + .save(&record(MemoryType::User, "Name", &format!("v{i}"), "x")) + .await + .unwrap(); + } + assert_eq!(vta.history_len("proj", "user/name"), MAX_HISTORY); + let key = MemoryKey::parse("user/name").unwrap(); + let history = store.history(&key).await.unwrap(); + assert_eq!(history.len(), MAX_HISTORY); + assert_eq!( + history[0].record.description, + format!("v{}", MAX_HISTORY + 1), + "newest first" + ); +} + +/// The caller read version n; somebody else saved since. The save is refused, +/// nothing is overwritten, and the winner comes back. +#[tokio::test] +async fn a_save_based_on_a_stale_read_is_rejected_with_the_winner() { + use vta_agent_memory::store::SaveOutcome; + + let vta = Arc::new(FakeMemoryKeyspace::default()); + let laptop = store_on(&vta, "proj"); + let phone = store_on(&vta, "proj"); + let key = laptop + .save(&record(MemoryType::User, "Editor", "uses vim", "x")) + .await + .unwrap(); + let read = laptop.get(&key).await.unwrap().unwrap().version; + + phone + .save(&record( + MemoryType::User, + "Editor", + "switched to helix", + "x", + )) + .await + .unwrap(); + + let outcome = laptop + .save_versioned( + &record(MemoryType::User, "Editor", "uses emacs", "x"), + Some(read), + ) + .await + .unwrap(); + let SaveOutcome::Conflict { conflict, .. } = outcome else { + panic!("a save from a stale read must not land: {outcome:?}"); + }; + let winner = conflict.current.expect("the winner").expect("verified"); + assert_eq!(winner.record.description, "switched to helix"); + assert_eq!(conflict.current_version, Some(winner.version)); + assert_eq!( + laptop.get(&key).await.unwrap().unwrap().record.description, + "switched to helix", + "nothing was overwritten" + ); + + // Retried against the winner's version, it lands. + let outcome = laptop + .save_versioned( + &record(MemoryType::User, "Editor", "uses emacs", "x"), + conflict.current_version, + ) + .await + .unwrap(); + assert!(matches!(outcome, SaveOutcome::Saved { .. }), "{outcome:?}"); +} + +/// Another writer lands between this store's read and its write. The VTA's +/// precondition catches it, and the winner comes back rather than being lost. +#[tokio::test] +async fn a_concurrent_save_in_the_read_write_window_is_rejected_by_the_vta() { + use vta_agent_memory::store::SaveOutcome; + + let vta = Arc::new(FakeMemoryKeyspace::default()); + let laptop = store_on(&vta, "proj"); + let phone = store_on(&vta, "proj"); + laptop + .save(&record(MemoryType::User, "Editor", "uses vim", "x")) + .await + .unwrap(); + + // What the phone would store, signed by the phone, slipped in after the + // laptop's read. + phone + .save(&record( + MemoryType::User, + "Editor", + "switched to helix", + "x", + )) + .await + .unwrap(); + let phones = vta.value("proj", "user/editor"); + laptop + .save(&record(MemoryType::User, "Editor", "back to vim", "x")) + .await + .unwrap(); + *vta.race.lock().unwrap() = Some(("proj".into(), "user/editor".into(), phones)); + + let outcome = laptop + .save_versioned(&record(MemoryType::User, "Editor", "uses emacs", "x"), None) + .await + .unwrap(); + let SaveOutcome::Conflict { conflict, .. } = outcome else { + panic!("the VTA's precondition must catch the race: {outcome:?}"); + }; + assert!( + conflict.reason.contains("versionMismatch"), + "{}", + conflict.reason + ); + let winner = conflict.current.unwrap().unwrap(); + assert_eq!(winner.record.description, "switched to helix"); + + // `save` (no expected version) reports the lost race as an error. + *vta.race.lock().unwrap() = Some(( + "proj".into(), + "user/editor".into(), + vta.value("proj", "user/editor"), + )); + let err = laptop + .save(&record(MemoryType::User, "Editor", "uses emacs", "x")) + .await + .unwrap_err(); + assert!( + format!("{err:#}").contains("nothing was overwritten"), + "{err:#}" + ); +} + +/// `expectedVersion: 0` means "this must be new". +#[tokio::test] +async fn expecting_a_new_memory_refuses_to_replace_an_existing_one() { + use vta_agent_memory::store::SaveOutcome; + + let (store, _vta) = store_with("proj"); + store + .save(&record(MemoryType::User, "Name", "first", "x")) + .await + .unwrap(); + let outcome = store + .save_versioned(&record(MemoryType::User, "Name", "second", "x"), Some(0)) + .await + .unwrap(); + assert!( + matches!(outcome, SaveOutcome::Conflict { .. }), + "{outcome:?}" + ); +} + +/// Retract keeps a tombstone that says why; recall leaves it out; get and +/// list still show it; the content survives only in history. +#[tokio::test] +async fn retracting_leaves_a_tombstone_and_forgetting_removes_everything() { + use vta_agent_memory::lifecycle::Status; + + let (store, vta) = store_with("proj"); + let key = store + .save(&record( + MemoryType::Feedback, + "Deploy fridays", + "fridays are fine for deploys", + "the body", + )) + .await + .unwrap(); + store + .set_status(&key, Status::Retracted, "the user never said this", None) + .await + .unwrap(); + + assert!( + store + .recall("deploy", None, 8) + .await + .unwrap() + .hits + .is_empty() + ); + let tomb = store + .get(&key) + .await + .unwrap() + .expect("a tombstone, not nothing"); + assert_eq!(tomb.standing.status, Some(Status::Retracted)); + assert!(tomb.record.body.is_empty(), "the content is withdrawn"); + assert_eq!(tomb.record.name, "Deploy fridays"); + let summary = tomb.summary(); + assert_eq!(summary["status"], "retracted"); + assert_eq!(summary["statusReason"], "the user never said this"); + assert_eq!(store.list().await.unwrap().entries.len(), 1); + assert_eq!( + store.history(&key).await.unwrap()[0].record.body, + "the body" + ); + + // Forget is the hard delete: memory and history both go. + store.forget(&key).await.unwrap(); + assert_eq!(vta.len(), 0); + assert_eq!(vta.history_len("proj", "feedback/deploy-fridays"), 0); + assert!(store.get(&key).await.unwrap().is_none()); +} + +/// Disputed and deprecated memories still recall — ranked below an equally +/// relevant active one, with their status and reason shown. +#[tokio::test] +async fn disputed_and_deprecated_memories_are_demoted_not_hidden() { + use vta_agent_memory::lifecycle::Status; + + let (store, _vta) = store_with("proj"); + for name in ["Indent a", "Indent b", "Indent c"] { + store + .save(&record( + MemoryType::User, + name, + "uses tabs for indentation", + "x", + )) + .await + .unwrap(); + } + let a = MemoryKey::parse("user/indent-a").unwrap(); + let b = MemoryKey::parse("user/indent-b").unwrap(); + store + .set_status(&a, Status::Disputed, "user says spaces now", None) + .await + .unwrap(); + store + .set_status(&b, Status::Deprecated, "replaced by indent c", None) + .await + .unwrap(); + + let hits = store + .recall("tabs indentation", None, 8) + .await + .unwrap() + .hits; + assert_eq!(hits.len(), 3); + assert_eq!(hits[0].entry.record.name, "Indent c"); + let disputed = hits.iter().find(|h| h.entry.key == a).unwrap(); + assert_eq!(disputed.entry.summary()["status"], "disputed"); + assert_eq!( + disputed.entry.summary()["statusReason"], + "user says spaces now" + ); + assert!(disputed.score < hits[0].score); +} + +/// `validUntil` past: recall leaves it out. `reviewAfter` past: recall keeps +/// it, flagged stale. +#[tokio::test] +async fn expired_memories_drop_out_and_stale_ones_are_flagged() { + let (store, _vta) = store_with("proj"); + let mut leave = record(MemoryType::Project, "On leave", "the user is on leave", "x"); + leave.valid_until = Some("2020-01-01T00:00:00+00:00".into()); + let mut style = record( + MemoryType::User, + "Leave style", + "prefers terse leave notes", + "x", + ); + style.review_after = Some("2020-01-01T00:00:00+00:00".into()); + let mut future = record( + MemoryType::User, + "Leave plan", + "leave planned for later", + "x", + ); + future.valid_until = Some("2999-01-01T00:00:00+00:00".into()); + for r in [&leave, &style, &future] { + store.save(r).await.unwrap(); + } + + let hits = store.recall("leave", None, 8).await.unwrap().hits; + let names: Vec<&str> = hits.iter().map(|h| h.entry.record.name.as_str()).collect(); + assert!(!names.contains(&"On leave"), "{names:?}"); + assert!(names.contains(&"Leave plan"), "{names:?}"); + let stale = hits + .iter() + .find(|h| h.entry.record.name == "Leave style") + .unwrap(); + assert_eq!(stale.entry.summary()["stale"], true); + + let listing = store.list().await.unwrap(); + let expired = listing + .entries + .iter() + .find(|e| e.record.name == "On leave") + .expect("list still shows it"); + assert_eq!(expired.summary()["expired"], true); +} + +/// A forged record can be retracted (that withdraws it) but not disputed or +/// deprecated, which would re-sign content nobody vouches for. +#[tokio::test] +async fn a_refused_record_can_only_be_retracted() { + use vta_agent_memory::lifecycle::Status; + + let (store, vta) = store_with("proj"); + let key = store + .save(&record(MemoryType::User, "Name", "d", "x")) + .await + .unwrap(); + let mut v: Value = serde_json::from_str(&vta.value("proj", "user/name")).unwrap(); + v["body"] = json!("forged"); + vta.plant("proj", "user/name", v.to_string()); + + let err = store + .set_status(&key, Status::Disputed, "looks wrong", None) + .await + .unwrap_err(); + assert!(format!("{err:#}").contains("only be retracted"), "{err:#}"); + + store + .set_status(&key, Status::Retracted, "forged", None) + .await + .unwrap(); + let listing = store.list().await.unwrap(); + assert!(listing.refused.is_empty(), "the forgery is withdrawn"); + assert_eq!(listing.entries[0].summary()["status"], "retracted"); +}