mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-13 23:08:09 +00:00
MCP: command_batch says it is not atomic, and the docs explain recovery (#3338)
Agents read the tool description, not docs/mcp.md, and the command_batch description only said it stops at the first error. It now also says the steps run in order and that earlier successful steps stay applied: there is no rollback. docs/mcp.md gives the stop_on_error default and a recovery recipe: inspect with doc_inspect, undo with edit.undo once per history entry (undo does not reverse files written to disk), or experiment on a copy made with image.duplicate or history.newDocument. A test checks the listed description. Claude-Session: https://claude.ai/code/session_01BSB7fMQUccqFQekksDGWpo Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
27222cf4b7
commit
48c1721470
3 files changed
+30
-4
No files matched your search
@@ -841,8 +841,9 @@ impl PhotocraftMcp {
|
||||
self.jobs_call("jobs.cancel", p.job.map_or_else(|| json!({}), |j| json!({"job": j})), false).await
|
||||
}
|
||||
|
||||
#[tool(description = "Run several engine commands in one call (fewer round trips). Returns {completed, failed, \
|
||||
results:[{ok, result|error}]}; stops at the first error unless stop_on_error is false.")]
|
||||
#[tool(description = "Run several engine commands in one call, in order (fewer round trips). Returns {completed, failed, \
|
||||
results:[{ok, result|error}]}; stops at the first error unless stop_on_error is false. \
|
||||
Not an atomic transaction: earlier successful steps stay applied (no rollback).")]
|
||||
async fn command_batch(&self, Parameters(p): Parameters<BatchParams>) -> Result<CallToolResult, McpError> {
|
||||
if p.steps.len() > MAX_BATCH_STEPS {
|
||||
return Ok(fail(format!("batch contains {} steps; maximum is {MAX_BATCH_STEPS}", p.steps.len())));
|
||||
|
||||
@@ -1040,6 +1040,18 @@ async fn bridge_reports_unreachable_app() {
|
||||
client.cancel().await.unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn command_batch_description_exposes_non_atomic_semantics_when_tools_are_listed() {
|
||||
let client = connect(PhotocraftMcp::headless()).await;
|
||||
let tools = client.list_all_tools().await.unwrap();
|
||||
let tool = tools.iter().find(|tool| tool.name == "command_batch").expect("command_batch tool");
|
||||
let description = tool.description.as_deref().expect("command_batch description");
|
||||
for contract in ["in order", "Not an atomic transaction", "first error", "stop_on_error is false", "no rollback"] {
|
||||
assert!(description.contains(contract), "command_batch description omits `{contract}`: {description}");
|
||||
}
|
||||
client.cancel().await.unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn command_batch_runs_steps_in_order() {
|
||||
let client = connect(PhotocraftMcp::headless()).await;
|
||||
|
||||
+15
-2
@@ -15,8 +15,9 @@ described in [development.md](development.md). UI tools require a running deskto
|
||||
- `command_list`: discover engine ids, parameter docs and current enabled state; accepts
|
||||
`filter` and `enabled_only`.
|
||||
- `command_run`: `id`, optional `params` and `wait`. Command parameters remain engine-defined.
|
||||
- `command_batch`: ordered `steps`, optional `stop_on_error`; one history step per command,
|
||||
not an atomic transaction. Existing per-step results and reply budgets are preserved.
|
||||
- `command_batch`: ordered `steps`, optional `stop_on_error` (default true); stops at the first
|
||||
error unless false. Not an atomic transaction: earlier successful steps stay applied, with
|
||||
no rollback. Existing per-step results and reply budgets are preserved.
|
||||
- `doc_inspect`: layer tree, history and selection; an empty session returns `document:null`.
|
||||
Every layer (including groups and nested children) reports raster-only `hasMask`,
|
||||
`hasVectorMask` for vector mask presence, and `vectorMaskEnabled` (`true`/`false` when
|
||||
@@ -29,6 +30,18 @@ Existing document/session/job/UI tools remain listed, including the documented
|
||||
and all four annotation hints. Generic command/control tools conservatively advertise writes;
|
||||
file saves may replace existing targets. Hints do not grant permission.
|
||||
|
||||
After a failed batch, `completed` and per-step `results` report successful steps, not a
|
||||
rolled-back transaction. Inspect the document and its history with `doc_inspect`; commands
|
||||
do not necessarily add one history entry each. For undoable edits, use `command_run` with
|
||||
`{"id":"edit.undo","params":{}}` once per undoable history entry added, inspecting history
|
||||
as you recover. Undo does not roll back files written to disk. If a reply-budget error says
|
||||
the current step may have completed, inspect its effects before retrying or undoing it.
|
||||
|
||||
Before experiments, use `command_run` with `image.duplicate` and `{}` params, or
|
||||
`history.newDocument` and `{}` params (copies the current state by default). Both create a
|
||||
new active document, leaving the source unchanged; run experimental edits on that copy.
|
||||
The optional `state` parameter of `history.newDocument` selects a retained historical state.
|
||||
|
||||
Unknown tool arguments return JSON-RPC `-32602`, naming the key, before execution. Batch
|
||||
step envelopes are strict too. Nested engine `params` retain their command-specific handling.
|
||||
Tool execution failures return `isError:true`. A worker panic becomes a tool error and the
|
||||
|
||||
Reference in new issue
Block a user