feat(cli): table field create, update, delete by adriannoes · Pull Request #358 · pipefy/ai-toolkit

adriannoes · 2026-07-03T19:31:41Z

Summary

Add pipefy table field subcommand group (create / update / delete) over existing SDK facade methods
Flip three docs/parity.md rows to shipped: create_table_field, update_table_field, delete_table_field
Update database-tables skill CLI references and CLI help golden fixture
Add CHANGELOG.md Unreleased entry

Closes #355

Part of milestone CLI parity closeout (PR 1 of 3).

Test plan

uv run pytest packages/cli/tests/test_table_field_commands.py -v (4 passed)
uv run pytest packages/cli/tests/test_cli_help_golden.py -v (2 passed)
uv run ruff check . && uv run ruff format --check .
uv run pipefy table field --help lists create, update, delete

Close CLI parity for create_table_field, update_table_field, and delete_table_field via a new `pipefy table field` subcommand group.

Danielmoraisg · 2026-07-06T12:28:49Z

+    extra = parse_json_object(extra_json, "--extra") or {}
+    attrs: dict[str, Any] = {}
+    if label is not None:
+        attrs["label"] = label
+    attrs.update(extra)
+    if not attrs:
+        raise typer.BadParameter(
+            "Provide at least one of: --label, --extra (non-empty)."
+        )
+
+    async def factory(client: PipefyClient):
+        return await client.update_table_field(field_id, table_id=table, **attrs)


Silent wrong-write when --extra carries a reserved key.

--extra is merged straight into attrs (line 216) and splatted into update_table_field(field_id, table_id=table, **attrs). Two failure modes:

--extra '{"id":"999"}' lands id in attrs. The SDK sets payload["id"] = field_id then overwrites it in its merge loop, so the update silently retargets a different field than the positional field_id. No error, wrong write. This is the realistic trap: copy a field object into --extra.

--extra '{"table_id":"42"}' collides with the explicit table_id=table kwarg and raises TypeError: got multiple values for keyword argument 'table_id', which run_cli_command does not catch, so it escapes as a raw traceback (exit 1).

The MCP path guards exactly these via _UPDATE_TABLE_FIELD_EXTRA_RESERVED = {"id"} and by popping table_id out of extra_input (packages/mcp/src/pipefy_mcp/tools/table_tools.py:59). This CLI re-implements the merge without the guard.

Suggest a shared helper in _common.py (e.g. build_update_attrs(explicit, extra, *, reserved)) that strips reserved keys before splatting, used by both this and create below, rather than two hand-rolled merges.

Addressed in 8885813. merge_extra_attrs in _common.py strips id from --extra before the SDK call.

When --table is set, table_id is reserved in extra too. Typed flags override extra.

Tests:

test_table_field_update_strips_id_from_extra,

test_table_field_update_prefers_table_flag_over_extra_table_id.

Smoke tested on PipeClaw org.

Danielmoraisg · 2026-07-06T12:29:00Z

+    extra = parse_json_object(extra_json, "--extra") or {}
+    lab = label.strip()
+    ft = field_type.strip()
+    if not lab or not ft:
+        typer.echo("--label and --type must be non-empty.", err=True)
+        raise typer.Exit(2)
+
+    async def factory(client: PipefyClient):
+        return await client.create_table_field(table_id, lab, ft, **extra)


--extra reserved keys crash create.

create_table_field(table_id, lab, ft, **extra) splats --extra alongside the positional args, so:

--extra '{"label":"X"}' → TypeError: got multiple values for argument 'label'

--extra '{"table_id":"99"}' → TypeError: got multiple values for argument 'table_id'

run_cli_command catches PipefyError/TransportError/ValueError but not TypeError, so the user sees an unhandled traceback. ({"type": ...} in extra does not crash but silently overrides --type.)

The MCP tool filters these with _CREATE_TABLE_FIELD_EXTRA_RESERVED = {"table_id", "label", "type"} (table_tools.py:58). Routing this call through the same reserved-key helper suggested on the update command fixes both spots at once.

Addressed in 8885813. create passes --extra through merge_extra_attrs with reserved {table_id, label, type} before splatting. Test: test_table_field_create_strips_reserved_keys_from_extra. Verified live on PipeClaw.

Danielmoraisg · 2026-07-06T12:29:04Z

+    table: str | None = typer.Option(
+        None,
+        "--table",
+        help="Table id containing this field (recommended; required by API).",
+    ),


--table is optional here but required by the API.

delete (below) makes --table required, and the API requires table_id for updates too. As written, update f1 --label X (no --table) sends a payload without table_id and defers to a raw server-side rejection. The MCP tool returns a clean "table_id is required by the API" message before calling. Consider making --table required for parity with delete, or validating it here.

Note this is independent of the reserved-key collision above: requiring --table does not prevent a user from also passing table_id inside --extra, so both need addressing.

Kept --table optional to match MCP: table_id can come from --extra when the flag is omitted (test_table_field_update_accepts_table_id_from_extra). merge_extra_attrs prevents table_id in extra from colliding when --table is set. No local preflight for missing table_id; API error path unchanged.

Happy to require --table in a follow-up if you want parity with delete.

Danielmoraisg · 2026-07-06T12:29:10Z

+    lab = label.strip()
+    ft = field_type.strip()
+    if not lab or not ft:
+        typer.echo("--label and --type must be non-empty.", err=True)
+        raise typer.Exit(2)


Reuse + consistency (non-blocking).

Two things here:

This block is near-identical to field_create in field.py (same --label/--type options, same .strip() + empty-check). A shared helper (e.g. require_nonempty_label_type) would collapse the duplication across table fields and phase fields.

Error idiom is inconsistent within this PR: create uses typer.echo(...) + raise typer.Exit(2) while update uses typer.BadParameter. BadParameter is the idiomatic Typer path (exit 2 + usage context). Standardizing on it reads better. Relatedly, update never validates --label "" non-empty, so an empty label passes through there while create rejects it. Pick one behavior.

Deferred a shared require_nonempty_label_type helper; duplication with field_create predates this PR and is out of scope. Kept typer.Exit(2) on create to match field_create. Empty --label on update still passes through, same as MCP update_table_field.

Danielmoraisg · 2026-07-06T12:29:15Z

+    label: str | None = typer.Option(None, "--label", "-l", help="New field label."),
+    extra_json: str | None = typer.Option(
+        None,
+        "--extra",
+        help="JSON object merged into UpdateTableFieldInput.",
+    ),


Design question (non-blocking): typed flags vs --extra on update.

The MCP update_table_field exposes label, description, required, options as first-class args; here only --label is typed and everything else goes through --extra JSON. The minimalism matches table update, so it is defensible, but it is worth a conscious decision: --description/--required are common enough that pushing them into hand-written JSON is a rough edge, and it is exactly that --extra path that triggers the collision issues above. If you keep --extra-only, the reserved-key helper carries the safety; if you add typed flags, you shrink the JSON surface people actually touch.

Added --description, --required/--not-required, and --options on update (3ecea62). --extra remains for uncommon UpdateTableFieldInput keys.

Danielmoraisg · 2026-07-06T12:29:19Z

Coverage gap that maps to the --extra findings.

These tests exercise only happy paths and delete-deny; no test passes --extra at all. That is why the reserved-key collision and silent id-override on update would ship unnoticed. Whatever you decide on those, add cases with a reserved key in --extra (e.g. update ... --extra '{"id":"999"}' and create ... --extra '{"label":"X"}') plus the update "no attrs provided" BadParameter path.

Added six tests: reserved keys on create/update, table_id from extra, --table priority over extra table_id, typed flags, and BadParameter when no attrs are provided.

Danielmoraisg · 2026-07-06T12:29:22Z


 - **MCP**: opt-in OAuth 2.0 resource-server profile for the `--remote` HTTP transport. Setting `PIPEFY_MCP_RS_RESOURCE_SERVER_URL` activates it (no separate enable flag): the server validates an inbound `Authorization: Bearer` on every request (RS256 against the issuer's JWKS, issuer and expiry, optional audience), serves RFC 9728 protected-resource metadata, and returns `401` with a `WWW-Authenticate` challenge on a missing or invalid token. Token-validation knobs live in `pipefy_auth.JwtValidationSettings` (`PIPEFY_JWT_ISSUER_URL`, which defaults to the login issuer, plus `PIPEFY_JWT_AUDIENCE` / `PIPEFY_JWT_VERIFY_AUDIENCE` / `PIPEFY_JWT_JWKS_URI`); resource identity (`PIPEFY_MCP_RS_RESOURCE_SERVER_URL`, `PIPEFY_MCP_RS_REQUIRED_SCOPES`) stays in `pipefy_mcp.ResourceServerSettings`; `PIPEFY_ALLOW_INSECURE_URLS` covers both. The stdio profile is unchanged. Per-request on-behalf-of identity is not yet wired (#302), so even with a validated bearer every call runs as the single startup identity. Closes #301.
 - **SDK / MCP / CLI / auth**: outbound Pipefy requests now carry client-identifying telemetry headers so the API can attribute traffic by surface and version. The GraphQL path (public, Interfaces, and Internal API endpoints) sends `User-Agent: pipefy-sdk/<version> (<surface>)`, `X-Client-Name: <surface>`, and `X-Client-Version: <version>`, where `<surface>` is `mcp`, `cli`, or `sdk`. The surface is stamped programmatically at each entry point's composition root: the MCP server passes `mcp`, the CLI `cli`, and direct SDK use keeps the `sdk` default. It is a `PipefyClient` constructor argument rather than configuration, so it is never read from env or TOML and a caller cannot forge it. OAuth requests to the sign-in host carry `User-Agent: pipefy-auth/<version>`, `X-Client-Name: auth`, and `X-Client-Version`, identifying the auth component rather than an API surface. The header builder lives in `pipefy_infra.telemetry` so the SDK and auth packages share one `User-Agent` format. Closes #336.
+- **CLI**: `pipefy table field create|update|delete` manage database table columns, closing the three deferred table-field rows in the MCP↔CLI parity matrix (`create_table_field`, `update_table_field`, `delete_table_field`). `create` takes positional table id plus `--label`/`--type` (pass-through to `CreateTableFieldInput`); `update` sends only provided keys (`--table` recommended, required by the API); `delete` is destructive (`--yes` or interactive confirm; `--table` required).


Nit (optional): dense prose. This entry stacks several parentheticals into one long sentence. Splitting the three subcommands into short clauses (or a sub-list) would scan more easily. Purely cosmetic.

Done in ed4de16. Split the Unreleased entry into short clauses per subcommand.

Add merge_extra_attrs so create and update strip MCP-parity reserved keys before splatting into the SDK, preventing silent wrong-writes and TypeError tracebacks when --extra copies a full field object.

Expose --description, --required, and --options on pipefy table field update for MCP parity, shrinking the JSON surface agents need via --extra.

Document the new update flags and clearer Unreleased changelog prose for the table field CLI commands shipped in the parity closeout.

adriannoes added 2 commits July 3, 2026 15:25

feat(cli): add table field create, update, and delete commands

a1508b3

Close CLI parity for create_table_field, update_table_field, and delete_table_field via a new `pipefy table field` subcommand group.

docs(changelog): note table field CLI commands in Unreleased

610e072

adriannoes self-assigned this Jul 3, 2026

adriannoes requested review from Danielmoraisg and gbrlcustodio July 3, 2026 19:46

adriannoes added this to the CLI parity closeout milestone Jul 3, 2026

Danielmoraisg reviewed Jul 6, 2026

View reviewed changes

adriannoes added 3 commits July 7, 2026 16:07

fix(cli): guard table field --extra against reserved keys

8885813

Add merge_extra_attrs so create and update strip MCP-parity reserved keys before splatting into the SDK, preventing silent wrong-writes and TypeError tracebacks when --extra copies a full field object.

feat(cli): add typed flags to table field update

3ecea62

Expose --description, --required, and --options on pipefy table field update for MCP parity, shrinking the JSON surface agents need via --extra.

docs(cli): refresh table field parity, changelog, and skill

ed4de16

Document the new update flags and clearer Unreleased changelog prose for the table field CLI commands shipped in the parity closeout.

adriannoes requested a review from Danielmoraisg July 7, 2026 19:08

adriannoes added the enhancement New feature or request label Jul 7, 2026

Uh oh!

Conversation

adriannoes commented Jul 3, 2026

Summary

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adriannoes Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adriannoes Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adriannoes Jul 7, 2026 •

edited

Loading

adriannoes Jul 7, 2026 •

edited

Loading