fix: surface upstream body on 5xx and unbreak review_urgent_order_requirements 415 (#203)

Two fixes driven by the 2026-05-26 MCP bug report.

1. utils.unwrap() now appends a truncated body excerpt to the error message
   when the response has no parsed body and a non-2xx status. StockTrim's
   Order Plan endpoint sometimes returns 500 with an HTML stack trace or
   415 with a plaintext reason; the previous "API error with status N"
   message was opaque without those bodies. Excerpts are bounded at 500
   chars with a "…[+N chars]" suffix so log lines stay readable, and
   UTF-8 decode errors fall back to a byte-count placeholder.

2. review_urgent_order_requirements built an OrderPlanFilterCriteriaDto
   (list-shaped, used by the V2 PO generation endpoint) and passed it to
   client.order_plan.query() which targets /api/OrderPlan and expects
   OrderPlanFilterCriteria (singular `location`/`supplier`/`category`).
   StockTrim rejected the wrong-shaped body with 415. Switch to the
   singular schema; for list-shaped MCP inputs, send the first element
   to the API and log `order_plan_multifilter_narrowed` so operators see
   that further filtering was dropped (multi-filter is an API limitation).

Tests:
- test_utils.py: 4 new cases for body-excerpt formatting (basic, long-body
  truncation, empty body, undecodable bytes).
- test_urgent_orders.py: 2 new cases pinning the OrderPlanFilterCriteria
  type (vs Dto) and the multi-element warning.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: dougborg

stocktrim_mcp_server/src/stocktrim_mcp_server/tools/workflows/urgent_orders.py

@@ -17,13 +17,45 @@
 from stocktrim_mcp_server.tools.preferences import load_preferences, resolve
 from stocktrim_mcp_server.tools.tool_result_utils import make_json_result
 from stocktrim_mcp_server.utils import unwrap_unset
-from stocktrim_public_api_client.client_types import UNSET
+from stocktrim_public_api_client.client_types import UNSET, Unset
+from stocktrim_public_api_client.generated.models.order_plan_filter_criteria import (
+    OrderPlanFilterCriteria,
+)
 from stocktrim_public_api_client.generated.models.order_plan_filter_criteria_dto import (
     OrderPlanFilterCriteriaDto,
 )
 
 logger = get_logger(__name__)
 
+
+_NARROWED_LOG_PREVIEW = 5
+
+
+def _first_or_unset(values: list[str] | None, field_name: str) -> str | Unset:
+    """Narrow a list-shaped MCP request field down to a single API-side filter.
+
+    StockTrim's /api/OrderPlan accepts only one location/supplier per call,
+    but the MCP request schema takes lists for forward compatibility. When
+    the caller supplies multiple values we keep the first and log a warning
+    so the call still succeeds (instead of 415-ing) and operators see that
+    further narrowing happened. The warning payload only includes a
+    bounded preview of the dropped values to keep structured logs small
+    when callers pass long lists.
+    """
+    if not values:
+        return UNSET
+    if len(values) > 1:
+        dropped = values[1:]
+        logger.warning(
+            "order_plan_multifilter_narrowed",
+            field=field_name,
+            kept=values[0],
+            dropped_count=len(dropped),
+            dropped_preview=dropped[:_NARROWED_LOG_PREVIEW],
+        )
+    return values[0]
+
+
 # ============================================================================
 # Tool 1: review_urgent_order_requirements
 # ============================================================================
@@ -100,6 +132,7 @@ async def _review_urgent_order_requirements_impl(
     """
     prefs = await load_preferences(context)
     days_threshold = resolve(request.days_threshold, prefs, "days_threshold", 30)
+    category = resolve(request.category, prefs, "category", None)
     # Use `is None` (not truthiness) so an explicit empty list from the caller
     # clears the filter for this call instead of silently falling back to the
     # stored preference. Same pattern below for supplier_codes.
@@ -122,12 +155,22 @@ async def _review_urgent_order_requirements_impl(
         # Get services from context (note: order_plan not in service layer yet, uses client directly)
         services = get_services(context)
 
-        # Build filter criteria for order plan query
-        # Note: order_plan.get_urgent_items() doesn't support all our filters,
-        # so we'll query with filters and filter by days threshold ourselves
-        filter_criteria = OrderPlanFilterCriteriaDto(
-            location_codes=location_codes or UNSET,
-            supplier_codes=supplier_codes or UNSET,
+        # The /api/OrderPlan endpoint expects an OrderPlanFilterCriteria
+        # (singular `location`/`supplier`/`category`), not the list-shaped
+        # OrderPlanFilterCriteriaDto used by the V2 PO generation endpoint.
+        # Passing the Dto here made StockTrim reject the body with 415
+        # (bug surfaced 2026-05-26). The API only supports one location/one
+        # supplier per call; the lists produced by the session-preference
+        # fallback above are narrowed to the first element here (logged as
+        # a warning) so the call no longer fails when callers supply more
+        # than one value.
+        location = _first_or_unset(location_codes, "location_codes")
+        supplier = _first_or_unset(supplier_codes, "supplier_codes")
+
+        filter_criteria = OrderPlanFilterCriteria(
+            location=location,
+            supplier=supplier,
+            category=category or UNSET,
         )
 
         # Query order plan (uses client directly as order_plan not in service layer)

stocktrim_mcp_server/tests/test_tools/test_workflows/test_urgent_orders.py

@@ -203,6 +203,116 @@ async def test_review_urgent_orders_with_cost_calculation(
     assert response.suppliers[0].total_estimated_cost == 1550.0
 
 
+@pytest.mark.asyncio
+async def test_review_urgent_orders_sends_singular_filter_criteria(
+    mock_urgent_context, urgent_order_item
+):
+    """The /api/OrderPlan endpoint expects OrderPlanFilterCriteria (singular
+    `location`/`supplier`/`category`), not OrderPlanFilterCriteriaDto. Sending
+    the Dto returns 415 from StockTrim (bug surfaced 2026-05-26).
+    """
+    from stocktrim_public_api_client.generated.models.order_plan_filter_criteria import (
+        OrderPlanFilterCriteria,
+    )
+
+    mock_client = mock_urgent_context.request_context.lifespan_context.client
+    mock_client.order_plan.query.return_value = [urgent_order_item]
+
+    request = ReviewUrgentOrdersRequest(
+        days_threshold=30,
+        location_codes=["WH-01"],
+        supplier_codes=["SUP-001"],
+        category="Widgets",
+    )
+    await _review(request, mock_urgent_context)
+
+    mock_client.order_plan.query.assert_called_once()
+    criteria = mock_client.order_plan.query.call_args.args[0]
+    assert isinstance(criteria, OrderPlanFilterCriteria)
+    # Singular fields mapped from the list-shaped request:
+    assert criteria.location == "WH-01"
+    assert criteria.supplier == "SUP-001"
+    assert criteria.category == "Widgets"
+
+
+@pytest.mark.asyncio
+async def test_review_urgent_orders_narrows_multifilter_with_warning(
+    mock_urgent_context, urgent_order_item
+):
+    """When multiple locations/suppliers are requested, only the first is
+    sent to the API (which doesn't support multi-filter) and a warning is
+    logged so operators see that the others were dropped."""
+    import structlog.testing
+
+    mock_client = mock_urgent_context.request_context.lifespan_context.client
+    mock_client.order_plan.query.return_value = [urgent_order_item]
+
+    request = ReviewUrgentOrdersRequest(
+        days_threshold=30,
+        location_codes=["WH-01", "WH-02"],
+        supplier_codes=["SUP-001", "SUP-002", "SUP-003"],
+    )
+    with structlog.testing.capture_logs() as captured:
+        await _review(request, mock_urgent_context)
+
+    criteria = mock_client.order_plan.query.call_args.args[0]
+    assert criteria.location == "WH-01"
+    assert criteria.supplier == "SUP-001"
+    # Both narrowings should have surfaced as warnings.
+    warnings = [
+        r
+        for r in captured
+        if r.get("log_level") == "warning"
+        and r.get("event") == "order_plan_multifilter_narrowed"
+    ]
+    by_field = {r["field"]: r for r in warnings}
+    assert set(by_field) == {"location_codes", "supplier_codes"}
+    # Counts and bounded previews replace the raw `dropped` list to keep
+    # structured logs small when callers pass long filter lists.
+    assert by_field["location_codes"]["dropped_count"] == 1
+    assert by_field["location_codes"]["dropped_preview"] == ["WH-02"]
+    assert by_field["supplier_codes"]["dropped_count"] == 2
+    assert by_field["supplier_codes"]["dropped_preview"] == ["SUP-002", "SUP-003"]
+
+
+@pytest.mark.asyncio
+async def test_review_urgent_orders_caps_warning_preview_for_long_lists(
+    mock_urgent_context, urgent_order_item
+):
+    """A very long list should still narrow to one value, but the warning
+    log only carries a bounded preview (not the full dropped list) so
+    structured logs don't bloat for outlier inputs."""
+    import structlog.testing
+
+    from stocktrim_mcp_server.tools.workflows.urgent_orders import (
+        _NARROWED_LOG_PREVIEW,
+    )
+
+    mock_client = mock_urgent_context.request_context.lifespan_context.client
+    mock_client.order_plan.query.return_value = [urgent_order_item]
+
+    long_locations = [f"WH-{i:03d}" for i in range(50)]
+    request = ReviewUrgentOrdersRequest(
+        days_threshold=30, location_codes=long_locations
+    )
+    with structlog.testing.capture_logs() as captured:
+        await _review(request, mock_urgent_context)
+
+    warning = next(
+        r
+        for r in captured
+        if r.get("event") == "order_plan_multifilter_narrowed"
+        and r.get("field") == "location_codes"
+    )
+    # 50 inputs → first kept, remaining 49 dropped; preview must be exactly
+    # the impl's cap (not a looser bound — looser would silently regress).
+    assert warning["dropped_count"] == 49
+    assert len(warning["dropped_preview"]) == _NARROWED_LOG_PREVIEW
+    # Preview starts at the right offset (first dropped element).
+    assert warning["dropped_preview"][0] == "WH-001"
+    assert warning["dropped_preview"][-1] == f"WH-{_NARROWED_LOG_PREVIEW:03d}"
+
+
 @pytest.mark.asyncio
 async def test_review_urgent_orders_filters_by_threshold(mock_urgent_context):
     """Test that items are filtered by days_threshold."""
@@ -470,7 +580,8 @@ async def test_review_urgent_orders_explicit_arg_wins_over_pref(mock_urgent_cont
 @pytest.mark.asyncio
 async def test_review_urgent_orders_falls_back_to_pref_supplier(mock_urgent_context):
     """When request.supplier_codes is omitted, the saved supplier_code
-    preference is wrapped into a one-item list and applied to the query."""
+    preference is applied to the (singular) OrderPlanFilterCriteria.supplier
+    sent to the /api/OrderPlan endpoint."""
     from stocktrim_mcp_server.tools.preferences import SessionPreferences
 
     pref = SessionPreferences(supplier_code="SUP-PREF")
@@ -485,7 +596,7 @@ async def test_review_urgent_orders_falls_back_to_pref_supplier(mock_urgent_cont
 
     mock_client.order_plan.query.assert_called_once()
     filter_criteria = mock_client.order_plan.query.call_args.args[0]
-    assert filter_criteria.supplier_codes == ["SUP-PREF"]
+    assert filter_criteria.supplier == "SUP-PREF"
 
 
 @pytest.mark.asyncio
@@ -506,7 +617,7 @@ async def test_review_urgent_orders_explicit_supplier_codes_override_pref(
     await _review(request, mock_urgent_context)
 
     filter_criteria = mock_client.order_plan.query.call_args.args[0]
-    assert filter_criteria.supplier_codes == ["SUP-EXPLICIT"]
+    assert filter_criteria.supplier == "SUP-EXPLICIT"
 
 
 @pytest.mark.asyncio
@@ -530,8 +641,8 @@ async def test_review_urgent_orders_explicit_empty_clears_pref(mock_urgent_conte
     # Empty list is falsy, so the helper sends UNSET (no filter) to the API.
     from stocktrim_public_api_client.client_types import UNSET
 
-    assert filter_criteria.location_codes is UNSET
-    assert filter_criteria.supplier_codes is UNSET
+    assert filter_criteria.location is UNSET
+    assert filter_criteria.supplier is UNSET
 
 
 @pytest.mark.asyncio

stocktrim_public_api_client/utils.py

@@ -9,6 +9,12 @@
 
 from .client_types import UNSET, Response, Unset
 
+# How many characters of the raw response body to attach to error messages
+# when the OpenAPI client could not parse it. Bounded so HTML stack-traces
+# and similar verbose bodies don't blow up log lines / MCP tool errors, but
+# long enough for the prefix to actually identify the problem.
+_BODY_EXCERPT_LIMIT = 500
+
 if TYPE_CHECKING:
     from .generated.models.problem_details import ProblemDetails
 
@@ -67,6 +73,64 @@ class ServerError(APIError):
     pass
 
 
+def _is_unsafe_control_char(c: str) -> bool:
+    """Identify ASCII / C1 control chars that would corrupt log output.
+
+    Covers the C0 range (``\\x00``-``\\x1f``) and DEL plus the C1 range
+    (``\\x7f``-``\\x9f``). Whitelists ``\\n``, ``\\r``, ``\\t`` — those are
+    legitimate text formatting and useful inside JSON/HTML body excerpts.
+    """
+    if c in "\n\r\t":
+        return False
+    code = ord(c)
+    return code < 0x20 or 0x7F <= code <= 0x9F
+
+
+def _body_excerpt(response: "Response[T]") -> str | None:
+    """Return a short, printable excerpt of the response body for error messages.
+
+    Returns ``None`` when the body is empty (so callers can omit the trailing
+    ``: <excerpt>`` from the message). Bodies that aren't valid UTF-8 (binary
+    blobs, etc.) fall back to a ``<N bytes, undecodable>`` placeholder so the
+    exception text stays printable. Control characters in otherwise-text
+    bodies (C0 range, C1 range, and DEL — see :func:`_is_unsafe_control_char`)
+    are escaped with ``\\xNN`` so they don't corrupt log lines. When the
+    body exceeds the limit a ``…[+N chars]`` suffix tells the caller how many
+    source characters were dropped.
+
+    The output length is strictly bounded by ``_BODY_EXCERPT_LIMIT`` (plus
+    the short ``…[+N chars]`` suffix). Without per-char accounting a body of
+    all control characters would expand 4x to ``4 * LIMIT`` after escaping;
+    the loop below stops adding chars once the *escaped* budget is spent and
+    reports the unconsumed source length in the suffix instead.
+    """
+    content = getattr(response, "content", None)
+    if not content:
+        return None
+    try:
+        text = content.decode("utf-8").strip()
+    except UnicodeDecodeError:
+        return f"<{len(content)} bytes, undecodable>"
+    if not text:
+        return None
+
+    pieces: list[str] = []
+    output_len = 0
+    consumed = 0
+    for c in text:
+        escaped_c = f"\\x{ord(c):02x}" if _is_unsafe_control_char(c) else c
+        if output_len + len(escaped_c) > _BODY_EXCERPT_LIMIT:
+            break
+        pieces.append(escaped_c)
+        output_len += len(escaped_c)
+        consumed += 1
+
+    excerpt = "".join(pieces)
+    if consumed == len(text):
+        return excerpt
+    return f"{excerpt}…[+{len(text) - consumed} chars]"
+
+
 @overload
 def unwrap(
     response: Response[T],
@@ -139,7 +203,10 @@ def unwrap(
         if not raise_on_error:
             return None
 
-        # Extract error message from ProblemDetails if available
+        # Extract error message from ProblemDetails if available; fall back
+        # to a generic message plus a body excerpt so 5xx responses with
+        # unparseable bodies (HTML stack traces, plain-text errors, etc.) are
+        # debuggable from the exception alone instead of opaque.
         if problem_details:
             title = unwrap_unset(problem_details.title)
             detail = unwrap_unset(problem_details.detail)
@@ -149,7 +216,9 @@ def unwrap(
                 else (title or detail or "Unknown error")
             )
         else:
-            message = f"API error with status {response.status_code}"
+            base = f"API error with status {response.status_code}"
+            excerpt = _body_excerpt(response)
+            message = f"{base}: {excerpt}" if excerpt else base
 
         # Raise specific exception based on status code
         if response.status_code == HTTPStatus.UNAUTHORIZED: