fix(federation): preserve backend pagination in federated list responses - #71
Conversation
There was a problem hiding this comment.
@Lang-Akshay I reviewed head 2537394c. The per-backend cursor approach is sound, but please address these before merge:
-
Do not turn backend failures into exhaustion.
fan_out_listdrops failed/unavailable backends, while the new cursor model treats absence as exhausted. If a backend times out during a resumed page, its incoming cursor is erased and the client can never reach its remaining items. Preserve that backend's prior cursor or fail the list request. -
Exercise the target MCP contract. The new fixture advertises
V_2024_11_05, and the integration test uses legacy initialization plus requests without the required per-request metadata. New protocol tests in this repository must target2026-07-28, useserver/discover, and include client metadata if available. -
Scope gateway cursors to the list operation. The token currently contains only backend positions, so a cursor returned by
tools/listis accepted byprompts/listorresources/listand forwarded upstream. Add and validate an operation discriminator so cross-operation cursors return-32602. -
Strengthen the multi-backend regression test. Both mocks use the same
"page2"cursor and exhaust simultaneously. Broadcasting one raw cursor to every backend would therefore still pass, and the test never resumes while only one backend is exhausted. Use backend-specific cursors, uneven page counts, and call counters. -
Run
cargo fmt --all. The currentfmtcheck fails inprompts.rs,resources.rs, andtools.rs.
Verification: focused pagination tests pass locally (2 integration + 5 unit). GitHub build, Clippy, tests, cargo-deny, and benchmarks pass; fmt is the remaining failing check.
…e with mcp specs Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
2537394 to
c782021
Compare
Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
…casrries no operation discriminator Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
Fix: cursor exhaustion on backend failure + cross-operation cursor leakageTwo bugs in the gateway's paginated list cursor model, both in Finding 1 — Backend timeout during resume treated as exhaustionRoot cause. Fix. // Preserve cursor for backends that were expected to continue but failed this page.
for (backend_name, prior_cursor) in &incoming_cursor.backends {
if !succeeded.contains(backend_name.as_str()) {
next_backends.entry(backend_name.clone()).or_insert_with(|| prior_cursor.clone());
}
}Finding 2 — No operation discriminator on
|
| Handler | op string |
|---|---|
list_tools |
"list_tools" |
list_resources |
"list_resources" |
list_resource_templates |
"list_resource_templates" |
list_prompts |
"list_prompts" |
Because the cursor is opaque and this project has no external users, the format change is not backward-compatible — existing in-flight cursors will be rejected as invalid, which is the correct safe-failure mode.
Tests added
| Test | Covers |
|---|---|
failed_backend_cursor_preserved_across_page |
Backend absent from success set keeps its prior cursor in the outgoing token |
cross_operation_cursor_rejected |
A list_tools cursor passed to decode_gateway_cursor(..., "list_prompts") returns INVALID_PARAMS |
Existing tests updated for the new signatures. All 57 tests pass, clippy clean.
gandhipratik203
left a comment
There was a problem hiding this comment.
Small thing worth a look: the cursor may never terminate. list_aggregation.rs:138-143 (and 190, 240, 289) carries a cursor forward for any backend not in succeeded, but fan_out_list can't distinguish "failed" from "never attempted". A backend removed from the VH gets filtered out at tools.rs:33-37, so it re-emits its cursor indefinitely. Might be worth handling those two cases separately?
Problem
Closes IBM/mcp-context-forge#5273
The federated list handlers (
tools/list,resources/list,resources/templates/list,prompts/list) had three compounding bugs:merge_tools(and the other three merge functions) always returnedwith_all_items(…)which hardwiresnext_cursor: None, silently dropping any pagination information the backends returned. Items past a backend's first page were permanently unreachable.PaginatedRequestParams::default().with_cursor(…), discarding any_metafields the client included in the original request. The fix clones the incoming request and replaces only the cursor field, so all other params survive to each backend.Solution
Gateway cursor format — the gateway wraps per-backend positions into a single opaque token for the MCP client:
Only backends that still have pages appear in the map; absent = exhausted. The token is opaque to clients (they must not parse it per MCP spec §Pagination).
Federated pagination algorithm:
Items from each round are merged and sorted as before. The gateway emits a
next_cursorwhenever at least one backend still has pages; it is omitted once all are exhausted. An undecodable cursor returns−32602 Invalid params(MCP spec requirement).Changes
list_aggregation.rsGatewayCursorstruct +decode_gateway_cursor+encode_next_cursor;fan_out_listclosure gains backend name param; all fourmerge_*functions return(Vec<Item>, Option<String>)mcp_service/tools.rs_meta); setnext_cursoron resultmcp_service/resources.rslist_resources+list_resource_templatesmcp_service/prompts.rslist_promptstests/support/paginating_mock.rsPaginatingServer— minimalServerHandlerreturning tools across 2 pagestests/support/mod.rspaginating_mocktests/gateway_pagination.rstokio::spawn(viabind_backend_port) to eliminate startup racedocs/book/src/mcp-routing-semantics.mdNo new crate dependencies —
serde_json(already available) encodes the cursor.Testing
New unit tests (
list_aggregation.rs):backend_next_cursor_is_preserved_in_gateway_cursor— a backendnext_cursorsurvives the round-trip through encode/decodeexhausted_backends_produce_no_next_cursor—next_cursor: Nonefrom backend → gateway omits cursorinvalid_cursor_returns_invalid_params_error— malformed cursor →−32602none_cursor_returns_default_empty_gateway_cursor—Noneinput → first-page defaultNew integration tests (
tests/gateway_pagination.rs):single_backend_pagination_all_tools_reachable— backend returns 2 pages; gateway client retrieves all 3 tools across 2list_toolscalls with no item droppedmulti_backend_exhausted_backend_not_requeried— two paginating backends; page 2 has exactly 2 tools (1 from each backend's second page), proving no re-query of exhausted backends; deduplication check confirms no items appear twiceBackend readiness: each mock binds its TCP port synchronously (before
tokio::spawn) so the port is guaranteed reserved beforeconnect_clientis called.Existing tests — zero regressions:
gateway_list_tools(2 tests) ✓gateway_prompts(2 tests) ✓gateway_resource_templates(2 tests) ✓Out of scope (per issue)
CPEX/plugin hook integration, tool/resource/prompt filtering, header passthrough rules, control-plane changes.