Skip to content

fix(prompts): accept string arguments for ticket_id #310

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

Open
zoispag wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(prompts): accept string arguments for ticket_id #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b115d07
fix(prompts): accept string arguments for ticket_id
zoispag Jul 30, 2026
e700308
test: use tuple form for parametrize argument names
zoispag Jul 30, 2026
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
4 changes: 2 additions & 2 deletions mcp_zammad/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2623,7 +2623,7 @@ def _setup_prompts(self) -> None:
"""Register all prompts with the MCP server."""

@self.mcp.prompt()
def analyze_ticket(ticket_id: int) -> str:
def analyze_ticket(ticket_id: str) -> str:
"""Generate a prompt to analyze a ticket.

Note: ticket_id must be the internal database ID (NOT the display number).
Expand All @@ -2642,7 +2642,7 @@ def analyze_ticket(ticket_id: int) -> str:
Use appropriate tools to gather any additional context about the customer or organization if needed."""

@self.mcp.prompt()
def draft_response(ticket_id: int, tone: str = "professional") -> str:
def draft_response(ticket_id: str, tone: str = "professional") -> str:
"""Generate a prompt to draft a response to a ticket.

Note: ticket_id must be the internal database ID (NOT the display number).
Expand Down
26 changes: 24 additions & 2 deletions tests/test_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,28 @@
assert "escalation_summary" in prompt_names


@pytest.mark.asyncio
@pytest.mark.parametrize(
("name", "arguments"),
[
("analyze_ticket", {"ticket_id": "$1"}),
("draft_response", {"ticket_id": "$1", "tone": "$2"}),
("escalation_summary", {"group": "$1"}),
],
)
async def test_prompts_render_with_non_numeric_arguments(name: str, arguments: dict[str, str]) -> None:
"""Prompt arguments arrive as strings, so they must not be coerced to int.

Check notice on line 245 in tests/test_server.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/test_server.py#L245 

Multi-line docstring summary should start at the second line (D213)

Clients that turn MCP prompts into slash commands render them with placeholder
values such as "$1" to discover their arguments. Annotating ticket_id as int made
FastMCP raise PromptError on those placeholders, so the prompts were unusable.
"""
result = await mcp.render_prompt(name, arguments)

rendered = result.messages[0].content.text # type: ignore[union-attr]
assert "$1" in rendered


Comment on lines +235 to +256

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Raise total coverage to the required threshold.

The supplied PR results report 88.42% coverage, below the repository’s 90% minimum. Add coverage for the remaining Python paths before merging.

As per coding guidelines, maintain test coverage at 90%+ for all Python code.

🧰 Tools
🪛 Ruff (0.16.0)

[warning] 237-237: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple

Use a tuple for the first argument

(PT006)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_server.py` around lines 235 - 256, Add tests covering the
remaining untested Python paths to raise overall coverage from 88.42% to at
least 90%. Extend the existing async prompt tests around mcp.render_prompt and
inspect related server functions for uncovered branches, preserving current
behavior while exercising each missing path.

Source: Coding guidelines

@pytest.mark.asyncio
async def test_initialization_failure():
"""Test that initialization handles failures gracefully."""
Expand Down Expand Up @@ -1579,13 +1601,13 @@

# Test analyze_ticket prompt
assert "analyze_ticket" in test_prompts
result = test_prompts["analyze_ticket"](ticket_id=123)
result = test_prompts["analyze_ticket"](ticket_id="123")
assert "analyze ticket with ID 123" in result
assert "get_ticket tool" in result

# Test draft_response prompt
assert "draft_response" in test_prompts
result = test_prompts["draft_response"](ticket_id=123, tone="friendly")
result = test_prompts["draft_response"](ticket_id="123", tone="friendly")
assert "draft a friendly response to ticket with ID 123" in result
assert "add_article" in result

Expand Down