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
8 changes: 5 additions & 3 deletions docs/mcp-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Filter rows in a CSV based on criteria that require judgment.
|-----------|------|----------|-------------|
| `task` | string | Yes | Screening criteria. Rows that meet the criteria pass. |
| `input_csv` | string | Yes | Absolute path to input CSV. |
| `response_schema` | object | No | JSON schema for custom fields. Default: `{passes: bool}`. |
| `response_schema` | object | No | JSON schema for custom fields. Default: `{"type": "object", "properties": {"passes": {"type": "boolean"}}}`. |

Returns `task_id` and `session_url`. Call `everyrow_progress` to monitor.

Expand Down Expand Up @@ -73,7 +73,7 @@ Run web research agents on each row.
|-----------|------|----------|-------------|
| `task` | string | Yes | Task for the agent to perform on each row. |
| `input_csv` | string | Yes | Absolute path to input CSV. |
| `response_schema` | object | No | JSON schema for structured output. Default: `{answer: str}`. |
| `response_schema` | object | No | JSON schema for structured output. Default: `{"type": "object", "properties": {"answer": {"type": "string"}}}`. |

Returns `task_id` and `session_url`. Call `everyrow_progress` to monitor.

Expand Down Expand Up @@ -128,6 +128,7 @@ All tools that accept `response_schema` take a JSON schema object:

```json
{
"type": "object",
"properties": {
"annual_revenue": {
"type": "integer",
Expand All @@ -142,7 +143,8 @@ All tools that accept `response_schema` take a JSON schema object:
}
```

Supported types: `string`, `integer`, `number`, `boolean`, `array`, `object`.
The top-level `type` must be `object`, and the `properties` must be non-empty.
The valid field types are: `string`, `integer`, `number`, `boolean`, `array`, `object`.

## Plugin

Expand Down
1 change: 1 addition & 0 deletions everyrow-mcp/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ readme = "README.md"
requires-python = ">=3.12"
dependencies = [
"everyrow>=0.3.0",
"jsonschema>=4.0.0",
"mcp[cli]>=1.0.0",
"pandas>=2.0.0",
"pydantic>=2.0.0,<3.0.0",
Expand Down
90 changes: 87 additions & 3 deletions everyrow-mcp/src/everyrow_mcp/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
screen_async,
)
from everyrow.session import create_session, get_session_url
from jsonschema import SchemaError
from jsonschema.validators import validator_for
from mcp.server.fastmcp import FastMCP
from mcp.types import TextContent, ToolAnnotations
from pydantic import BaseModel, ConfigDict, Field, create_model, field_validator
Expand Down Expand Up @@ -132,6 +134,13 @@ def validate_input_csv(cls, v: str) -> str:
validate_csv_path(v)
return v

@field_validator("response_schema")
@classmethod
def validate_response_schema(
cls, v: dict[str, Any] | None
) -> dict[str, Any] | None:
return _validate_response_schema(v)


class RankInput(BaseModel):
"""Input for the rank operation."""
Expand Down Expand Up @@ -163,6 +172,13 @@ def validate_input_csv(cls, v: str) -> str:
validate_csv_path(v)
return v

@field_validator("response_schema")
@classmethod
def validate_response_schema(
cls, v: dict[str, Any] | None
) -> dict[str, Any] | None:
return _validate_response_schema(v)


class ScreenInput(BaseModel):
"""Input for the screen operation."""
Expand All @@ -185,6 +201,13 @@ def validate_input_csv(cls, v: str) -> str:
validate_csv_path(v)
return v

@field_validator("response_schema")
@classmethod
def validate_response_schema(
cls, v: dict[str, Any] | None
) -> dict[str, Any] | None:
return _validate_screen_response_schema(v)


class DedupeInput(BaseModel):
"""Input for the dedupe operation."""
Expand Down Expand Up @@ -894,6 +917,65 @@ async def everyrow_results(params: ResultsInput) -> list[TextContent]:
return [TextContent(type="text", text=f"Error retrieving results: {e!r}")]


def _validate_response_schema(schema: dict[str, Any] | None) -> dict[str, Any] | None:
"""Validate response_schema is a JSON Schema object schema.

If it's valid, return it. Otherwise, raise an exception.
"""
if schema is None:
return None

validator_cls = validator_for(schema)
try:
validator_cls.check_schema(schema)
except SchemaError as exc:
raise ValueError(
f"Invalid JSON Schema in response_schema: {exc.message}"
) from exc

schema_type = schema.get("type")
if schema_type not in (None, "object"):
raise ValueError(
"response_schema must describe an object response (top-level 'type' must be 'object')"
)

properties = schema.get("properties")
if not isinstance(properties, dict) or not properties:
raise ValueError(
"response_schema must include a non-empty top-level 'properties' object"
)

for field_name, field_def in properties.items():
if not isinstance(field_def, dict):
raise ValueError(
f"Invalid property schema for '{field_name}': expected an object."
)

return schema


def _validate_screen_response_schema(
schema: dict[str, Any] | None,
) -> dict[str, Any] | None:
"""Validate screen response_schema includes at least one boolean property.

If it's valid, return it. Otherwise, raise an exception.
"""
validated_schema = _validate_response_schema(schema)
if validated_schema is None:
return None

properties = validated_schema["properties"]
has_boolean_property = any(
isinstance(field_def, dict) and field_def.get("type") == "boolean"
for field_def in properties.values()
)
if not has_boolean_property:
raise ValueError("response_schema must include at least one boolean property")

return validated_schema


JSON_TYPE_MAP = {
"string": str,
"integer": int,
Expand All @@ -910,13 +992,15 @@ def _schema_to_model(name: str, schema: dict[str, Any]) -> type[BaseModel]:
This allows the MCP client to pass arbitrary response schemas without
needing to define Python classes.
"""
properties = schema.get("properties", schema)
properties = schema["properties"]
required = set(schema.get("required", []))

fields: dict[str, Any] = {}
for field_name, field_def in properties.items():
if field_name.startswith("_") or not isinstance(field_def, dict):
continue
if not isinstance(field_def, dict):
raise ValueError(
f"Invalid property schema for '{field_name}': expected an object."
)

field_type_str = field_def.get("type", "string")
python_type = JSON_TYPE_MAP.get(field_type_str, str)
Comment on lines 996 to 1006
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The new _validate_response_schema function allows underscore-prefixed field names, but Pydantic's create_model later silently ignores them, leading to unexpected data loss.
Severity: MEDIUM

Suggested Fix

Update the _validate_response_schema function to iterate through the properties in the schema and explicitly raise a ValueError if any field name starts with an underscore, similar to how other schema validations are performed.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: everyrow-mcp/src/everyrow_mcp/server.py#L992-L1006

Potential issue: The new validation function `_validate_response_schema` does not
prohibit field names that start with an underscore. While the old code explicitly
skipped these fields, the new logic allows them to pass validation. However, the
downstream call to Pydantic's `create_model` in `_schema_to_model` will silently ignore
any fields prefixed with an underscore, only issuing a `RuntimeWarning`. This
discrepancy means a schema with a field like `_internal_id` will be considered valid,
but the corresponding data will not be captured, resulting in silent data loss.

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down
108 changes: 108 additions & 0 deletions everyrow-mcp/tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@ def test_all_types(self):
model = _schema_to_model("AllTypes", schema)
assert len(model.model_fields) == 4

def test_rejects_non_object_property_schema(self):
"""Property definitions must be JSON Schema objects."""
schema = {
"type": "object",
"properties": {"score": "number"},
}

with pytest.raises(ValueError, match="Invalid property schema"):
_schema_to_model("BadSchema", schema)


class TestInputValidation:
"""Tests for input validation."""
Expand Down Expand Up @@ -119,6 +129,104 @@ def test_merge_input_validates_both_csvs(self, tmp_path: Path):
right_csv=str(tmp_path / "nonexistent.csv"),
)

def test_agent_input_rejects_empty_response_schema(self, tmp_path: Path):
csv_file = tmp_path / "test.csv"
csv_file.write_text("a,b\n1,2\n")

with pytest.raises(
ValidationError, match="must include a non-empty top-level 'properties'"
):
AgentInput(
task="test",
input_csv=str(csv_file),
response_schema={},
)

def test_agent_input_rejects_shorthand_response_schema(self, tmp_path: Path):
"""response_schema must be JSON Schema, not a field map."""
csv_file = tmp_path / "test.csv"
csv_file.write_text("a,b\n1,2\n")

with pytest.raises(
ValidationError, match="must include a non-empty top-level 'properties'"
):
AgentInput(
task="test",
input_csv=str(csv_file),
response_schema={"population": "string", "year": "string"},
)

def test_tool_inputs_accept_example_schemas(self, tmp_path: Path):
csv_file = tmp_path / "test.csv"
csv_file.write_text("a,b\n1,2\n")

ScreenInput(
task="test",
input_csv=str(csv_file),
response_schema={
"type": "object",
"properties": {
"passes": {
"type": "boolean",
},
},
},
)
AgentInput(
task="test",
input_csv=str(csv_file),
response_schema={
"type": "object",
"properties": {
"answer": {
"type": "string",
},
},
},
)
AgentInput(
task="test",
input_csv=str(csv_file),
response_schema={
"type": "object",
"properties": {
"annual_revenue": {
"type": "integer",
"description": "Annual revenue in USD",
},
"employee_count": {
"type": "integer",
"description": "Number of employees",
},
},
"required": ["annual_revenue"],
},
)

def test_screen_input_requires_boolean_property(self, tmp_path: Path):
"""Screen schemas must include at least one boolean property."""
csv_file = tmp_path / "test.csv"
csv_file.write_text("a,b\n1,2\n")

with pytest.raises(ValidationError, match="must include at least one boolean"):
ScreenInput(
task="test",
input_csv=str(csv_file),
response_schema={
"type": "object",
"properties": {"reason": {"type": "string"}},
},
)

ScreenInput(
task="test",
input_csv=str(csv_file),
response_schema={
"type": "object",
"properties": {"pass": {"type": "boolean"}},
},
)


def _make_mock_task(task_id=None):
"""Create a mock EveryrowTask with a task_id."""
Expand Down
2 changes: 2 additions & 0 deletions uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.