From c8fe0eb7b315e4ca97cb0ef08978cea76d94d3c0 Mon Sep 17 00:00:00 2001 From: Fini Date: Mon, 20 Jul 2026 21:34:20 +0800 Subject: [PATCH] fix(ai): mark rolled-back design batches as errors --- crates/op-ai/src/chat_provider.rs | 5 ++- .../op-host-services/src/chat_canvas_tools.rs | 19 +++++--- .../op-host-services/src/chat_tool_result.rs | 35 +++++++++++++++ .../src/design_agent_tool_result_tests.rs | 44 +++++++++++++++++++ crates/op-host-services/src/lib.rs | 3 ++ 5 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 crates/op-host-services/src/chat_tool_result.rs create mode 100644 crates/op-host-services/src/design_agent_tool_result_tests.rs diff --git a/crates/op-ai/src/chat_provider.rs b/crates/op-ai/src/chat_provider.rs index 3908ed529..31813bc20 100644 --- a/crates/op-ai/src/chat_provider.rs +++ b/crates/op-ai/src/chat_provider.rs @@ -331,8 +331,9 @@ pub struct ChatToolDef { /// Result of executing one chat tool call. `content` is the JSON the /// model sees as the tool result (TS shape: `{"success":true,"data":…}` -/// or `{"success":false,"error":…}`); `is_error` marks transport-level -/// failure so Anthropic `tool_result` blocks can set `is_error`. +/// or `{"success":false,"error":…}`); `is_error` marks an unsuccessful +/// tool outcome (semantic validation/rollback or executor failure) so +/// Anthropic `tool_result` blocks can set `is_error`. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ChatToolResult { pub content: String, diff --git a/crates/op-host-services/src/chat_canvas_tools.rs b/crates/op-host-services/src/chat_canvas_tools.rs index 1232495a2..fe9e87767 100644 --- a/crates/op-host-services/src/chat_canvas_tools.rs +++ b/crates/op-host-services/src/chat_canvas_tools.rs @@ -23,6 +23,7 @@ use op_mcp::{ToolRegistry, ToolResponse}; use std::collections::HashSet; use crate::chat_modify_sanitize::sanitize_modify_replacement; +use crate::chat_tool_result::{rolled_back_transaction_message, structured_error_result}; /// TS `maxTurns` for the chat agent loop (`ai-chat-handlers.ts:254`). pub const MAX_TOOL_TURNS: usize = 20; @@ -171,6 +172,19 @@ pub(crate) fn execute_with_registry( json, .. } => { + let data = match json { + Some(raw) => serde_json::from_str::(&raw) + .unwrap_or(serde_json::Value::String(raw)), + None => serde_json::to_value(&result).unwrap_or(serde_json::Value::Null), + }; + // `batch_design` reports transactional rollbacks as structured + // JSON so callers retain the failing lines and resend hint. That + // is still a tool failure: no command landed and the agent loop + // must receive `is_error`, otherwise the transcript paints a + // green success card for an unchanged document. + if let Some(message) = rolled_back_transaction_message(&data) { + return (structured_error_result(message, data), false); + } let mut mutated = false; if let Some(cmd) = command { if state.apply(cmd.clone()) { @@ -182,11 +196,6 @@ pub(crate) fn execute_with_registry( ); } } - let data = match json { - Some(raw) => serde_json::from_str::(&raw) - .unwrap_or(serde_json::Value::String(raw)), - None => serde_json::to_value(&result).unwrap_or(serde_json::Value::Null), - }; let envelope = serde_json::json!({ "success": true, "data": data }); ( ChatToolResult { diff --git a/crates/op-host-services/src/chat_tool_result.rs b/crates/op-host-services/src/chat_tool_result.rs new file mode 100644 index 000000000..779d300fe --- /dev/null +++ b/crates/op-host-services/src/chat_tool_result.rs @@ -0,0 +1,35 @@ +//! Semantic result classification shared by in-process chat tool surfaces. + +use op_ai::chat_provider::ChatToolResult; + +pub(crate) fn rolled_back_transaction_message(data: &serde_json::Value) -> Option { + let object = data.as_object()?; + if object.get("applied") != Some(&serde_json::Value::Bool(false)) { + return None; + } + let errors = object.get("errors")?.as_array()?; + if errors.is_empty() { + return None; + } + Some( + object + .get("hint") + .and_then(serde_json::Value::as_str) + .unwrap_or( + "Tool transaction rolled back; inspect data.errors and resend the corrected batch", + ) + .to_string(), + ) +} + +pub(crate) fn structured_error_result(message: String, data: serde_json::Value) -> ChatToolResult { + let envelope = serde_json::json!({ + "success": false, + "error": message, + "data": data, + }); + ChatToolResult { + content: envelope.to_string(), + is_error: true, + } +} diff --git a/crates/op-host-services/src/design_agent_tool_result_tests.rs b/crates/op-host-services/src/design_agent_tool_result_tests.rs new file mode 100644 index 000000000..8c57cbed8 --- /dev/null +++ b/crates/op-host-services/src/design_agent_tool_result_tests.rs @@ -0,0 +1,44 @@ +use op_editor_core::EditorState; + +use crate::design_agent_tools::execute_design_tool; + +#[test] +fn rolled_back_batch_is_an_error_and_preserves_structured_feedback() { + let mut state = EditorState::new(); + let before = serde_json::to_value(state.active_children()).expect("initial document snapshot"); + let args = serde_json::json!({ + "operations": concat!( + "root=I(null,{type:'frame',name:'Never Applied',width:120,height:80})\n", + "U(\"missing-node\",{x:5})" + ) + }) + .to_string(); + + let (result, mutated) = execute_design_tool(&mut state, "batch_design", &args); + + assert!(result.is_error, "rolled-back batch must be a tool error"); + assert!(!mutated, "rolled-back batch must not report a mutation"); + assert_eq!( + serde_json::to_value(state.active_children()).expect("final document snapshot"), + before, + "the transaction must leave the document unchanged" + ); + + let envelope: serde_json::Value = + serde_json::from_str(&result.content).expect("chat tool result envelope"); + assert_eq!(envelope["success"], false, "{envelope}"); + assert_eq!(envelope["data"]["applied"], false, "{envelope}"); + assert_eq!( + envelope["data"]["errors"] + .as_array() + .expect("structured errors") + .len(), + 1, + "{envelope}" + ); + let hint = envelope["data"]["hint"] + .as_str() + .expect("structured resend hint"); + assert!(hint.contains("rolled back"), "{hint}"); + assert_eq!(envelope["error"], hint, "{envelope}"); +} diff --git a/crates/op-host-services/src/lib.rs b/crates/op-host-services/src/lib.rs index d27f2e5a4..25c9e30f4 100644 --- a/crates/op-host-services/src/lib.rs +++ b/crates/op-host-services/src/lib.rs @@ -41,12 +41,15 @@ mod chat_subprocess_parse; pub mod chat_subprocess_quirks; mod chat_subprocess_safety; pub mod chat_system_prompt; +mod chat_tool_result; pub mod cli_model_discovery; pub mod cli_provider_probe; mod cli_resolver_windows; mod design_agent_diagnostics; #[cfg(test)] mod design_agent_reflow_tests; +#[cfg(test)] +mod design_agent_tool_result_tests; pub mod design_agent_tools; pub mod design_context; pub mod design_md_llm;