Skip to content

Commit fdc2e54

Browse files
authored
fix(billing-cost-management-mcp-server): treat empty pagination token as last page (#4714)
* fix(billing-cost-management-mcp-server): treat empty pagination token as last page Some AWS APIs, such as Savings Plans DescribeSavingsPlansOfferingRates, return nextToken: "" on the final page instead of omitting it. paginate_aws_response already stopped looping on a falsy token, but its metadata used `is not None` checks, so it reported has_more: true, complete_dataset: false, and next_token: "" after fetching every page. A caller trusting has_more would then retry with an unusable token. Treat any falsy token as no more pages in the metadata. A non-empty token at the max_pages cutoff still reports has_more: true. * fix(billing-cost-management-mcp-server): apply empty-token check to credits and SP recommendations get_credit_allocation_history and get_savings_plans_purchase_recommendation build their own pagination metadata instead of using paginate_aws_response, and had the same `is not None` checks. Treat a falsy token as the last page there too.
1 parent 7f5fbd8 commit fdc2e54

7 files changed

Lines changed: 170 additions & 9 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3838
- Added an optional `context` parameter to the Cost Explorer `get_dimension_values` operation, so `SAVINGS_PLANS` returns values scoped to the plans an account owns rather than to its usage
3939

4040
### Fixed
41+
- Fixed pagination metadata reporting more pages when an AWS API returns an empty-string pagination token on its last page. Some APIs (e.g. Savings Plans `DescribeSavingsPlansOfferingRates`) return `nextToken: ""` rather than omitting it, which made `paginate_aws_response` report `has_more: true`, `complete_dataset: false`, and `next_token: ""` even though all pages had been fetched. The metadata now treats any falsy token as no more pages (`has_more: false`, `complete_dataset: true`, `next_token: null`). A non-empty token returned when the `max_pages` limit is reached still reports `has_more: true` with that token. The `get_credit_allocation_history` and `get_savings_plans_purchase_recommendation` tools, which build their own pagination metadata instead of using `paginate_aws_response`, carried the same bug and are fixed the same way.
4142
- Fixed `get_savings_plans_utilization` returning "No Savings Plans utilization data found" on every call (#3351). It read `SavingsPlansUtilizations` where the API returns `SavingsPlansUtilizationsByTime`, and behind that early return the figures were read from each row's top level as `{Amount, Unit}` mappings, but the API nests them under `Utilization`, `Savings` and `AmortizedCommitment` as decimal strings. Rows and `Total` now pass through as the API sends them, preserving decimal precision and the previously discarded blocks
4243
- Fixed `get_savings_plans_utilization_details` reporting every plan's utilization and savings as zero, from the same two causes. Rows now pass through as the API sends them, including the previously discarded `AmortizedCommitment` block and the whole of `Attributes` -- superseding the `summary` block, which read lowercase keys the API does not send, and exposing the owning `AccountId`
4344
- Corrected AWS Compute Optimizer recommendation response field names so EC2, Auto Scaling group, Lambda, and RDS tools return actual values instead of null. Fixed the shared savings-opportunity parser (`savingsOpportunityPercentage`), projected utilization metrics, EC2/RDS idle flags, nested ASG instance types, and the RDS instance/storage recommendation schema.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -370,11 +370,11 @@ async def _paginate_get_credit_allocation_history(
370370
await ctx.info(f'Fetched {pages_fetched} page(s), {len(rows)} allocation records')
371371

372372
pagination = {
373-
'complete_dataset': resume_token is None,
373+
'complete_dataset': not resume_token,
374374
'pages_fetched': pages_fetched,
375375
'total_results': len(rows),
376-
'has_more': resume_token is not None,
377-
'next_token': resume_token,
376+
'has_more': bool(resume_token),
377+
'next_token': resume_token or None,
378378
'duration_ms': int(duration_ms),
379379
}
380380
completeness = {'partial_results': partial_results, 'failed_months': failed_months}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,11 +301,11 @@ async def get_savings_plans_purchase_recommendation(
301301
data = {k: v for k, v in first_page.items() if k != 'NextPageToken'}
302302
data['SavingsPlansPurchaseRecommendation'] = merged_recommendation
303303
data['pagination'] = {
304-
'complete_dataset': current_token is None,
304+
'complete_dataset': not current_token,
305305
'pages_fetched': pages_fetched,
306306
'total_results': len(all_details),
307-
'has_more': current_token is not None,
308-
'next_token': current_token,
307+
'has_more': bool(current_token),
308+
'next_token': current_token or None,
309309
'duration_ms': int(duration_ms),
310310
}
311311

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/utilities/aws_service_base.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -365,11 +365,11 @@ async def paginate_aws_response(
365365

366366
# Create pagination metadata
367367
pagination_metadata = {
368-
'complete_dataset': current_token is None,
368+
'complete_dataset': not current_token,
369369
'pages_fetched': pages_fetched,
370370
'total_results': len(all_results),
371-
'has_more': current_token is not None,
372-
'next_token': current_token,
371+
'has_more': bool(current_token),
372+
'next_token': current_token or None,
373373
'duration_ms': int(duration_ms),
374374
}
375375

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,34 @@ async def test_accumulates_pages(self, mock_context, sample_allocation):
631631
assert result['data']['pagination']['has_more'] is False
632632
assert result['data']['pagination']['complete_dataset'] is True
633633

634+
@pytest.mark.asyncio
635+
async def test_empty_string_token_on_last_page_reports_complete(
636+
self, mock_context, sample_allocation
637+
):
638+
"""An empty-string token on the last page reads as complete, not as more pages.
639+
640+
Some AWS APIs return ``nextToken: ""`` on the final page rather than omitting it. The
641+
metadata must treat that as the end of the data rather than offering ``""`` as a resume
642+
token the caller cannot use. Every page the paginator yields is consumed, so no extra
643+
request is made beyond them.
644+
"""
645+
client = _get_credit_allocation_history_client(
646+
[
647+
{'creditAllocationHistoryList': [sample_allocation], 'nextToken': 'page-2'},
648+
{'creditAllocationHistoryList': [sample_allocation], 'nextToken': ''},
649+
]
650+
)
651+
652+
result = await _call_get_credit_allocation_history(
653+
mock_context, client, billing_period='2026-06'
654+
)
655+
656+
pagination = result['data']['pagination']
657+
assert pagination['pages_fetched'] == 2
658+
assert pagination['has_more'] is False
659+
assert pagination['next_token'] is None
660+
assert pagination['complete_dataset'] is True
661+
634662
@pytest.mark.asyncio
635663
async def test_paging_controls_go_to_the_paginator(self, mock_context, sample_allocation):
636664
"""Paging controls travel in PaginationConfig, not in the operation parameters."""

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,53 @@ async def test_max_pages_stops_early_and_reports_the_token(self, mock_context, m
298298
assert result['data']['pagination']['has_more'] is True
299299
assert result['data']['pagination']['next_token'] == 'more-to-come'
300300

301+
async def test_empty_string_token_on_last_page_ends_pagination(
302+
self, mock_context, mock_ce_client
303+
):
304+
"""An empty-string token on the final page reads as complete, not as more pages.
305+
306+
Some AWS APIs return ``NextPageToken: ""`` on the last page rather than omitting it. The
307+
loop already stops on a falsy token, so no request is made for an empty page, but the
308+
metadata must report the run as complete rather than offering ``""`` as a resume token.
309+
"""
310+
311+
def page(detail_id, token):
312+
return {
313+
'SavingsPlansPurchaseRecommendation': {
314+
'SavingsPlansPurchaseRecommendationDetails': [
315+
{'RecommendationDetailId': detail_id}
316+
]
317+
},
318+
'NextPageToken': token,
319+
}
320+
321+
mock_ce_client.get_savings_plans_purchase_recommendation.side_effect = [
322+
page('detail-1', 'token-1'),
323+
page('detail-2', ''),
324+
]
325+
326+
result = await get_savings_plans_purchase_recommendation(
327+
mock_context,
328+
mock_ce_client,
329+
'COMPUTE_SP',
330+
'ONE_YEAR',
331+
'NO_UPFRONT',
332+
'THIRTY_DAYS',
333+
None,
334+
None,
335+
None,
336+
None,
337+
None,
338+
)
339+
340+
# The empty-token page is consumed and no further request is made for an empty token.
341+
assert mock_ce_client.get_savings_plans_purchase_recommendation.call_count == 2
342+
pagination = result['data']['pagination']
343+
assert pagination['has_more'] is False
344+
assert pagination['next_token'] is None
345+
assert pagination['complete_dataset'] is True
346+
assert pagination['total_results'] == 2
347+
301348
async def test_a_page_without_details_is_not_treated_as_a_list(
302349
self, mock_context, mock_ce_client
303350
):

‎src/billing-cost-management-mcp-server/tests/utilities/test_aws_service_base.py‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
format_response,
3030
get_date_range,
3131
handle_aws_error,
32+
paginate_aws_response,
3233
parse_json,
3334
validate_date_format,
3435
)
@@ -396,3 +397,87 @@ def test_format_error_response_with_classification(self):
396397
'operation': 'getCostAndUsage',
397398
'service': 'Cost Explorer',
398399
}
400+
401+
402+
class TestPaginateAwsResponse:
403+
"""Tests for paginate_aws_response pagination metadata."""
404+
405+
@pytest.mark.asyncio
406+
async def test_last_page_empty_string_token(self):
407+
"""Test that an empty-string token on the last page reports no more pages."""
408+
# Setup
409+
ctx = AsyncMock()
410+
# Some AWS APIs (e.g. Savings Plans DescribeSavingsPlansOfferingRates)
411+
# return an empty-string token on the final page rather than omitting it.
412+
api_function = MagicMock(return_value={'Items': [1, 2, 3], 'NextToken': ''})
413+
414+
# Execute
415+
results, metadata = await paginate_aws_response(
416+
ctx,
417+
'TestOperation',
418+
api_function,
419+
{},
420+
'Items',
421+
)
422+
423+
# Assert
424+
assert results == [1, 2, 3]
425+
assert metadata['has_more'] is False
426+
assert metadata['complete_dataset'] is True
427+
assert metadata['next_token'] is None
428+
assert metadata['pages_fetched'] == 1
429+
assert metadata['total_results'] == 3
430+
431+
@pytest.mark.asyncio
432+
async def test_max_pages_hit_with_token(self):
433+
"""Test that hitting max_pages with a real token still reports more pages."""
434+
# Setup
435+
ctx = AsyncMock()
436+
api_function = MagicMock(
437+
side_effect=[
438+
{'Items': [1, 2], 'NextToken': 'token-1'},
439+
{'Items': [3, 4], 'NextToken': 'token-2'},
440+
]
441+
)
442+
443+
# Execute
444+
results, metadata = await paginate_aws_response(
445+
ctx,
446+
'TestOperation',
447+
api_function,
448+
{},
449+
'Items',
450+
max_pages=2,
451+
)
452+
453+
# Assert
454+
assert results == [1, 2, 3, 4]
455+
assert metadata['has_more'] is True
456+
assert metadata['complete_dataset'] is False
457+
assert metadata['next_token'] == 'token-2'
458+
assert metadata['pages_fetched'] == 2
459+
assert metadata['total_results'] == 4
460+
461+
@pytest.mark.asyncio
462+
async def test_missing_token_key(self):
463+
"""Test that a missing token key reports no more pages (unchanged behavior)."""
464+
# Setup
465+
ctx = AsyncMock()
466+
api_function = MagicMock(return_value={'Items': [1, 2, 3]})
467+
468+
# Execute
469+
results, metadata = await paginate_aws_response(
470+
ctx,
471+
'TestOperation',
472+
api_function,
473+
{},
474+
'Items',
475+
)
476+
477+
# Assert
478+
assert results == [1, 2, 3]
479+
assert metadata['has_more'] is False
480+
assert metadata['complete_dataset'] is True
481+
assert metadata['next_token'] is None
482+
assert metadata['pages_fetched'] == 1
483+
assert metadata['total_results'] == 3

0 commit comments

Comments
 (0)