diff --git a/packages/mcp/src/pipefy_mcp/tools/pipe_tool_helpers.py b/packages/mcp/src/pipefy_mcp/tools/pipe_tool_helpers.py index ba716d5e..a0b64ce7 100644 --- a/packages/mcp/src/pipefy_mcp/tools/pipe_tool_helpers.py +++ b/packages/mcp/src/pipefy_mcp/tools/pipe_tool_helpers.py @@ -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 ( @@ -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). diff --git a/packages/mcp/src/pipefy_mcp/tools/pipe_tools.py b/packages/mcp/src/pipefy_mcp/tools/pipe_tools.py index 042649ae..31a6ef7c 100644 --- a/packages/mcp/src/pipefy_mcp/tools/pipe_tools.py +++ b/packages/mcp/src/pipefy_mcp/tools/pipe_tools.py @@ -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, @@ -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: @@ -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). diff --git a/packages/mcp/tests/tools/test_pipe_tool_helpers.py b/packages/mcp/tests/tools/test_pipe_tool_helpers.py index 40747e76..d0b37cbc 100644 --- a/packages/mcp/tests/tools/test_pipe_tool_helpers.py +++ b/packages/mcp/tests/tools/test_pipe_tool_helpers.py @@ -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, @@ -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, @@ -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 @@ -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 # ============================================================================= diff --git a/packages/mcp/tests/tools/test_pipe_tools.py b/packages/mcp/tests/tools/test_pipe_tools.py index d2be3964..9e9fe0b9 100644 --- a/packages/mcp/tests/tools/test_pipe_tools.py +++ b/packages/mcp/tests/tools/test_pipe_tools.py @@ -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: diff --git a/packages/sdk/src/pipefy_sdk/queries/pipe_config_queries.py b/packages/sdk/src/pipefy_sdk/queries/pipe_config_queries.py index 77f4d5c2..0b72f2f4 100644 --- a/packages/sdk/src/pipefy_sdk/queries/pipe_config_queries.py +++ b/packages/sdk/src/pipefy_sdk/queries/pipe_config_queries.py @@ -221,6 +221,7 @@ } } actions { + actionId phaseFieldId } } @@ -247,6 +248,7 @@ } } actions { + actionId phaseFieldId } } diff --git a/packages/sdk/src/pipefy_sdk/queries/pipe_queries.py b/packages/sdk/src/pipefy_sdk/queries/pipe_queries.py index 5cd377ac..f37dcaf8 100644 --- a/packages/sdk/src/pipefy_sdk/queries/pipe_queries.py +++ b/packages/sdk/src/pipefy_sdk/queries/pipe_queries.py @@ -57,6 +57,7 @@ organizations { id name + pipesCount pipes(name_search: $nameSearch) { id name diff --git a/packages/sdk/src/pipefy_sdk/services/pipe_service.py b/packages/sdk/src/pipefy_sdk/services/pipe_service.py index 709bfe25..aaea7d13 100644 --- a/packages/sdk/src/pipefy_sdk/services/pipe_service.py +++ b/packages/sdk/src/pipefy_sdk/services/pipe_service.py @@ -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.""" @@ -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 @@ -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} diff --git a/packages/sdk/tests/services/pipefy/test_pipe_config_service.py b/packages/sdk/tests/services/pipefy/test_pipe_config_service.py index 74de1c9f..31ecc441 100644 --- a/packages/sdk/tests/services/pipefy/test_pipe_config_service.py +++ b/packages/sdk/tests/services/pipefy/test_pipe_config_service.py @@ -922,7 +922,7 @@ async def test_get_field_conditions_success(mock_settings): }, ], }, - "actions": [{"phaseFieldId": "pf-9"}], + "actions": [{"actionId": "hide", "phaseFieldId": "pf-9"}], }, ], }, @@ -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 @@ -945,7 +946,7 @@ 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) @@ -953,6 +954,7 @@ async def test_get_field_condition_success(mock_settings): 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 diff --git a/packages/sdk/tests/services/test_pipe_service.py b/packages/sdk/tests/services/test_pipe_service.py index 8c0b7fb0..95050a13 100644 --- a/packages/sdk/tests/services/test_pipe_service.py +++ b/packages/sdk/tests/services/test_pipe_service.py @@ -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)