From 8eddc7d787386a756f1ab1882f0b88aa93deccd7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 18 Sep 2026 07:13:21 +0000 Subject: [PATCH 1/6] test(mcp): prove tracedecay_rename_symbol behavior Co-authored-by: Zack Jackson --- .../mcp_handler_test/rename_symbol_test.rs | 490 +++++++++++++----- 1 file changed, 355 insertions(+), 135 deletions(-) diff --git a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs index bcac85a1d2..b43c3d29fd 100644 --- a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs +++ b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs @@ -16,6 +16,95 @@ use std::path::Path; use std::time::Duration; use tracedecay_mcp::ToolResult; +const PRICING_BEFORE: &str = r#"//! pricing +pub struct LineItem { + pub unit_price: u64, + pub quantity: u32, +} + +/// Grand total in cents. +pub fn compute_grand_total(items: &[LineItem]) -> u64 { + let mut total = 0u64; + for item in items { + total += item.unit_price * item.quantity as u64; + } + total +} + +pub fn tally(items: &[LineItem]) -> u64 { + compute_grand_total(items) +} +"#; + +const PRICING_AFTER: &str = r#"//! pricing +pub struct LineItem { + pub unit_price: u64, + pub quantity: u32, +} + +/// Grand total in cents. +pub fn calculate_total_cents(items: &[LineItem]) -> u64 { + let mut total = 0u64; + for item in items { + total += item.unit_price * item.quantity as u64; + } + total +} + +pub fn tally(items: &[LineItem]) -> u64 { + calculate_total_cents(items) +} +"#; + +/// Single-hunk preview the dry run must return for `PRICING_BEFORE` → `PRICING_AFTER`. +const PRICING_DIFF: &str = "\ +--- src/pricing.rs +@@ -5,14 +5,14 @@ + } + + /// Grand total in cents. +-pub fn compute_grand_total(items: &[LineItem]) -> u64 { +- let mut total = 0u64; +- for item in items { +- total += item.unit_price * item.quantity as u64; +- } +- total +-} +- +-pub fn tally(items: &[LineItem]) -> u64 { +- compute_grand_total(items) ++pub fn calculate_total_cents(items: &[LineItem]) -> u64 { ++ let mut total = 0u64; ++ for item in items { ++ total += item.unit_price * item.quantity as u64; ++ } ++ total ++} ++ ++pub fn tally(items: &[LineItem]) -> u64 { ++ calculate_total_cents(items) + } +"; + +const ORDERS_BEFORE: &str = r#"//! orders +use crate::pricing::LineItem; + +pub fn quantity(items: &[LineItem]) -> usize { + items.len() +} +"#; + +const ORDERS_CROSS_MODULE: &str = r#"//! orders +use crate::pricing::{LineItem, compute_grand_total}; + +pub fn order_total(items: &[LineItem]) -> u64 { + compute_grand_total(items) +} +"#; + +const BLOCKED_MESSAGE: &str = + "rename blocked by stale, ambiguous, unsupported, or colliding evidence"; + /// A pricing crate whose caller shares the target's module, so both declaration /// and call are extraction-attested by the production graph. The nested module /// deliberately contains no target spelling; cross-module unresolved names are @@ -33,32 +122,61 @@ async fn rename_fixture(project: &Path) { ) .unwrap(); fs::write(project.join("src/nested/mod.rs"), "pub mod orders;\n").unwrap(); - fs::write( - project.join("src/pricing.rs"), - "//! pricing\n\ - pub struct LineItem {\n pub unit_price: u64,\n pub quantity: u32,\n}\n\n\ - /// Grand total in cents.\n\ - pub fn compute_grand_total(items: &[LineItem]) -> u64 {\n\ - \x20 let mut total = 0u64;\n\ - \x20 for item in items {\n\ - \x20 total += item.unit_price * item.quantity as u64;\n\ - \x20 }\n\ - \x20 total\n\ - }\n\n\ - pub fn tally(items: &[LineItem]) -> u64 {\n\ - \x20 compute_grand_total(items)\n\ - }\n", - ) - .unwrap(); - fs::write( - project.join("src/nested/orders.rs"), - "//! orders\n\ - use crate::pricing::LineItem;\n\n\ - pub fn quantity(items: &[LineItem]) -> usize {\n\ - \x20 items.len()\n\ - }\n", - ) - .unwrap(); + fs::write(project.join("src/pricing.rs"), PRICING_BEFORE).unwrap(); + fs::write(project.join("src/nested/orders.rs"), ORDERS_BEFORE).unwrap(); +} + +fn assert_workspace_unchanged(project: &Path) { + assert_eq!( + fs::read_to_string(project.join("src/pricing.rs")).unwrap(), + PRICING_BEFORE + ); + assert_eq!( + fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(), + ORDERS_BEFORE + ); +} + +/// Caller-visible site fields. Identity digests and byte offsets are omitted +/// because they are addresses, not the rename the caller observes. +fn visible_sites(payload: &Value) -> Vec { + payload["sites"] + .as_array() + .map(|sites| { + sites + .iter() + .map(|site| { + json!({ + "kind": site["kind"], + "disposition": site["disposition"], + "file": site["file"], + "line": site["line"], + "expected_bytes": site["expected_bytes"], + "replacement_bytes": site["replacement_bytes"], + "reason": site["reason"], + }) + }) + .collect() + }) + .unwrap_or_default() +} + +fn visible_hazards(payload: &Value) -> Vec { + payload["hazards"] + .as_array() + .map(|hazards| { + hazards + .iter() + .map(|hazard| { + json!({ + "kind": hazard["kind"], + "blocking": hazard["blocking"], + "message": hazard["message"], + }) + }) + .collect() + }) + .unwrap_or_default() } /// Runs `tracedecay_rename_preview` for `symbol` and returns the exact node @@ -183,41 +301,78 @@ async fn test_rename_symbol_dry_run_default_reports_plan_and_writes_nothing() { rename_fixture(project).await; let (cg, _env) = init_test_project(project).await; - let before_pricing = fs::read_to_string(project.join("src/pricing.rs")).unwrap(); - let before_orders = fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(); - let node = preview_node(&cg, "compute_grand_total").await; + assert_eq!(node["name"], "compute_grand_total"); + assert_eq!(node["kind"], "function"); + assert_eq!(node["file"], "src/pricing.rs"); + assert_eq!( + node["qualified_name"], + "src/pricing.rs::compute_grand_total" + ); + let p = preview_rename(&cg, &node, "calculate_total_cents").await; assert_eq!(p["success"], true, "payload: {p}"); assert_eq!(p["dry_run"], true, "default must be a dry run: {p}"); + assert_eq!( + p["message"], "dry run. Nothing written; preview only (rename previewed)", + "payload: {p}" + ); + assert_eq!(p["symbol"], "src/pricing.rs::compute_grand_total", "{p}"); + assert_eq!(p["old_name"], "compute_grand_total"); + assert_eq!(p["new_name"], "calculate_total_cents"); assert_eq!( p["preview_digest"], p["expected_state"], "the accepted preview must echo the exact candidate-state CAS digest: {p}" ); - let files: Vec<&str> = p["files"] - .as_array() - .unwrap() - .iter() - .map(|f| f["file"].as_str().unwrap()) - .collect(); - assert!(files.contains(&"src/pricing.rs"), "files: {files:?}\n{p}"); - assert_eq!(files.len(), 1, "only graph-bound files may be edited: {p}"); - assert!( - p["reference_count"].as_u64().unwrap() >= 1, - "the caller must be graph-attested: {p}" + assert_eq!( + p["files"], + json!([{ "file": "src/pricing.rs", "replaced_count": 2 }]), + "{p}" ); - let diff = p["diff"].as_str().unwrap(); - assert!(diff.contains("calculate_total_cents"), "diff: {diff}"); - - // The dry run wrote nothing. + assert_eq!(p["reference_count"], 1, "{p}"); assert_eq!( - fs::read_to_string(project.join("src/pricing.rs")).unwrap(), - before_pricing + p["dispositions"], + json!({ "changed": 2, "unchanged": 0, "skipped": 0, "blocked": 0 }), + "{p}" ); assert_eq!( - fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(), - before_orders + visible_sites(&p), + json!([ + { + "kind": "declaration", + "disposition": "changed", + "file": "src/pricing.rs", + "line": 8, + "expected_bytes": "compute_grand_total", + "replacement_bytes": "calculate_total_cents", + "reason": "exact graph-bound occurrence" + }, + { + "kind": "resolved_call", + "disposition": "changed", + "file": "src/pricing.rs", + "line": 17, + "expected_bytes": "compute_grand_total", + "replacement_bytes": "calculate_total_cents", + "reason": "exact graph-bound occurrence" + } + ]), + "{p}" + ); + assert_eq!( + p["impact"], + json!({ + "callers": ["src/pricing.rs::tally"], + "reexports": [], + "affected_files": ["src/pricing.rs"], + "affected_tests": [] + }), + "{p}" ); + assert_eq!(p["diff"], PRICING_DIFF, "diff: {}", p["diff"]); + assert_eq!(visible_hazards(&p), Vec::::new(), "{p}"); + + assert_workspace_unchanged(project); } #[tokio::test] @@ -241,26 +396,23 @@ async fn test_rename_symbol_apply_rewrites_declaration_and_callers() { .unwrap(); let p = rename_payload(&result); assert_eq!(p["success"], true, "payload: {p}"); - assert_ne!(p["dry_run"], json!(true), "payload: {p}"); + assert_eq!(p["replayed"], false, "payload: {p}"); assert_eq!(p["message"], "rename applied", "payload: {p}"); - - let pricing = fs::read_to_string(project.join("src/pricing.rs")).unwrap(); - assert!( - pricing.contains("pub fn calculate_total_cents"), - "declaration renamed: {pricing}" - ); - assert!( - !pricing.contains("compute_grand_total"), - "old name gone from declaration: {pricing}" + assert_eq!(p["old_name"], "compute_grand_total"); + assert_eq!(p["new_name"], "calculate_total_cents"); + assert_eq!( + p["files"], + json!([{ "file": "src/pricing.rs", "replaced_count": 2 }]), + "{p}" ); - let orders = fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(); - assert!( - pricing.contains("calculate_total_cents(items)"), - "caller renamed: {pricing}" + + assert_eq!( + fs::read_to_string(project.join("src/pricing.rs")).unwrap(), + PRICING_AFTER ); - assert!( - !orders.contains("compute_grand_total"), - "unrelated module remains free of the old name: {orders}" + assert_eq!( + fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(), + ORDERS_BEFORE ); // An exact idempotent replay returns the durable receipt without attempting @@ -271,6 +423,23 @@ async fn test_rename_symbol_apply_rewrites_declaration_and_callers() { let p2 = rename_payload(&result2); assert_eq!(p2["success"], true, "idempotent replay: {p2}"); assert_eq!(p2["replayed"], true, "idempotent replay: {p2}"); + assert_eq!( + p2["operation"], "use-case.application.source-edit.rename-symbol", + "{p2}" + ); + assert_eq!(p2["files"], json!(["src/pricing.rs"]), "{p2}"); + assert_eq!(p2["change_count"], 2, "{p2}"); + assert_eq!(p2["finding_count"], 0, "{p2}"); + assert_eq!(p2["durable_metadata_only"], true, "{p2}"); + assert_eq!( + p2["message"], "source edit completed; detailed edit output was not retained", + "{p2}" + ); + assert_eq!( + fs::read_to_string(project.join("src/pricing.rs")).unwrap(), + PRICING_AFTER, + "replay must not rewrite the applied source" + ); } #[tokio::test] @@ -307,6 +476,21 @@ async fn test_rename_symbol_stale_tree_refuses_before_writing() { .unwrap(); let p = rename_payload(&result); assert_eq!(p["success"], false, "stale evidence must refuse: {p}"); + assert_eq!( + p["message"], BLOCKED_MESSAGE, + "stale evidence must refuse: {p}" + ); + assert_eq!( + visible_hazards(&p), + json!([ + { + "kind": "stale_evidence", + "blocking": true, + "message": "src/pricing.rs no longer matches the admitted graph generation" + } + ]), + "{p}" + ); assert_eq!( p["effect"]["execution"]["termination"], "failed", "source drift must terminate before the effect: {p}" @@ -339,8 +523,6 @@ async fn test_rename_symbol_denies_invalid_and_colliding_names() { rename_fixture(project).await; let (cg, _env) = init_test_project(project).await; - let before_pricing = fs::read_to_string(project.join("src/pricing.rs")).unwrap(); - let before_orders = fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(); let node = preview_node(&cg, "compute_grand_total").await; // A denied preview has no acceptance to apply. @@ -350,13 +532,20 @@ async fn test_rename_symbol_denies_invalid_and_colliding_names() { .unwrap(); let p = rename_payload(&result); assert_eq!(p["success"], false, "invalid name must be denied: {p}"); - assert!( - p["hazards"] - .as_array() - .is_some_and(|hazards| hazards.iter().any(|hazard| { - hazard["kind"] == "invalid_identifier" && hazard["blocking"] == true - })), - "denial must retain the typed invalid-identifier hazard: {p}" + assert_eq!(p["dry_run"], true, "{p}"); + assert_eq!(p["new_name"], "not an identifier"); + assert_eq!( + p["message"], "rename requires valid old and new identifiers", + "{p}" + ); + assert_eq!( + visible_hazards(&p), + json!([{ + "kind": "invalid_identifier", + "blocking": true, + "message": "rename requires valid old and new identifiers" + }]), + "{p}" ); // Identical to the old name. @@ -366,32 +555,53 @@ async fn test_rename_symbol_denies_invalid_and_colliding_names() { .unwrap(); let p = rename_payload(&result); assert_eq!(p["success"], false, "same-name rename must be denied: {p}"); + assert_eq!( + p["message"], "new name is identical to the bound old name", + "{p}" + ); + assert_eq!( + visible_hazards(&p), + json!([{ + "kind": "invalid_identifier", + "blocking": true, + "message": "new name is identical to the bound old name" + }]), + "{p}" + ); // Collides with an identifier already present in a touched file. + let collision_message = "`tally` already occurs in src/pricing.rs; collision, shadowing, or changed resolution is possible"; let collision = rename_args(&node, "tally"); let result = handle_tool_call(&cg, "tracedecay_rename_symbol", collision, None, None) .await .unwrap(); let p = rename_payload(&result); assert_eq!(p["success"], false, "collision must be denied: {p}"); - assert!( - p["hazards"] - .as_array() - .is_some_and(|hazards| hazards.iter().any(|hazard| { - hazard["kind"] == "namespace_collision" && hazard["blocking"] == true - })), - "denial must retain the typed namespace-collision hazard: {p}" - ); - - // Every denial wrote nothing. - assert_eq!( - fs::read_to_string(project.join("src/pricing.rs")).unwrap(), - before_pricing - ); + assert_eq!(p["message"], BLOCKED_MESSAGE, "{p}"); + assert_eq!(p["new_name"], "tally"); assert_eq!( - fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(), - before_orders + visible_hazards(&p), + json!([ + { + "kind": "namespace_collision", + "blocking": true, + "message": collision_message + }, + { + "kind": "shadowing", + "blocking": true, + "message": collision_message + }, + { + "kind": "changed_resolution", + "blocking": true, + "message": collision_message + } + ]), + "{p}" ); + + assert_workspace_unchanged(project); } #[tokio::test] @@ -400,17 +610,7 @@ async fn test_rename_symbol_blocks_unresolved_cross_module_spelling() { let project_root = dir.path().join("project"); let project = project_root.as_path(); rename_fixture(project).await; - fs::write( - project.join("src/nested/orders.rs"), - "//! orders\n\ - use crate::pricing::{LineItem, compute_grand_total};\n\n\ - pub fn order_total(items: &[LineItem]) -> u64 {\n\ - \x20 compute_grand_total(items)\n\ - }\n", - ) - .unwrap(); - let before_pricing = fs::read_to_string(project.join("src/pricing.rs")).unwrap(); - let before_orders = fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(); + fs::write(project.join("src/nested/orders.rs"), ORDERS_CROSS_MODULE).unwrap(); let (cg, _env) = init_test_project(project).await; let node = preview_node(&cg, "compute_grand_total").await; @@ -426,27 +626,61 @@ async fn test_rename_symbol_blocks_unresolved_cross_module_spelling() { let payload = rename_payload(&result); assert_eq!(payload["success"], false, "unresolved spelling: {payload}"); - assert!( - payload["hazards"].as_array().is_some_and(|hazards| hazards - .iter() - .any(|hazard| { hazard["kind"] == "ambiguous_symbol" && hazard["blocking"] == true })), - "unresolved spelling must be a blocking graph hazard: {payload}" + assert_eq!(payload["dry_run"], true, "{payload}"); + assert_eq!(payload["message"], BLOCKED_MESSAGE, "{payload}"); + assert_eq!( + visible_sites(&payload) + .into_iter() + .filter(|site| site["file"] == "src/nested/orders.rs") + .collect::>(), + json!([ + { + "kind": "unresolved_text", + "disposition": "blocked", + "file": "src/nested/orders.rs", + "line": 2, + "expected_bytes": "compute_grand_total", + "replacement_bytes": "compute_grand_total", + "reason": "unresolved code spelling may bind this symbol" + }, + { + "kind": "unresolved_text", + "disposition": "blocked", + "file": "src/nested/orders.rs", + "line": 5, + "expected_bytes": "compute_grand_total", + "replacement_bytes": "compute_grand_total", + "reason": "unresolved code spelling may bind this symbol" + } + ]), + "{payload}" ); - assert!( - payload["sites"] - .as_array() - .is_some_and(|sites| sites.iter().any(|site| { - site["file"] == "src/nested/orders.rs" && site["kind"] == "unresolved_text" - })), - "hazard must identify the unresolved cross-module site: {payload}" + assert_eq!( + visible_hazards(&payload) + .into_iter() + .filter(|hazard| hazard["kind"] == "ambiguous_symbol") + .collect::>(), + json!([ + { + "kind": "ambiguous_symbol", + "blocking": true, + "message": "unresolved code spelling may bind this symbol" + }, + { + "kind": "ambiguous_symbol", + "blocking": true, + "message": "unresolved code spelling may bind this symbol" + } + ]), + "{payload}" ); assert_eq!( fs::read_to_string(project.join("src/pricing.rs")).unwrap(), - before_pricing + PRICING_BEFORE ); assert_eq!( fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(), - before_orders + ORDERS_CROSS_MODULE ); } @@ -463,8 +697,6 @@ async fn test_rename_symbol_publication_failure_preserves_preimage() { rename_fixture(project).await; let (cg, _env) = init_test_project(project).await; - let before_pricing = fs::read_to_string(project.join("src/pricing.rs")).unwrap(); - let before_orders = fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(); let node = preview_node(&cg, "compute_grand_total").await; let preview = preview_rename(&cg, &node, "calculate_total_cents").await; @@ -484,31 +716,19 @@ async fn test_rename_symbol_publication_failure_preserves_preimage() { // Restore permissions before asserting so the tempdir always cleans up. fs::set_permissions(&src_dir, writable).unwrap(); - // The apply failed, either as a typed error or a failed durable effect, - // and never reported success. - match apply { - Ok(result) => { - let p = rename_payload(&result); - assert_ne!(p["success"], json!(true), "payload: {p}"); - } - Err(error) => { - let message = error.to_string(); - assert!( - message.contains("rename aborted") || message.contains("reconciliation"), - "unexpected failure shape: {message}" - ); - } - } + // Publication refusal is a typed tool result, not a successful rename. + let result = apply.expect("publication failure must still return a tool result"); + let p = rename_payload(&result); + assert_eq!(p["success"], false, "payload: {p}"); - // The workspace is byte-identical to the preimage. assert_eq!( fs::read_to_string(project.join("src/pricing.rs")).unwrap(), - before_pricing, + PRICING_BEFORE, "declaration file must be untouched" ); assert_eq!( fs::read_to_string(project.join("src/nested/orders.rs")).unwrap(), - before_orders, + ORDERS_BEFORE, "published caller must be rolled back to its preimage" ); } From 41b2a131cb8130d56d413313d7af61a69e174d44 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 18 Sep 2026 08:37:03 +0000 Subject: [PATCH 2/6] test(mcp): call rename symbol over tools/call The proof now drives tracedecay_rename_symbol through the production MCP server's tools/call, not the test-only dispatcher. Co-authored-by: Zack Jackson --- .../mcp_handler_test/rename_symbol_test.rs | 171 ++++++++++-------- 1 file changed, 96 insertions(+), 75 deletions(-) diff --git a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs index b43c3d29fd..5ab1bfe5ca 100644 --- a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs +++ b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs @@ -1,20 +1,19 @@ -//! `tracedecay_rename_symbol`, apply-grade rename bound to preview evidence. +//! `tracedecay_rename_symbol` as a host calls it: one `tools/call` on the +//! production MCP server the daemon composition mounts. //! //! The preview (`tracedecay_rename_preview`) reports the exact node identity; //! the apply consumes it and must succeed only while that evidence still //! matches the live tree: staleness refuses, invalid targets are denied, and a -//! partial-failure apply restores every already-written preimage. +//! publication failure leaves every file byte-identical to its preimage. -use crate::support::*; use crate::support::{ - handle_production_source_edit_tool_call as handle_tool_call, - init_production_source_edit_project as init_test_project, + ProductionSourceEditFixture, extract_first_json_content, + init_production_source_edit_project as init_test_project, test_temp_dir, }; use serde_json::{Value, json}; use std::fs; use std::path::Path; use std::time::Duration; -use tracedecay_mcp::ToolResult; const PRICING_BEFORE: &str = r#"//! pricing pub struct LineItem { @@ -179,23 +178,64 @@ fn visible_hazards(payload: &Value) -> Vec { .unwrap_or_default() } +/// One production `tools/call`. JSON is the public `format` a host requests +/// when it wants the structured payload; a protocol error is not a rename. +async fn call_json( + fixture: &ProductionSourceEditFixture, + tool_name: &str, + arguments: Value, +) -> Value { + let response = tools_call(fixture, tool_name, arguments) + .await + .unwrap_or_else(|error| panic!("{tool_name} did not answer tools/call: {error}")); + let result = response + .result + .as_ref() + .unwrap_or_else(|| panic!("{tool_name} returned no tools/call result: {response:?}")); + extract_first_json_content(result) +} + +async fn tools_call( + fixture: &ProductionSourceEditFixture, + tool_name: &str, + mut arguments: Value, +) -> Result { + if let Some(object) = arguments.as_object_mut() { + object + .entry("format".to_owned()) + .or_insert_with(|| json!("json")); + } + let response = fixture + .harness + .call_tool(&fixture.project_root, tool_name, arguments) + .await + .map_err(|error| error.to_string())?; + if let Some(error) = &response.error { + return Err(format!("{error:?}")); + } + Ok(response) +} + /// Runs `tracedecay_rename_preview` for `symbol` and returns the exact node /// identity the apply must be bound to. -async fn preview_node(cg: &ProductionSourceEditFixture, symbol: &str) -> Value { +async fn preview_node(fixture: &ProductionSourceEditFixture, symbol: &str) -> Value { let deadline = tokio::time::Instant::now() + Duration::from_secs(20); let search = loop { - match handle_tool_call( - cg, + match tools_call( + fixture, "tracedecay_find_exact_symbol", json!({ "name": symbol, "limit": 20 }), - None, - None, ) .await { - Ok(result) => break result, + Ok(response) => { + let result = response.result.as_ref().unwrap_or_else(|| { + panic!("exact symbol lookup returned no tools/call result: {response:?}") + }); + break extract_first_json_content(result); + } Err(error) - if error.to_string().contains("code-graph-unavailable") + if error.contains("code-graph-unavailable") && tokio::time::Instant::now() < deadline => { tokio::time::sleep(Duration::from_millis(10)).await; @@ -203,7 +243,6 @@ async fn preview_node(cg: &ProductionSourceEditFixture, symbol: &str) -> Value { Err(error) => panic!("exact symbol lookup failed: {error}"), } }; - let search: Value = serde_json::from_str(extract_text(&search.value)).unwrap(); let node_id = search["matches"] .as_array() .and_then(|matches| { @@ -216,21 +255,17 @@ async fn preview_node(cg: &ProductionSourceEditFixture, symbol: &str) -> Value { .unwrap_or_else(|| { panic!("symbol {symbol:?} missing from production code graph: {search}") }); - let result = handle_tool_call( - cg, + let payload = call_json( + fixture, "tracedecay_rename_preview", json!({ "node_id": node_id }), - None, - None, ) - .await - .unwrap(); - let payload = extract_first_json_content(&result.value); + .await; let node = payload["node"].clone(); - assert!(node["id"].is_string(), "preview node identity: {payload}"); - assert!( - node["qualified_name"].is_string(), - "preview must report the qualified name the apply binds to: {payload}" + assert_eq!(node["id"], node_id, "preview node identity: {payload}"); + assert_eq!( + node["name"], symbol, + "preview must report the looked-up symbol: {payload}" ); node } @@ -247,17 +282,17 @@ fn rename_args(node: &Value, new_name: &str) -> Value { }) } -async fn preview_rename(cg: &ProductionSourceEditFixture, node: &Value, new_name: &str) -> Value { - let result = handle_tool_call( - cg, +async fn preview_rename( + fixture: &ProductionSourceEditFixture, + node: &Value, + new_name: &str, +) -> Value { + let payload = call_json( + fixture, "tracedecay_rename_symbol", rename_args(node, new_name), - None, - None, ) - .await - .unwrap(); - let payload = rename_payload(&result); + .await; assert_eq!(payload["success"], true, "rename preview: {payload}"); assert_eq!(payload["dry_run"], true, "rename preview: {payload}"); assert_eq!( @@ -288,11 +323,6 @@ fn accepted_apply_args(node: &Value, new_name: &str, preview: &Value, key: &str) }) } -fn rename_payload(result: &ToolResult) -> Value { - let text = extract_text(&result.value); - serde_json::from_str(text).unwrap_or_else(|e| panic!("rename payload not JSON: {e}\n{text}")) -} - #[tokio::test] async fn test_rename_symbol_dry_run_default_reports_plan_and_writes_nothing() { let dir = test_temp_dir(); @@ -391,10 +421,7 @@ async fn test_rename_symbol_apply_rewrites_declaration_and_callers() { &preview, "rename.apply-and-replay", ); - let result = handle_tool_call(&cg, "tracedecay_rename_symbol", args.clone(), None, None) - .await - .unwrap(); - let p = rename_payload(&result); + let p = call_json(&cg, "tracedecay_rename_symbol", args.clone()).await; assert_eq!(p["success"], true, "payload: {p}"); assert_eq!(p["replayed"], false, "payload: {p}"); assert_eq!(p["message"], "rename applied", "payload: {p}"); @@ -417,10 +444,7 @@ async fn test_rename_symbol_apply_rewrites_declaration_and_callers() { // An exact idempotent replay returns the durable receipt without attempting // to reinterpret the now-retired node identity. - let result2 = handle_tool_call(&cg, "tracedecay_rename_symbol", args, None, None) - .await - .unwrap(); - let p2 = rename_payload(&result2); + let p2 = call_json(&cg, "tracedecay_rename_symbol", args).await; assert_eq!(p2["success"], true, "idempotent replay: {p2}"); assert_eq!(p2["replayed"], true, "idempotent replay: {p2}"); assert_eq!( @@ -471,10 +495,7 @@ async fn test_rename_symbol_stale_tree_refuses_before_writing() { &preview, "rename.stale-tree", ); - let result = handle_tool_call(&cg, "tracedecay_rename_symbol", args, None, None) - .await - .unwrap(); - let p = rename_payload(&result); + let p = call_json(&cg, "tracedecay_rename_symbol", args).await; assert_eq!(p["success"], false, "stale evidence must refuse: {p}"); assert_eq!( p["message"], BLOCKED_MESSAGE, @@ -527,10 +548,7 @@ async fn test_rename_symbol_denies_invalid_and_colliding_names() { // A denied preview has no acceptance to apply. let invalid = rename_args(&node, "not an identifier"); - let result = handle_tool_call(&cg, "tracedecay_rename_symbol", invalid, None, None) - .await - .unwrap(); - let p = rename_payload(&result); + let p = call_json(&cg, "tracedecay_rename_symbol", invalid).await; assert_eq!(p["success"], false, "invalid name must be denied: {p}"); assert_eq!(p["dry_run"], true, "{p}"); assert_eq!(p["new_name"], "not an identifier"); @@ -550,10 +568,7 @@ async fn test_rename_symbol_denies_invalid_and_colliding_names() { // Identical to the old name. let same = rename_args(&node, "compute_grand_total"); - let result = handle_tool_call(&cg, "tracedecay_rename_symbol", same, None, None) - .await - .unwrap(); - let p = rename_payload(&result); + let p = call_json(&cg, "tracedecay_rename_symbol", same).await; assert_eq!(p["success"], false, "same-name rename must be denied: {p}"); assert_eq!( p["message"], "new name is identical to the bound old name", @@ -572,10 +587,7 @@ async fn test_rename_symbol_denies_invalid_and_colliding_names() { // Collides with an identifier already present in a touched file. let collision_message = "`tally` already occurs in src/pricing.rs; collision, shadowing, or changed resolution is possible"; let collision = rename_args(&node, "tally"); - let result = handle_tool_call(&cg, "tracedecay_rename_symbol", collision, None, None) - .await - .unwrap(); - let p = rename_payload(&result); + let p = call_json(&cg, "tracedecay_rename_symbol", collision).await; assert_eq!(p["success"], false, "collision must be denied: {p}"); assert_eq!(p["message"], BLOCKED_MESSAGE, "{p}"); assert_eq!(p["new_name"], "tally"); @@ -614,16 +626,12 @@ async fn test_rename_symbol_blocks_unresolved_cross_module_spelling() { let (cg, _env) = init_test_project(project).await; let node = preview_node(&cg, "compute_grand_total").await; - let result = handle_tool_call( + let payload = call_json( &cg, "tracedecay_rename_symbol", rename_args(&node, "calculate_total_cents"), - None, - None, ) - .await - .unwrap(); - let payload = rename_payload(&result); + .await; assert_eq!(payload["success"], false, "unresolved spelling: {payload}"); assert_eq!(payload["dry_run"], true, "{payload}"); @@ -701,9 +709,15 @@ async fn test_rename_symbol_publication_failure_preserves_preimage() { let preview = preview_rename(&cg, &node, "calculate_total_cents").await; // `src/` read-only blocks the temp-file publish of `src/pricing.rs`. + // The guard restores write permission even if the tool call panics, so + // the temp directory can still be removed. let src_dir = project.join("src"); let writable = fs::metadata(&src_dir).unwrap().permissions(); fs::set_permissions(&src_dir, fs::Permissions::from_mode(0o555)).unwrap(); + let _restore = RestoreWrite { + path: src_dir, + permissions: writable, + }; let args = accepted_apply_args( &node, @@ -711,14 +725,8 @@ async fn test_rename_symbol_publication_failure_preserves_preimage() { &preview, "rename.publication-failure", ); - let apply = handle_tool_call(&cg, "tracedecay_rename_symbol", args, None, None).await; - - // Restore permissions before asserting so the tempdir always cleans up. - fs::set_permissions(&src_dir, writable).unwrap(); - // Publication refusal is a typed tool result, not a successful rename. - let result = apply.expect("publication failure must still return a tool result"); - let p = rename_payload(&result); + let p = call_json(&cg, "tracedecay_rename_symbol", args).await; assert_eq!(p["success"], false, "payload: {p}"); assert_eq!( @@ -732,3 +740,16 @@ async fn test_rename_symbol_publication_failure_preserves_preimage() { "published caller must be rolled back to its preimage" ); } + +#[cfg(unix)] +struct RestoreWrite { + path: std::path::PathBuf, + permissions: fs::Permissions, +} + +#[cfg(unix)] +impl Drop for RestoreWrite { + fn drop(&mut self) { + fs::set_permissions(&self.path, self.permissions.clone()).unwrap(); + } +} From 9776017eb284f99f8011d60242293b6d71fa85fb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 18 Sep 2026 08:56:51 +0000 Subject: [PATCH 3/6] test(mcp): compare rename sites as JSON arrays Vec and serde_json::Value do not compare, so the proof assertions never compiled. Compare the observed arrays to literal JSON. Co-authored-by: Zack Jackson --- .../mcp_handler_test/rename_symbol_test.rs | 100 ++++++++++-------- 1 file changed, 57 insertions(+), 43 deletions(-) diff --git a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs index 5ab1bfe5ca..3071eb75dd 100644 --- a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs +++ b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs @@ -138,44 +138,60 @@ fn assert_workspace_unchanged(project: &Path) { /// Caller-visible site fields. Identity digests and byte offsets are omitted /// because they are addresses, not the rename the caller observes. -fn visible_sites(payload: &Value) -> Vec { - payload["sites"] - .as_array() - .map(|sites| { - sites - .iter() - .map(|site| { - json!({ - "kind": site["kind"], - "disposition": site["disposition"], - "file": site["file"], - "line": site["line"], - "expected_bytes": site["expected_bytes"], - "replacement_bytes": site["replacement_bytes"], - "reason": site["reason"], +fn visible_sites(payload: &Value) -> Value { + Value::Array( + payload["sites"] + .as_array() + .map(|sites| { + sites + .iter() + .map(|site| { + json!({ + "kind": site["kind"], + "disposition": site["disposition"], + "file": site["file"], + "line": site["line"], + "expected_bytes": site["expected_bytes"], + "replacement_bytes": site["replacement_bytes"], + "reason": site["reason"], + }) }) - }) - .collect() - }) - .unwrap_or_default() + .collect() + }) + .unwrap_or_default(), + ) } -fn visible_hazards(payload: &Value) -> Vec { - payload["hazards"] - .as_array() - .map(|hazards| { - hazards - .iter() - .map(|hazard| { - json!({ - "kind": hazard["kind"], - "blocking": hazard["blocking"], - "message": hazard["message"], +fn visible_hazards(payload: &Value) -> Value { + Value::Array( + payload["hazards"] + .as_array() + .map(|hazards| { + hazards + .iter() + .map(|hazard| { + json!({ + "kind": hazard["kind"], + "blocking": hazard["blocking"], + "message": hazard["message"], + }) }) - }) - .collect() - }) - .unwrap_or_default() + .collect() + }) + .unwrap_or_default(), + ) +} + +fn matching(items: &Value, pred: impl Fn(&Value) -> bool) -> Value { + Value::Array( + items + .as_array() + .into_iter() + .flatten() + .filter(|item| pred(item)) + .cloned() + .collect(), + ) } /// One production `tools/call`. JSON is the public `format` a host requests @@ -400,7 +416,7 @@ async fn test_rename_symbol_dry_run_default_reports_plan_and_writes_nothing() { "{p}" ); assert_eq!(p["diff"], PRICING_DIFF, "diff: {}", p["diff"]); - assert_eq!(visible_hazards(&p), Vec::::new(), "{p}"); + assert_eq!(visible_hazards(&p), json!([]), "{p}"); assert_workspace_unchanged(project); } @@ -637,10 +653,9 @@ async fn test_rename_symbol_blocks_unresolved_cross_module_spelling() { assert_eq!(payload["dry_run"], true, "{payload}"); assert_eq!(payload["message"], BLOCKED_MESSAGE, "{payload}"); assert_eq!( - visible_sites(&payload) - .into_iter() - .filter(|site| site["file"] == "src/nested/orders.rs") - .collect::>(), + matching(&visible_sites(&payload), |site| { + site["file"] == "src/nested/orders.rs" + }), json!([ { "kind": "unresolved_text", @@ -664,10 +679,9 @@ async fn test_rename_symbol_blocks_unresolved_cross_module_spelling() { "{payload}" ); assert_eq!( - visible_hazards(&payload) - .into_iter() - .filter(|hazard| hazard["kind"] == "ambiguous_symbol") - .collect::>(), + matching(&visible_hazards(&payload), |hazard| { + hazard["kind"] == "ambiguous_symbol" + }), json!([ { "kind": "ambiguous_symbol", From 387a6154be95da88c7084d0c1d8cb2b30264ce2f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 18 Sep 2026 09:04:00 +0000 Subject: [PATCH 4/6] test(mcp): lock observed rename tools/call results The production tools/call payload is the proof: no trailing newline on the preview diff, every stale hazard, a resolved cross-module call beside a blocked import, and the durable replay receipt. Co-authored-by: Zack Jackson --- .../mcp_handler_test/rename_symbol_test.rs | 137 +++++++++++++----- 1 file changed, 103 insertions(+), 34 deletions(-) diff --git a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs index 3071eb75dd..385f39e238 100644 --- a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs +++ b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs @@ -82,8 +82,7 @@ const PRICING_DIFF: &str = "\ + +pub fn tally(items: &[LineItem]) -> u64 { + calculate_total_cents(items) - } -"; + }"; const ORDERS_BEFORE: &str = r#"//! orders use crate::pricing::LineItem; @@ -182,18 +181,6 @@ fn visible_hazards(payload: &Value) -> Value { ) } -fn matching(items: &Value, pred: impl Fn(&Value) -> bool) -> Value { - Value::Array( - items - .as_array() - .into_iter() - .flatten() - .filter(|item| pred(item)) - .cloned() - .collect(), - ) -} - /// One production `tools/call`. JSON is the public `format` a host requests /// when it wants the structured payload; a protocol error is not a rename. async fn call_json( @@ -463,18 +450,32 @@ async fn test_rename_symbol_apply_rewrites_declaration_and_callers() { let p2 = call_json(&cg, "tracedecay_rename_symbol", args).await; assert_eq!(p2["success"], true, "idempotent replay: {p2}"); assert_eq!(p2["replayed"], true, "idempotent replay: {p2}"); + assert_eq!(p2["failed"], false, "idempotent replay: {p2}"); assert_eq!( - p2["operation"], "use-case.application.source-edit.rename-symbol", + p2["message"], "source edit completed; detailed edit output was not retained", "{p2}" ); - assert_eq!(p2["files"], json!(["src/pricing.rs"]), "{p2}"); - assert_eq!(p2["change_count"], 2, "{p2}"); - assert_eq!(p2["finding_count"], 0, "{p2}"); - assert_eq!(p2["durable_metadata_only"], true, "{p2}"); assert_eq!( - p2["message"], "source edit completed; detailed edit output was not retained", + p2["effect"]["payload"]["operation"], "use-case.application.source-edit.rename-symbol", + "{p2}" + ); + assert_eq!( + p2["effect"]["payload"]["files"], + json!(["src/pricing.rs"]), + "{p2}" + ); + assert_eq!(p2["effect"]["payload"]["change_count"], 2, "{p2}"); + assert_eq!(p2["effect"]["payload"]["finding_count"], 0, "{p2}"); + assert_eq!( + p2["effect"]["payload"]["durable_metadata_only"], true, "{p2}" ); + assert_eq!(p2["effect"]["payload"]["success"], true, "{p2}"); + assert_eq!( + p2["effect"]["execution"]["termination"], "partial", + "a replay reports the stored partial receipt: {p2}" + ); + assert_eq!(p2["effect"]["receipt"]["outcome"], "partial", "{p2}"); assert_eq!( fs::read_to_string(project.join("src/pricing.rs")).unwrap(), PRICING_AFTER, @@ -524,6 +525,41 @@ async fn test_rename_symbol_stale_tree_refuses_before_writing() { "kind": "stale_evidence", "blocking": true, "message": "src/pricing.rs no longer matches the admitted graph generation" + }, + { + "kind": "stale_evidence", + "blocking": true, + "message": "target graph evidence no longer resolves in src/pricing.rs" + }, + { + "kind": "stale_evidence", + "blocking": true, + "message": "target graph evidence no longer resolves in src/pricing.rs" + }, + { + "kind": "ambiguous_symbol", + "blocking": true, + "message": "unresolved code spelling may bind this symbol" + }, + { + "kind": "stale_evidence", + "blocking": true, + "message": "rename apply requires the exact accepted preview identity, plan, repository, and graph revisions" + } + ]), + "{p}" + ); + assert_eq!( + visible_sites(&p), + json!([ + { + "kind": "unresolved_text", + "disposition": "blocked", + "file": "src/pricing.rs", + "line": 17, + "expected_bytes": "compute_grand_total", + "replacement_bytes": "compute_grand_total", + "reason": "unresolved code spelling may bind this symbol" } ]), "{p}" @@ -652,10 +688,32 @@ async fn test_rename_symbol_blocks_unresolved_cross_module_spelling() { assert_eq!(payload["success"], false, "unresolved spelling: {payload}"); assert_eq!(payload["dry_run"], true, "{payload}"); assert_eq!(payload["message"], BLOCKED_MESSAGE, "{payload}"); + assert_eq!(payload["reference_count"], 2, "{payload}"); + assert_eq!( + payload["files"], + json!([ + { "file": "src/nested/orders.rs", "replaced_count": 1 }, + { "file": "src/pricing.rs", "replaced_count": 2 } + ]), + "{payload}" + ); assert_eq!( - matching(&visible_sites(&payload), |site| { - site["file"] == "src/nested/orders.rs" + payload["dispositions"], + json!({ "changed": 3, "unchanged": 0, "skipped": 0, "blocked": 1 }), + "{payload}" + ); + assert_eq!( + payload["impact"], + json!({ + "callers": ["src/nested/orders.rs::order_total", "src/pricing.rs::tally"], + "reexports": [], + "affected_files": ["src/nested/orders.rs", "src/pricing.rs"], + "affected_tests": [] }), + "{payload}" + ); + assert_eq!( + visible_sites(&payload), json!([ { "kind": "unresolved_text", @@ -667,27 +725,38 @@ async fn test_rename_symbol_blocks_unresolved_cross_module_spelling() { "reason": "unresolved code spelling may bind this symbol" }, { - "kind": "unresolved_text", - "disposition": "blocked", + "kind": "resolved_call", + "disposition": "changed", "file": "src/nested/orders.rs", "line": 5, "expected_bytes": "compute_grand_total", - "replacement_bytes": "compute_grand_total", - "reason": "unresolved code spelling may bind this symbol" + "replacement_bytes": "calculate_total_cents", + "reason": "exact graph-bound occurrence" + }, + { + "kind": "declaration", + "disposition": "changed", + "file": "src/pricing.rs", + "line": 8, + "expected_bytes": "compute_grand_total", + "replacement_bytes": "calculate_total_cents", + "reason": "exact graph-bound occurrence" + }, + { + "kind": "resolved_call", + "disposition": "changed", + "file": "src/pricing.rs", + "line": 17, + "expected_bytes": "compute_grand_total", + "replacement_bytes": "calculate_total_cents", + "reason": "exact graph-bound occurrence" } ]), "{payload}" ); assert_eq!( - matching(&visible_hazards(&payload), |hazard| { - hazard["kind"] == "ambiguous_symbol" - }), + visible_hazards(&payload), json!([ - { - "kind": "ambiguous_symbol", - "blocking": true, - "message": "unresolved code spelling may bind this symbol" - }, { "kind": "ambiguous_symbol", "blocking": true, From b757257f1f0eb37973ee5d267f282e6f412eec38 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 18 Sep 2026 10:08:04 +0000 Subject: [PATCH 5/6] ci: rerun checks after cancelled queue The ready-for-review run waited an hour for a runner and was cancelled before any step started. Co-authored-by: Zack Jackson From 6188c257541dde62ec81b2c655e5576a7ca1383e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 18 Sep 2026 10:12:50 +0000 Subject: [PATCH 6/6] test(mcp): note rename diff has no trailing newline An empty retrigger commit did not start CI. This records the observed diff shape and pushes a real tree change. Co-authored-by: Zack Jackson --- .../tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs index 385f39e238..987d9e6102 100644 --- a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs +++ b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/rename_symbol_test.rs @@ -56,6 +56,7 @@ pub fn tally(items: &[LineItem]) -> u64 { "#; /// Single-hunk preview the dry run must return for `PRICING_BEFORE` → `PRICING_AFTER`. +/// The production server omits a trailing newline after the final context line. const PRICING_DIFF: &str = "\ --- src/pricing.rs @@ -5,14 +5,14 @@