fix: Set readOnlyHint=False on create_ipaas_connection write tool - #646
fix: Set readOnlyHint=False on create_ipaas_connection write tool#646VedantMadane wants to merge 2 commits into
Conversation
- Match other write tools: ToolAnnotations(readOnlyHint=False, openWorldHint=True) - create_ipaas_connection upserts credentials and must not look read-only Fixes pipefy#644 Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
|
Thanks for the contribution! Can you please review the failed jobs @VedantMadane |
There was a problem hiding this comment.
Thank you for this first-time contribution. You closed the one write tool that still omitted readOnlyHint, which is exactly issue 644, and False is the right hint for a credential upsert.
Reviewed at a11fa0c. DCO passed. GitHub lint and test are red on format.
Verdict: needs changes into dev. Please switch the base off main first.
Required before merge
- Wrap the
create_ipaas_connectiondecorator soruff format --checkpasses. Detail and a one-click fix are on the thread onpackages/mcp/src/pipefy_mcp/tools/ipaas_tools.py:402.
Decisions I would make
- Change this PR's base to
dev. We will mergedevintomainthis week, and that is how this fix should land. Merging tomainnow would skip that train and force a back-merge.
Optional
Your call on all of these.
- Extend
test_connection_auth_url_is_not_read_onlysocreate_ipaas_connectionassertsread_only_hint is False. In this repo's Python model an omitted hint isNone, sois Falseactually catches a regression.
What worked well
- The annotation matches a real write,
meta=REMOTEstayed put, anddocs/mcp/tools/ipaas.mdalready listed the tool as not read-only.
| return await _run_ipaas_tool(ctx, pipe_id, work) | ||
|
|
||
| @mcp.tool(annotations=ToolAnnotations(openWorldHint=True), meta=REMOTE) | ||
| @mcp.tool(annotations=ToolAnnotations(readOnlyHint=False, openWorldHint=True), meta=REMOTE) |
There was a problem hiding this comment.
Blocking. Adding readOnlyHint=False on one line pushed this decorator to 99 columns. ruff format --check fails locally and on GitHub (Would reformat), so the lint and test jobs never get past format. The sibling get_ipaas_connection_auth_url already wraps the same annotations.
I would wrap this decorator the same way.
Done when:
uv run ruff format --check packages/mcp/src/pipefy_mcp/tools/ipaas_tools.pyis clean- the GitHub lint job can proceed past the format step
| @mcp.tool(annotations=ToolAnnotations(readOnlyHint=False, openWorldHint=True), meta=REMOTE) | |
| @mcp.tool( | |
| annotations=ToolAnnotations(readOnlyHint=False, openWorldHint=True), | |
| meta=REMOTE, | |
| ) |
There was a problem hiding this comment.
Wrapped the decorator to match get_ipaas_connection_auth_url and extended test_connection_auth_url_is_not_read_only so create_ipaas_connection asserts read_only_hint is False. Also retargeted base to dev.
Fixed in dc6da86.
Break the @mcp.tool decorator across lines to match sibling tools and satisfy ruff format --check. Assert create_ipaas_connection read_only_hint is False in the existing connection annotations test. Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
|
Addressed review feedback:
Commit: dc6da86 |
3 participants
Summary
Set readOnlyHint=False on create_ipaas_connection write tool
Changes
Fixes #644