From 8b21d727615e51fa4162eb9da082e3ea373b0aa1 Mon Sep 17 00:00:00 2001 From: neil Date: Wed, 12 Aug 2026 11:59:02 +0800 Subject: [PATCH 1/6] fix(mcp): emit spec-compliant outputSchema for note_get --- flicknote-cli/src/main_tests/mcp.rs | 69 +++++++++++++++++++++++++++++ flicknote-cli/src/mcp/server.rs | 1 + 2 files changed, 70 insertions(+) diff --git a/flicknote-cli/src/main_tests/mcp.rs b/flicknote-cli/src/main_tests/mcp.rs index 4bd19da..2b872e5 100644 --- a/flicknote-cli/src/main_tests/mcp.rs +++ b/flicknote-cli/src/main_tests/mcp.rs @@ -346,6 +346,75 @@ async fn mcp_server_exposes_stable_tool_contract() { assert!(project_get["inputSchema"]["properties"].get("id").is_none()); } +/// Walk a JSON Schema and fail if a schema position holds a bare boolean +/// (e.g. `"metadata": true`). Boolean *keywords* such as +/// `additionalProperties: false` are left untouched, matching how strict MCP +/// clients parse `outputSchema`. +fn assert_schema_has_no_boolean_terms(schema: &serde_json::Value, tool: &str) { + match schema { + serde_json::Value::Bool(value) => { + panic!("tool {tool}: outputSchema contains boolean schema term {value}") + } + serde_json::Value::Object(map) => { + for (key, value) in map { + let is_named_schema_map = matches!( + key.as_str(), + "properties" | "patternProperties" | "$defs" | "definitions" + ); + let is_schema_array = + matches!(key.as_str(), "allOf" | "anyOf" | "oneOf" | "prefixItems"); + let is_single_schema = matches!( + key.as_str(), + "items" + | "propertyNames" + | "contains" + | "not" + | "if" + | "then" + | "else" + | "unevaluatedItems" + | "unevaluatedProperties" + | "contentSchema" + ); + if is_named_schema_map { + for (_name, sub) in value.as_object().unwrap() { + assert_schema_has_no_boolean_terms(sub, tool); + } + } else if is_schema_array { + for sub in value.as_array().unwrap() { + assert_schema_has_no_boolean_terms(sub, tool); + } + } else if is_single_schema { + assert_schema_has_no_boolean_terms(value, tool); + } + } + } + serde_json::Value::Array(values) => { + for value in values { + assert_schema_has_no_boolean_terms(value, tool); + } + } + serde_json::Value::Null | serde_json::Value::String(_) | serde_json::Value::Number(_) => {} + } +} + +#[tokio::test] +async fn mcp_tool_output_schemas_are_strict_client_compatible() { + let mut harness = McpHarness::start().await; + let tools = harness.tools().await; + assert!(!tools.is_empty()); + for tool in &tools { + let name = tool["name"].as_str().unwrap(); + let output = &tool["outputSchema"]; + assert_eq!( + output["type"], + serde_json::json!("object"), + "tool {name}: outputSchema root type must be object" + ); + assert_schema_has_no_boolean_terms(output, name); + } +} + #[tokio::test] async fn mcp_note_queries_use_short_ids_and_hide_uuid() { let mut harness = McpHarness::start().await; diff --git a/flicknote-cli/src/mcp/server.rs b/flicknote-cli/src/mcp/server.rs index 67907d8..24d1f14 100644 --- a/flicknote-cli/src/mcp/server.rs +++ b/flicknote-cli/src/mcp/server.rs @@ -189,6 +189,7 @@ impl FlickNoteMcp { #[tool( name = "note_get", description = "Get one note with editable content, metadata, extractions, and section tree.", + output_schema = object_output_schema(), annotations(read_only_hint = true) )] async fn note_get( From bd93800fd65048e32fe011028c146de2c6c6c601 Mon Sep 17 00:00:00 2001 From: neil Date: Wed, 12 Aug 2026 12:04:24 +0800 Subject: [PATCH 2/6] test(mcp): simplify outputSchema contract check --- flicknote-cli/src/main_tests/mcp.rs | 63 +++++------------------------ 1 file changed, 9 insertions(+), 54 deletions(-) diff --git a/flicknote-cli/src/main_tests/mcp.rs b/flicknote-cli/src/main_tests/mcp.rs index 2b872e5..8cf4edf 100644 --- a/flicknote-cli/src/main_tests/mcp.rs +++ b/flicknote-cli/src/main_tests/mcp.rs @@ -346,63 +346,10 @@ async fn mcp_server_exposes_stable_tool_contract() { assert!(project_get["inputSchema"]["properties"].get("id").is_none()); } -/// Walk a JSON Schema and fail if a schema position holds a bare boolean -/// (e.g. `"metadata": true`). Boolean *keywords* such as -/// `additionalProperties: false` are left untouched, matching how strict MCP -/// clients parse `outputSchema`. -fn assert_schema_has_no_boolean_terms(schema: &serde_json::Value, tool: &str) { - match schema { - serde_json::Value::Bool(value) => { - panic!("tool {tool}: outputSchema contains boolean schema term {value}") - } - serde_json::Value::Object(map) => { - for (key, value) in map { - let is_named_schema_map = matches!( - key.as_str(), - "properties" | "patternProperties" | "$defs" | "definitions" - ); - let is_schema_array = - matches!(key.as_str(), "allOf" | "anyOf" | "oneOf" | "prefixItems"); - let is_single_schema = matches!( - key.as_str(), - "items" - | "propertyNames" - | "contains" - | "not" - | "if" - | "then" - | "else" - | "unevaluatedItems" - | "unevaluatedProperties" - | "contentSchema" - ); - if is_named_schema_map { - for (_name, sub) in value.as_object().unwrap() { - assert_schema_has_no_boolean_terms(sub, tool); - } - } else if is_schema_array { - for sub in value.as_array().unwrap() { - assert_schema_has_no_boolean_terms(sub, tool); - } - } else if is_single_schema { - assert_schema_has_no_boolean_terms(value, tool); - } - } - } - serde_json::Value::Array(values) => { - for value in values { - assert_schema_has_no_boolean_terms(value, tool); - } - } - serde_json::Value::Null | serde_json::Value::String(_) | serde_json::Value::Number(_) => {} - } -} - #[tokio::test] async fn mcp_tool_output_schemas_are_strict_client_compatible() { let mut harness = McpHarness::start().await; let tools = harness.tools().await; - assert!(!tools.is_empty()); for tool in &tools { let name = tool["name"].as_str().unwrap(); let output = &tool["outputSchema"]; @@ -411,7 +358,15 @@ async fn mcp_tool_output_schemas_are_strict_client_compatible() { serde_json::json!("object"), "tool {name}: outputSchema root type must be object" ); - assert_schema_has_no_boolean_terms(output, name); + for (property, subschema) in output["properties"] + .as_object() + .unwrap_or(&serde_json::Map::new()) + { + assert!( + subschema.is_object(), + "tool {name}: outputSchema property {property} must be an object, got {subschema}" + ); + } } } From fb237720ba2abf80cb9b2d514e492af901d20979 Mon Sep 17 00:00:00 2001 From: neil Date: Wed, 12 Aug 2026 12:33:49 +0800 Subject: [PATCH 3/6] fix(mcp): advertise precise object-rooted output schemas --- flicknote-cli/src/main_tests/mcp.rs | 187 ++++++++++++++++++++++++++-- flicknote-cli/src/mcp/dto.rs | 82 +++++++++++- flicknote-cli/src/mcp/server.rs | 29 +---- 3 files changed, 267 insertions(+), 31 deletions(-) diff --git a/flicknote-cli/src/main_tests/mcp.rs b/flicknote-cli/src/main_tests/mcp.rs index 8cf4edf..c799f9f 100644 --- a/flicknote-cli/src/main_tests/mcp.rs +++ b/flicknote-cli/src/main_tests/mcp.rs @@ -346,6 +346,49 @@ async fn mcp_server_exposes_stable_tool_contract() { assert!(project_get["inputSchema"]["properties"].get("id").is_none()); } +/// Fail if a schema position holds a bare boolean (e.g. `"metadata": true`). +/// Boolean *keyword* values such as `additionalProperties: false` are not +/// schema positions and are left untouched. +fn assert_no_boolean_schema_terms(schema: &serde_json::Value, tool: &str) { + match schema { + serde_json::Value::Bool(_) => { + panic!("tool {tool}: outputSchema contains a boolean schema term") + } + serde_json::Value::Object(map) => { + for (key, value) in map { + let is_named_schemas = + matches!(key.as_str(), "properties" | "patternProperties" | "$defs"); + let is_schema_list = + matches!(key.as_str(), "allOf" | "anyOf" | "oneOf" | "prefixItems"); + let is_single_schema = matches!( + key.as_str(), + "items" + | "propertyNames" + | "contains" + | "not" + | "if" + | "then" + | "else" + | "unevaluatedItems" + | "unevaluatedProperties" + ); + if is_named_schemas { + for subschema in value.as_object().unwrap().values() { + assert_no_boolean_schema_terms(subschema, tool); + } + } else if is_schema_list { + for subschema in value.as_array().unwrap() { + assert_no_boolean_schema_terms(subschema, tool); + } + } else if is_single_schema { + assert_no_boolean_schema_terms(value, tool); + } + } + } + _ => {} + } +} + #[tokio::test] async fn mcp_tool_output_schemas_are_strict_client_compatible() { let mut harness = McpHarness::start().await; @@ -358,16 +401,146 @@ async fn mcp_tool_output_schemas_are_strict_client_compatible() { serde_json::json!("object"), "tool {name}: outputSchema root type must be object" ); - for (property, subschema) in output["properties"] + assert_no_boolean_schema_terms(output, name); + } + + let list = tools + .iter() + .find(|tool| tool["name"] == "note_list") + .unwrap(); + assert!( + list["outputSchema"]["properties"]["notes"]["items"].is_object(), + "note_list outputSchema must advertise the notes array item schema" + ); + let projects = tools + .iter() + .find(|tool| tool["name"] == "project_list") + .unwrap(); + assert!( + projects["outputSchema"]["properties"]["projects"]["items"].is_object(), + "project_list outputSchema must advertise the projects array item schema" + ); +} + +#[tokio::test] +async fn mcp_note_get_output_schema_advertises_detail_structure() { + let mut harness = McpHarness::start().await; + let tools = harness.tools().await; + let note_get = tools + .iter() + .find(|tool| tool["name"] == "note_get") + .unwrap(); + let output = ¬e_get["outputSchema"]; + let properties = output["properties"].as_object().unwrap(); + for required_property in ["content", "metadata", "extractions", "sections"] { + assert!( + properties.contains_key(required_property), + "note_get outputSchema must advertise {required_property}" + ); + } + let required = output["required"].as_array().unwrap(); + for required_field in ["content", "extractions", "sections"] { + assert!( + required.iter().any(|field| field == required_field), + "note_get outputSchema must require {required_field}" + ); + } + let metadata = &properties["metadata"]; + assert!( + metadata.is_object(), + "note_get metadata schema must be an object-form term, got {metadata}" + ); + let metadata_types = metadata["type"].as_array().unwrap(); + for json_type in ["null", "object", "array", "string", "number", "boolean"] { + assert!( + metadata_types.iter().any(|allowed| allowed == json_type), + "note_get metadata schema must represent arbitrary JSON including {json_type}" + ); + } +} + +#[tokio::test] +async fn mcp_note_source_output_schema_advertises_all_views() { + let mut harness = McpHarness::start().await; + let tools = harness.tools().await; + let note_source = tools + .iter() + .find(|tool| tool["name"] == "note_source") + .unwrap(); + let output = ¬e_source["outputSchema"]; + assert_eq!( + output["type"], + serde_json::json!("object"), + "note_source outputSchema root type must be object" + ); + let variants = output["oneOf"].as_array().unwrap(); + assert_eq!( + variants.len(), + 3, + "note_source must advertise rendered, raw, and info variants" + ); + let mut views = BTreeSet::new(); + let mut rendered_fields = BTreeSet::new(); + let mut raw_value_types = Vec::new(); + let mut info_fields = BTreeSet::new(); + for variant in variants { + let view = variant["properties"]["view"]["enum"][0].as_str().unwrap(); + views.insert(view.to_string()); + assert!( + variant["required"] + .as_array() + .unwrap() + .iter() + .any(|field| field == "view"), + "{view} variant must retain the view discriminator in required" + ); + let fields: BTreeSet = variant["properties"] .as_object() - .unwrap_or(&serde_json::Map::new()) - { - assert!( - subschema.is_object(), - "tool {name}: outputSchema property {property} must be an object, got {subschema}" - ); + .unwrap() + .keys() + .cloned() + .collect(); + match view { + "rendered" => rendered_fields = fields, + "raw" => { + raw_value_types = variant["properties"]["value"]["type"] + .as_array() + .unwrap() + .clone() + } + "info" => info_fields = fields, + other => panic!("unexpected source view {other}"), } } + assert_eq!( + views, + BTreeSet::from(["rendered", "raw", "info"].map(String::from)) + ); + for field in [ + "view", + "source_type", + "range_unit", + "total_count", + "selected_start", + "selected_end", + "content", + ] { + assert!( + rendered_fields.contains(field), + "rendered variant must advertise {field}" + ); + } + assert!( + raw_value_types.contains(&serde_json::json!("null")) + && raw_value_types.contains(&serde_json::json!("object")), + "raw variant value must represent arbitrary JSON including null" + ); + for field in ["view", "source_type", "range_unit", "count"] { + assert!( + info_fields.contains(field), + "info variant must advertise {field}" + ); + } } #[tokio::test] diff --git a/flicknote-cli/src/mcp/dto.rs b/flicknote-cli/src/mcp/dto.rs index 9bee3b2..7070cfd 100644 --- a/flicknote-cli/src/mcp/dto.rs +++ b/flicknote-cli/src/mcp/dto.rs @@ -1,8 +1,11 @@ +use std::borrow::Cow; + use flicknote_core::services::dto::{ ExtractionDto, NoteArchiveResult, NoteDetail, NoteMutationResult, NoteSummary, ProjectDto, SectionDto, }; -use rmcp::schemars::JsonSchema; +use flicknote_core::services::source::SourceResult; +use rmcp::schemars::{JsonSchema, Schema, SchemaGenerator}; use serde::Serialize; #[derive(Debug, Serialize, JsonSchema)] @@ -54,6 +57,7 @@ pub(super) struct McpNoteDetail { #[serde(flatten)] pub note: McpNoteSummary, pub content: String, + #[schemars(schema_with = "arbitrary_json_schema")] pub metadata: Option, pub extractions: Vec, pub sections: Vec, @@ -71,6 +75,82 @@ impl From for McpNoteDetail { } } +/// Object-form JSON Schema term for arbitrary JSON values. +/// +/// `serde_json::Value` derives a bare boolean schema term (`true`) that strict +/// MCP clients reject. This term keeps arbitrary values unconstrained while +/// remaining a parseable JSON Schema object. `null` is included because +/// `Option` fields serialize as `null` when absent. +fn arbitrary_json_schema(_generator: &mut SchemaGenerator) -> Schema { + serde_json::from_value(serde_json::json!({ + "type": ["array", "boolean", "integer", "null", "number", "object", "string"], + })) + .expect("arbitrary-json schema is valid JSON Schema") +} + +/// MCP boundary result for source queries. +/// +/// Serializes identically to `SourceResult` (a `view`-tagged union) while +/// advertising an object-rooted JSON Schema whose variants describe each +/// source view and use object-form terms for the arbitrary raw value, both of +/// which strict MCP clients require. +#[derive(Debug, Serialize)] +#[serde(transparent)] +pub(super) struct McpSourceResult(pub(super) SourceResult); + +impl From for McpSourceResult { + fn from(result: SourceResult) -> Self { + Self(result) + } +} + +impl JsonSchema for McpSourceResult { + fn schema_name() -> Cow<'static, str> { + Cow::Borrowed("McpSourceResult") + } + + fn json_schema(_generator: &mut SchemaGenerator) -> Schema { + serde_json::from_value(serde_json::json!({ + "type": "object", + "oneOf": [ + { + "type": "object", + "properties": { + "view": {"type": "string", "enum": ["rendered"]}, + "source_type": {"type": "string"}, + "range_unit": {"type": "string"}, + "total_count": {"type": "integer", "format": "uint", "minimum": 0}, + "selected_start": {"type": "integer", "format": "uint", "minimum": 0}, + "selected_end": {"type": "integer", "format": "uint", "minimum": 0}, + "content": {"type": "string"} + }, + "required": ["view", "source_type", "range_unit", "total_count", "selected_start", "selected_end", "content"] + }, + { + "type": "object", + "properties": { + "view": {"type": "string", "enum": ["raw"]}, + "source_type": {"type": "string"}, + "value": {"type": ["array", "boolean", "integer", "null", "number", "object", "string"]} + }, + "required": ["view", "source_type", "value"] + }, + { + "type": "object", + "properties": { + "view": {"type": "string", "enum": ["info"]}, + "source_type": {"type": "string"}, + "range_unit": {"type": "string"}, + "count": {"type": "integer", "format": "uint", "minimum": 0} + }, + "required": ["view", "source_type", "range_unit", "count"] + } + ] + })) + .expect("source-result schema is valid JSON Schema") + } +} + #[derive(Debug, Serialize, JsonSchema)] pub(super) struct McpNoteMutationResult { pub note: McpNoteSummary, diff --git a/flicknote-cli/src/mcp/server.rs b/flicknote-cli/src/mcp/server.rs index 24d1f14..cbf7ad1 100644 --- a/flicknote-cli/src/mcp/server.rs +++ b/flicknote-cli/src/mcp/server.rs @@ -13,14 +13,14 @@ use flicknote_core::services::source::SourceResult; use flicknote_sync::ipc::{AppRequest, AppResult, DaemonClient}; use rmcp::handler::server::router::tool::ToolRouter; use rmcp::handler::server::wrapper::Parameters; -use rmcp::model::{CallToolResult, Implementation, JsonObject, ServerCapabilities, ServerInfo}; +use rmcp::model::{CallToolResult, Implementation, ServerCapabilities, ServerInfo}; use rmcp::schemars::JsonSchema; use rmcp::{Json, ServerHandler, ServiceExt, tool, tool_handler, tool_router}; use serde::Serialize; use super::dto::{ McpNoteArchiveResult, McpNoteDetail, McpNoteListResult, McpNoteMutationResult, McpNoteSummary, - McpProjectDto, McpProjectListResult, + McpProjectDto, McpProjectListResult, McpSourceResult, }; use super::error::tool_error; use super::note_tools::*; @@ -100,22 +100,6 @@ fn structured(result: Result) -> Result, CallToolRes result.map(Json).map_err(|error| tool_error(&error)) } -/// Minimal, spec-compliant output schema (`type: "object"`). -/// -/// MCP 2025-era clients validate `outputSchema.type` strictly and reject root -/// types other than `"object"` (e.g. the `"array"` schemas derived from list -/// return types, or the `oneOf` union derived from `SourceResult`). Each tool's -/// description documents the actual result shape, so this declaration stays -/// valid for strict clients without inventing structure. -fn object_output_schema() -> Arc { - let mut schema = JsonObject::new(); - schema.insert( - "type".to_string(), - serde_json::Value::String("object".to_string()), - ); - Arc::new(schema) -} - #[tool_router(router = tool_router)] impl FlickNoteMcp { #[tool( @@ -189,7 +173,6 @@ impl FlickNoteMcp { #[tool( name = "note_get", description = "Get one note with editable content, metadata, extractions, and section tree.", - output_schema = object_output_schema(), annotations(read_only_hint = true) )] async fn note_get( @@ -227,21 +210,21 @@ impl FlickNoteMcp { #[tool( name = "note_source", description = "Read stored source data as rendered content, raw JSON/text, or compact info. Normal notes often have no source data; use note_get for editable content. Use info then a 1-based range for large text or meeting sources.", - output_schema = object_output_schema(), annotations(read_only_hint = true) )] async fn note_source( &self, Parameters(params): Parameters, - ) -> Result, CallToolResult> { + ) -> Result, CallToolResult> { structured( - self.call(AppRequest::NoteSource { + self.call::(AppRequest::NoteSource { id: params.id.to_string(), archived: params.archived, view: params.view, range: params.range, }) - .await, + .await + .map(Into::into), ) } From 26ec35a526dbf2cb62bda82e0f6ea0475ce74375 Mon Sep 17 00:00:00 2001 From: neil Date: Wed, 12 Aug 2026 12:41:42 +0800 Subject: [PATCH 4/6] refactor(mcp): deduplicate arbitrary-JSON schema type list --- flicknote-cli/src/mcp/dto.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/flicknote-cli/src/mcp/dto.rs b/flicknote-cli/src/mcp/dto.rs index 7070cfd..cede764 100644 --- a/flicknote-cli/src/mcp/dto.rs +++ b/flicknote-cli/src/mcp/dto.rs @@ -75,17 +75,20 @@ impl From for McpNoteDetail { } } +/// Every JSON value type plus `null` (for `Option` fields). Used wherever +/// `serde_json::Value` appears so the schema stays an object-form term. +const ARBITRARY_JSON_TYPES: [&str; 7] = [ + "array", "boolean", "integer", "null", "number", "object", "string", +]; + /// Object-form JSON Schema term for arbitrary JSON values. /// /// `serde_json::Value` derives a bare boolean schema term (`true`) that strict /// MCP clients reject. This term keeps arbitrary values unconstrained while -/// remaining a parseable JSON Schema object. `null` is included because -/// `Option` fields serialize as `null` when absent. +/// remaining a parseable JSON Schema object. fn arbitrary_json_schema(_generator: &mut SchemaGenerator) -> Schema { - serde_json::from_value(serde_json::json!({ - "type": ["array", "boolean", "integer", "null", "number", "object", "string"], - })) - .expect("arbitrary-json schema is valid JSON Schema") + serde_json::from_value(serde_json::json!({ "type": ARBITRARY_JSON_TYPES })) + .expect("arbitrary-json schema is valid JSON Schema") } /// MCP boundary result for source queries. @@ -131,7 +134,7 @@ impl JsonSchema for McpSourceResult { "properties": { "view": {"type": "string", "enum": ["raw"]}, "source_type": {"type": "string"}, - "value": {"type": ["array", "boolean", "integer", "null", "number", "object", "string"]} + "value": {"type": ARBITRARY_JSON_TYPES} }, "required": ["view", "source_type", "value"] }, From 4de82d71dedec713bfd38e8c1cb5e57450ca30b2 Mon Sep 17 00:00:00 2001 From: neil Date: Wed, 12 Aug 2026 12:55:20 +0800 Subject: [PATCH 5/6] refactor(mcp): derive source schema from boundary DTO --- flicknote-cli/src/main_tests/mcp.rs | 13 ++- flicknote-cli/src/mcp/dto.rs | 124 ++++++++++++++++------------ flicknote-cli/src/mcp/server.rs | 3 +- 3 files changed, 84 insertions(+), 56 deletions(-) diff --git a/flicknote-cli/src/main_tests/mcp.rs b/flicknote-cli/src/main_tests/mcp.rs index c799f9f..7bde520 100644 --- a/flicknote-cli/src/main_tests/mcp.rs +++ b/flicknote-cli/src/main_tests/mcp.rs @@ -484,7 +484,18 @@ async fn mcp_note_source_output_schema_advertises_all_views() { let mut raw_value_types = Vec::new(); let mut info_fields = BTreeSet::new(); for variant in variants { - let view = variant["properties"]["view"]["enum"][0].as_str().unwrap(); + let view_schema = &variant["properties"]["view"]; + let view = view_schema["const"] + .as_str() + .or_else(|| { + view_schema["enum"] + .as_array() + .and_then(|values| values.first()) + .and_then(|value| value.as_str()) + }) + .unwrap_or_else(|| { + panic!("variant must declare its view discriminator via const or enum") + }); views.insert(view.to_string()); assert!( variant["required"] diff --git a/flicknote-cli/src/mcp/dto.rs b/flicknote-cli/src/mcp/dto.rs index cede764..ea2b81c 100644 --- a/flicknote-cli/src/mcp/dto.rs +++ b/flicknote-cli/src/mcp/dto.rs @@ -1,10 +1,12 @@ -use std::borrow::Cow; +use std::sync::Arc; use flicknote_core::services::dto::{ ExtractionDto, NoteArchiveResult, NoteDetail, NoteMutationResult, NoteSummary, ProjectDto, SectionDto, }; use flicknote_core::services::source::SourceResult; +use rmcp::handler::server::tool::schema_for_output; +use rmcp::model::JsonObject; use rmcp::schemars::{JsonSchema, Schema, SchemaGenerator}; use serde::Serialize; @@ -93,65 +95,79 @@ fn arbitrary_json_schema(_generator: &mut SchemaGenerator) -> Schema { /// MCP boundary result for source queries. /// -/// Serializes identically to `SourceResult` (a `view`-tagged union) while -/// advertising an object-rooted JSON Schema whose variants describe each -/// source view and use object-form terms for the arbitrary raw value, both of -/// which strict MCP clients require. -#[derive(Debug, Serialize)] -#[serde(transparent)] -pub(super) struct McpSourceResult(pub(super) SourceResult); +/// Mirrors `SourceResult`'s `view`-tagged variants so that the advertised +/// output schema and the serialized `structuredContent` evolve together. The +/// raw `value` keeps an object-form arbitrary-JSON term (its only schema +/// customization); the explicit object root is applied by +/// [`source_output_schema`]. +#[derive(Debug, Serialize, JsonSchema)] +#[serde(tag = "view", rename_all = "snake_case")] +pub(super) enum McpSourceResult { + Rendered { + source_type: String, + range_unit: String, + total_count: usize, + selected_start: usize, + selected_end: usize, + content: String, + }, + Raw { + source_type: String, + #[schemars(schema_with = "arbitrary_json_schema")] + value: serde_json::Value, + }, + Info { + source_type: String, + range_unit: String, + count: usize, + }, +} impl From for McpSourceResult { fn from(result: SourceResult) -> Self { - Self(result) + match result { + SourceResult::Rendered { + source_type, + range_unit, + total_count, + selected_start, + selected_end, + content, + } => Self::Rendered { + source_type, + range_unit, + total_count, + selected_start, + selected_end, + content, + }, + SourceResult::Raw { source_type, value } => Self::Raw { source_type, value }, + SourceResult::Info { + source_type, + range_unit, + count, + } => Self::Info { + source_type, + range_unit, + count, + }, + } } } -impl JsonSchema for McpSourceResult { - fn schema_name() -> Cow<'static, str> { - Cow::Borrowed("McpSourceResult") - } - - fn json_schema(_generator: &mut SchemaGenerator) -> Schema { - serde_json::from_value(serde_json::json!({ - "type": "object", - "oneOf": [ - { - "type": "object", - "properties": { - "view": {"type": "string", "enum": ["rendered"]}, - "source_type": {"type": "string"}, - "range_unit": {"type": "string"}, - "total_count": {"type": "integer", "format": "uint", "minimum": 0}, - "selected_start": {"type": "integer", "format": "uint", "minimum": 0}, - "selected_end": {"type": "integer", "format": "uint", "minimum": 0}, - "content": {"type": "string"} - }, - "required": ["view", "source_type", "range_unit", "total_count", "selected_start", "selected_end", "content"] - }, - { - "type": "object", - "properties": { - "view": {"type": "string", "enum": ["raw"]}, - "source_type": {"type": "string"}, - "value": {"type": ARBITRARY_JSON_TYPES} - }, - "required": ["view", "source_type", "value"] - }, - { - "type": "object", - "properties": { - "view": {"type": "string", "enum": ["info"]}, - "source_type": {"type": "string"}, - "range_unit": {"type": "string"}, - "count": {"type": "integer", "format": "uint", "minimum": 0} - }, - "required": ["view", "source_type", "range_unit", "count"] - } - ] - })) - .expect("source-result schema is valid JSON Schema") - } +/// Object-rooted output schema for `McpSourceResult`. +/// +/// Derives the union from the boundary DTO (so schema and serialization stay +/// in sync) and applies the single local compatibility fix: internally tagged +/// enum unions derive without an explicit root `type`, which strict MCP +/// clients reject. +pub(super) fn source_output_schema() -> Arc { + let mut schema = (*schema_for_output::()).clone(); + schema.insert( + "type".to_string(), + serde_json::Value::String("object".to_string()), + ); + Arc::new(schema) } #[derive(Debug, Serialize, JsonSchema)] diff --git a/flicknote-cli/src/mcp/server.rs b/flicknote-cli/src/mcp/server.rs index cbf7ad1..bc7e107 100644 --- a/flicknote-cli/src/mcp/server.rs +++ b/flicknote-cli/src/mcp/server.rs @@ -20,7 +20,7 @@ use serde::Serialize; use super::dto::{ McpNoteArchiveResult, McpNoteDetail, McpNoteListResult, McpNoteMutationResult, McpNoteSummary, - McpProjectDto, McpProjectListResult, McpSourceResult, + McpProjectDto, McpProjectListResult, McpSourceResult, source_output_schema, }; use super::error::tool_error; use super::note_tools::*; @@ -210,6 +210,7 @@ impl FlickNoteMcp { #[tool( name = "note_source", description = "Read stored source data as rendered content, raw JSON/text, or compact info. Normal notes often have no source data; use note_get for editable content. Use info then a 1-based range for large text or meeting sources.", + output_schema = source_output_schema(), annotations(read_only_hint = true) )] async fn note_source( From ea6eedafa6b90a74dff594db6a17c12afb76a083 Mon Sep 17 00:00:00 2001 From: neil Date: Wed, 12 Aug 2026 13:13:27 +0800 Subject: [PATCH 6/6] test(mcp): make boolean-schema walker cover all subschema positions --- flicknote-cli/src/main_tests/mcp.rs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/flicknote-cli/src/main_tests/mcp.rs b/flicknote-cli/src/main_tests/mcp.rs index 7bde520..02cca88 100644 --- a/flicknote-cli/src/main_tests/mcp.rs +++ b/flicknote-cli/src/main_tests/mcp.rs @@ -346,9 +346,16 @@ async fn mcp_server_exposes_stable_tool_contract() { assert!(project_get["inputSchema"]["properties"].get("id").is_none()); } -/// Fail if a schema position holds a bare boolean (e.g. `"metadata": true`). -/// Boolean *keyword* values such as `additionalProperties: false` are not -/// schema positions and are left untouched. +/// Fail if any schema position holds a bare boolean schema (e.g. +/// `"metadata": true`). +/// +/// JSON Schema itself allows booleans as schema shorthand — `true` accepts +/// anything, `additionalProperties: false` forbids extra keys. This test is a +/// proxy for strict MCP clients that reject boolean schema terms, so every +/// schema position (including `additionalProperties` and +/// `unevaluatedProperties`) must stay an object-form term. The walk covers the +/// subschema keywords of JSON Schema 2020-12, skipping non-schema keywords +/// such as `required`, `enum`, and `type`. fn assert_no_boolean_schema_terms(schema: &serde_json::Value, tool: &str) { match schema { serde_json::Value::Bool(_) => { @@ -356,13 +363,21 @@ fn assert_no_boolean_schema_terms(schema: &serde_json::Value, tool: &str) { } serde_json::Value::Object(map) => { for (key, value) in map { - let is_named_schemas = - matches!(key.as_str(), "properties" | "patternProperties" | "$defs"); + let is_named_schemas = matches!( + key.as_str(), + "properties" + | "patternProperties" + | "$defs" + | "definitions" + | "dependentSchemas" + ); let is_schema_list = matches!(key.as_str(), "allOf" | "anyOf" | "oneOf" | "prefixItems"); let is_single_schema = matches!( key.as_str(), "items" + | "additionalProperties" + | "unevaluatedProperties" | "propertyNames" | "contains" | "not" @@ -370,7 +385,7 @@ fn assert_no_boolean_schema_terms(schema: &serde_json::Value, tool: &str) { | "then" | "else" | "unevaluatedItems" - | "unevaluatedProperties" + | "contentSchema" ); if is_named_schemas { for subschema in value.as_object().unwrap().values() {