refactor(mcp): typed Rx refs for prefab Confirm args + pin no-SendMessage on Cancel#85
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the MCP prefab preview-card builders to use a centralized, typed Rx helper for preview.* tool-argument bindings, and adds a regression test ensuring Cancel buttons don’t reintroduce chat-noise by firing SendMessage.
Changes:
- Introduced
_preview_ref(field) -> Rxand routed all 4 preview-card ConfirmCallTool(arguments=...)bindings through it. - Added
test_cancel_buttons_do_not_send_chat_messagesto assert Cancel buttons never include asendMessageaction in theironClickchain.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
statuspro_mcp_server/src/statuspro_mcp/tools/prefab_ui.py |
Centralizes preview.* reactive argument references via _preview_ref and updates Confirm CallTool argument wiring in all preview cards. |
statuspro_mcp_server/tests/tools/test_prefab_ui.py |
Adds a regression test that scans preview-card envelopes to ensure Cancel buttons don’t send chat messages. |
f8860b3 to
36ee222
Compare
…sage on Cancel Closes #54, closes #53. **#54 — Typed Rx refs for CallTool template args.** The four Confirm-button ``CallTool`` actions used bare-string templates like ``"{{ preview.order_id }}"`` to bind iframe state into tool arguments. A typo (``"{{ preivew.order_id }}"``) would silently expand to empty and the host would call the tool with a missing arg, with no signal on our side. Added a ``_preview_ref(field) -> Rx`` helper next to the existing action builders; the four builders now go through it: "order_id": _preview_ref("order_id"), "status_code": _preview_ref("new_status_code"), ... Wire shape is identical (``Rx`` serializes to the same template string), so all existing tests continue to pass. The win is a single namespaced choke-point and refactor-safe references — the ``preview.`` prefix can no longer be typo'd, and field names are greppable. **#53 — Pin no-SendMessage on Cancel.** PR #83 already replaced the ``SendMessage("Cancel the X")`` chain with ``[SetState("cancelled", True), ShowToast(...)]``, but no test enforced that contract. Added ``test_cancel_buttons_do_not_send_chat_messages`` which walks every preview card's envelope, finds each Cancel button, and asserts no ``sendMessage`` action in the on_click chain. Pinning so a future regression that re-introduces SendMessage on Cancel surfaces here instead of as chat-noise in production. Co-Authored-By: Claude <noreply@anthropic.com>
36ee222 to
eec6641
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #54, closes #53.
#54 — Typed
Rxrefs forCallTooltemplate args. The four Confirm-buttonCallToolactions used bare-string templates like"{{ preview.order_id }}"to bind iframe state into tool arguments. A typo ("{{ preivew.order_id }}") would silently expand to empty and the host would call the tool with a missing arg — opaque failure. Added a_preview_ref(field) -> Rxhelper; the four builders now route through it:Wire shape is identical (
Rxserializes to the same template string), so all 16 existing tests continue to pass. The win is a single namespaced choke-point: thepreview.prefix can no longer be typo'd, and field names are greppable.#53 — Pin no-SendMessage on Cancel. PR #83 already replaced the old
SendMessage("Cancel the X")chain with[SetState("cancelled", True), ShowToast(...)], but no test enforced that contract. Addedtest_cancel_buttons_do_not_send_chat_messageswhich walks every preview card's envelope, finds each Cancel button, and asserts nosendMessageaction in the on_click chain.Test plan
uv run poe check— 300/300 Python tests pass (was 299, +1 new test)"{{ preview.X }}"still pass — Rx serializes to the same template stringtest_cancel_buttons_do_not_send_chat_messageswould fail if any builder regresses toSendMessageon Cancel🤖 Generated with Claude Code