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
33 changes: 33 additions & 0 deletions packages/mcp/src/pipefy_mcp/tools/pipe_tool_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from typing import Any, Literal, cast

from pipefy_sdk import CardSearch, PipefyClient
from pipefy_sdk.models.comment import MAX_COMMENT_TEXT_LENGTH
from pydantic import ValidationError
from typing_extensions import TypedDict

from pipefy_mcp.tools.destructive_tool_guard import (
Expand All @@ -24,6 +26,37 @@ class UserCancelledError(Exception):
"""Raised when a user cancels an interactive flow."""


ADD_CARD_COMMENT_VALIDATION_FALLBACK_MSG = (
"Invalid input. Please provide a valid 'card_id' and non-empty 'text'."
)


def message_for_add_card_comment_validation_error(
exc: ValidationError,
*,
raw_text: str,
) -> str:
"""Map Pydantic errors from ``CommentInput`` to a user-visible MCP message.

Args:
exc: Validation failure from constructing ``CommentInput``.
raw_text: Original ``text`` argument (length hint if the error omits input).

Returns:
Actionable English message; long-text failures cite the 1000-character cap.
"""
for err in exc.errors():
if err.get("type") not in ("string_too_long", "too_long"):
continue
loc = err.get("loc") or ()
if "text" not in loc:
continue
inp = err.get("input")
got = len(inp) if isinstance(inp, str) else len(raw_text)
return f"text exceeds {MAX_COMMENT_TEXT_LENGTH}-character limit (got {got})."
return ADD_CARD_COMMENT_VALIDATION_FALLBACK_MSG


# The ``Legacy*SuccessPayload`` TypedDicts below describe the flag=false shape
# only. Under the default ``PIPEFY_MCP_UNIFIED_ENVELOPE=true``, helpers return
# ``ToolSuccessPayload`` instead (see ADR-0001).
Expand Down
11 changes: 8 additions & 3 deletions packages/mcp/src/pipefy_mcp/tools/pipe_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
map_delete_card_error_to_message,
map_delete_comment_error_to_message,
map_update_comment_error_to_message,
message_for_add_card_comment_validation_error,
)
from pipefy_mcp.tools.relation_tool_helpers import (
build_relation_error_payload,
Expand Down Expand Up @@ -302,9 +303,11 @@ async def add_card_comment(
# Privacy: never log the full comment text (it may contain sensitive data).
try:
comment_input = CommentInput(card_id=card_id, text=text)
except ValidationError:
except ValidationError as exc:
return build_add_card_comment_error_payload(
message="Invalid input. Please provide a valid 'card_id' and non-empty 'text'."
message=message_for_add_card_comment_validation_error(
exc, raw_text=text
)
)

try:
Expand Down Expand Up @@ -1054,7 +1057,9 @@ async def search_pipes(
Without a name filter, each organization returns at most ``max_pipes_per_org``
pipes (capped 1--500) to avoid huge responses. With a name filter, the API
receives a server-side ``name_search`` hint; results are still capped per org
after scoring. Check ``search_limits`` and per-org ``pipes_truncated`` when present.
after scoring. Check ``search_limits`` and per-org ``pipes_truncated`` when
present; when unfiltered, ``pipes_truncated`` is True if the API returned fewer
pipes than ``pipesCount`` for that org (incomplete visible list vs org total).

Args:
pipe_name: Optional pipe name to search for (case-insensitive partial match).
Expand Down
29 changes: 29 additions & 0 deletions packages/mcp/tests/tools/test_pipe_tool_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from unittest.mock import AsyncMock, MagicMock

import pytest
from pipefy_sdk import CommentInput
from pydantic import ValidationError

from pipefy_mcp.tools.graphql_error_helpers import (
extract_error_strings,
Expand All @@ -16,6 +18,7 @@
with_debug_suffix,
)
from pipefy_mcp.tools.pipe_tool_helpers import (
ADD_CARD_COMMENT_VALIDATION_FALLBACK_MSG,
FIND_CARDS_EMPTY_MESSAGE,
UserCancelledError,
_filter_editable_field_definitions,
Expand All @@ -27,6 +30,7 @@
find_label_dependents,
map_add_card_comment_error_to_message,
map_delete_card_error_to_message,
message_for_add_card_comment_validation_error,
)
from pipefy_mcp.tools.tool_error_envelope import tool_error

Expand Down Expand Up @@ -81,6 +85,31 @@ def test_build_add_card_comment_success_payload_parametrized_flag(envelope_flag)
assert out == {"success": True, "comment_id": "42"}


# =============================================================================
# message_for_add_card_comment_validation_error
# =============================================================================


@pytest.mark.unit
def test_message_for_add_card_comment_validation_error_string_too_long():
long_text = "x" * 1001
with pytest.raises(ValidationError) as exc_info:
CommentInput(card_id=1, text=long_text)
msg = message_for_add_card_comment_validation_error(
exc_info.value, raw_text=long_text
)
assert "1000" in msg
assert "got 1001" in msg


@pytest.mark.unit
def test_message_for_add_card_comment_validation_error_fallback_for_blank_text():
with pytest.raises(ValidationError) as exc_info:
CommentInput(card_id=1, text=" ")
msg = message_for_add_card_comment_validation_error(exc_info.value, raw_text=" ")
assert msg == ADD_CARD_COMMENT_VALIDATION_FALLBACK_MSG


# =============================================================================
# extract_error_strings
# =============================================================================
Expand Down
22 changes: 22 additions & 0 deletions packages/mcp/tests/tools/test_pipe_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1302,6 +1302,28 @@ async def test_api_exception_returns_mapped_error_payload(
payload
) or "card_id" in tool_error_message(payload)

@pytest.mark.parametrize("client_session", [None], indirect=True)
async def test_validation_error_text_over_limit_returns_explicit_length_message(
self,
client_session,
mock_pipefy_client,
extract_payload,
):
"""Comment text over max length surfaces cap and length, not generic card_id hint."""
long_text = "a" * 1001
async with client_session as session:
result = await session.call_tool(
"add_card_comment",
{"card_id": 123, "text": long_text},
)
assert result.isError is False
mock_pipefy_client.add_card_comment.assert_not_called()
payload = extract_payload(result)
assert payload["success"] is False
msg = tool_error_message(payload)
assert "1000" in msg
assert "got 1001" in msg


@pytest.mark.anyio
class TestGetPhaseFieldsTool:
Expand Down
2 changes: 2 additions & 0 deletions packages/sdk/src/pipefy_sdk/queries/pipe_config_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@
}
}
actions {
actionId
phaseFieldId
}
}
Expand All @@ -247,6 +248,7 @@
}
}
actions {
actionId
phaseFieldId
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/sdk/src/pipefy_sdk/queries/pipe_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
organizations {
id
name
pipesCount
pipes(name_search: $nameSearch) {
id
name
Expand Down
26 changes: 23 additions & 3 deletions packages/sdk/src/pipefy_sdk/services/pipe_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@ def _clamp_max_pipes_per_org(value: int) -> int:
return max(SEARCH_PIPES_MAX_PER_ORG_MIN, min(SEARCH_PIPES_MAX_PER_ORG_CAP, value))


def _coerce_org_pipes_count(raw: object) -> int | None:
"""Parse GraphQL ``Organization.pipesCount`` for truncation checks."""
if raw is None:
return None
try:
return int(raw)
except (TypeError, ValueError):
return None


class PipeService(BasePipefyClient):
"""Service for Pipe-related operations."""

Expand Down Expand Up @@ -112,6 +122,11 @@ async def search_pipes(
If pipe_name is provided, only pipes matching the name are included,
sorted by match score (best matches first).
May include ``search_limits`` describing caps applied.

When ``pipe_name`` is omitted, per-org ``pipes_truncated`` and
``search_limits.pipes_truncated`` are True if the client sliced the list
to ``max_pipes_per_org`` **or** the API returned fewer ``pipes`` than
``pipesCount`` (user-visible subset vs org total).
"""
stripped = pipe_name.strip() if pipe_name else None
name_search = stripped if stripped else None
Expand All @@ -132,17 +147,22 @@ async def search_pipes(
organizations: list[dict] = []
for org in raw_orgs:
pipes = list(org.get("pipes") or [])
truncated = len(pipes) > per_org_cap
if truncated:
pipes_count = _coerce_org_pipes_count(org.get("pipesCount"))
truncated = len(pipes) > per_org_cap or (
pipes_count is not None and len(pipes) < pipes_count
)
if len(pipes) > per_org_cap:
pipes = pipes[:per_org_cap]
limits["pipes_truncated"] = True
row: dict = {
"id": org.get("id"),
"name": org.get("name"),
"pipes": pipes,
}
if pipes_count is not None:
row["pipesCount"] = pipes_count
if truncated:
row["pipes_truncated"] = True
limits["pipes_truncated"] = True
organizations.append(row)
return {"organizations": organizations, "search_limits": limits}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,7 @@ async def test_get_field_conditions_success(mock_settings):
},
],
},
"actions": [{"phaseFieldId": "pf-9"}],
"actions": [{"actionId": "hide", "phaseFieldId": "pf-9"}],
},
],
},
Expand All @@ -932,6 +932,7 @@ async def test_get_field_conditions_success(mock_settings):

query, variables = service.execute_query.call_args[0]
assert query is GET_FIELD_CONDITIONS_QUERY
assert "actionId" in GET_FIELD_CONDITIONS_QUERY.loc.source.body
assert variables == {"phaseId": "404"}
assert result == api_payload

Expand All @@ -945,14 +946,15 @@ async def test_get_field_condition_success(mock_settings):
"name": "Rule B",
"phase": {"id": "88", "name": "Form"},
"condition": {"expressions": []},
"actions": [],
"actions": [{"actionId": "hide", "phaseFieldId": "pf-9"}],
},
}
service = _make_service(mock_settings, api_payload)
result = await service.get_field_condition("fc-2")

query, variables = service.execute_query.call_args[0]
assert query is GET_FIELD_CONDITION_QUERY
assert "actionId" in GET_FIELD_CONDITION_QUERY.loc.source.body
assert variables == {"id": "fc-2"}
assert result == api_payload

Expand Down
23 changes: 23 additions & 0 deletions packages/sdk/tests/services/test_pipe_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,29 @@ async def test_search_pipes_truncates_per_org_when_over_cap(mock_settings):
assert result["search_limits"]["pipes_truncated"] is True


@pytest.mark.unit
@pytest.mark.asyncio
async def test_search_pipes_truncates_per_org_when_api_returns_fewer_than_pipes_count(
mock_settings,
):
"""When API returns fewer pipes than Organization.pipesCount, flag as truncated."""
pipes = [{"id": str(i), "name": f"P{i}"} for i in range(10)]
mock_orgs = [{"id": "1", "name": "Org", "pipesCount": 271, "pipes": pipes}]
service = _make_service(mock_settings, {"organizations": mock_orgs})
result = await service.search_pipes(max_pipes_per_org=500)

assert len(result["organizations"][0]["pipes"]) == 10
assert result["organizations"][0]["pipes_truncated"] is True
assert result["organizations"][0]["pipesCount"] == 271
assert result["search_limits"]["pipes_truncated"] is True


@pytest.mark.unit
def test_search_pipes_query_selects_pipes_count():
printed = print_ast(SEARCH_PIPES_QUERY)
assert "pipesCount" in printed


@pytest.mark.unit
def test_get_phase_fields_query_selects_internal_id_and_uuid():
printed = print_ast(GET_PHASE_FIELDS_QUERY)
Expand Down