Merge pull request #115 from gbrlcustodio/fix/smoke-pre-merge-mcp-1-2

fix: smoke pre-merge — field condition actionId, search_pipes truncation, comment length UX

Author: adriannoes

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).

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).

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
 # =============================================================================

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:

packages/sdk/src/pipefy_sdk/queries/pipe_config_queries.py

@@ -221,6 +221,7 @@
                     }
                 }
                 actions {
+                    actionId
                     phaseFieldId
                 }
             }
@@ -247,6 +248,7 @@
                 }
             }
             actions {
+                actionId
                 phaseFieldId
             }
         }

packages/sdk/src/pipefy_sdk/queries/pipe_queries.py

@@ -57,6 +57,7 @@
         organizations {
             id
             name
+            pipesCount
             pipes(name_search: $nameSearch) {
                 id
                 name

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}
 

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,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
 

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)