fix(billing-cost-management-mcp-server): read rec-details templates as UTF-8 (#4815)

* fix(billing-cost-management-mcp-server): read rec-details templates as UTF-8

get_template_for_recommendation opened the bundled recommendation
templates with the locale's default encoding. All nine templates contain
non-ASCII characters, so on a host whose default is not UTF-8 (for
example Windows with cp1252) eight of them came back with mojibake, and
the ecs_service template could not be decoded at all. The surrounding
except/pass turned that into None, so rec-details silently dropped the
template for EcsService recommendations.

Open the template with encoding='utf-8'. Add tests that load every
bundled template under a simulated cp1252 default, so they also fail on
UTF-8 platforms without the fix, and a CHANGELOG entry.

* test(billing-cost-management-mcp-server): correct legacy_default_encoding docstring

The docstring said the fixture also changes Path.read_text(). It does only
on Python 3.11+: on 3.10 pathlib binds io.open at import time, so patching
io.open does not reach it. The tests exercise the builtin open() that
get_template_for_recommendation uses and fail without the fix on 3.10,
3.11 and 3.12; only the description was wrong.

---------

Co-authored-by: finklen-amazon <finklen@amazon.com>

Author: OsamaAnsar

src/billing-cost-management-mcp-server/CHANGELOG.md

@@ -59,6 +59,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - Cost Explorer `getCostAndUsage` and `getCostAndUsageWithResources` now validate `metrics` (a list of metric names or a JSON array string) and reject anything else with a `validation_error` before calling Cost Explorer. The parsed list, with the `["UnblendedCost"]` default applied, is now what is recorded with a result stored to session SQL; previously the raw argument was recorded, so an omitted `metrics` was stored as `null` and a JSON string as a string.
 - **Breaking:** `aws-pricing` `get_pricing_from_api` results returned inline now keep each product's `terms` as the Price List API returned it. Each term was reduced to `effectiveDate` and, per price dimension, `description`, `unit` and `pricePerUnit`, which dropped `termAttributes` (`LeaseContractLength`, `PurchaseOption`, `OfferingClass`), `beginRange`/`endRange`, `rateCode`, `offerTermCode`, `sku` and `appliesTo`, so Reserved prices for different lease lengths and purchase options could not be told apart and tiered prices lost their tier bounds. `terms` is now keyed by offer term and price dimension code, as in the API, and inline results are larger. Results stored in session SQL already kept the full terms and are unchanged.
+- The `rec-details` tool reads its recommendation templates as UTF-8 instead of the host's default encoding. On a host whose default is not UTF-8 (for example Windows with cp1252) most templates were returned with garbled characters, and the `ecs_service` template was dropped without any error.
 - Cost Explorer (`getCostAndUsage`, `getCostAndUsageWithResources`, `getDimensionValues`, `getTagsOrValues`, `getCostCategories`), Cost Optimization Hub list operations and Compute Optimizer `get_*_recommendations` returned only the first page unless the caller passed `max_pages` or `next_token`, so a truncated result looked complete. They now fetch every page unless `max_items` is passed.
 - `cost-comparison` requested 10 comparisons per page although the API allows 2,000 (10 for drivers), multiplying the number of paid Cost Explorer requests.
 - `list-cost-allocation-tags` and `list-cost-category-definitions` paged to the end with no limit whenever a `next_token` was passed without `max_pages`.

src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/recommendation_details_tools.py

@@ -442,7 +442,7 @@ def get_template_for_recommendation(
         )
 
         if os.path.exists(template_path):
-            with open(template_path, 'r') as f:
+            with open(template_path, 'r', encoding='utf-8') as f:
                 return f.read()
     except Exception:
         # Template loading is optional, so we'll silently continue if not found

src/billing-cost-management-mcp-server/tests/tools/test_recommendation_details_tools.py

@@ -24,7 +24,10 @@
 """
 
 import botocore.session
+import builtins
+import io
 import pytest
+from awslabs.billing_cost_management_mcp_server.tools import recommendation_details_tools
 from awslabs.billing_cost_management_mcp_server.tools.recommendation_details_tools import (
     format_base_recommendation,
     format_timestamp,
@@ -42,6 +45,7 @@
 )
 from datetime import datetime, timezone
 from fastmcp import Context
+from pathlib import Path
 from unittest.mock import AsyncMock, MagicMock, mock_open, patch
 
 
@@ -1203,6 +1207,80 @@ def test_get_template_for_recommendation_unsupported_type(self):
         assert result is None
 
 
+TEMPLATES_DIR = (
+    Path(recommendation_details_tools.__file__).parent.parent
+    / 'templates'
+    / 'recommendation_templates'
+)
+
+# Recommendation that selects each bundled template
+TEMPLATE_CASES = {
+    'savings_plans': {'actionType': 'PurchaseSavingsPlans'},
+    'reserved_instances': {'actionType': 'PurchaseReservedInstances'},
+    'idle': {'actionType': 'Stop'},
+    'ec2_instance': {'currentResourceType': 'Ec2Instance'},
+    'ec2_asg': {'currentResourceType': 'Ec2AutoScalingGroup'},
+    'ebs_volume': {'currentResourceType': 'EbsVolume'},
+    'ecs_service': {'currentResourceType': 'EcsService'},
+    'lambda_function': {'currentResourceType': 'LambdaFunction'},
+    'rds_database': {'currentResourceType': 'RdsDbInstance'},
+}
+
+
+@pytest.fixture
+def legacy_default_encoding(monkeypatch):
+    """Make text-mode file I/O behave like a Windows host whose default encoding is cp1252.
+
+    ``open()`` called without an ``encoding`` uses cp1252 (and, on Python 3.11+,
+    ``Path.read_text()`` too; on 3.10 ``pathlib`` binds ``io.open`` at import time, so only
+    the builtin ``open()`` is affected, which is what the code under test uses). This
+    reproduces such a host on any platform, including when the test run itself has
+    UTF-8 mode enabled.
+    """
+    real_open = io.open
+
+    def legacy_open(
+        file,
+        mode='r',
+        buffering=-1,
+        encoding=None,
+        errors=None,
+        newline=None,
+        closefd=True,
+        opener=None,
+    ):
+        if 'b' not in mode and encoding in (None, 'locale'):
+            encoding = 'cp1252'
+        return real_open(file, mode, buffering, encoding, errors, newline, closefd, opener)
+
+    def legacy_text_encoding(encoding, stacklevel=2):
+        return 'locale' if encoding is None else encoding
+
+    monkeypatch.setattr(builtins, 'open', legacy_open)
+    monkeypatch.setattr(io, 'open', legacy_open)
+    monkeypatch.setattr(io, 'text_encoding', legacy_text_encoding)
+
+
+class TestGetTemplateForRecommendationEncoding:
+    """The bundled templates are UTF-8 and must read the same whatever the host's default is."""
+
+    def test_every_bundled_template_has_a_case(self):
+        """Guard: a new template file must be added to TEMPLATE_CASES."""
+        assert {path.stem for path in TEMPLATES_DIR.glob('*.template')} == set(TEMPLATE_CASES)
+
+    @pytest.mark.parametrize('name', sorted(TEMPLATE_CASES))
+    def test_template_is_read_as_utf8(self, name, legacy_default_encoding):
+        """Each template is returned exactly as stored, not as None or mojibake."""
+        expected = (TEMPLATES_DIR / f'{name}.template').read_text(encoding='utf-8')
+
+        result = get_template_for_recommendation(TEMPLATE_CASES[name], {})
+
+        assert result == expected
+        # Every bundled template has non-ASCII characters; make sure the comparison above
+        # is exercising a decoding difference.
+        assert not expected.isascii()
+
+
 @pytest.mark.asyncio
 class TestProcessRecommendation:
     """Tests for process_recommendation function."""