Skip to content

refactor(mcp): typed Rx refs for prefab Confirm args + pin no-SendMessage on Cancel #85

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dougborg merged 1 commit into from
May 20, 2026
Merged

refactor(mcp): typed Rx refs for prefab Confirm args + pin no-SendMessage on Cancel #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 36 additions & 18 deletions statuspro_mcp_server/src/statuspro_mcp/tools/prefab_ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,24 @@ def _status_chip(code: str | None, name: str | None, color: str | None) -> None:
}


def _preview_ref(field: str) -> Rx:
"""Build a reactive reference to ``preview.<field>`` in iframe state.

The Confirm-button ``CallTool`` actions wire iframe state into tool
arguments via templates like ``{{ preview.order_id }}``. Bare strings
are fragile: a typo in the prefix (``{{ preivew.order_id }}``) silently
expands to empty and the host calls the tool with a missing arg.

This helper doesn't validate the field at runtime — ``Rx`` accepts any
string. The win is structural: the ``preview.`` prefix exists in
exactly one place (this function body), so prefix typos at call sites
are impossible. Field-name typos like ``_preview_ref("ordr_id")`` are
still possible, but the field name is now a bare identifier that's
greppable and refactor-safe (vs. embedded in a template string).
"""
return Rx(f"preview.{field}")


def _build_confirm_action(
tool: str,
arguments: dict[str, Any],
Expand Down Expand Up @@ -512,12 +530,12 @@ def build_status_change_preview_ui(
confirm_action=_build_confirm_action(
"update_order_status",
{
"order_id": "{{ preview.order_id }}",
"status_code": "{{ preview.new_status_code }}",
"comment": "{{ preview.comment }}",
"public": "{{ preview.public }}",
"email_customer": "{{ preview.email_customer }}",
"email_additional": "{{ preview.email_additional }}",
"order_id": _preview_ref("order_id"),
"status_code": _preview_ref("new_status_code"),
"comment": _preview_ref("comment"),
"public": _preview_ref("public"),
"email_customer": _preview_ref("email_customer"),
"email_additional": _preview_ref("email_additional"),
"confirm": True,
},
),
Expand Down Expand Up @@ -636,9 +654,9 @@ def build_comment_preview_ui(preview: dict[str, Any]) -> PrefabApp:
confirm_action=_build_confirm_action(
"add_order_comment",
{
"order_id": "{{ preview.order_id }}",
"comment": "{{ preview.comment }}",
"public": "{{ preview.public }}",
"order_id": _preview_ref("order_id"),
"comment": _preview_ref("comment"),
"public": _preview_ref("public"),
"confirm": True,
},
),
Expand Down Expand Up @@ -689,9 +707,9 @@ def build_due_date_change_preview_ui(preview: dict[str, Any]) -> PrefabApp:
confirm_action=_build_confirm_action(
"update_order_due_date",
{
"order_id": "{{ preview.order_id }}",
"due_date": "{{ preview.new_due_date }}",
"due_date_to": "{{ preview.new_due_date_to }}",
"order_id": _preview_ref("order_id"),
"due_date": _preview_ref("new_due_date"),
"due_date_to": _preview_ref("new_due_date_to"),
"confirm": True,
},
),
Expand Down Expand Up @@ -759,12 +777,12 @@ def build_bulk_status_change_preview_ui(preview: dict[str, Any]) -> PrefabApp:
confirm_action=_build_confirm_action(
"bulk_update_order_status",
{
"order_ids": "{{ preview.order_ids }}",
"status_code": "{{ preview.target_status_code }}",
"comment": "{{ preview.comment }}",
"public": "{{ preview.public }}",
"email_customer": "{{ preview.email_customer }}",
"email_additional": "{{ preview.email_additional }}",
"order_ids": _preview_ref("order_ids"),
"status_code": _preview_ref("target_status_code"),
"comment": _preview_ref("comment"),
"public": _preview_ref("public"),
"email_customer": _preview_ref("email_customer"),
"email_additional": _preview_ref("email_additional"),
"confirm": True,
},
),
Expand Down
139 changes: 91 additions & 48 deletions statuspro_mcp_server/tests/tools/test_prefab_ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,59 @@ def _find_tool_calls(envelope: dict[str, Any]) -> list[dict[str, Any]]:
]


# Smallest-viable preview fixtures for the four mutation cards. Used by tests
# that pin contracts across every preview card (state-machine guards,
# Cancel-no-SendMessage). Kept module-level so a new "every card must…"
# assertion can iterate over them without copying ~45 lines per test.
_PREVIEW_BUILDER_FIXTURES: list[tuple[Any, dict[str, Any]]] = [
(
build_status_change_preview_ui,
{
"order_id": 1,
"current_status_code": "st000002",
"new_status_code": "st000003",
"comment": None,
"public": False,
"email_customer": True,
"email_additional": False,
"valid": True,
"viable_status_codes": ["st000003"],
},
),
(
build_comment_preview_ui,
{
"order_id": 1,
"order_summary": {"id": 1},
"comment": "hi",
"public": False,
},
),
(
build_due_date_change_preview_ui,
{
"order_id": 1,
"order_summary": {"id": 1},
"current_due_date": "2026-01-01",
"new_due_date": "2026-02-01",
},
),
(
build_bulk_status_change_preview_ui,
{
"order_ids": [1, 2],
"order_count": 2,
"target_status_code": "st000003",
"target_status_name": "Shipped",
"comment": None,
"public": False,
"email_customer": True,
"email_additional": False,
},
),
]


def test_build_orders_table_ui_renders_with_drill_down_action():
app = build_orders_table_ui(
[
Expand Down Expand Up @@ -397,54 +450,7 @@ def test_preview_cards_wire_double_click_guard():
is the spam guard; the explicit ``disabled={{ pending || cancelled }}``
binding on the button is belt-and-suspenders.
"""
builders = [
(
build_status_change_preview_ui,
{
"order_id": 1,
"current_status_code": "st000002",
"new_status_code": "st000003",
"comment": None,
"public": False,
"email_customer": True,
"email_additional": False,
"valid": True,
"viable_status_codes": ["st000003"],
},
),
(
build_comment_preview_ui,
{
"order_id": 1,
"order_summary": {"id": 1},
"comment": "hi",
"public": False,
},
),
(
build_due_date_change_preview_ui,
{
"order_id": 1,
"order_summary": {"id": 1},
"current_due_date": "2026-01-01",
"new_due_date": "2026-02-01",
},
),
(
build_bulk_status_change_preview_ui,
{
"order_ids": [1, 2],
"order_count": 2,
"target_status_code": "st000003",
"target_status_name": "Shipped",
"comment": None,
"public": False,
"email_customer": True,
"email_additional": False,
},
),
]
for builder, preview in builders:
for builder, preview in _PREVIEW_BUILDER_FIXTURES:
envelope = _envelope(builder(preview))
state = envelope.get("state") or {}
# The four state slots the footer's If/Elif blocks bind to.
Expand Down Expand Up @@ -563,6 +569,43 @@ def test_retry_button_has_double_click_guard():
)


def test_cancel_buttons_do_not_send_chat_messages():
"""Cancel on every preview card must dismiss client-side (SetState +
optional toast) — never fire ``SendMessage``.

SendMessage round-trips through the LLM and shows up as a fake user
message in the chat, which is noisy and unnecessary now that Confirm
fires the apply directly via CallTool (no agent middleman). Pinning
this so a future regression that re-introduces SendMessage on Cancel
surfaces here instead of as chat-noise in production.
"""
for builder, preview in _PREVIEW_BUILDER_FIXTURES:
envelope = _envelope(builder(preview))
cancel_buttons = [
n
for n in _walk_nodes(envelope)
if n.get("type") == "Button" and n.get("label") == "Cancel"
]
assert cancel_buttons, f"{builder.__name__}: no Cancel button found in envelope"
for cancel in cancel_buttons:
# ``_build_cancel_action`` returns ``list[Action]``, so the
# serialized on_click is always a list — no defensive coercion
# needed.
on_click = cancel.get("onClick") or []
assert isinstance(on_click, list), (
f"{builder.__name__}: Cancel on_click must be a list; got {on_click!r}"
)
send_messages = [
a
for a in on_click
if isinstance(a, dict) and a.get("action") == "sendMessage"
]
assert not send_messages, (
f"{builder.__name__}: Cancel button must not fire SendMessage; "
f"got {send_messages}"
)


def test_status_change_preview_invalid_does_not_seed_apply_state():
"""When ``valid=False`` the Confirm button is replaced with a "See viable
transitions" button — the apply rail isn't live, so the pending/applied
Expand Down