From a251ccc273d73cacb271b9775e52f0c35eb1f4db Mon Sep 17 00:00:00 2001 From: Dongcheng Lin Date: Sat, 24 Oct 2026 18:00:19 +0900 Subject: [PATCH] feat(mcp): apply a reversible edit in one call --- crates/agent/src/capabilities.rs | 3 +- crates/agent/src/protocol.rs | 11 +- crates/agent/src/session.rs | 4 + crates/agent/src/session/apply.rs | 70 ++++++++ crates/agent/src/session/apply_tests.rs | 172 +++++++++++++++++++ crates/agent/src/session/recovery.rs | 12 +- crates/agent/src/session/result_context.rs | 2 +- crates/agent/src/tool_effects.rs | 2 +- crates/agent/src/tool_effects_tests.rs | 4 +- docs/src/content/docs/reference/mcp.md | 13 ++ docs/src/content/docs/zh-cn/reference/mcp.md | 10 ++ 11 files changed, 293 insertions(+), 10 deletions(-) create mode 100644 crates/agent/src/session/apply.rs create mode 100644 crates/agent/src/session/apply_tests.rs diff --git a/crates/agent/src/capabilities.rs b/crates/agent/src/capabilities.rs index 6d4b0e03..4395f68c 100644 --- a/crates/agent/src/capabilities.rs +++ b/crates/agent/src/capabilities.rs @@ -192,6 +192,7 @@ fn tool_index() -> Vec { "plotium_properties_inspect"=>(vec!["properties.inspect"],"ToolResult.value: {property,canonical_label,tier,aggregate,readings,schema?,default_value?}; schema and default_value move out of readings when every reading shares them; ToolResult.targets lists only skips and failures","ids expand by property, or exact Series TargetRef[]"), "plotium_plan"=>(EDIT_OPERATIONS.to_vec(),"{plan_id,project_context?,basis_revision?,preflight:ToolPlan|edit_preflight{ToolPlan without request and frozen_targets,summary:{compatible,skipped,failed},targets:non-compatible PlannedTarget[]}|creation_preflight{required_authority,created_target_count,targets,outputs,binding_plan{creation,binding_count,row_counts,bindings:issue_diagnostics[]}}|property_preflight{required_authority,estimated_changes,summary:{compatible,skipped,failed},targets:non-compatible PlannedTarget[]}|save_preflight|export_preflight,save_id?,export_id?,import?:{status,items},transform?:{status,output},update?:{status,output,before_table_revision,changed}}; native import or table_data_only import/transform; import omits ids/targets, transform requires one DatasetId in ids; prepared is not committed","rename: dataset/canvas/content IDs; properties: ids or exact Series targets; transform: one DatasetId; creation: explicit parent inside parameters, no top-level selector"), "plotium_execute"=>(EDIT_OPERATIONS.to_vec(),"ToolResult for edits; plot.create adds summary and keeps exceptional targets, full bindings/layout and preflight-only defaults; properties.set/set_many/reset add summary {succeeded,skipped,failed,by_reason} and keep only non-succeeded targets and value.writes; figure.edit, plot.series.*, scientific plots, rename, themes and dataset.delete add summary {succeeded,skipped,failed} and keep only non-succeeded targets; {save:SaveReceipt} or {export:ExportReceipt} for external writes. Submitted plan is consumed even on failure","one-use plan_id"), + "plotium_apply"=>(crate::session::apply::APPLY_OPERATIONS.to_vec(),"Same as plotium_execute for the applied operation; nothing is retained to execute again","Same as plotium_plan"), "plotium_document_inspect"=>(vec!["document.inspect"],"ToolResult.value: self {revision,node:{target,kind,name,visible,ancestors,child_count,...}}; children {revision,target,items,total,next_offset,truncated}","document, canvas, panel, content or exact Series TargetRef"), "plotium_table_columns"=>(vec!["table.columns"],"ToolResult.value: {dataset_id,table_id,table_revision,snapshot_fingerprint,fields,columns,total_rows,diagnostics,inference,freshness,analyses,provenance?}","explicit DatasetId and table RevisionId"), "plotium_table_read_rows"=>(vec!["table.read_rows"],"ToolResult.value: {dataset_id,table_id,table_revision,revision,column_ids,offset,requested_rows,returned_rows,total_rows,next_offset,end,rows:[{row_id,values,sources?}]} ; typed 64-bit integer values are decimal strings","explicit DatasetId, table RevisionId and ColumnId projection"), @@ -204,7 +205,7 @@ fn tool_index() -> Vec { }; let mut entry = json!({"name":tool.name,"operations":operations, "result_contract":result,"targets":targets, - "required_permission":if tool.name=="plotium_execute"{"operation_specific: reversible_modify or per_request_desktop_save/export_authorization"}else if tool.name=="plotium_plan"{"operation_specific: read or reversible_modify"}else if matches!(tool.name.as_ref(),"plotium_preview_plan"|"plotium_tasks_cancel") {"reversible_modify"} else{"read"}}); + "required_permission":if tool.name=="plotium_apply"{"reversible_modify"}else if tool.name=="plotium_execute"{"operation_specific: reversible_modify or per_request_desktop_save/export_authorization"}else if tool.name=="plotium_plan"{"operation_specific: read or reversible_modify"}else if matches!(tool.name.as_ref(),"plotium_preview_plan"|"plotium_tasks_cancel") {"reversible_modify"} else{"read"}}); // plan and execute accept every editable operation; the index lists them once. if matches!(tool.name.as_ref(), "plotium_plan" | "plotium_execute") { entry.as_object_mut().expect("local entry").remove("operations"); diff --git a/crates/agent/src/protocol.rs b/crates/agent/src/protocol.rs index 68c22354..05e1f46c 100644 --- a/crates/agent/src/protocol.rs +++ b/crates/agent/src/protocol.rs @@ -15,7 +15,7 @@ impl ServerHandler for Handler { fn get_info(&self) -> ServerConfig { ServerConfig::new(ServerCapabilities::builder().enable_tools().build()) .with_server_info(Implementation::new("Plotium", env!("CARGO_PKG_VERSION"))) - .with_instructions("Known operations can fetch their static detail directly; otherwise browse the plotium_capabilities_get index. Read plotium_get_state first. Supply its project_context as expected_project_context on project queries and planning. On project_changed, reread state and replan; never reuse targets or plans. For known exact targets, committed synchronous results provide project_context and after_revision for the next call. Refresh state after desktop edits or Undo. Never replay a tool call after a disconnection.") + .with_instructions("Known operations can fetch their static detail directly; otherwise browse the plotium_capabilities_get index. Read plotium_get_state first. Supply its project_context as expected_project_context on project queries and planning. On project_changed, reread state and replan; never reuse targets or plans. For known exact targets, committed synchronous results provide project_context and after_revision for the next call. Refresh state after desktop edits or Undo. For a reversible edit, plotium_apply plans and commits in one call. Never replay a tool call after a disconnection.") } async fn list_tools( &self, @@ -146,6 +146,15 @@ pub(crate) fn definitions() -> Vec { let schema = json!({"type": "object", "properties": properties, "required": required, "additionalProperties": false}); Tool::new(name, description, schema.as_object().expect("local schema is an object").clone()) }).collect(); + let mut apply = tools + .iter() + .find(|t| t.name == "plotium_plan") + .expect("plan tool") + .input_schema + .as_ref() + .clone(); + apply["properties"]["tool_id"] = json!({"enum": crate::session::apply::APPLY_OPERATIONS}); + tools.push(Tool::new("plotium_apply", "Plan and commit one reversible edit in a single call, with plotium_plan inputs; returns the plotium_execute result. Saving, export, deletions, imports, table and scientific operations use plotium_plan then plotium_execute. After a disconnection read state: an unchanged revision means nothing was committed. Never replay.", apply)); tools.push(Tool::new("plotium_document_inspect", "Inspect bounded document structure and source bindings by full stable target. view self returns details; children pages direct children. Pagination is only valid with view children. Never uses selection.", document.as_object().expect("local schema is an object").clone())); let mut diagnostic = plotium_core::automation::figure_diagnostic_schema(); diagnostic["properties"]["expected_revision"] = json!({"type":"integer","minimum":0}); diff --git a/crates/agent/src/session.rs b/crates/agent/src/session.rs index 4ef906e1..34d66293 100644 --- a/crates/agent/src/session.rs +++ b/crates/agent/src/session.rs @@ -1,5 +1,6 @@ //! Desktop-thread capability boundary. No project state crosses the bridge. use std::collections::HashMap; +pub(crate) mod apply; mod candidate; mod compact; mod creation_response; @@ -232,6 +233,9 @@ impl Session { mut arguments: Value, blocked: bool, ) -> Result { + if name == "plotium_apply" { + return self.apply(app, arguments, blocked); + } if name == "plotium_properties_list" { return crate::property_catalog::respond(arguments); } diff --git a/crates/agent/src/session/apply.rs b/crates/agent/src/session/apply.rs new file mode 100644 index 00000000..73816095 --- /dev/null +++ b/crates/agent/src/session/apply.rs @@ -0,0 +1,70 @@ +//! One call that plans and commits a reversible edit. It reuses the plan and +//! execute paths unchanged, so preflight, permissions, revision checks, Undo +//! and audit records are exactly those of the two-step form. +use super::*; + +/// Reversible edits that write no files and start no background work. +/// Deletions keep the two-step form so callers review them first. +pub(crate) const APPLY_OPERATIONS: [&str; 10] = [ + TOOL_SET, + TOOL_SET_MANY, + TOOL_RESET, + FIGURE_EDIT, + PLOT_CREATE, + "plot.series.append", + "plot.series.bind", + "plot.series.materialize_default", + "resource.rename", + "figure.apply_theme", +]; + +const DELETIONS: [&str; 3] = ["content_delete", "panel_delete", "series_delete"]; + +impl Session { + pub(super) fn apply( + &mut self, + app: &mut PlotiumApp, + arguments: Value, + blocked: bool, + ) -> Result { + let tool_id = arguments.get("tool_id").and_then(Value::as_str); + if !tool_id.is_some_and(|id| APPLY_OPERATIONS.contains(&id)) { + return Err(Failure::new( + "unsupported_operation", + "plotium_apply accepts reversible edits only; plan this operation with plotium_plan and submit it with plotium_execute", + ) + .at_field("tool_id")); + } + let deletes = arguments + .pointer("/parameters/edits") + .and_then(Value::as_array) + .is_some_and(|edits| { + edits + .iter() + .any(|e| e["op"].as_str().is_some_and(|op| DELETIONS.contains(&op))) + }); + if tool_id == Some(FIGURE_EDIT) && deletes { + return Err(Failure::new( + "unsupported_operation", + "Deletions are reviewed before they run; use plotium_plan, then plotium_execute", + ) + .at_field("parameters.edits")); + } + if !self.allow_modify { + return Err(Failure::new( + "permission_denied", + "Switch to Read and modify before applying edits", + )); + } + let plan = self.call_inner(app, "plotium_plan", arguments, blocked)?; + let id = plan["plan_id"] + .as_str() + .expect("plans have an id") + .to_owned(); + self.call_inner(app, "plotium_execute", json!({ "plan_id": id }), blocked) + } +} + +#[cfg(test)] +#[path = "apply_tests.rs"] +mod tests; diff --git a/crates/agent/src/session/apply_tests.rs b/crates/agent/src/session/apply_tests.rs new file mode 100644 index 00000000..0bfc360e --- /dev/null +++ b/crates/agent/src/session/apply_tests.rs @@ -0,0 +1,172 @@ +use super::*; +use plotium_core::{ + data::*, + state::{Dataset, TableDataset}, +}; + +fn fixture() -> (PlotiumApp, Session, Value) { + let mut app = PlotiumApp::new_with_settings(Default::default()); + let columns = [ + ("x", vec![Some(0.), Some(1.), Some(2.)]), + ("y", vec![Some(1.), Some(3.), Some(2.)]), + ] + .into_iter() + .map(|(name, values)| { + ( + ColumnSchema::new(name, LogicalType::Float64), + ColumnChunk::optional_f64(values), + ) + }) + .collect(); + let table = + TableDataset::from_materialized(columns, vec![], None, vec![], "plotium.test.apply") + .unwrap(); + app.doc.datasets.push(Dataset::Table(Box::new(table))); + let table = app.doc.datasets[0].as_table().unwrap(); + let r = &table.typed_state.envelope.revision; + let c = &r.snapshot.schema.columns; + let parameters = json!({"parent":plotium_core::automation::document_root_target(),"name":"P", + "series":[{"kind":"line","source":{"dataset_id":table.resource_id, + "field_id":app.doc.datasets[0].default_field_id().unwrap(),"table_id":r.table_id, + "table_revision":r.id,"x_column_id":c[0].id,"y_column_id":c[1].id,"y_error_column_id":null}}]}); + let mut session = Session::default(); + session.set_allow_modify(true); + (app, session, parameters) +} + +fn call( + session: &mut Session, + app: &mut PlotiumApp, + name: &str, + tool: &str, + parameters: Value, +) -> std::result::Result { + let request = args(app, tool, parameters); + session.call(app, name, request) +} + +fn args(app: &PlotiumApp, tool: &str, parameters: Value) -> Value { + json!({"expected_project_context":app.session.project_context, + "expected_revision":app.doc.automation_revision,"tool_id":tool,"parameters":parameters}) +} + +#[test] +fn apply_commits_like_plan_then_execute() { + let (mut two_step, mut s2, create) = fixture(); + let plan = call( + &mut s2, + &mut two_step, + "plotium_plan", + PLOT_CREATE, + create.clone(), + ) + .unwrap(); + let executed = s2 + .call( + &mut two_step, + "plotium_execute", + json!({"plan_id":plan["plan_id"]}), + ) + .unwrap(); + let (mut one_step, mut s1, create) = fixture(); + let applied = call(&mut s1, &mut one_step, "plotium_apply", PLOT_CREATE, create).unwrap(); + assert!(s1.plans.is_empty()); + for key in ["tool_id", "before_revision", "after_revision", "summary"] { + assert_eq!(applied[key], executed[key], "{key}"); + } + assert_eq!( + applied["project_context"], + json!(one_step.session.project_context) + ); + assert_eq!( + one_step.doc.automation_runs.len(), + two_step.doc.automation_runs.len() + ); + assert_eq!( + one_step.session.undo_stack.len(), + two_step.session.undo_stack.len() + ); + let target = applied["value"]["bindings"][0]["target"].clone(); + let write = json!({"writes":[{"target":target,"key":"series.line.color","value":"#123456"}]}); + let styled = call( + &mut s1, + &mut one_step, + "plotium_apply", + TOOL_SET_MANY, + write, + ) + .unwrap(); + assert_eq!(styled["summary"]["succeeded"], 1); + let steps = one_step.session.undo_stack.len(); + one_step.undo(); + assert_eq!(one_step.session.undo_stack.len(), steps - 1); +} + +#[test] +fn apply_refuses_deletions_files_read_only_and_stale_requests_without_retaining_plans() { + let (mut app, mut session, create) = fixture(); + let dataset = app.doc.datasets[0].resource_id().to_string(); + for (tool, parameters) in [ + (DATASET_DELETE, json!({})), + ("project.save", json!({"path":"/tmp/never.plotium"})), + ("data.import", json!({"paths":["/tmp/never.csv"]})), + ] { + let mut request = args(&app, tool, parameters); + if tool == DATASET_DELETE { + request["ids"] = json!([dataset]); + } + let error = session + .call(&mut app, "plotium_apply", request) + .unwrap_err(); + assert_eq!(error.code, "unsupported_operation", "{tool}"); + } + let deletion = json!({"canvas":{"resource":{"id":uuid::Uuid::new_v4(),"kind":"plotium.canvas"}}, + "edits":[{"op":"content_delete","target":{"resource":{"id":"c/1","kind":"plotium.canvas.object"}}}]}); + let error = call( + &mut session, + &mut app, + "plotium_apply", + FIGURE_EDIT, + deletion, + ) + .unwrap_err(); + assert_eq!(error.code, "unsupported_operation"); + let mut stale = args(&app, PLOT_CREATE, create.clone()); + stale["expected_revision"] = json!(app.doc.automation_revision + 1); + let error = session.call(&mut app, "plotium_apply", stale).unwrap_err(); + assert_eq!(error.code, "stale_revision"); + let recovery = error.value()["error"]["recovery"].clone(); + assert_eq!(recovery["committed"], false); + assert_eq!(recovery["submission"], "not_submitted"); + session.set_allow_modify(false); + let error = call(&mut session, &mut app, "plotium_apply", PLOT_CREATE, create).unwrap_err(); + assert_eq!(error.code, "permission_denied"); + assert!(session.plans.is_empty()); + assert!(app.doc.canvases.is_empty()); + assert_eq!(app.doc.automation_revision, 0); +} + +#[test] +fn interrupted_apply_reports_an_unknown_outcome() { + let request = json!({"tool_id":PLOT_CREATE,"parameters":{}}); + for code in ["request_cancelled", "connection_stopped"] { + let recovery = Failure::new(code, "interrupted") + .for_request("plotium_apply", &request) + .value()["error"]["recovery"] + .clone(); + assert_eq!(recovery["submission"], "unknown", "{code}"); + assert_eq!(recovery["committed"], Value::Null, "{code}"); + assert_eq!(recovery["completion"], "unknown", "{code}"); + assert_eq!(recovery["replay_allowed"], false, "{code}"); + let planned = Failure::new(code, "interrupted") + .for_request("plotium_plan", &request) + .value()["error"]["recovery"] + .clone(); + assert_eq!(planned["committed"], false, "{code}"); + } + let rejected = Failure::new("busy", "Desktop queue is full") + .for_request("plotium_apply", &request) + .value()["error"]["recovery"] + .clone(); + assert_eq!(rejected["committed"], false); +} diff --git a/crates/agent/src/session/recovery.rs b/crates/agent/src/session/recovery.rs index 72654742..e46f6e90 100644 --- a/crates/agent/src/session/recovery.rs +++ b/crates/agent/src/session/recovery.rs @@ -101,7 +101,8 @@ pub(super) fn for_error(code: &str) -> Value { impl Failure { pub(crate) fn for_request(mut self, name: &str, arguments: &Value) -> Self { - if name == "plotium_plan" + let planning = matches!(name, "plotium_plan" | "plotium_apply"); + if planning && self.code == "invalid_parameters" && self.details.as_ref().is_some_and(|details| { details["field"].is_null() @@ -116,7 +117,7 @@ impl Failure { let field = self.details.as_ref().and_then(|d| d["field"].as_str()); let query = if name == "plotium_capabilities_get" { Some(json!({})) - } else if name == "plotium_plan" + } else if planning && !matches!( field, Some("expected_project_context" | "expected_revision") @@ -163,8 +164,11 @@ impl Failure { } } self.recovery = Some(value); - if name == "plotium_plan" { - // Preflight can retain candidates, but never submits or commits execution. + // A desktop error means nothing was committed: apply plans and executes + // atomically. A cancelled or disconnected apply may still have committed. + let unknown = name == "plotium_apply" + && matches!(self.code, "request_cancelled" | "connection_stopped"); + if planning && !unknown { self.not_submitted() } else { self diff --git a/crates/agent/src/session/result_context.rs b/crates/agent/src/session/result_context.rs index 26f47b94..0edb5582 100644 --- a/crates/agent/src/session/result_context.rs +++ b/crates/agent/src/session/result_context.rs @@ -3,7 +3,7 @@ use super::*; pub(super) fn annotate(app: &PlotiumApp, name: &str, value: &mut Value) { // Receipt revisions describe their own lifecycle, possibly in another project. // A submitted task's after_revision is not evidence of a committed edit. - if name == "plotium_execute" + if matches!(name, "plotium_execute" | "plotium_apply") && value["tool_id"].is_string() && value["before_revision"].is_number() && value["after_revision"] == json!(app.doc.automation_revision) diff --git a/crates/agent/src/tool_effects.rs b/crates/agent/src/tool_effects.rs index 0a2e33a3..d616e9f4 100644 --- a/crates/agent/src/tool_effects.rs +++ b/crates/agent/src/tool_effects.rs @@ -32,7 +32,7 @@ impl ToolEffect { | "plotium_export_status" => Self::Read, "plotium_figure_diagnose" => Self::Diagnostic, "plotium_plan" => Self::Prepare, - "plotium_execute" => Self::Execute, + "plotium_execute" | "plotium_apply" => Self::Execute, "plotium_tasks_cancel" => Self::Cancel, _ => unreachable!("every advertised tool needs an effect classification"), } diff --git a/crates/agent/src/tool_effects_tests.rs b/crates/agent/src/tool_effects_tests.rs index 25a7eb70..f8e01052 100644 --- a/crates/agent/src/tool_effects_tests.rs +++ b/crates/agent/src/tool_effects_tests.rs @@ -24,11 +24,11 @@ fn every_advertised_tool_has_explicit_conservative_annotations() { "plotium_export_status", ]; let definitions = protocol::definitions(); - assert_eq!(definitions.len(), reads.len() + 4); + assert_eq!(definitions.len(), reads.len() + 5); for tool in definitions { let (read, destructive, idempotent, external) = match tool.name.as_ref() { "plotium_plan" => (false, false, false, true), - "plotium_execute" => (false, true, false, true), + "plotium_execute" | "plotium_apply" => (false, true, false, true), "plotium_tasks_cancel" => (false, true, true, false), "plotium_figure_diagnose" => (true, false, false, false), name if reads.contains(&name) => (true, false, true, false), diff --git a/docs/src/content/docs/reference/mcp.md b/docs/src/content/docs/reference/mcp.md index f85b1a4c..ab2fd378 100644 --- a/docs/src/content/docs/reference/mcp.md +++ b/docs/src/content/docs/reference/mcp.md @@ -300,6 +300,19 @@ execution results carry `summary` counts of succeeded, skipped and failed targets, list only skipped or failed rows in `targets`, and report changed resources in `produced` and `modified`. +`plotium_apply` takes the same inputs as `plotium_plan` and plans and commits one +reversible edit in a single call, returning the `plotium_execute` result. It +accepts `properties.set`, `properties.set_many`, `properties.reset`, +`figure.edit` without deletions, `plot.create`, `plot.series.append`, +`plot.series.bind`, `plot.series.materialize_default`, `resource.rename` and +`figure.apply_theme`, and requires Read and modify. Preflight, revision and +permission checks, Undo and run records are those of the two-step form, and no +plan is retained. Saving, export, deletions, imports, table and scientific +operations still use `plotium_plan` and `plotium_execute`, as does previewing a +candidate. A rejected apply commits nothing. After a disconnection, read state: +a revision equal to the request's `expected_revision` means the edit was not +committed; otherwise inspect the targets. Never replay the call. + A queued or submitted scientific task is not a committed edit; its immediate `after_revision` must not be reused. Only a terminal `succeeded` receipt in the known project context with `end_revision` permits reuse. Check scientific diff --git a/docs/src/content/docs/zh-cn/reference/mcp.md b/docs/src/content/docs/zh-cn/reference/mcp.md index 8da549ce..f0c963b4 100644 --- a/docs/src/content/docs/zh-cn/reference/mcp.md +++ b/docs/src/content/docs/zh-cn/reference/mcp.md @@ -249,6 +249,16 @@ stale/context 错误或结果不确定时，仍须读取状态。执行继续检 失败的目标。它们的执行结果用 `summary` 统计成功、跳过和失败数，`targets` 只列跳过或 失败的行，已修改资源见 `produced` 和 `modified`。 +`plotium_apply` 的输入与 `plotium_plan` 相同，一次调用完成一项可撤销编辑的规划和提交， +返回 `plotium_execute` 的结果。它接受 `properties.set`、`properties.set_many`、 +`properties.reset`、不含删除的 `figure.edit`、`plot.create`、`plot.series.append`、 +`plot.series.bind`、`plot.series.materialize_default`、`resource.rename` 和 +`figure.apply_theme`，需要“读取和修改”权限。预检、revision 与权限检查、Undo 和运行记录 +都与两步形式相同，且不保留计划。保存、导出、删除、导入、表格和科学操作仍用 +`plotium_plan` 加 `plotium_execute`，预览候选图也是如此。被拒绝的 apply 不提交任何内容。 +断线后先读状态：revision 等于请求的 `expected_revision` 表示编辑没有提交，否则检查目标； +不要重放这次调用。 + 科学任务排队或提交并不代表修改已提交，不可复用即时响应的 `after_revision`。 只有已知项目上下文中终态为 `succeeded` 且包含 `end_revision` 的回执才允许复用。 科学有效性与收敛情况须单独检查：任务成功并不证明拟合有效。失败、取消、失效、