Skip to content

Commit 5a1a21c

Browse files
authored
Merge pull request #114 from gbrlcustodio/fix/smoke-findings
fix: smoke QA hardening (phase fields, invites, AI preflight)
2 parents a9d3980 + 2873cab commit 5a1a21c

35 files changed

Lines changed: 1107 additions & 66 deletions

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
3737

3838
- **CLI**: `pipefy agent update` resolves slug-style `fieldId` values in behaviors for error-path enrichment the same way as the happy path (via `PipefyClient.update_ai_agent`), so `RECORD_NOT_SAVED` diagnostics do not falsely blame slug tokens as unknown pipe fields.
3939
- **CLI / MCP**: `field-condition create` / `update` accept legacy `actionId: "hidden"` on condition actions; the SDK normalizes to `hide` before mutations.
40+
- **SDK**: `PipeConfigService.update_phase_field` accepts optional `phase_id` / `pipe_id` and resolves a slug-like `field_id` to the field's `uuid` (injected as `input.uuid` while the slug stays as `input.id`, matching Pipefy's `UpdatePhaseFieldInput` contract). The pipe-wide lookup runs phase fetches concurrently via `asyncio.gather`; partial phase-fetch failures raise an actionable `ValueError` instead of returning an ambiguous match. Surfaced through MCP `update_phase_field(phase_id=…, pipe_id=…)` and CLI `pipefy field update --extra '{"phase_id":"…"}'`.
41+
- **MCP**: `delete_phase_field` preview now enumerates `dependents.field_conditions` even when the rule only references the field in expression `field_address` (not just `actions[].phaseFieldId`); the condition tree walker has a defensive depth cap of 16.
42+
- **SDK / MCP**: `PipefyClient.get_automation_logs_by_repo` short-circuits to an empty page when the pipe has no automations (was returning `MULTIPLE_INVALID_INPUT: Automation_ids can't be blank` from the API).
43+
- **SDK / MCP**: `invite_members` validates each row with a new `MemberInvite` Pydantic model (`EmailStr` + non-blank `role_name`, lowercase normalization, `extra="forbid"`) and raises a single-line `ValueError` pointing at the offending field. MCP surfaces it as `INVALID_ARGUMENTS`.
44+
- **SDK**: `ai_preflight.validate_ai_automation_prompt_sdk` flags overlap when the same `%{internal_id}` appears both in the prompt and in `field_ids`, in English, citing the API rejection message.
45+
- **MCP**: `find_records` returns the unified envelope `pagination={has_more, end_cursor, page_size}` (snake_case) when the unified envelope flag is on, matching `get_table_records`.
46+
- **MCP / CLI docs**: `create_card`, `create_table_record`, `clone_pipe`, and `create_field_condition` docstrings clarify title-derivation quirks, async clone phases, and the `phaseFieldId` discovery path.
47+
- **SDK**: `MemberInvite` lives at `pipefy_sdk.MemberInvite` (re-exported in the top-level `__all__`); `slug_like_field_token` / `looks_like_uuid_token` extracted to `pipefy_sdk.utils.field_tokens` for reuse across services.
4048

4149
### Removed

docs/mcp/tools/database-tables.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Tables, records (rows), and schema columns (table fields) for org Database Table
66

77
- Same conventions as pipe building: `introspect_type` on inputs such as `CreateTableFieldInput` / `UpdateTableFieldInput`, `debug=true` on mutations, `extra_input` where the tool exposes it.
88
- **IDs:** `table_id`, record IDs, and related parameters are **strings** in GraphQL (numeric strings for many org tables, or opaque tokens such as `fIVcd19N`). Prefer quoted strings in MCP/JSON; unquoted numeric JSON may be coerced where the tool uses the shared ID type. See [Pipes & cards — Pipefy IDs](pipes-and-cards.md#pipefy-ids-type-safety).
9-
- **Pagination:** `get_table_records` and `find_records` support `first` / `after`. Read `pageInfo.hasNextPage` and `pageInfo.endCursor` from the response and pass `after=endCursor` for the next page (default page size for listing records is 50; caps apply — see tool docstrings).
9+
- **Pagination:** `get_table_records` and `find_records` support `first` / `after`. With the unified MCP envelope, read top-level `pagination.has_more` and `pagination.end_cursor` (and `pagination.page_size`) and pass `after=end_cursor` for the next page (default page size for listing records is 50; caps apply — see tool docstrings).
1010

1111
---
1212

docs/parity.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ This matrix is the source of truth for **MCP tool ↔ `pipefy` CLI** coverage. I
6464
| `export_pipe_report` | `pipefy report-pipe export` | shipped | Exports + reports. |
6565
| `fill_card_phase_fields` || deferred | Card bulk fill; not in FR-5.2 launch command list (may follow **5.2**). |
6666
| `find_cards` | `pipefy card find` | shipped | Task **5.2** (`--pipe`, `--field`, `--value`). |
67-
| `find_records` | `pipefy record find` | shipped | Task **5.7** (`--filter` JSON with `field_id` + `field_value`). |
67+
| `find_records` | `pipefy record find` | shipped | Task **5.7** (`--filter` JSON with `field_id` + `field_value`). Unified MCP envelope: top-level `pagination` uses `has_more` / `end_cursor` / `page_size` (same as `get_table_records`). |
6868
| `get_agents_usage` | `pipefy usage agents` | shipped | Task **10.3** (`--organization`, `--from`, `--to`, optional `--filters` / `--search` / `--sort` JSON). |
6969
| `get_ai_agent` | `pipefy agent get` | shipped | AI Agents domain. |
7070
| `get_ai_agent_log_details` | `pipefy agent logs get` | shipped | AI Agents domain. |
@@ -135,7 +135,7 @@ This matrix is the source of truth for **MCP tool ↔ `pipefy` CLI** coverage. I
135135
| `update_label` | `pipefy label update` | shipped | Task **5.8**. |
136136
| `update_organization_report` | `pipefy report-org update` | shipped | Organization reports. |
137137
| `update_phase` | `pipefy phase update` | shipped | Task **5.4**. |
138-
| `update_phase_field` | `pipefy field update` | shipped | Task **5.5** (``--extra`` JSON). |
138+
| `update_phase_field` | `pipefy field update` | shipped | Task **5.5** (``--extra`` JSON). Optional ``phase_id`` / ``pipe_id`` in ``--extra`` resolve slug ``field_id`` to ``internal_id`` when ``uuid`` is omitted. |
139139
| `update_pipe` | `pipefy pipe update` | shipped | Task **5.3** (`--name`, `--icon`, `--color`, `--preferences` JSON). |
140140
| `update_pipe_relation` | `pipefy relation pipe update` | shipped | Task **5.10**. |
141141
| `update_pipe_report` | `pipefy report-pipe update` | shipped | Reports domain. |

packages/cli/src/pipefy_cli/skills/pipefy-database-tables.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ Tables, records (rows), schema columns (table fields), and attachments for Pipef
1818
## Cross-cutting patterns
1919

2020
- Same conventions as pipe building: `introspect_type` on inputs such as `CreateTableFieldInput` / `UpdateTableFieldInput`, `debug=true` on mutations.
21-
- **Pagination:** `get_table_records` and `find_records` support `first` / `after`. Read `pageInfo.hasNextPage` and `pageInfo.endCursor` from the response and pass `after=endCursor` for the next page (default page size is 50).
21+
- **Pagination:** `get_table_records` and `find_records` support `first` / `after`. With the unified MCP envelope, read top-level `pagination.has_more` and `pagination.end_cursor` (and `pagination.page_size`) and pass `after=end_cursor` for the next page (default page size is 50).
2222
- **Destructive deletes** (`delete_table`, `delete_table_record`, `delete_table_field`) use a **mandatory two-step flow**: call without `confirm=true` first (preview), then with `confirm=true` after user approves.
2323

2424
---

packages/cli/tests/test_cli_tasks_10_11.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,14 @@ def test_ai_automation_validate_prompt_json(
7373
mock_client.get_pipe_with_preferences = AsyncMock(
7474
return_value={
7575
"pipe": {
76-
"phases": [{"fields": [{"internal_id": "9", "id": "f", "label": "L"}]}],
76+
"phases": [
77+
{
78+
"fields": [
79+
{"internal_id": "9", "id": "f", "label": "L"},
80+
{"internal_id": "42", "id": "o", "label": "Out"},
81+
]
82+
}
83+
],
7784
"start_form_fields": [],
7885
"preferences": {"aiAgentsEnabled": True},
7986
"organizationId": "300",
@@ -98,7 +105,7 @@ def test_ai_automation_validate_prompt_json(
98105
"--prompt",
99106
"Hello %{9}",
100107
"--field-ids",
101-
'["9"]',
108+
'["42"]',
102109
"--event-id",
103110
"card_created",
104111
"--json",
@@ -521,15 +528,21 @@ def test_ai_automation_update_auto_fetches_prompt_when_omitted(
521528
):
522529
"""When ``--prompt``/``--field-ids`` are omitted, the CLI re-uses current values for pre-flight only."""
523530
oauth_env("ai-up-auto")
524-
existing = _ai_automation_row("Summarize: %{9}", ["9"])
531+
# Prompt references field 9 as input; output field 88 is distinct so overlap preflight passes.
532+
existing = _ai_automation_row("Summarize: %{9}", ["88"])
525533
mock_client = MagicMock()
526534
mock_client.ai_automation_available = True
527535
mock_client.get_automation = AsyncMock(return_value=existing)
528536
mock_client.get_pipe_with_preferences = AsyncMock(
529537
return_value={
530538
"pipe": {
531539
"phases": [
532-
{"fields": [{"internal_id": "9", "id": "f9", "label": "L"}]}
540+
{
541+
"fields": [
542+
{"internal_id": "9", "id": "f9", "label": "L"},
543+
{"internal_id": "88", "id": "f88", "label": "Out"},
544+
]
545+
}
533546
],
534547
"start_form_fields": [],
535548
"preferences": {"aiAgentsEnabled": True},

packages/cli/tests/test_phase_field_commands.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,41 @@ def test_field_create_json(runner, clean_pipefy_env, saved_cwd, oauth_env):
118118
)
119119

120120

121+
def test_field_update_forwards_extra_phase_id_for_slug_resolution(
122+
runner, clean_pipefy_env, saved_cwd, oauth_env
123+
):
124+
"""``--extra '{"phase_id": ...}'`` reaches ``client.update_phase_field`` verbatim.
125+
126+
Locks the CLI side of the smoke-2026-05-15 slug-resolution fix: the SDK can only
127+
map ``"priority"`` to its ``internal_id`` if the CLI forwards ``phase_id`` as a
128+
keyword argument.
129+
"""
130+
oauth_env("fld-upd")
131+
mock_client = MagicMock()
132+
mock_client.update_phase_field = AsyncMock(
133+
return_value={"updatePhaseField": {"phase_field": {"id": "429358624"}}}
134+
)
135+
with patch(
136+
"pipefy_cli.commands._common.get_authenticated_client",
137+
return_value=mock_client,
138+
):
139+
result = runner.invoke(
140+
app,
141+
[
142+
"field",
143+
"update",
144+
"priority",
145+
"--extra",
146+
'{"label": "Priority", "phase_id": "343162749"}',
147+
"--json",
148+
],
149+
)
150+
assert result.exit_code == 0, result.stdout
151+
mock_client.update_phase_field.assert_awaited_once_with(
152+
"priority", label="Priority", phase_id="343162749"
153+
)
154+
155+
121156
def test_field_delete_yes(runner, clean_pipefy_env, saved_cwd, oauth_env):
122157
oauth_env("fld-del")
123158
mock_client = MagicMock()

packages/mcp/src/pipefy_mcp/tools/field_condition_tools.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,10 +212,11 @@ async def create_field_condition(
212212
objects with ``structure_id``, ``field_address``, ``operation``, ``value``) and
213213
``expressions_structure`` (array of arrays of string indices).
214214
Discover via: ``get_phase_fields(phase_id)[].internal_id`` for ``field_address``.
215-
actions: List of ``FieldConditionActionInput`` dicts; use ``phaseFieldId`` (often the
216-
field's ``internal_id`` from ``get_phase_fields``). Each action must include
217-
``actionId`` (``hide`` or ``show``); legacy ``hidden`` is mapped to ``hide``.
218-
Discover via: ``get_phase_fields(phase_id)[].internal_id`` for ``phaseFieldId``.
215+
actions: List of ``FieldConditionActionInput`` dicts; each needs ``phaseFieldId``.
216+
``phaseFieldId`` is usually the field's ``internal_id`` from ``get_phase_fields``,
217+
same pattern as ``fill_card_phase_fields`` and ``create_card_relation``.
218+
Each action must include ``actionId`` (``hide`` or ``show``); legacy ``hidden`` is
219+
mapped to ``hide``.
219220
name: Rule display name. Required by the API; may also be provided via
220221
``extra_input={"name": ...}`` for back-compat.
221222
extra_input: Optional extra keys for ``createFieldConditionInput`` (e.g. ``index``).

packages/mcp/src/pipefy_mcp/tools/member_tool_helpers.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,16 @@ def build_member_success_payload(
4545
return cast(MemberMutationSuccessPayload, payload)
4646

4747

48-
def build_member_error_payload(*, message: str) -> dict[str, Any]:
48+
def build_member_error_payload(
49+
*, message: str, code: str | None = None
50+
) -> dict[str, Any]:
4951
"""``success: False`` with ``error`` text.
5052
5153
Args:
5254
message: User-visible failure reason.
55+
code: Optional machine-readable error code (e.g. ``INVALID_ARGUMENTS``).
5356
"""
54-
return tool_error(message)
57+
return tool_error(message, code=code)
5558

5659

5760
def handle_member_tool_graphql_error(

packages/mcp/src/pipefy_mcp/tools/member_tools.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ async def invite_members(
6464
)
6565
try:
6666
raw = await client.invite_members(pipe_id, members)
67+
except ValueError as exc:
68+
return build_member_error_payload(
69+
message=str(exc),
70+
code="INVALID_ARGUMENTS",
71+
)
6772
except Exception as exc: # noqa: BLE001
6873
return handle_member_tool_graphql_error(
6974
exc,

packages/mcp/src/pipefy_mcp/tools/pipe_config_tool_helpers.py

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,32 @@ def field_condition_actions_error_message(
282282
return None
283283

284284

285+
_MAX_CONDITION_TREE_DEPTH = 16
286+
287+
288+
def _condition_tree_references_targets(
289+
obj: Any, targets: set[str], depth: int = 0
290+
) -> bool:
291+
"""Return True if any node in the condition subtree references one of the targets."""
292+
if not targets:
293+
return False
294+
if depth > _MAX_CONDITION_TREE_DEPTH:
295+
return False
296+
if isinstance(obj, dict):
297+
for key, val in obj.items():
298+
if key in ("field_address", "phaseFieldId") and val is not None:
299+
if str(val).strip() in targets:
300+
return True
301+
if _condition_tree_references_targets(val, targets, depth + 1):
302+
return True
303+
return False
304+
if isinstance(obj, list):
305+
return any(
306+
_condition_tree_references_targets(item, targets, depth + 1) for item in obj
307+
)
308+
return False
309+
310+
285311
async def find_phase_field_dependents(
286312
client: PipefyClient,
287313
*,
@@ -290,7 +316,7 @@ async def find_phase_field_dependents(
290316
field_uuid: str | None,
291317
field_slug: str | None,
292318
) -> list[dict[str, Any]]:
293-
"""Return field conditions on ``phase_id`` whose actions reference the field."""
319+
"""Return field conditions on ``phase_id`` that reference the field in actions or conditions."""
294320
try:
295321
payload = await client.get_field_conditions(phase_id)
296322
except Exception: # noqa: BLE001
@@ -303,19 +329,26 @@ async def find_phase_field_dependents(
303329
if not isinstance(cond, dict):
304330
continue
305331
actions = cond.get("actions") or []
332+
matched = False
306333
for action in actions:
307334
if not isinstance(action, dict):
308335
continue
309336
raw_pf = action.get("phaseFieldId")
310337
if raw_pf is not None and str(raw_pf) in targets:
311-
out.append(
312-
{
313-
"id": cond.get("id"),
314-
"name": cond.get("name"),
315-
"action_count": len(actions),
316-
}
317-
)
338+
matched = True
318339
break
340+
if not matched and _condition_tree_references_targets(
341+
cond.get("condition"), targets
342+
):
343+
matched = True
344+
if matched:
345+
out.append(
346+
{
347+
"id": cond.get("id"),
348+
"name": cond.get("name"),
349+
"action_count": len(actions) if isinstance(actions, list) else 0,
350+
}
351+
)
319352
return out
320353

321354

0 commit comments

Comments
 (0)