From ea9c5e6750605ecc1b42b312897177f0021cabb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Andrade?= <95101505+viniciusdsandrade@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:17:53 -0300 Subject: [PATCH] feat(mcp): add status discriminant to memory_handoff_accept (fixes #988) - Adds HandoffAcceptStatus enum ('claimed' | 'consumed_by_hook' | 'none_pending') returned alongside 'handoff' in memory_handoff_accept (#988, split from #920). - Adds ReaderPool::handoff_claimed_by_live_session to verify if the caller's live session already received the handoff via SessionStart hook. - Reports 'consumed_by_hook' only when the caller forwards its session id (e.g., Claude Code with --session-aware or OpenCode 2) and matches the scope, live session, and owner filters. - Reports 'none_pending' when no handoff was pending or for static clients where the hook claim cannot be confirmed for that exact session. - Updates documentation, routing skill, ARCHITECTURE.md, security boundaries (row 4g), and CHANGELOG.md. --- CHANGELOG.md | 10 +- .../routing_skills/ai-memory-handoff/SKILL.md | 2 +- crates/ai-memory-hooks/src/router.rs | 6 +- crates/ai-memory-mcp/src/server.rs | 216 ++++++++++++++++-- .../tests/suite/handoff_admission.rs | 2 +- .../tests/suite/handoff_identity.rs | 128 +++++++++++ crates/ai-memory-store/src/reader.rs | 45 ++++ .../tests/suite/handoff_ownership.rs | 116 ++++++++++ docs/ARCHITECTURE.md | 2 +- docs/security-boundaries.md | 1 + docs/usage.md | 9 + 11 files changed, 510 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e21d8a17..4f002734 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -260,7 +260,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `docs/airgapped-install.md`, and the README docs table. The on-box `ai-memory backup --to ` command (`docs/lifecycle-ops.md#backup`) is unchanged. (#950) - +- `memory_handoff_accept` returns a `status` next to `handoff`: `claimed` when + the call took the handoff, `consumed_by_hook` when the calling session's own + SessionStart already claimed it (so it is in that session's context), and + `none_pending` when nothing is left to claim. A bare `{"handoff": null}` used + to cover both of the last two. `consumed_by_hook` needs the session id the + hook claimed under, which Claude Code's `install-mcp --session-aware` bridge + and OpenCode 2 forward; the answer stays inside the caller's project and + handoff ownership, so a forwarded id cannot report another operator's or + another session's claim. The `handoff` field is unchanged. (#988, #920) ### Changed - `ai-memory purge-session` without `--confirm` now previews what a confirmed diff --git a/crates/ai-memory-core/src/routing_skills/ai-memory-handoff/SKILL.md b/crates/ai-memory-core/src/routing_skills/ai-memory-handoff/SKILL.md index 2522006f..6d942b9a 100644 --- a/crates/ai-memory-core/src/routing_skills/ai-memory-handoff/SKILL.md +++ b/crates/ai-memory-core/src/routing_skills/ai-memory-handoff/SKILL.md @@ -17,7 +17,7 @@ Use this skill for single-use cross-session handoffs. Handoffs are for the next ## Single-use handoff behavior -The SessionStart hook usually fetches and consumes any pending handoff before the agent sees its first prompt. If the current context already contains a pending handoff block, answer from that block directly. Do not call the accept tool again to find it in another project, because handoffs are single-use and the tool will normally return null after SessionStart consumed it. +The SessionStart hook usually fetches and consumes any pending handoff before the agent sees its first prompt. If the current context already contains a pending handoff block, answer from that block directly. Do not call the accept tool again to find it in another project, because handoffs are single-use: after SessionStart consumed it the tool returns no handoff, with `status` `consumed_by_hook` when the client forwards its session id and `none_pending` otherwise. If no pending handoff block is visible, inspect with `memory_handoff_list` first. Listing does not claim or expire anything. When the user asks where we left off, claim one listed row with `memory_handoff_accept` and that `handoff_id`, using the client-aware project scope below. Do not treat list as a second accept path. diff --git a/crates/ai-memory-hooks/src/router.rs b/crates/ai-memory-hooks/src/router.rs index 3164d1bc..0d8851dd 100644 --- a/crates/ai-memory-hooks/src/router.rs +++ b/crates/ai-memory-hooks/src/router.rs @@ -2013,15 +2013,15 @@ fn render_handoff_markdown(h: &Handoff) -> String { // Agent-facing reading instructions. This block is the // load-bearing UX fix — without it, agents call - // memory_handoff_accept again, get `null` (single-use + // memory_handoff_accept again, get no handoff (single-use // already consumed by this hook), and conclude "no handoff" // *despite this content being right in their context*. buf.push_str( "\n---\n\ _**To the receiving agent:** this content IS the pending \ handoff — already consumed by the SessionStart hook. A \ - subsequent `memory_handoff_accept` call will return \ - `{ \"handoff\": null }` (single-use). When the user asks \ + subsequent `memory_handoff_accept` call returns no handoff \ + (single-use). When the user asks \ \"where did we leave off?\" or \"any pending handoff?\", \ answer from THIS content; do NOT re-call the tool. Call \ `memory_query` / `memory_recent` only for additional \ diff --git a/crates/ai-memory-mcp/src/server.rs b/crates/ai-memory-mcp/src/server.rs index fed28e68..cd15657a 100644 --- a/crates/ai-memory-mcp/src/server.rs +++ b/crates/ai-memory-mcp/src/server.rs @@ -258,7 +258,7 @@ developer, user, and canonical project instructions.\n\ before you see your first prompt; if a block starting with \ '📥 ai-memory: pending handoff' is anywhere in your context, \ THAT is the handoff — answer from it directly, don't re-call \ - this tool (it'll return null because handoffs are single-use). \ + this tool (it'll return no handoff because handoffs are single-use). \ When no prepended block is visible, inspect with memory_handoff_list \ first, then pass the listed `handoff_id` to claim that exact row; \ omitting `handoff_id` still claims the latest eligible open handoff. \ @@ -1255,6 +1255,22 @@ struct HandoffAcceptArgs { handoff_id: Option, } +/// Why `memory_handoff_accept` did or did not return a handoff. `handoff: null` +/// alone meant both "nothing to claim" and "your own SessionStart already +/// claimed it", which only a paragraph of tool description told apart (#920). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +enum HandoffAcceptStatus { + /// This call claimed the handoff it returns. + Claimed, + /// The calling session's own SessionStart claimed it, so it is already in + /// that session's context. Reported only when the request carries the + /// session id the hook claimed under. + ConsumedByHook, + /// Nothing is left for this caller to claim. + NonePending, +} + #[derive(Debug, Serialize, Deserialize, schemars::JsonSchema)] struct HandoffListArgs { /// Also list handoffs that belong to OTHER operators. Off by default: @@ -4549,17 +4565,16 @@ impl AiMemoryServer { '📥 ai-memory: pending handoff from previous session' anywhere \ in your context, that IS the handoff. \ \ - A subsequent call to this tool will return `{ \"handoff\": null }` \ - because the hook already consumed it. Do NOT interpret null as \ - 'no handoff exists' — check your context for the prepended block \ - first, and answer the user from there. Call this tool only when \ - you BOTH don't see a prepended block AND the user explicitly asks \ - for a handoff (e.g. a hook script ran with no stdout capture). \ - Prefer memory_handoff_list first in that case, then pass the listed \ - `handoff_id` here to claim that exact row. Omitting `handoff_id` \ - claims the latest eligible open handoff. \ - \ - Returns the handoff body only when THIS call wins the claim.")] + `status`: `claimed` (THIS call won it; see `handoff`), \ + `consumed_by_hook` (your SessionStart took it; answer from that \ + block), `none_pending` (nothing to claim, or a hook took it for a \ + client that does not forward its session id: do NOT answer 'no \ + handoff exists' before checking your context). Call this tool \ + only when you BOTH don't see a prepended block AND the user \ + explicitly asks for a handoff (e.g. a hook script ran with no \ + stdout capture). Prefer memory_handoff_list first in that case, \ + then pass the listed `handoff_id` here to claim that exact row. \ + Omitting `handoff_id` claims the latest eligible open handoff.")] async fn memory_handoff_accept( &self, Parameters(args): Parameters, @@ -4595,10 +4610,13 @@ impl AiMemoryServer { .handoff_id .as_deref() .map(str::trim) - .filter(|id| !id.is_empty()); - let handoff = if let Some(id) = requested_id { - let handoff_id = HandoffId::from_str(id) - .map_err(|e| McpError::internal_error(format!("invalid handoff_id: {e}"), None))?; + .filter(|id| !id.is_empty()) + .map(|id| { + HandoffId::from_str(id) + .map_err(|e| McpError::internal_error(format!("invalid handoff_id: {e}"), None)) + }) + .transpose()?; + let handoff = if let Some(handoff_id) = requested_id { self.reader .handoff_by_id_in_scope(ws, proj, handoff_id, owner_filter.clone()) .await @@ -4611,7 +4629,10 @@ impl AiMemoryServer { .map_err(|e| McpError::internal_error(e.to_string(), None))? }; match handoff { - None => ok_json(&serde_json::json!({ "handoff": null })), + None => { + self.unclaimed_handoff(ws, proj, &aps_actor, requested_id, owner_filter) + .await + } Some(h) => { // Admission is asked here, not before the lookup: the routine // outcome of this tool is `{"handoff": null}` — the tool's own @@ -4641,21 +4662,66 @@ impl AiMemoryServer { accepting_agent: AgentKind::Other, accepting_session: None, accepting_user: actor_user.clone(), - owner_filter, + owner_filter: owner_filter.clone(), receiving_cwd, }) .await .map_err(|e| McpError::internal_error(e.to_string(), None))?; if claimed { self.notify_operation_observers(admission.as_ref()); - ok_json(&serde_json::json!({ "handoff": h })) + ok_json(&serde_json::json!({ + "handoff": h, + "status": HandoffAcceptStatus::Claimed, + })) } else { - ok_json(&serde_json::json!({ "handoff": null })) + // The racing claimant may be this session's own SessionStart. + self.unclaimed_handoff(ws, proj, &aps_actor, requested_id, owner_filter) + .await } } } } + /// `memory_handoff_accept`'s answer when this call claimed nothing. + /// + /// `consumed_by_hook` needs proof that the caller's own session took the + /// baton at SessionStart: the session id the hook claimed under, which only + /// a session-aware client forwards. Without it the answer is `none_pending` + /// rather than a guess from some other session's claim, since pointing an + /// agent at a block that is not in its context is the failure the status + /// exists to remove. A requested `handoff_id` counts only when it is the + /// row the session took. + async fn unclaimed_handoff( + &self, + workspace_id: WorkspaceId, + project_id: ProjectId, + actor: &ai_memory_core::ActorKey, + requested: Option, + owner_filter: ai_memory_core::OwnerFilter, + ) -> Result { + let claimed_at_start = match actor.session_id.as_deref() { + Some(native) => self + .reader + .handoff_claimed_by_live_session( + workspace_id, + project_id, + SessionId::from_native(native), + owner_filter, + ) + .await + .map_err(|e| McpError::internal_error(e.to_string(), None))?, + None => None, + }; + let status = match (claimed_at_start, requested) { + (Some(claimed), Some(requested)) if claimed != requested => { + HandoffAcceptStatus::NonePending + } + (Some(_), _) => HandoffAcceptStatus::ConsumedByHook, + (None, _) => HandoffAcceptStatus::NonePending, + }; + ok_json(&serde_json::json!({ "handoff": null, "status": status })) + } + /// Cancel a mistaken open handoff by exact id. #[tool(description = "Cancel/discard a mistakenly-created OPEN handoff by \ exact `handoff_id` returned from `memory_handoff_begin` or \ @@ -13015,6 +13081,7 @@ mod tests { .unwrap(); assert!(accept_text.contains("left mid-refactor")); assert!(accept_text.contains("what max channel size?")); + assert!(accept_text.contains("\"status\": \"claimed\"")); // Second accept returns null (handoff is now accepted). let again = server @@ -13037,6 +13104,8 @@ mod tests { .map(|t| t.text.clone()) .unwrap(); assert!(again_text.contains("\"handoff\": null")); + // Taken by this server's own MCP call, not by a SessionStart hook. + assert!(again_text.contains("\"status\": \"none_pending\"")); } /// `Handoff` is grouped internally into `HandoffScope`/`HandoffOrigin`/ @@ -14905,6 +14974,113 @@ mod tests { text.contains("\"handoff\": null"), "expected handoff=null in: {text}", ); + assert!( + text.contains("\"status\": \"none_pending\""), + "expected status=none_pending in: {text}", + ); + } + + /// A requested `handoff_id` that some other caller took must not be + /// reported as `consumed_by_hook` just because this session's SessionStart + /// took a different baton: the agent would look for the wrong block. + #[tokio::test] + async fn memory_handoff_accept_by_id_reports_the_hook_only_for_its_own_row() { + let (_tmp, store, server, ws, pj) = setup_server().await; + let insert = |summary: &str| NewHandoff { + workspace_id: ws, + project_id: pj, + from_session_id: None, + from_agent: AgentKind::ClaudeCode, + to_agent: None, + cwd: None, + summary: summary.into(), + open_questions: Vec::new(), + next_steps: Vec::new(), + files_touched: Vec::new(), + owner_user: None, + }; + let by_hook = store + .writer + .insert_handoff(insert("taken at session start")) + .await + .unwrap(); + let by_other = store + .writer + .insert_handoff(insert("taken by another caller")) + .await + .unwrap(); + + let session = SessionId::from_native("claude-session-1"); + store + .writer + .begin_session(ai_memory_core::NewSession { + occurred_at: None, + id: session, + workspace_id: ws, + project_id: pj, + agent_kind: AgentKind::ClaudeCode, + cwd: None, + actor_user: None, + }) + .await + .unwrap(); + for (handoff_id, accepting_session) in [(by_hook, Some(session)), (by_other, None)] { + let claimed = store + .writer + .accept_handoff(ai_memory_core::HandoffAcceptance { + handoff_id, + workspace_id: ws, + project_id: pj, + accepting_agent: AgentKind::ClaudeCode, + accepting_session, + accepting_user: None, + owner_filter: ai_memory_core::OwnerFilter::Unattributed, + receiving_cwd: None, + }) + .await + .unwrap(); + assert!(claimed); + } + + let status_for = |handoff_id: Option| { + let server = server.clone(); + async move { + let mut parts = test_parts_default(); + parts.headers.insert( + "x-memory-actor-session-id", + axum::http::HeaderValue::from_static("claude-session-1"), + ); + let result = server + .memory_handoff_accept( + Parameters(HandoffAcceptArgs { + cwd: None, + project: None, + workspace: None, + any_owner: None, + handoff_id: handoff_id.map(|id| id.to_string()), + }), + OptionalParts(parts), + ) + .await + .unwrap(); + let text = result + .content + .first() + .and_then(|c| c.as_text()) + .map(|t| t.text.clone()) + .unwrap(); + let value: serde_json::Value = serde_json::from_str(&text).unwrap(); + assert!(value["handoff"].is_null(), "nothing is open: {text}"); + value["status"].as_str().unwrap().to_owned() + } + }; + assert_eq!(status_for(Some(by_hook)).await, "consumed_by_hook"); + assert_eq!(status_for(None).await, "consumed_by_hook"); + assert_eq!( + status_for(Some(by_other)).await, + "none_pending", + "the row this session asked for went to another caller", + ); } /// `memory_query` clamps `limit` into [1, 100]. Anyone sending diff --git a/crates/ai-memory-mcp/tests/suite/handoff_admission.rs b/crates/ai-memory-mcp/tests/suite/handoff_admission.rs index 0a18d75e..0a6cc308 100644 --- a/crates/ai-memory-mcp/tests/suite/handoff_admission.rs +++ b/crates/ai-memory-mcp/tests/suite/handoff_admission.rs @@ -289,7 +289,7 @@ async fn accept_with_nothing_pending_tells_no_webhook() { .await; assert_eq!( result, - json!({ "handoff": null }), + json!({ "handoff": null, "status": "none_pending" }), "nothing was pending, so nothing is accepted", ); diff --git a/crates/ai-memory-mcp/tests/suite/handoff_identity.rs b/crates/ai-memory-mcp/tests/suite/handoff_identity.rs index 611121fe..226a5f0a 100644 --- a/crates/ai-memory-mcp/tests/suite/handoff_identity.rs +++ b/crates/ai-memory-mcp/tests/suite/handoff_identity.rs @@ -326,3 +326,131 @@ async fn legacy_unowned_handoffs_stay_visible_to_everyone() { ); } } + +/// `memory_handoff_accept`'s `status`, checked against the body: a handoff +/// comes back exactly when the status says this call claimed one. +async fn accept_status(router: &Router, headers: &[(&str, &str)]) -> String { + let got = call( + router, + "memory_handoff_accept", + json!({ "workspace": "default", "project": "scratch" }), + headers, + ) + .await; + let status = got + .get("status") + .and_then(Value::as_str) + .unwrap_or_else(|| panic!("no status in {got}")) + .to_owned(); + assert_eq!( + got.get("handoff").is_some_and(|h| !h.is_null()), + status == "claimed", + "the body and the status disagree: {got}", + ); + status +} + +fn with_session( + mut headers: Vec<(&'static str, &'static str)>, + session: &'static str, +) -> Vec<(&'static str, &'static str)> { + headers.push(("x-memory-actor-session-id", session)); + headers +} + +/// `consumed_by_hook` tells an agent the baton is already in its context, so it +/// is said only to the session whose SessionStart claimed it. The session id on +/// the request is routing data a client can forge: a colleague forwarding that +/// id, the same operator from another session, and a request with no session +/// at all must each hear `none_pending`, not a pointer at someone else's context. +#[tokio::test] +async fn consumed_by_hook_is_reported_only_to_the_session_that_claimed_it() { + use ai_memory_core::{AgentKind, HandoffAcceptance, NewSession, OwnerFilter, SessionId}; + + let h = harness(Some("dj"), true).await; + let alice = proxied("x-memory-actor-user", "alice"); + assert_eq!(accept_status(&h.http, &alice).await, "none_pending"); + + begin(&h.http, "alices-baton", &alice).await; + let rows = h + .store + .reader + .list_handoffs(h.ws, h.proj, None, OwnerFilter::Any, 10) + .await + .expect("list"); + let owner = rows[0] + .origin + .owner_user + .clone() + .expect("a proxied operator owns the baton"); + + // What the SessionStart hook does: claim the baton for Alice's session under + // the native id her session-aware client then forwards on every MCP call. + let session = SessionId::from_native("alice-claude-session"); + h.store + .writer + .begin_session(NewSession { + occurred_at: None, + id: session, + workspace_id: h.ws, + project_id: h.proj, + agent_kind: AgentKind::ClaudeCode, + cwd: None, + actor_user: Some(owner.clone()), + }) + .await + .expect("receiving session"); + let claimed = h + .store + .writer + .accept_handoff(HandoffAcceptance { + handoff_id: rows[0].scope.id, + workspace_id: h.ws, + project_id: h.proj, + accepting_agent: AgentKind::ClaudeCode, + accepting_session: Some(session), + accepting_user: Some(owner.clone()), + owner_filter: OwnerFilter::User(owner), + receiving_cwd: None, + }) + .await + .expect("session-start claim"); + assert!(claimed); + + assert_eq!( + accept_status( + &h.http, + &with_session(alice.clone(), "alice-claude-session") + ) + .await, + "consumed_by_hook", + "the session that received the baton must be told it is in its context", + ); + for (label, headers) in [ + ( + "bob forwarding alice's session id", + with_session( + proxied("x-memory-actor-user", "bob"), + "alice-claude-session", + ), + ), + ( + "alice from another session", + with_session(alice.clone(), "alice-other-session"), + ), + ("alice with no session id", alice.clone()), + ] { + assert_eq!( + accept_status(&h.http, &headers).await, + "none_pending", + "{label} was pointed at a handoff that is not in its context", + ); + } + + // A baton left after the session started is still claimed normally. + begin(&h.http, "a-later-baton", &alice).await; + assert_eq!( + accept_status(&h.http, &with_session(alice, "alice-claude-session")).await, + "claimed", + ); +} diff --git a/crates/ai-memory-store/src/reader.rs b/crates/ai-memory-store/src/reader.rs index 966066e4..81329e31 100644 --- a/crates/ai-memory-store/src/reader.rs +++ b/crates/ai-memory-store/src/reader.rs @@ -6084,6 +6084,51 @@ impl ReaderPool { .await } + /// The handoff `session` claimed in this scope while it is still running — + /// its own SessionStart delivery, so the baton is already in that session's + /// context — or `None`. + /// + /// The session id is a routing coordinate the caller supplies, never + /// identity: the scope and owner predicates bound the answer, so a forged + /// id can only confirm a claim on a row the caller could have claimed + /// itself. A receiving session holds at most one baton, and one that ended + /// no longer has a context the baton could be in. + /// + /// # Errors + /// Propagates any SQL or pool error. + pub async fn handoff_claimed_by_live_session( + &self, + workspace_id: WorkspaceId, + project_id: ProjectId, + session: SessionId, + owner_filter: OwnerFilter, + ) -> StoreResult> { + self.with_conn(move |conn| { + let (owner_clause, owner_param) = handoff_owner_sql(&owner_filter, 4); + let sql = format!( + "SELECT id FROM handoffs \ + WHERE workspace_id = ?1 AND project_id = ?2 \ + AND state = 'accepted' AND accepted_by_session = ?3{owner_clause} \ + AND EXISTS (SELECT 1 FROM sessions s \ + WHERE s.id = ?3 AND s.ended_at IS NULL) \ + ORDER BY accepted_at DESC LIMIT 1" + ); + let mut binds: Vec<&dyn rusqlite::ToSql> = vec![ + workspace_id.as_bytes(), + project_id.as_bytes(), + session.as_bytes(), + ]; + if let Some(owner) = owner_param.as_ref() { + binds.push(owner); + } + let id: Option> = conn + .query_row(&sql, binds.as_slice(), |row| row.get(0)) + .optional()?; + Ok(id.map(|id| HandoffId::from_slice(&id)).transpose()?) + }) + .await + } + /// Snapshot the database to `dest_path` using SQLite's online backup /// API. The source DB stays writable for the duration of the copy. /// diff --git a/crates/ai-memory-store/tests/suite/handoff_ownership.rs b/crates/ai-memory-store/tests/suite/handoff_ownership.rs index e727d652..98839a65 100644 --- a/crates/ai-memory-store/tests/suite/handoff_ownership.rs +++ b/crates/ai-memory-store/tests/suite/handoff_ownership.rs @@ -956,3 +956,119 @@ async fn open_session_lookup_is_owner_scoped() { None, ); } + +/// `memory_handoff_accept` reports `consumed_by_hook` from this lookup, keyed on +/// a session id the caller supplies. That id is routing data, not identity, so +/// the scope and owner predicates are what bound the answer: a forged id, a +/// foreign project, another session or an ended one must all come back empty, +/// while the session that really claimed the baton at SessionStart sees it. +#[tokio::test] +async fn a_session_start_claim_is_confirmed_only_to_its_own_live_session() { + use ai_memory_core::{HandoffState, NewSession, SessionId}; + + let tmp = tempfile::tempdir().unwrap(); + let store = Store::open(tmp.path()).unwrap(); + let (ws, proj) = scope(&store).await; + let other_proj = store + .writer + .get_or_create_project(ws, "other-app".to_string(), None) + .await + .unwrap(); + + let begin = |id: SessionId| NewSession { + occurred_at: None, + id, + workspace_id: ws, + project_id: proj, + agent_kind: AgentKind::ClaudeCode, + cwd: None, + actor_user: Some(operator("alice")), + }; + let receiver = SessionId::from_native("alice-claude-session"); + let sibling = SessionId::from_native("alice-other-session"); + for id in [receiver, sibling] { + store.writer.begin_session(begin(id)).await.unwrap(); + } + + let id = store + .writer + .insert_handoff(handoff(ws, proj, "alice's baton", Some("alice"))) + .await + .unwrap(); + let claimed = store + .writer + .accept_handoff(HandoffAcceptance { + accepting_session: Some(receiver), + ..acceptance( + id, + ws, + proj, + Some(operator("alice")), + filter_for("alice"), + None, + ) + }) + .await + .unwrap(); + assert!(claimed, "the receiving session claims the baton at start"); + + let lookup = |proj: ProjectId, session: SessionId, filter: OwnerFilter| { + let reader = store.reader.clone(); + async move { + reader + .handoff_claimed_by_live_session(ws, proj, session, filter) + .await + .unwrap() + } + }; + + // Control: the session that claimed it, as its owner. + assert_eq!( + lookup(proj, receiver, filter_for("alice")).await, + Some(id), + "the receiving session must be told its own SessionStart claimed the baton", + ); + assert_eq!( + lookup(proj, receiver, OwnerFilter::Any).await, + Some(id), + "root recovery sees the claim too", + ); + + // Bob names Alice's session: the owner filter still hides her row. + assert_eq!( + lookup(proj, receiver, filter_for("bob")).await, + None, + "a forged session id must not confirm another operator's claim", + ); + assert_eq!( + lookup(proj, receiver, OwnerFilter::Unattributed).await, + None, + "an unattributed caller must not see an owned claim", + ); + // The right session, the wrong project. + assert_eq!( + lookup(other_proj, receiver, filter_for("alice")).await, + None, + "a claim in one project must not be reported in another", + ); + // Alice again, from a session that claimed nothing. + assert_eq!( + lookup(proj, sibling, filter_for("alice")).await, + None, + "another session of the same operator did not receive this baton", + ); + + // Once the session ends there is no context the baton could be in. + store.writer.end_session(receiver, None).await.unwrap(); + let row = store.reader.handoff_by_id(id).await.unwrap().unwrap(); + assert_eq!( + (row.lifecycle.state, row.lifecycle.accepted_by_session), + (HandoffState::Accepted, Some(receiver)), + "the claim itself must survive the end, so only liveness can hide it", + ); + assert_eq!( + lookup(proj, receiver, filter_for("alice")).await, + None, + "an ended session must not be told the baton is in its context", + ); +} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 19c8d6cc..3858edb5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -456,7 +456,7 @@ invariants below. | `memory_explore` | read-only | LLM prose digest over the briefing snapshot, degrading to JSON without a provider. An optional `reasoning` tier (`minimal` (default) / `low` / `medium` / `high` / `max`) scales the digest's max-token budget off its base (16 000; same 1x/1.5x/2x/3x/4x mapping as `memory_query`); inert on the no-provider briefing-only path, and `minimal` is byte-identical. | | `memory_handoff_begin` | destructive | Open an owner-scoped handoff for the next agent; `shared=true` deliberately publishes it to the project. Optional `workspace` + `project` targets a named sibling workspace/project. | | `memory_handoff_list` | read-only | List open own/shared handoffs with inspectable body and identity fields; does not claim or expire. Root-only `any_owner=true` recovers across operators. Optional `workspace` + `project` targets a named sibling workspace/project. | -| `memory_handoff_accept` | destructive | Fetch + ack an open own/shared handoff. Pass `handoff_id` from `memory_handoff_list` to claim that exact row; omitting it still claims the latest eligible open handoff (automatic handoffs are cwd-matched). Root-only `any_owner=true` recovers across operators. Optional `workspace` + `project` targets a named sibling workspace/project. | +| `memory_handoff_accept` | destructive | Fetch + ack an open own/shared handoff. Pass `handoff_id` from `memory_handoff_list` to claim that exact row; omitting it still claims the latest eligible open handoff (automatic handoffs are cwd-matched). Root-only `any_owner=true` recovers across operators. Optional `workspace` + `project` targets a named sibling workspace/project. Returns `handoff` plus `status`: `claimed` (this call won it), `consumed_by_hook` (the calling session's own SessionStart claimed it, so it is already in context; needs the session id the hook claimed under, i.e. a session-aware client), or `none_pending`. | | `memory_handoff_cancel` | destructive | Mark an exact visible open handoff id expired when it was created by mistake; root-only `any_owner=true` recovers across operators. | | `memory_message_send` | destructive | Send a directed cross-project message into another project's inbox (V64). Requires `to_workspace` + `to_project`; the recipient must already exist (fail-closed, never created). Body is secret-scrubbed and size-capped. The one tool that crosses project isolation on purpose. | | `memory_message_list` | read-only | List pending mail for this project — `box="inbox"` (poppable, default) or `box="outbox"` (cancellable). Bodies are untrusted cross-project input. | diff --git a/docs/security-boundaries.md b/docs/security-boundaries.md index c1b9da75..ef786384 100644 --- a/docs/security-boundaries.md +++ b/docs/security-boundaries.md @@ -33,6 +33,7 @@ boundary not yet built. | 4d | Native session rebind / drifted-end is owner+agent+cwd CAS | `ai-memory-store/src/ops.rs` — `session.moved` rebind gated on `agent==OpenCode` + `ended_at IS NULL` + lexical `normalize_cwd(stored)==normalize_cwd(from)`; `expire_same_cwd_auto_handoffs` AND-gated on `owner_user IS ?4 AND id IS NOT ?5` and on the source not being another still-open session (row 4e, #883); drifted `SessionEnd` ends only same owner+agent+cwd | `ops.rs` `native_move_rebinds_live_session_only_from_its_current_cwd`, `native_move_rejects_foreign_owner_and_ignores_completed_replay`, `drifted_session_end_ends_only_the_same_actor_agent_and_cwd` (#865) | STRONG | | 4e | A live session's baton stays its own until the session is quiet | `ai-memory-store/src/ops.rs` - `expire_same_cwd_auto_handoffs` spares batons of other open sessions; `accept_handoff_in_transaction(.., busy_since)` re-checks inside the claim that the source is not an open session with observations after the cutoff, and its sweep `expire_superseded_auto_handoffs(.., busy_since)` spares a busy open session's baton (every open session's when `None`, the explicit MCP accept); `reader.rs` `startup_handoff` selects with the same cutoff; `ai-memory-hooks/src/router.rs` `LIVE_BATON_QUIET_PERIOD` | `multi_session.rs` `parallel_live_sessions_keep_their_own_checkpoint_batons`, `startup_claim_rechecks_the_source_and_sweeps_only_quiet_open_batons`; `router.rs` `opencode_parallel_live_batons_are_owned_and_wait_for_quiet` - each fails with its guard removed | STRONG | | 4f | Finalize-on-exit closes only a session the run provably owns (invariant #16) | `ai-memory-cli/src/commands/run.rs` `finalize_hookless_session` runs only for `harness.lacks_session_end_hook()` harnesses (Command Code, Kiro v2/v3, Antigravity); `own_native_session` returns a session only when it is named, chosen before the spawn, or linked under this run's own id in this checkout — a merely *discovered* native session (possibly a concurrent launch's) is never finalized | `run.rs` `a_session_linked_during_the_run_wins_over_discovery` — `own_native_session` returns `None` for the unlinked and other-checkout (`nested`) cases, so a foreign/concurrent session is refused; the named/linked control is finalized (#944) | STRONG | +| 4g | `memory_handoff_accept` `status` never points a caller at another session's baton | `ai-memory-store/src/reader.rs` `handoff_claimed_by_live_session` — the caller-supplied session id (header or native `_meta`, routing-only per row 6b) selects which claim to check, but the answer is bounded by the resolved `(workspace_id, project_id)`, the authenticated `handoff_owner_sql` owner filter, `state='accepted'`, and a still-open receiving session; `ai-memory-mcp/src/server.rs` `unclaimed_handoff` reports `consumed_by_hook` only on that match (and, for a requested `handoff_id`, only when it is the matched row), `none_pending` otherwise | `handoff_ownership.rs` `a_session_start_claim_is_confirmed_only_to_its_own_live_session` (forged session id under another operator, unattributed reader, foreign project, sibling session and ended session all empty; owner and root controls see the claim); `ai-memory-mcp` `tests/suite/handoff_identity.rs` `consumed_by_hook_is_reported_only_to_the_session_that_claimed_it` (over the production transport: a colleague forwarding the session id, the owner from another session, and a request with no session id all get `none_pending`; the claiming session gets `consumed_by_hook`) | STRONG | | 5a | Pages shared: `author_id` is never a read filter (invariant #16) | `ai-memory-store/src/reader.rs` `search_pages`/`page_body_by_ids` — `author_id` is an attribution JOIN only, never a WHERE term | `multi_session.rs` — a page with a **non-null** `author_id` (operator A) is readable by operator B in the same project | STRONG | | 5b | Page supersession (loser stays reachable) | `ai-memory-store/src/ops.rs` `upsert_page_in_tx` — demote `is_latest=0` (never delete) + `supersedes` chain | `multi_session.rs` `concurrent_writes_to_one_path_supersede_rather_than_destroy`; `retrieval_superseded.rs` | STRONG | | 6 | Active-project pointer (PerActor, no clobber) | `ai-memory-core/src/active_project.rs` `set_for`/`lookup_for` (fail-closed on `Mismatch`) | `active_project.rs` `parallel_harnesses_of_one_user_keep_separate_pointers`, `two_operators_never_read_each_others_pointer`, `a_session_mismatch_fails_closed_once_anything_has_been_keyed` | STRONG | diff --git a/docs/usage.md b/docs/usage.md index d9ec888f..3f21d79d 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -45,6 +45,15 @@ session never calls one, call `memory_handoff_list` then `memory_handoff_accept` with the listed `handoff_id`. Zero should do that on resume. Listing does not claim the row. +`memory_handoff_accept` says why it returned no handoff. Its `status` is +`claimed` when the call took one, `consumed_by_hook` when the calling +session's own SessionStart already did (the handoff is in that session's +context), and `none_pending` when nothing is left to claim. Only a client that +forwards its session id on MCP calls can be told `consumed_by_hook`: Claude +Code through `install-mcp --session-aware`, or OpenCode 2. Any other client +gets `none_pending` after the hook consumed the handoff, so its agent still +checks its context for the delivered block first. + On a server that distinguishes operators, handoffs belong to their creator by default: the next session for that operator sees their own plus deliberately shared rows, never a teammate's. Use `shared: true` on