diff --git a/.changesets/fix_fix_validate_exec_doc_validation.md b/.changesets/fix_fix_validate_exec_doc_validation.md new file mode 100644 index 00000000..7c9ff17e --- /dev/null +++ b/.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 \ No newline at end of file diff --git a/crates/apollo-mcp-server/src/introspection/tools/execute.rs b/crates/apollo-mcp-server/src/introspection/tools/execute.rs index d221cab5..2f8702b3 100644 --- a/crates/apollo-mcp-server/src/introspection/tools/execute.rs +++ b/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, } diff --git a/crates/apollo-mcp-server/src/introspection/tools/validate.rs b/crates/apollo-mcp-server/src/introspection/tools/validate.rs index 8da3c785..17a66051 100644 --- a/crates/apollo-mcp-server/src/introspection/tools/validate.rs +++ b/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>>> = 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()); + } } diff --git a/e2e/mcp-server-tester/local-operations/tool-tests.yaml b/e2e/mcp-server-tester/local-operations/tool-tests.yaml index 6d088681..9d9c83d8 100644 --- a/e2e/mcp-server-tester/local-operations/tool-tests.yaml +++ b/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`' \ No newline at end of file + 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' \ No newline at end of file diff --git a/e2e/mcp-server-tester/pq-manifest/tool-tests.yaml b/e2e/mcp-server-tester/pq-manifest/tool-tests.yaml index 6d088681..e2b2fe04 100644 --- a/e2e/mcp-server-tester/pq-manifest/tool-tests.yaml +++ b/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`' \ No newline at end of file + 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'