From 4eda22f8ceeaded74ceee059c8453c38df081eb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tr=E1=BA=A7n=20Quang=20=C4=90=C3=A3ng?= Date: Sun, 17 May 2026 12:59:14 +0000 Subject: [PATCH] fix(serve): honor --palace; fix MCP unknown-tool message; document sweep Three release-blocker findings from end-to-end testing: 1. mpr --palace serve silently ignored --palace and always used the global ~/.config/mempalace/config.json. Thread palace_arg through run_server and let it override config.palace_path so users connecting their AI client to a project palace can do so without re-init'ing the global config. ~/ expansion mirrors resolve_palace_path. 2. tools/call for an unknown tool name returned JSON-RPC -32601 with message='tools/call' (echoing the method name, not the bad tool). Switch to -32602 (invalid_params, which is what a bad 'name' param actually is) and surface 'Unknown tool: ' so MCP clients get an actionable error. 3. mpr sweep had no description in --help. Add a clap doc comment so it stops looking like an undocumented hidden command. Adds 4 regression tests: - test_unknown_tool_returns_invalid_params_with_tool_name - test_resolve_palace_override_expands_tilde_home - test_resolve_palace_override_absolute_path_passthrough - test_resolve_palace_override_relative_path_passthrough cargo fmt clean, clippy -D warnings 0, 424 tests pass. --- crates/core/src/cli.rs | 4 +- crates/core/src/mcp_server.rs | 70 ++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/crates/core/src/cli.rs b/crates/core/src/cli.rs index 8e7334b..c774506 100644 --- a/crates/core/src/cli.rs +++ b/crates/core/src/cli.rs @@ -259,7 +259,9 @@ enum Commands { read_only: bool, }, + /// Re-ingest a file or directory of mined drawers into the palace (idempotent). Sweep { + /// File or directory to sweep into the palace. target: PathBuf, #[arg(long)] palace: Option, @@ -1962,7 +1964,7 @@ pub fn run() -> Result<()> { } Commands::Mcp => cmd_mcp(palace_arg), Commands::Serve { read_only } => { - crate::mcp_server::run_server(*read_only)?; + crate::mcp_server::run_server(palace_arg, *read_only)?; } Commands::Sweep { target, palace } => cmd_sweep(target, palace.as_deref())?, } diff --git a/crates/core/src/mcp_server.rs b/crates/core/src/mcp_server.rs index 532bcbf..97c97f0 100644 --- a/crates/core/src/mcp_server.rs +++ b/crates/core/src/mcp_server.rs @@ -318,9 +318,10 @@ fn make_dispatch(state: Arc) -> impl Fn(String, JsonObject) -> DynResu "mempalace_graph_stats" => tool_graph_stats(&state, args), "mempalace_diary_read" => tool_diary_read(&state, args), "mempalace_diary_write" => tool_diary_write(&state, args), - _ => Err(ErrorData::method_not_found::< - rmcp::model::CallToolRequestMethod, - >()), + other => Err(ErrorData::invalid_params( + format!("Unknown tool: {}", other), + None, + )), } }) as DynResult } @@ -1266,8 +1267,11 @@ fn kg_path(state: &AppState) -> std::path::PathBuf { // Run entry point // --------------------------------------------------------------------------- -pub fn run_server(read_only: bool) -> anyhow::Result<()> { - let config = crate::Config::load()?; +pub fn run_server(palace_override: Option<&str>, read_only: bool) -> anyhow::Result<()> { + let mut config = crate::Config::load()?; + if let Some(p) = palace_override { + config.palace_path = resolve_palace_override(p); + } let server = MempalaceServer::new(AppState::new(config, read_only)?); let (stdin, stdout) = stdio(); let rt = Runtime::new()?; @@ -1279,6 +1283,15 @@ pub fn run_server(read_only: bool) -> anyhow::Result<()> { Ok(()) } +fn resolve_palace_override(raw: &str) -> std::path::PathBuf { + if let Some(rest) = raw.strip_prefix("~/") { + if let Some(home) = std::env::var_os("HOME") { + return std::path::PathBuf::from(home).join(rest); + } + } + std::path::PathBuf::from(raw) +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -2494,6 +2507,53 @@ mod tests { ); } + #[test] + fn test_unknown_tool_returns_invalid_params_with_tool_name() { + // Regression: previously this returned -32601 with message="tools/call", + // which echoed the JSON-RPC method instead of naming the bad tool. + // It should now be -32602 with message="Unknown tool: ". + let state = test_state(); + let err = dispatch(&state, "definitely_not_a_real_tool", json!({})) + .expect_err("unknown tool name should error"); + assert_eq!(err.code.0, -32602); + let message = err.message.as_ref(); + assert!( + message.contains("Unknown tool"), + "message should mention 'Unknown tool', got: {message}" + ); + assert!( + message.contains("definitely_not_a_real_tool"), + "message should name the bogus tool, got: {message}" + ); + // Must not echo the JSON-RPC method name as the message. + assert!( + !message.starts_with("tools/call"), + "message should not echo the JSON-RPC method, got: {message}" + ); + } + + #[test] + fn test_resolve_palace_override_expands_tilde_home() { + std::env::set_var("HOME", "/tmp/fake_home_42"); + let resolved = resolve_palace_override("~/palace"); + assert_eq!( + resolved, + std::path::PathBuf::from("/tmp/fake_home_42/palace") + ); + } + + #[test] + fn test_resolve_palace_override_absolute_path_passthrough() { + let resolved = resolve_palace_override("/var/lib/mp_palace"); + assert_eq!(resolved, std::path::PathBuf::from("/var/lib/mp_palace")); + } + + #[test] + fn test_resolve_palace_override_relative_path_passthrough() { + let resolved = resolve_palace_override("./relative/palace"); + assert_eq!(resolved, std::path::PathBuf::from("./relative/palace")); + } + #[test] fn test_add_drawer_accepts_unknown_custom_metadata_keys() { // `mempalace_add_drawer` uses `#[serde(flatten)] custom_metadata`,