fix(mcp): return get_screenshot as MCP ImageContent block (#205)
The get_screenshot MCP tool serialized its PNG through OkJson, which wraps the payload in a text content block. Vision-capable MCP clients (Copilot, Claude Code) therefore received a large base64 string as text instead of an image, making the screenshot unusable for visual reasoning (openpencil issue #204). - Add ToolOutcome::OkImageContent carrying base64 + mime_type plus an optional metadata JSON string - Thread an optional ImageContent through ToolResponse::Ok and emit it as an MCP {"type":"image","data":...,"mimeType":...} content block in tool_response_to_json before any text block - get_screenshot now returns OkImageContent (image/png) while retaining image_base64 in its text metadata for the in-app chat-agent path - Update the get_screenshot schema description to state the use case (PNG image for visual verification) without the base64 implementation detail
This commit is contained in:
parent
c658706d04
commit
db93a2ac74
|
|
@ -297,7 +297,7 @@ fn delete_node_tool_def() -> ChatToolDef {
|
|||
fn get_screenshot_tool_def() -> ChatToolDef {
|
||||
ChatToolDef {
|
||||
name: "get_screenshot".into(),
|
||||
description: "Render a node to a base64 PNG".into(),
|
||||
description: "Render a node to a PNG image for visual verification".into(),
|
||||
level: "read".into(),
|
||||
input_schema_json: r#"{"type":"object","properties":{"nodeId":{"type":"string"}}}"#.into(),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ pub const TOOL_SCHEMAS: &[&str] = &[
|
|||
r#"{"name":"get_guidelines","description":"Return OpenPencil product-design guidelines. Default category is guide, which reads topic. category=style resolves an OpenPencil style using flat scalar string args.","inputSchema":{"type":"object","properties":{"category":{"type":"string","enum":["guide","style"],"default":"guide","description":"guide returns the existing topic guideline path; style resolves the style catalog"},"topic":{"type":"string","enum":["web-app","mobile","code-to-design"],"description":"Guide topic for category=guide"},"name":{"type":"string","description":"Style catalog name for category=style"},"colorPalette":{"type":"string","description":"Style palette name for category=style"},"roundness":{"type":"string","description":"Roundness profile for category=style"},"elevation":{"type":"string","description":"Elevation profile for category=style"},"headings":{"type":"string","description":"Heading font family for category=style"},"body":{"type":"string","description":"Body font family for category=style"},"captions":{"type":"string","description":"Caption font family for category=style"},"data":{"type":"string","description":"Data font family for category=style"},"decorativeImagery":{"type":"string","description":"Optional decorative imagery guidance for category=style"}}}}"#,
|
||||
r#"{"name":"spawn_agents","description":"Split a large design task into parallel subtasks. Each config item gives a prompt, the container node(s) to fill, and the styleguide + guideline NAMES to pass to the subagent (subagents cannot search styleguides). Returns the spawned agent ids. Execution runs the subagents in parallel.","inputSchema":{"type":"object","properties":{"config":{"type":"array","items":{"type":"object","properties":{"prompt":{"type":"string"},"containerNodes":{"type":"array","items":{"type":"string"}},"styleguideName":{"type":"string"},"guidelineNames":{"type":"array","items":{"type":"string"}}},"required":["prompt","styleguideName"]}}},"required":["config"]}}"#,
|
||||
r#"{"name":"ToolSearch","description":"Discover tools by keyword or exact selection. Use 'select:Name1,Name2' to load specific tools, or a keyword query to search names+descriptions.","inputSchema":{"type":"object","properties":{"query":{"type":"string"},"max_results":{"type":"integer","minimum":1,"default":5}},"required":["query"]}}"#,
|
||||
r#"{"name":"get_screenshot","description":"Render a node to a base64 PNG for visual verification. nodeId may be \"root\" for the active page's top node.","inputSchema":{"type":"object","properties":{"nodeId":{"type":"string","description":"Node id, or \"root\""}},"required":["nodeId"]}}"#,
|
||||
r#"{"name":"get_screenshot","description":"Render a node to a PNG image for visual verification. nodeId may be \"root\" for the active page's top node.","inputSchema":{"type":"object","properties":{"nodeId":{"type":"string","description":"Node id, or \"root\""}},"required":["nodeId"]}}"#,
|
||||
r#"{"name":"export_item","description":"Export a page, arbitrary node, or current selection to base64-encoded image/PDF bytes.","inputSchema":{"type":"object","properties":{"itemId":{"type":"string"},"format":{"type":"string","enum":["png","jpeg","webp","pdf"]},"scale":{"type":"number"}},"required":["format"]}}"#,
|
||||
r#"{"name":"export_nodes","description":"Export one or more nodes to base64-encoded image/PDF bytes. format is one of png|jpeg|webp|pdf.","inputSchema":{"type":"object","properties":{"nodeIds":{"type":"array","items":{"type":"string"}},"format":{"type":"string","enum":["png","jpeg","webp","pdf"]},"scale":{"type":"number"}},"required":["nodeIds","format"]}}"#,
|
||||
r#"{"name":"get_active_theme","description":"Return the active theme axis pinning per axis.","inputSchema":{"type":"object","properties":{},"additionalProperties":false}}"#,
|
||||
|
|
|
|||
|
|
@ -74,11 +74,18 @@ impl McpTool for GetScreenshot {
|
|||
// (see `export/screenshot.rs` and `export/tests.rs`).
|
||||
let image_base64 = base64::engine::general_purpose::STANDARD.encode(&bytes);
|
||||
|
||||
let out = serde_json::json!({
|
||||
"image_base64": image_base64,
|
||||
let metadata = serde_json::json!({
|
||||
"nodeId": node_id,
|
||||
"format": "png",
|
||||
// include image_base64 in metadata for the in-app chat agent path —
|
||||
// the MCP path uses the `image` field on ToolResponse::Ok separately
|
||||
"image_base64": image_base64,
|
||||
});
|
||||
ToolOutcome::OkJson(out.to_string())
|
||||
ToolOutcome::OkImageContent {
|
||||
image_base64,
|
||||
mime_type: "image/png".into(),
|
||||
metadata_json: Some(metadata.to_string()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -148,15 +155,23 @@ mod tests {
|
|||
)]);
|
||||
let tool = tool_from_scene(scene);
|
||||
match call(&tool, "root") {
|
||||
ToolOutcome::OkJson(json) => {
|
||||
let v: serde_json::Value = serde_json::from_str(&json).expect("valid JSON");
|
||||
let b64 = v["image_base64"].as_str().expect("image_base64 field");
|
||||
assert!(!b64.is_empty(), "image_base64 must not be empty");
|
||||
let bytes = decode_base64(b64);
|
||||
ToolOutcome::OkImageContent {
|
||||
image_base64,
|
||||
mime_type,
|
||||
metadata_json,
|
||||
} => {
|
||||
assert_eq!(mime_type, "image/png");
|
||||
assert!(!image_base64.is_empty(), "image_base64 must not be empty");
|
||||
let bytes = decode_base64(&image_base64);
|
||||
assert_eq!(&bytes[..8], PNG_MAGIC, "must be a PNG payload");
|
||||
assert_eq!(v["format"], "png");
|
||||
// Metadata carries nodeId + format + image_base64 (for chat agent path).
|
||||
let meta: serde_json::Value =
|
||||
serde_json::from_str(&metadata_json.expect("metadata")).expect("valid JSON");
|
||||
assert_eq!(meta["format"], "png");
|
||||
assert_eq!(meta["nodeId"], "n1");
|
||||
assert!(!meta["image_base64"].as_str().unwrap_or("").is_empty());
|
||||
}
|
||||
other => panic!("expected OkJson, got {other:?}"),
|
||||
other => panic!("expected OkImageContent, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -181,12 +196,16 @@ mod tests {
|
|||
]);
|
||||
let tool = tool_from_scene(scene);
|
||||
match call(&tool, "r2") {
|
||||
ToolOutcome::OkJson(json) => {
|
||||
let v: serde_json::Value = serde_json::from_str(&json).expect("valid JSON");
|
||||
let bytes = decode_base64(v["image_base64"].as_str().expect("field"));
|
||||
ToolOutcome::OkImageContent {
|
||||
image_base64,
|
||||
mime_type,
|
||||
..
|
||||
} => {
|
||||
assert_eq!(mime_type, "image/png");
|
||||
let bytes = decode_base64(&image_base64);
|
||||
assert_eq!(&bytes[..8], PNG_MAGIC, "must be a PNG payload");
|
||||
}
|
||||
other => panic!("expected OkJson, got {other:?}"),
|
||||
other => panic!("expected OkImageContent, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -41,20 +41,37 @@ pub fn response_to_json(r: &ToolResponse) -> String {
|
|||
/// result — NOT a JSON-RPC `error` (those are reserved for transport/parse
|
||||
/// failures, still emitted via [`response_to_json`]). External MCP clients
|
||||
/// (Claude Code / Codex) require this envelope.
|
||||
///
|
||||
/// When `image` is `Some`, an MCP `ImageContent` block is emitted BEFORE
|
||||
/// any text block so vision-capable MCP clients receive the image as
|
||||
/// multimodal content instead of a base64 string buried in JSON text.
|
||||
pub fn tool_response_to_json(r: &ToolResponse) -> String {
|
||||
let (id_repr, body) = match r {
|
||||
ToolResponse::Ok {
|
||||
id, result, json, ..
|
||||
id,
|
||||
result,
|
||||
json,
|
||||
image,
|
||||
..
|
||||
} => {
|
||||
// Nested-JSON read result rides verbatim in the text block;
|
||||
// otherwise the flat string-map is encoded as the text.
|
||||
let mut content_blocks: Vec<String> = Vec::new();
|
||||
// Image block first (when present) so vision models see it.
|
||||
if let Some(img) = image {
|
||||
content_blocks.push(format!(
|
||||
r#"{{"type":"image","data":{},"mimeType":{}}}"#,
|
||||
json_escape(&img.data),
|
||||
json_escape(&img.mime_type),
|
||||
));
|
||||
}
|
||||
// Text block: nested JSON or flat string-map.
|
||||
let text = match json {
|
||||
Some(raw) => json_escape(raw),
|
||||
None => json_escape(&btree_to_json(result)),
|
||||
};
|
||||
content_blocks.push(format!(r#"{{"type":"text","text":{text}}}"#));
|
||||
(
|
||||
id_to_json(id),
|
||||
format!(r#""result":{{"content":[{{"type":"text","text":{text}}}]}}"#),
|
||||
format!(r#""result":{{"content":[{}]}}"#, content_blocks.join(",")),
|
||||
)
|
||||
}
|
||||
ToolResponse::Err { id, message, .. } => (
|
||||
|
|
|
|||
|
|
@ -314,6 +314,17 @@ pub struct ToolCall {
|
|||
pub arguments: BTreeMap<String, String>,
|
||||
}
|
||||
|
||||
/// MCP `ImageContent` block payload — carried by `ToolResponse::Ok`
|
||||
/// so the serializer can emit a proper `{"type":"image",…}` content
|
||||
/// block instead of wrapping base64 bytes in a text block.
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub struct ImageContent {
|
||||
/// Base64-encoded image bytes (no data-URI prefix).
|
||||
pub data: String,
|
||||
/// MIME type, e.g. `"image/png"`.
|
||||
pub mime_type: String,
|
||||
}
|
||||
|
||||
/// Tool response — either a structured result object or an error.
|
||||
/// Errors are typed enough for the LLM client to recover (e.g.
|
||||
/// `MissingArgument` vs `InvalidArgument` vs `ToolFailed`).
|
||||
|
|
@ -334,6 +345,13 @@ pub enum ToolResponse {
|
|||
/// (the flat map) — so read tools match TS's arbitrary-JSON
|
||||
/// shapes byte-for-byte. `None` ⇒ use the flat-map encoding.
|
||||
json: Option<String>,
|
||||
/// When `Some`, the serializer emits an MCP `ImageContent` block
|
||||
/// (`{"type":"image","data":"<base64>","mimeType":"..."}`) in the
|
||||
/// `content[]` array BEFORE any text block (from `json` or
|
||||
/// `result`). Tools like `get_screenshot` use this so MCP clients
|
||||
/// (Copilot, Claude Code, …) receive the image as multimodal
|
||||
/// content instead of a base64 string buried in JSON text.
|
||||
image: Option<ImageContent>,
|
||||
},
|
||||
Err {
|
||||
id: RequestId,
|
||||
|
|
@ -382,6 +400,18 @@ pub enum ToolOutcome {
|
|||
/// results (`fixes[]`, `layoutSnapshot`, `results[]`, …) while still
|
||||
/// mutating the document. The host applies `command`; the client sees `json`.
|
||||
OkJsonWithCommand(String, EditorCommand),
|
||||
/// A tool returning a base64-encoded image as an MCP `ImageContent` block.
|
||||
/// `image_base64` is the raw base64 (no data-URI prefix);
|
||||
/// `mime_type` is e.g. `"image/png"`. An optional `metadata_json`
|
||||
/// string is emitted as a separate `text` content block (like
|
||||
/// `debug_screenshot`'s pretty-printed metadata block). MCP clients
|
||||
/// that support vision (Copilot, Claude Code, …) receive the image as
|
||||
/// multimodal content instead of a base64 string in JSON text.
|
||||
OkImageContent {
|
||||
image_base64: String,
|
||||
mime_type: String,
|
||||
metadata_json: Option<String>,
|
||||
},
|
||||
Err(ToolErrorCode, String),
|
||||
}
|
||||
|
||||
|
|
@ -423,24 +453,42 @@ impl ToolRegistry {
|
|||
result,
|
||||
command: None,
|
||||
json: None,
|
||||
image: None,
|
||||
},
|
||||
ToolOutcome::OkWithCommand(result, command) => ToolResponse::Ok {
|
||||
id: call.id,
|
||||
result,
|
||||
command: Some(command),
|
||||
json: None,
|
||||
image: None,
|
||||
},
|
||||
ToolOutcome::OkJson(json) => ToolResponse::Ok {
|
||||
id: call.id,
|
||||
result: BTreeMap::new(),
|
||||
command: None,
|
||||
json: Some(json),
|
||||
image: None,
|
||||
},
|
||||
ToolOutcome::OkJsonWithCommand(json, command) => ToolResponse::Ok {
|
||||
id: call.id,
|
||||
result: BTreeMap::new(),
|
||||
command: Some(command),
|
||||
json: Some(json),
|
||||
image: None,
|
||||
},
|
||||
ToolOutcome::OkImageContent {
|
||||
image_base64,
|
||||
mime_type,
|
||||
metadata_json,
|
||||
} => ToolResponse::Ok {
|
||||
id: call.id,
|
||||
result: BTreeMap::new(),
|
||||
command: None,
|
||||
json: metadata_json,
|
||||
image: Some(ImageContent {
|
||||
data: image_base64,
|
||||
mime_type,
|
||||
}),
|
||||
},
|
||||
ToolOutcome::Err(code, message) => ToolResponse::Err {
|
||||
id: call.id,
|
||||
|
|
|
|||
|
|
@ -97,6 +97,7 @@ fn response_to_json_ok_payload() {
|
|||
result,
|
||||
command: None,
|
||||
json: None,
|
||||
image: None,
|
||||
};
|
||||
let j = response_to_json(&r);
|
||||
assert!(j.contains(r#""jsonrpc":"2.0""#));
|
||||
|
|
@ -129,6 +130,7 @@ fn tool_response_to_json_wraps_ok_in_mcp_content_envelope() {
|
|||
result,
|
||||
command: None,
|
||||
json: None,
|
||||
image: None,
|
||||
};
|
||||
let j = tool_response_to_json(&r);
|
||||
assert!(j.contains(r#""id":7"#), "{j}");
|
||||
|
|
@ -148,6 +150,7 @@ fn ok_json_rides_verbatim_in_both_serializers() {
|
|||
result: BTreeMap::new(),
|
||||
command: None,
|
||||
json: Some(nested.to_string()),
|
||||
image: None,
|
||||
};
|
||||
// tools/call envelope: text block holds the nested JSON verbatim.
|
||||
assert_eq!(tool_text(&tool_response_to_json(&r)), nested);
|
||||
|
|
@ -159,6 +162,62 @@ fn ok_json_rides_verbatim_in_both_serializers() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_response_to_json_emits_image_content_block_when_image_is_some() {
|
||||
// When `image` is `Some`, the serializer emits an ImageContent block
|
||||
// BEFORE the text block so vision-capable MCP clients receive the
|
||||
// image as multimodal content.
|
||||
let r = ToolResponse::Ok {
|
||||
id: RequestId::Num(3),
|
||||
result: BTreeMap::new(),
|
||||
command: None,
|
||||
json: Some(r#"{"nodeId":"n1","format":"png"}"#.into()),
|
||||
image: Some(ImageContent {
|
||||
data: "iVBORw0KGgo=".into(),
|
||||
mime_type: "image/png".into(),
|
||||
}),
|
||||
};
|
||||
let j = tool_response_to_json(&r);
|
||||
// Parse the JSON-RPC envelope to verify structure.
|
||||
let v: serde_json::Value = serde_json::from_str(&j).expect("valid JSON-RPC");
|
||||
let content = v["result"]["content"].as_array().expect("content array");
|
||||
assert_eq!(content.len(), 2, "expected image + text blocks");
|
||||
// First block must be the image.
|
||||
assert_eq!(content[0]["type"], "image");
|
||||
assert_eq!(content[0]["data"], "iVBORw0KGgo=");
|
||||
assert_eq!(content[0]["mimeType"], "image/png");
|
||||
// Second block must be text with the metadata.
|
||||
assert_eq!(content[1]["type"], "text");
|
||||
let text_val = content[1]["text"].as_str().expect("text string");
|
||||
let meta: serde_json::Value =
|
||||
serde_json::from_str(text_val).expect("metadata parses as JSON");
|
||||
assert_eq!(meta["nodeId"], "n1");
|
||||
assert_eq!(meta["format"], "png");
|
||||
// No isError for successful results.
|
||||
assert!(v["result"].get("isError").is_none(), "{j}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_response_to_json_image_without_text_metadata_still_emits_blocks() {
|
||||
// When image is set but json/result are empty, the text block is still
|
||||
// present (with empty object) — MCP clients expect at least one text
|
||||
// block for tool result reporting.
|
||||
let r = ToolResponse::Ok {
|
||||
id: RequestId::Num(4),
|
||||
result: BTreeMap::new(),
|
||||
command: None,
|
||||
json: None,
|
||||
image: Some(ImageContent {
|
||||
data: "AAAA".into(),
|
||||
mime_type: "image/png".into(),
|
||||
}),
|
||||
};
|
||||
let j = tool_response_to_json(&r);
|
||||
assert!(j.contains(r#""type":"image""#), "{j}");
|
||||
assert!(j.contains(r#""type":"text""#), "{j}");
|
||||
assert!(j.contains(r#""data":"AAAA""#), "{j}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_response_to_json_marks_tool_error_with_iserror() {
|
||||
// A tool-level failure is an `isError` result, NOT a JSON-RPC error.
|
||||
|
|
|
|||
Loading…
Reference in a new issue