Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changesets/fix_fix_validate_exec_doc_validation.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}

Expand Down
17 changes: 16 additions & 1 deletion crates/apollo-mcp-server/src/introspection/tools/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")],
Expand All @@ -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(),
))
});

Expand Down Expand Up @@ -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());
}
}
16 changes: 15 additions & 1 deletion e2e/mcp-server-tester/local-operations/tool-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
16 changes: 15 additions & 1 deletion e2e/mcp-server-tester/pq-manifest/tool-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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'