fix(mcp): advertise precise object-rooted output schemas

flicknote-cli/src/main_tests/mcp.rs

@@ -346,6 +346,229 @@ async fn mcp_server_exposes_stable_tool_contract() {
     assert!(project_get["inputSchema"]["properties"].get("id").is_none());
 }
 
+/// 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(_) => {
+            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"
+                        | "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"
+                        | "if"
+                        | "then"
+                        | "else"
+                        | "unevaluatedItems"
+                        | "contentSchema"
+                );
+                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;
+    let tools = harness.tools().await;
+    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_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 = &note_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 = &note_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_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"]
+                .as_array()
+                .unwrap()
+                .iter()
+                .any(|field| field == "view"),
+            "{view} variant must retain the view discriminator in required"
+        );
+        let fields: BTreeSet<String> = variant["properties"]
+            .as_object()
+            .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]
 async fn mcp_note_queries_use_short_ids_and_hide_uuid() {
     let mut harness = McpHarness::start().await;

flicknote-cli/src/mcp/dto.rs

@@ -1,8 +1,13 @@
+use std::sync::Arc;
+
 use flicknote_core::services::dto::{
     ExtractionDto, NoteArchiveResult, NoteDetail, NoteMutationResult, NoteSummary, ProjectDto,
     SectionDto,
 };
-use rmcp::schemars::JsonSchema;
+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;
 
 #[derive(Debug, Serialize, JsonSchema)]
@@ -54,6 +59,7 @@ pub(super) struct McpNoteDetail {
     #[serde(flatten)]
     pub note: McpNoteSummary,
     pub content: String,
+    #[schemars(schema_with = "arbitrary_json_schema")]
     pub metadata: Option<serde_json::Value>,
     pub extractions: Vec<ExtractionDto>,
     pub sections: Vec<SectionDto>,
@@ -71,6 +77,99 @@ impl From<NoteDetail> 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.
+fn arbitrary_json_schema(_generator: &mut SchemaGenerator) -> 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.
+///
+/// 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<SourceResult> for McpSourceResult {
+    fn from(result: SourceResult) -> Self {
+        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,
+            },
+        }
+    }
+}
+
+/// 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<JsonObject> {
+    let mut schema = (*schema_for_output::<McpSourceResult>()).clone();
+    schema.insert(
+        "type".to_string(),
+        serde_json::Value::String("object".to_string()),
+    );
+    Arc::new(schema)
+}
+
 #[derive(Debug, Serialize, JsonSchema)]
 pub(super) struct McpNoteMutationResult {
     pub note: McpNoteSummary,