refactor(mcp): strip Pydantic noise from tool validation errors (#703)

[abhinyaay]

Summary

Closes #703.

Six MCP tool paths returned str(ValidationError) directly, leaking the Pydantic model name, an input_value= echo of the caller's arguments, and an errors.pydantic.dev URL to the agent:

• create_ai_agent / update_ai_agent (ai_agent_tools.py)
• create_ai_automation / update_ai_automation (ai_automation_tools.py)
• the condition parse and create_send_task_automation (automation_tools.py)

They now route through a shared format_validation_error_message() in validation_helpers.py, which renders exc.errors() as loc: msg clauses (with missing / extra_forbidden special-cased) and none of that noise. The existing _format_validation_errors in the validation envelope is refactored to delegate to the same helper, so the clause renderers collapse toward one. Non-breaking: the tools still fail in the same cases with the same error envelope — only the message text gets shorter.

Scoping note: portal_element_validation_error (portal_tool_helpers.py) is left as-is. It carries portal-specific clause handling (a value_error inner-cause unwrap and a literal-error message for type) backed by its own tests, so folding it in would change that behaviour. Happy to do it in a follow-up if you'd prefer a single renderer.

Test plan

• uv run pytest -m "not integration" — full packages/mcp suite: 2058 passed, 6 skipped
• uv run ruff check + uv run ruff format --check on the changed files — clean
• Manual smoke (if applicable): callable via Cursor MCP

Added format_validation_error_message unit tests plus a per-tool-family wiring test (create/update ai_agent, create/update ai_automation, the condition path, and create_send_task_automation) asserting the surfaced message has no input_value= and no errors.pydantic.dev. Each was confirmed to fail before the fix and pass after.

Docs / skills

• docs/parity.md updated when MCP ↔ CLI coverage changed
• Affected skills/ updated in this PR (or a paired PR)
• No docs/skills update needed — this only shortens error-message text; no MCP ↔ CLI coverage change.

Legal / contributions

• Commits include DCO sign-off (git commit -s)
• Regulated-domain skills include COMPLIANCE.md when applicable

[mocha06]

Fix these in this PR. Each comment points at something the reader cannot see.

Lines are at 1071129d.

Comments and names that point at the plan, the MR, or the release

• packages/mcp/tests/tools/test_ai_agent_tools.py:2588: "must not leak pydantic noise (MCP: tools return raw Pydantic ValidationError text from SDK input models #703)"
• packages/mcp/tests/tools/test_ai_automation_tools.py:1606: "must not leak pydantic noise (MCP: tools return raw Pydantic ValidationError text from SDK input models #703)"
• packages/mcp/tests/tools/test_automation_tools.py:536: "# The raw pydantic rendering must not leak to the agent (issue MCP: tools return raw Pydantic ValidationError text from SDK input models #703)."
• packages/mcp/tests/tools/test_automation_tools.py:1285: "the message stays clean (MCP: tools return raw Pydantic ValidationError text from SDK input models #703)"

No other test under packages/mcp/tests cites an issue number. The sentences stand on their own without it.

[mocha06]

Review against ground truth

The six paths no longer leak the model name, the input_value= echo, or the errors.pydantic.dev URL. The shared helper still keeps Pydantic's Value error, prefix on every validator error, which is the common case for these models (22 validators). One finding, inline.

The scoping decision

• The value_error unwrap in portal_element_validation_error is not portal-specific. It is how Pydantic wraps every ValueError a validator raises. Moving it into the shared helper is the fix for the finding, and lets the portal formatter delegate too.

Findings

1 finding: packages/mcp/src/pipefy_mcp/tools/validation_helpers.py:157.

4 cleanup items, posted as one note.

Read at

• pipefy/ai-toolkit @ 1071129d (PR head), base origin/dev @ 447df7b0. Probes: the real SDK input models through the new helper, pydantic 2.13.4; a mutation check on create_ai_agent (red with str(exc), green with the helper).
• Sibling repositories read-only: none cited in the comments. The hosted deployment and the Copilot consumer were checked for code that parses these messages; neither reads the message text.

[mocha06]

When a behavior prompt has a placeholder with no value, the error still starts with Value error, .

Fix: for a value_error row, use ctx["error"] as the message, as portal_element_validation_error does at portal_tool_helpers.py:245-250. Add one validator-error case to the new tests.

User scenario

1. A user in Claude Desktop asks the agent to create an AI agent. One behavior prompt contains {{customer}} and no value for it.
2. The agent calls create_ai_agent.
3. The user reads behaviors: Value error, Behavior contains {{placeholders}} but no template_params .... They expected the sentence to start at Behavior contains.

How

1. The three input models behind the six paths raise ValueError in 22 places.
2. Pydantic turns each one into a value_error row whose msg is Value error, plus the original text.
3. This line copies that msg into the clause unchanged.
4. The new tests send only empty lists and an empty string. Those are too_short rows and never reach this branch.

Evidence

• packages/mcp/src/pipefy_mcp/tools/validation_helpers.py:157 @ 1071129d (the line under review)
• packages/mcp/src/pipefy_mcp/tools/portal_tool_helpers.py:245-250 @ 1071129d (the existing unwrap; it is generic Pydantic structure, not portal-specific)
• grep -c "raise ValueError": models/ai_agent.py 17, models/ai_automation.py 4, models/send_task_automation.py 1 (the 22 validators)
• Probe, pydantic 2.13.4, CreateAiAgentInput(..., behaviors=[{"name": "b", "prompt": "hi {{missing}}"}]) through the new helper:

behaviors: Value error, Behavior contains {{placeholders}} but no template_params (or placeholders) dict was provided on this behavior.

• Same probe with the unwrap applied:

behaviors: Behavior contains {{placeholders}} but no template_params (or placeholders) dict was provided on this behavior.

• The unwrap changes only value_error rows. Nine inputs probed (missing, extra_forbidden, too_short, assertion_error, int_parsing, null, three value_error): output identical for the six non-value_error cases.
• grep -rn "Value error" packages/mcp/tests → no test pins the prefix.
• test_ai_agent_tools.py:2600, test_ai_automation_tools.py:1619, test_automation_tools.py:1294 @ 1071129d (inputs behaviors: [], field_ids: [], recipients: ""; each renders as too_short)

[adriannoes]

Thank you for the contribution. The six validation paths now share one formatter, and the shorter message still names the field and the problem.

Verdict: merge with notes into dev.

Optional

Your call on both of these.

• Two notes are on their lines in the diff.

What worked well

• One helper renders the six inner validation errors and the argument envelope. The envelope still uses the same clause loop as before.

Review path

Reviewed 1071129d against 447df7b0. CI on that head is green. A local session of this head ran the hygiene tests (24 passed) and the six invalid tool calls. Each returned success: false, named the field and the problem, and created no resource.

[adriannoes]

Optional. assert "items" in message stays green if the renderer returns only the field name and drops the constraint text.

Assert items: and a stable fragment of the min-length message. Done when: a renderer that drops the msg half fails this test.

Details

• The missing and extra cases in the next test lock the full clause.
• This line is the only lock on the normal loc: msg branch.
• The tool-family tests check that input_value= and pydantic.dev are absent. They do not check the clause text.
• A live call on this head already returned the full min-length sentence. This unit test would still pass if that sentence were dropped.

[adriannoes]

Optional. This docstring cites issue 703. Tracker numbers in source comments go stale. This repo keeps them in the PR and the commits.

Drop (#703) on this line, on the class docstring in test_ai_automation_tools.py, on the condition comment in test_automation_tools.py, and on the send-task test docstring. Done when: those four tests contain no issue number.

Suggested change

	"""The inner SDK-model ValidationError must not leak pydantic noise (#703)."""
	"""The inner SDK-model ValidationError must not leak pydantic noise."""

Details

• The same token is on test_ai_automation_tools.py line 1606, test_automation_tools.py line 536 (issue #703), and line 1285.
• test_validation_helpers.py and the production modules do not cite the issue.
• The PR title, the PR body, and both commit subjects may keep the issue number.

[abhinyaay]

Thanks for the review. Addressed in the latest commit: dropped the issue-number citations from all four flagged tests — the two must not leak pydantic noise docstrings, the # The raw pydantic rendering must not leak to the agent comment, and the the message stays clean docstring. Each sentence stands on its own now. DCO/lint green.

[mocha06]

@abhinyaay Verified at 5389f6ba: the four issue-number citations are gone and no test under packages/mcp/tests cites one. Cleanup closed. That also covers @adriannoes's note on the same lines.

Two threads are still open at this head:

• My inline finding on validation_helpers.py:157: the Value error, prefix still leaks on every validator error.
• @adriannoes's note on test_validation_helpers.py:141: the line still reads assert "items" in message, so a renderer that drops the constraint text keeps this test green.