fix: Validate ExecutableDocument in validate tool (#329)

The validate tool was parsing the operation passed in to it against the schema but it wasn't performing the validate function on the `ExecutableDocument` returned by the Parser. This led to cases where missing required arguments were not caught by the Tool.

This change also updates the input schema to the execute tool to make it more clear to the LLM that it needs to provide a valid JSON object

Author: swcollard

.changesets/fix_fix_validate_exec_doc_validation.md

@@ -0,0 +1,7 @@
+### fix: Validate ExecutableDocument in validate tool - @swcollard PR #329
+
+Contains fixes for https://github.com/apollographql/apollo-mcp-server/issues/327
+
+The validate tool was parsing the operation passed in to it against the schema but it wasn't performing the validate function on the ExecutableDocument returned by the Parser. This led to cases where missing required arguments were not caught by the Tool.
+
+This change also updates the input schema to the execute tool to make it more clear to the LLM that it needs to provide a valid JSON object

crates/apollo-mcp-server/src/introspection/tools/execute.rs

@@ -26,7 +26,8 @@ pub struct Input {
     /// The GraphQL operation
     query: String,
 
-    /// The variable values
+    /// The variable values represented as JSON
+    #[schemars(schema_with = "String::json_schema", default)]
     variables: Option<Value>,
 }
 

crates/apollo-mcp-server/src/introspection/tools/validate.rs

@@ -64,6 +64,8 @@ impl Validate {
         let schema_guard = self.schema.lock().await;
         Parser::new()
             .parse_executable(&schema_guard, input.operation.as_str(), "operation.graphql")
+            .map_err(|e| McpError::new(ErrorCode::INVALID_PARAMS, e.to_string(), None))?
+            .validate(&schema_guard)
             .map_err(|e| McpError::new(ErrorCode::INVALID_PARAMS, e.to_string(), None))?;
         Ok(CallToolResult {
             content: vec![Content::text("Operation is valid")],
@@ -80,7 +82,11 @@ mod tests {
     static SCHEMA: std::sync::LazyLock<Arc<Mutex<Valid<Schema>>>> =
         std::sync::LazyLock::new(|| {
             Arc::new(Mutex::new(
-                Schema::parse_and_validate("type Query { id: ID! }", "schema.graphql").unwrap(),
+                Schema::parse_and_validate(
+                    "type Query { id: ID! hello(name: String!): String! }",
+                    "schema.graphql",
+                )
+                .unwrap(),
             ))
         });
 
@@ -110,4 +116,13 @@ mod tests {
         });
         assert!(validate.execute(input).await.is_err());
     }
+
+    #[tokio::test]
+    async fn validate_invalid_argument() {
+        let validate = Validate::new(SCHEMA.clone());
+        let input = json!({
+            "operation": "query { hello }"
+        });
+        assert!(validate.execute(input).await.is_err());
+    }
 }

e2e/mcp-server-tester/local-operations/tool-tests.yaml

@@ -62,4 +62,18 @@ tools:
       expect:
         success: false
         error:
-          contains: 'Error: type `Launch` does not have a field `invalid`'
+          contains: 'Error: type `Launch` does not have a field `invalid`'
+
+    - name: 'Validates a launches query with an missing argument'
+      tool: 'validate'
+      params:
+        operation: >
+          query Agency {
+            agency {
+              id
+            }
+          }
+      expect:
+        success: false
+        error:
+          contains: 'Error: the required argument `Query.agency(id:)` is not provided'

e2e/mcp-server-tester/pq-manifest/tool-tests.yaml

@@ -62,4 +62,18 @@ tools:
       expect:
         success: false
         error:
-          contains: 'Error: type `Launch` does not have a field `invalid`'
+          contains: 'Error: type `Launch` does not have a field `invalid`'
+
+    - name: 'Validates a launches query with an missing argument'
+      tool: 'validate'
+      params:
+        operation: >
+          query Agency {
+            agency {
+              id
+            }
+          }
+      expect:
+        success: false
+        error:
+          contains: 'Error: the required argument `Query.agency(id:)` is not provided'