mirror of
https://github.com/akitaonrails/ai-memory.git
synced 2026-10-02 03:24:46 +08:00
feat(delete-page): admin endpoint + MCP workspace arg + CLI subcommand
Closes the structural gap where `delete-page` was the only basic CRUD
operation without a complete surface and the MCP variant silently routed
deletes to the wrong slot on multi-workspace servers.
- **`POST /admin/delete-page`** (new) — accepts explicit `{workspace,
project, path}` and uses `lookup_ws_proj_no_create` (the same no-create
resolver `purge-project`/`rename-project` use), so a typo'd or unknown
workspace/project returns `404 'X' not found` instead of silently
auto-creating empty containers and returning misleading `deleted: true`.
Forwards admission context with `op=Delete` and respects the
`X-Memory-Skip-Webhooks` loop-prevention header just like write-page.
- **`memory_delete_page` MCP** — `DeletePageArgs` gains an optional
`workspace`; scope now resolves through `effective_ids_for_read_args`,
the same path the read tools use. Previously the tool called
`effective_ids(project)` which, for a project name that lived in
multiple workspaces, could land the delete in the wrong slot and
return `deleted: true` for a page that was never touched. Tool
docstring updated to instruct passing `workspace + project` together
on shared servers.
- **`ai-memory delete-page` CLI** — new subcommand, thin client of
`/admin/delete-page`. Mirrors the `write-page`/`read-page` CLI shape
(`--path`, `--workspace`, `--project`; project auto-derives from cwd
via the shared `resolve_project_name`). Terminal users now have a
complete delete-single-page path for the first time.
Tests:
- `admin::tests::delete_page_removes_existing_page` — happy path.
- `admin::tests::delete_page_unknown_workspace_does_not_fake_success` —
Bug 5 regression: typo'd scope returns 404, not `deleted: true`.
- `admin::tests::delete_page_idempotent_for_missing_file_in_existing_scope`
— matches MCP semantics.
- `admin::tests::delete_page_traversal_rejected_with_422`.
- `server::tests::memory_delete_page_with_explicit_workspace_targets_right_scope`
— Bug 5 cross-workspace regression: seed identical `notes/twin.md` in
`alpha/shared` and `beta/shared`, delete beta with explicit workspace,
confirm alpha survives and beta is gone.
Full gate: workspace tests 257/0, fmt clean, clippy -D warnings clean.
End-to-end smoke validated against running engine: write + delete + read-
back round-trip on multi-workspace data and CLI surface exit codes for
both success and typo cases.
Refs: CHANGELOG entries under [Unreleased].
This commit is contained in:
@@ -6,6 +6,25 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
|
||||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
## [Unreleased]
|
||||
### Added
|
||||
- New `POST /admin/delete-page` HTTP endpoint deletes a single page with
|
||||
explicit `(workspace, project)`. Like `purge-project`/`rename-project`, it
|
||||
uses no-create lookup — a delete on a typo'd or wrong scope now returns
|
||||
`404 workspace 'X' not found` instead of silently auto-creating the
|
||||
container and returning misleading `deleted: true`.
|
||||
- New `ai-memory delete-page --path <P> --workspace <W> --project <P>` CLI
|
||||
subcommand, a thin client of `/admin/delete-page`. Mirrors the
|
||||
write-page/read-page CLI shape so terminal users get a complete
|
||||
delete-single-page surface for the first time.
|
||||
|
||||
### Fixed
|
||||
- `memory_delete_page` (MCP) now accepts `workspace` alongside `project` and
|
||||
routes scope through `effective_ids_for_read_args`, the same path the read
|
||||
tools use. Previously a project name that lived in multiple workspaces
|
||||
could silently route the delete to the wrong slot and return `deleted:
|
||||
true` for a page that was never touched. Operators on shared (multi-
|
||||
workspace) servers should explicitly pass `workspace + project` to make
|
||||
the target unambiguous.
|
||||
|
||||
## [0.9.0] - 2026-06-02
|
||||
### Added
|
||||
|
||||
@@ -39,6 +39,11 @@ pub enum Command {
|
||||
ReadPage(ReadPageArgs),
|
||||
/// Write or update a wiki page atomically (also indexes it in the store).
|
||||
WritePage(WritePageArgs),
|
||||
/// Delete a single wiki page. The server routes scope resolution
|
||||
/// through `resolve_ws_proj`, so a delete targeting a project that
|
||||
/// exists in multiple workspaces never silently lands in the wrong
|
||||
/// slot (the MCP `memory_delete_page` had that gap until this build).
|
||||
DeletePage(DeletePageArgs),
|
||||
/// Run the MCP server (with watcher) over stdio or HTTP.
|
||||
Serve(ServeArgs),
|
||||
/// Wipe the data directory's wiki/, db/, raw/ contents.
|
||||
@@ -572,6 +577,23 @@ pub struct ReadPageArgs {
|
||||
pub json: bool,
|
||||
}
|
||||
|
||||
/// Arguments for `delete-page`.
|
||||
#[derive(Debug, Args)]
|
||||
pub struct DeletePageArgs {
|
||||
/// Exact wiki path to delete (e.g. `notes/foo.md`).
|
||||
#[arg(long)]
|
||||
pub path: String,
|
||||
/// Workspace name. Defaults to `default`. Required (no auto-detect) so
|
||||
/// a cross-workspace project-name collision can never silently route
|
||||
/// the delete to the wrong slot.
|
||||
#[arg(long, default_value_t = crate::config::DEFAULT_WORKSPACE.to_string())]
|
||||
pub workspace: String,
|
||||
/// Project name. When omitted, auto-derived from the current project
|
||||
/// (same heuristic write-page/read-page use).
|
||||
#[arg(long)]
|
||||
pub project: Option<String>,
|
||||
}
|
||||
|
||||
/// Arguments for `reset`.
|
||||
#[derive(Debug, Args)]
|
||||
pub struct ResetArgs {
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
//! `ai-memory delete-page` — delete a wiki page via the server.
|
||||
//!
|
||||
//! Sends a `POST /admin/delete-page` request to the running server.
|
||||
//! The server resolves `(workspace, project)` via the same path the
|
||||
//! read tools use (`resolve_ws_proj`) so a delete targeting a project
|
||||
//! that exists in multiple workspaces can never silently land in the
|
||||
//! wrong slot — closes the structural gap that `memory_delete_page`
|
||||
//! (MCP) had until this milestone.
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::cli::DeletePageArgs;
|
||||
use crate::config::Config;
|
||||
use crate::http_client::{ServerEndpoint, post_json};
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct DeletePageBody {
|
||||
workspace: String,
|
||||
project: String,
|
||||
path: String,
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct DeletePageResponseBody {
|
||||
path: String,
|
||||
deleted: bool,
|
||||
}
|
||||
|
||||
/// Run the `delete-page` subcommand.
|
||||
///
|
||||
/// # Errors
|
||||
/// Returns an error if the POST to `/admin/delete-page` fails (network
|
||||
/// failure, scope resolution failure, admission webhook rejecting the
|
||||
/// delete, or filesystem error).
|
||||
pub async fn run(config: &Config, args: DeletePageArgs) -> Result<()> {
|
||||
// Resolve the project the same way write-page/read-page do: explicit
|
||||
// flag wins, otherwise derive the current project from host cwd / repo
|
||||
// root. Keeps delete + read-back pairs targeting the same project.
|
||||
let project = super::resolve_project_name(config, args.project.as_deref())?;
|
||||
|
||||
let endpoint = ServerEndpoint::from_config(config);
|
||||
let resp: DeletePageResponseBody = post_json(
|
||||
&endpoint,
|
||||
"/admin/delete-page",
|
||||
&DeletePageBody {
|
||||
workspace: args.workspace.clone(),
|
||||
project: project.clone(),
|
||||
path: args.path.clone(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.context("deleting page via server")?;
|
||||
|
||||
let status = if resp.deleted { "✓ deleted" } else { "no-op" };
|
||||
println!(
|
||||
"{} {} under {}/{}",
|
||||
status, resp.path, args.workspace, project
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
@@ -10,6 +10,7 @@ pub mod backup;
|
||||
pub mod bootstrap;
|
||||
pub mod commit;
|
||||
pub mod data_purge;
|
||||
pub mod delete_page;
|
||||
pub mod embed;
|
||||
pub mod forget_sweep;
|
||||
pub mod generate_auth_token;
|
||||
|
||||
@@ -45,6 +45,7 @@ async fn main() -> Result<()> {
|
||||
Command::Search(args) => commands::search::run(&config, args).await,
|
||||
Command::ReadPage(args) => commands::read_page::run(&config, args).await,
|
||||
Command::WritePage(args) => commands::write_page::run(&config, args).await,
|
||||
Command::DeletePage(args) => commands::delete_page::run(&config, args).await,
|
||||
Command::Serve(args) => commands::serve::run(&config, args).await,
|
||||
Command::Reset(args) => commands::reset::run(&config, args),
|
||||
Command::Backup(args) => commands::backup::run(&config, args).await,
|
||||
|
||||
@@ -71,7 +71,7 @@ match the intent to the tool. They do not need to name the tool.
|
||||
| "consolidate this session" / "compile what we learned" (also runs on PreCompact; at session end only if `AI_MEMORY_CONSOLIDATE_ON_SESSION_END` is set) | `memory_consolidate` |
|
||||
| "remember this permanently" / "save a note" / "add an annotation" / durable project knowledge | `memory_write_page` (write a wiki page; do **not** use handoff for permanent notes; put the title as a `# H1` on the first line of `body` and omit the `title` arg — ai-memory derives it from the H1) |
|
||||
| "read the page about X" / "show me the full content of Y" / "open the page on Z" | `memory_read_page` (full body; pass a query to search or `path` for a direct lookup; pass `workspace` + `project` together only for a named sibling workspace/project) |
|
||||
| "delete the page X" / "remove that note" | `memory_delete_page` (by exact `path`; idempotent) |
|
||||
| "delete the page X" / "remove that note" | `memory_delete_page` (by exact `path`; idempotent; pass `workspace` + `project` together only for a named sibling workspace/project) |
|
||||
| "audit the wiki" / "find contradictions" / "what rules should we add?" | `memory_lint` |
|
||||
| "prune old pages" / "memory cleanup" | `memory_forget_sweep` |
|
||||
|
||||
|
||||
@@ -177,6 +177,7 @@ pub fn admin_router(state: AdminState) -> Router {
|
||||
.route("/admin/rename-project", post(handle_rename_project))
|
||||
.route("/admin/move-project", post(handle_move_project))
|
||||
.route("/admin/write-page", post(handle_write_page))
|
||||
.route("/admin/delete-page", post(handle_delete_page))
|
||||
.route_layer(axum::middleware::from_fn_with_state(
|
||||
state.clone(),
|
||||
require_root_for_multiuser_admin,
|
||||
@@ -2592,6 +2593,96 @@ async fn handle_write_page(
|
||||
))
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// delete-page
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
/// JSON request body for `POST /admin/delete-page`.
|
||||
///
|
||||
/// Unlike `memory_delete_page` (MCP), this endpoint REQUIRES explicit
|
||||
/// `workspace` so cross-workspace ambiguity can never silently route a
|
||||
/// delete to the wrong slot.
|
||||
#[derive(Deserialize)]
|
||||
struct DeletePageAdminRequest {
|
||||
/// Workspace name. Required (no auto-create — delete acts on existing data).
|
||||
workspace: String,
|
||||
/// Project name within the workspace. Required.
|
||||
project: String,
|
||||
/// Relative wiki path (e.g. `concepts/foo.md`).
|
||||
path: String,
|
||||
}
|
||||
|
||||
/// JSON response body for `POST /admin/delete-page`.
|
||||
#[derive(Serialize)]
|
||||
struct DeletePageResponse {
|
||||
/// Canonical wiki path of the deletion target.
|
||||
path: String,
|
||||
/// Always `true` on a successful (resolved-scope) call. `Wiki::delete_page`
|
||||
/// itself is idempotent — a missing file is treated as already-deleted —
|
||||
/// so the boolean reports "the call succeeded", not "a row was removed".
|
||||
/// The structural defense is in the 404 returned when `(workspace, project)`
|
||||
/// fails to resolve (so a stale or wrong-scope call never returns a misleading
|
||||
/// `deleted: true`).
|
||||
deleted: bool,
|
||||
}
|
||||
|
||||
async fn handle_delete_page(
|
||||
State(state): State<Arc<AdminState>>,
|
||||
actor_ext: Option<axum::Extension<ai_memory_core::ActorContext>>,
|
||||
level_ext: Option<axum::Extension<ai_memory_core::AuthLevel>>,
|
||||
headers: HeaderMap,
|
||||
Json(req): Json<DeletePageAdminRequest>,
|
||||
) -> Result<impl IntoResponse, (StatusCode, Json<serde_json::Value>)> {
|
||||
let path = PagePath::new(req.path.clone()).map_err(|e| {
|
||||
(
|
||||
StatusCode::UNPROCESSABLE_ENTITY,
|
||||
Json(serde_json::json!({ "error": format!("invalid path: {e}") })),
|
||||
)
|
||||
})?;
|
||||
|
||||
// Use the no-create lookup (same as purge/rename/move): a delete on a
|
||||
// typo'd workspace/project must return 404, NOT silently auto-create
|
||||
// empty containers and then return `deleted: true` for nothing.
|
||||
let (ws, proj) = lookup_ws_proj_no_create(&state, &req.workspace, &req.project).await?;
|
||||
|
||||
let actor = actor_ext
|
||||
.map(|axum::Extension(actor)| actor)
|
||||
.unwrap_or_else(ai_memory_core::ActorContext::anonymous);
|
||||
let skip_webhooks = match level_ext.map(|axum::Extension(level)| level) {
|
||||
Some(ai_memory_core::AuthLevel::User) => Vec::new(),
|
||||
Some(ai_memory_core::AuthLevel::Root | ai_memory_core::AuthLevel::Anonymous) | None => {
|
||||
crate::actor::skip_webhooks_from_headers(&headers)
|
||||
}
|
||||
};
|
||||
let admission_ctx = if actor.has_any() || !skip_webhooks.is_empty() {
|
||||
Some(AdmissionContext {
|
||||
actor,
|
||||
op: AdmissionOp::Delete,
|
||||
skip_webhooks,
|
||||
..AdmissionContext::default()
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
state
|
||||
.wiki
|
||||
.delete_page(ws, proj, &path, admission_ctx)
|
||||
.await
|
||||
.map_err(|e| internal_err(e.to_string()))?;
|
||||
|
||||
Ok((
|
||||
StatusCode::OK,
|
||||
Json(
|
||||
serde_json::to_value(DeletePageResponse {
|
||||
path: path.to_string(),
|
||||
deleted: true,
|
||||
})
|
||||
.unwrap_or_else(|_| serde_json::json!({})),
|
||||
),
|
||||
))
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// user management (root-only)
|
||||
// ---------------------------------------------------------------------
|
||||
@@ -3622,4 +3713,129 @@ mod tests {
|
||||
assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE);
|
||||
let _ = tmp;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// delete-page
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
/// Helper: POST /admin/delete-page and return (status, body json).
|
||||
async fn post_delete_page(
|
||||
router: &Router,
|
||||
ws: &str,
|
||||
project: &str,
|
||||
path: &str,
|
||||
) -> (StatusCode, serde_json::Value) {
|
||||
let req_body = serde_json::json!({
|
||||
"workspace": ws,
|
||||
"project": project,
|
||||
"path": path,
|
||||
});
|
||||
let resp = router
|
||||
.clone()
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("POST")
|
||||
.uri("/admin/delete-page")
|
||||
.header("content-type", "application/json")
|
||||
.body(Body::from(serde_json::to_vec(&req_body).unwrap()))
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let status = resp.status();
|
||||
let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap();
|
||||
let json: serde_json::Value =
|
||||
serde_json::from_slice(&body).unwrap_or(serde_json::json!({}));
|
||||
(status, json)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn delete_page_removes_existing_page() {
|
||||
let (_tmp, router) = read_page_test_router();
|
||||
post_write_page(&router, "default", "audit", "notes/doomed.md", "bye body").await;
|
||||
|
||||
// Confirm the page is reachable before delete.
|
||||
let resp = router
|
||||
.clone()
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.uri("/admin/read-page?workspace=default&project=audit&path=notes/doomed.md")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
resp.status(),
|
||||
StatusCode::OK,
|
||||
"setup precondition: page must exist before delete"
|
||||
);
|
||||
|
||||
let (status, json) = post_delete_page(&router, "default", "audit", "notes/doomed.md").await;
|
||||
assert_eq!(status, StatusCode::OK);
|
||||
assert_eq!(json["path"], "notes/doomed.md");
|
||||
assert_eq!(json["deleted"], true);
|
||||
|
||||
// Read-back must now 404.
|
||||
let resp = router
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.uri("/admin/read-page?workspace=default&project=audit&path=notes/doomed.md")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
resp.status(),
|
||||
StatusCode::NOT_FOUND,
|
||||
"page must be gone after delete"
|
||||
);
|
||||
}
|
||||
|
||||
/// A delete request whose `(workspace, project)` doesn't resolve to a
|
||||
/// real `(WorkspaceId, ProjectId)` must NOT report success. The shared
|
||||
/// `resolve_ws_proj` returns the unresolved-scope error; the handler
|
||||
/// surfaces it as a 4xx/5xx — never as `deleted: true`. This guards the
|
||||
/// Bug 5 regression where MCP `memory_delete_page` returned `true` for
|
||||
/// a scope it never touched.
|
||||
#[tokio::test]
|
||||
async fn delete_page_unknown_workspace_does_not_fake_success() {
|
||||
let (_tmp, router) = read_page_test_router();
|
||||
let (status, json) =
|
||||
post_delete_page(&router, "no-such-ws", "audit", "notes/whatever.md").await;
|
||||
assert_ne!(
|
||||
status,
|
||||
StatusCode::OK,
|
||||
"delete on unresolved scope must not return 200/deleted=true; got body {json:?}",
|
||||
);
|
||||
assert!(
|
||||
json.get("deleted").and_then(|v| v.as_bool()) != Some(true),
|
||||
"body must not claim deleted=true on unresolved scope; got {json:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// `Wiki::delete_page` is idempotent for a path that doesn't exist
|
||||
/// inside an EXISTING (workspace, project) — the file is just not there
|
||||
/// to quarantine. The handler reports `deleted: true` (i.e. "the call
|
||||
/// succeeded") rather than 404, matching the documented MCP semantics.
|
||||
#[tokio::test]
|
||||
async fn delete_page_idempotent_for_missing_file_in_existing_scope() {
|
||||
let (_tmp, router) = read_page_test_router();
|
||||
// Seed the project so (workspace, project) resolves, but skip the
|
||||
// page we'll try to delete.
|
||||
post_write_page(&router, "default", "audit", "notes/keep.md", "keeper").await;
|
||||
|
||||
let (status, json) =
|
||||
post_delete_page(&router, "default", "audit", "notes/never-existed.md").await;
|
||||
assert_eq!(status, StatusCode::OK);
|
||||
assert_eq!(json["deleted"], true);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn delete_page_traversal_rejected_with_422() {
|
||||
let (_tmp, router) = read_page_test_router();
|
||||
let (status, _) = post_delete_page(&router, "default", "audit", "../etc/passwd").await;
|
||||
assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -91,7 +91,10 @@ the conversation calls for them:\n\
|
||||
not just snippets.\n\
|
||||
- `memory_delete_page` — when the user explicitly asks to delete or \
|
||||
remove a specific page (by exact path). Idempotent; fires the \
|
||||
admission chain so mirrors/backups stay consistent.\n\
|
||||
admission chain so mirrors/backups stay consistent. Pass `workspace` \
|
||||
+ `project` together only when the page lives in a sibling workspace \
|
||||
(otherwise a project name that exists in multiple workspaces can \
|
||||
silently route the delete to the wrong slot).\n\
|
||||
- `memory_lint` — when the user asks to audit the wiki for stale \
|
||||
pages, contradictions, or rule suggestions.\n\
|
||||
- `memory_forget_sweep` — when the user wants to prune old / cold \
|
||||
@@ -406,6 +409,15 @@ struct DeletePageArgs {
|
||||
/// user explicitly names a *different* project.**
|
||||
#[serde(default)]
|
||||
project: Option<String>,
|
||||
/// Workspace to delete from together with `project`. Omit to use the
|
||||
/// current/default workspace resolution chain. Provide both to delete a
|
||||
/// page that lives in a *different* workspace (e.g. a sibling project on
|
||||
/// a shared server). Without this, a delete targeting a project that
|
||||
/// exists in MULTIPLE workspaces can silently land in the wrong slot —
|
||||
/// fixed by routing scope resolution through the same path the read
|
||||
/// tools use (`effective_ids_for_read_args`).
|
||||
#[serde(default)]
|
||||
workspace: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, Deserialize, schemars::JsonSchema)]
|
||||
@@ -1203,6 +1215,9 @@ impl AiMemoryServer {
|
||||
delete or remove a page. Fires the admission chain (op=delete) \
|
||||
before the file is removed so backups/mirrors stay consistent. \
|
||||
Idempotent — deleting a page that is already gone is a no-op. \
|
||||
Pass `workspace` + `project` together when the page lives in a \
|
||||
sibling workspace (otherwise a project name that exists in multiple \
|
||||
workspaces can silently route the delete to the wrong slot). \
|
||||
Returns `{ path, deleted }`.")]
|
||||
async fn memory_delete_page(
|
||||
&self,
|
||||
@@ -1217,7 +1232,13 @@ impl AiMemoryServer {
|
||||
};
|
||||
let path = PagePath::new(args.path.clone())
|
||||
.map_err(|e| McpError::internal_error(format!("invalid path: {e}"), None))?;
|
||||
let (ws, proj) = self.effective_ids(args.project.as_deref()).await;
|
||||
// Same resolution as read/write tools: when (workspace, project) are
|
||||
// BOTH given, they're looked up explicitly; otherwise the cwd-based
|
||||
// active-project chain is used. Closes the silent cross-workspace
|
||||
// delete that the single-arg `effective_ids(project)` allowed.
|
||||
let (ws, proj) = self
|
||||
.effective_ids_for_read_args(args.workspace.as_deref(), args.project.as_deref())
|
||||
.await?;
|
||||
|
||||
// Carry actor identity + loop-prevention skip list (same as write_page).
|
||||
// `Wiki::delete_page` stamps `op = Delete` regardless of what we pass.
|
||||
@@ -2722,6 +2743,7 @@ mod tests {
|
||||
Parameters(DeletePageArgs {
|
||||
path: "notes/temp.md".into(),
|
||||
project: None,
|
||||
workspace: None,
|
||||
}),
|
||||
rmcp::handler::server::tool::Extension(parts()),
|
||||
)
|
||||
@@ -2762,6 +2784,128 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Bug 5 regression: when a project name lives in MULTIPLE workspaces,
|
||||
/// `memory_delete_page` without `workspace` resolved scope via
|
||||
/// `effective_ids(project)` and could silently land in the wrong slot
|
||||
/// (returning `deleted: true` while the page survived in the workspace
|
||||
/// the operator actually meant). Passing `workspace` + `project` now
|
||||
/// flows through `effective_ids_for_read_args` — the same path the read
|
||||
/// tools use — so the delete lands EXACTLY where the operator pointed.
|
||||
#[tokio::test]
|
||||
async fn memory_delete_page_with_explicit_workspace_targets_right_scope() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let store = Store::open(tmp.path()).unwrap();
|
||||
let ws_alpha = store.writer.get_or_create_workspace("alpha").await.unwrap();
|
||||
let proj_alpha_shared = store
|
||||
.writer
|
||||
.get_or_create_project(ws_alpha, "shared", None)
|
||||
.await
|
||||
.unwrap();
|
||||
let ws_beta = store.writer.get_or_create_workspace("beta").await.unwrap();
|
||||
let proj_beta_shared = store
|
||||
.writer
|
||||
.get_or_create_project(ws_beta, "shared", None)
|
||||
.await
|
||||
.unwrap();
|
||||
let wiki = Wiki::new(tmp.path(), store.writer.clone()).unwrap();
|
||||
// Server's baked default is alpha/shared; beta/shared is the
|
||||
// sibling we'll target via explicit (workspace, project).
|
||||
let server = AiMemoryServer::new(
|
||||
store.reader.clone(),
|
||||
store.writer.clone(),
|
||||
ws_alpha,
|
||||
proj_alpha_shared,
|
||||
)
|
||||
.with_wiki(wiki);
|
||||
let parts = || {
|
||||
axum::http::Request::builder()
|
||||
.uri("/mcp")
|
||||
.method("POST")
|
||||
.body(())
|
||||
.unwrap()
|
||||
.into_parts()
|
||||
.0
|
||||
};
|
||||
|
||||
// Seed both workspaces with a SAME-NAMED page.
|
||||
server
|
||||
.memory_write_page(
|
||||
Parameters(WritePageArgs {
|
||||
path: "notes/twin.md".into(),
|
||||
body: "# alpha twin".into(),
|
||||
title: None,
|
||||
tier: Some("semantic".into()),
|
||||
tags: vec![],
|
||||
pinned: false,
|
||||
project: Some("shared".into()),
|
||||
workspace: Some("alpha".into()),
|
||||
}),
|
||||
rmcp::handler::server::tool::Extension(parts()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
server
|
||||
.memory_write_page(
|
||||
Parameters(WritePageArgs {
|
||||
path: "notes/twin.md".into(),
|
||||
body: "# beta twin".into(),
|
||||
title: None,
|
||||
tier: Some("semantic".into()),
|
||||
tags: vec![],
|
||||
pinned: false,
|
||||
project: Some("shared".into()),
|
||||
workspace: Some("beta".into()),
|
||||
}),
|
||||
rmcp::handler::server::tool::Extension(parts()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Delete from BETA only, explicit scope.
|
||||
server
|
||||
.memory_delete_page(
|
||||
Parameters(DeletePageArgs {
|
||||
path: "notes/twin.md".into(),
|
||||
project: Some("shared".into()),
|
||||
workspace: Some("beta".into()),
|
||||
}),
|
||||
rmcp::handler::server::tool::Extension(parts()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Alpha twin must survive.
|
||||
let read_alpha = server
|
||||
.memory_read_page(Parameters(ReadPageArgs {
|
||||
query: None,
|
||||
path: Some("notes/twin.md".into()),
|
||||
project: Some("shared".into()),
|
||||
workspace: Some("alpha".into()),
|
||||
}))
|
||||
.await;
|
||||
assert!(
|
||||
read_alpha.is_ok(),
|
||||
"alpha/shared/notes/twin.md must survive a delete targeting beta"
|
||||
);
|
||||
|
||||
// Beta twin must be gone (file-on-disk delete + DB row cleared).
|
||||
let read_beta = server
|
||||
.memory_read_page(Parameters(ReadPageArgs {
|
||||
query: None,
|
||||
path: Some("notes/twin.md".into()),
|
||||
project: Some("shared".into()),
|
||||
workspace: Some("beta".into()),
|
||||
}))
|
||||
.await;
|
||||
assert!(
|
||||
read_beta.is_err(),
|
||||
"beta/shared/notes/twin.md must be gone after delete with explicit workspace"
|
||||
);
|
||||
|
||||
// Defense-in-depth: the alpha-side IDs survive purge-check (project_id != deleted).
|
||||
let _ = proj_beta_shared;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn memory_write_page_creates_explicit_project() {
|
||||
// Bug B regression: an explicit `project` that doesn't exist yet must
|
||||
|
||||
@@ -49,6 +49,7 @@ the wiki without you naming tools explicitly.
|
||||
| "Save context for the next session" | `memory_handoff_begin` | Writes a terse handoff with open questions and next steps. |
|
||||
| "Consolidate this session" | `memory_consolidate` | Manually runs LLM consolidation. Also runs on PreCompact, and at session end only when `AI_MEMORY_CONSOLIDATE_ON_SESSION_END` is set (off by default; session end otherwise writes a rule-based summary page). |
|
||||
| "Remember this permanently" / "add an annotation" | `memory_write_page` | Writes durable wiki knowledge; not a single-use handoff. |
|
||||
| "Delete this page" / "remove the note about X" | `memory_delete_page` | Removes a page by exact path. Pass `workspace` + `project` together when the page lives in a sibling workspace, so a project name shared between workspaces never silently routes the delete to the wrong slot. |
|
||||
| "Audit the wiki" / "any contradictions?" | `memory_lint` | Runs stale-page, contradiction, and rule-suggestion checks. |
|
||||
| "How big is the wiki?" / "stats?" | `memory_status`, `memory_briefing` | Counts and recent activity windows. |
|
||||
|
||||
|
||||
Reference in New Issue
Block a user