56 lines
2.4 KiB
Markdown
56 lines
2.4 KiB
Markdown
---
|
|
id: review-002-con17-mcp-pagination
|
|
name: Bound MCP tools/list pagination (max pages + deadline) (CON-14)
|
|
status: completed
|
|
depends_on: []
|
|
scope: narrow
|
|
risk: low
|
|
impact: component
|
|
level: implementation
|
|
tags: [adapters, review-002, mcp]
|
|
---
|
|
|
|
## Description
|
|
|
|
Review 002 CON-14 [major]. `from_mcp/mod.rs:114-119` delegates to
|
|
rmcp 1.8.0's `Peer::list_all_tools`, which loops `while
|
|
cursor.is_some()` with **no page cap and no timeout** (verified against
|
|
rmcp source `service/client.rs:390-407`). A malicious or buggy server
|
|
that always returns `next_cursor: Some(...)` (cycling or ignoring the
|
|
cursor) hangs `import()` indefinitely while appending every page's
|
|
tools → unbounded memory growth alongside the hang. Tools from
|
|
completed pages are not lost (extend per page), but nothing in alkhttp
|
|
bounds the loop. The paging test that exists (from_mcp_integration.rs)
|
|
terminates after 3 pages — the cycling shape is untested.
|
|
|
|
## Acceptance Criteria
|
|
|
|
- [ ] Replace `list_all_tools` with a bounded loop over `list_tools`:
|
|
hard page-count cap + overall deadline (constants or config;
|
|
pick generous defaults — e.g. 100 pages / 60 s — and note them
|
|
in the module doc)
|
|
- [ ] Budget trips → `AdapterError::DiscoveryFailed`-family error
|
|
naming the budget (pages seen, tools accumulated), no partial
|
|
import left behind (the existing import-fails-closed behavior
|
|
holds)
|
|
- [ ] Integration test: a paging server that cycles its cursor
|
|
(always `next_cursor: Some("a")`) → import terminates with the
|
|
clean error, bounded time (no hang)
|
|
- [ ] Existing 3-page pagination test unchanged and passing
|
|
(well-formed pagination still imports all pages)
|
|
- [ ] `cargo test --features mcp`, `cargo clippy --all-features --all-targets -- -D warnings`,
|
|
`cargo fmt --check` pass
|
|
|
|
## References
|
|
|
|
- docs/reviews/002-post-remediation-review.md (Part F', CON-14)
|
|
- src/adapters/from_mcp/mod.rs:114-119 (the delegation), rmcp 1.8.0 src/service/client.rs:390-407 (the unbounded loop)
|
|
- tests/from_mcp_integration.rs:302-317 (the existing paging server)
|
|
- tasks/adapters/review-001-consumer-adapter-hygiene.md (CON-01's pagination work this bounds)
|
|
|
|
## Notes
|
|
|
|
Small, self-contained, feature-gated (`mcp`). Consider (optional) a
|
|
`max_tools` sanity cap as well — same budget family, one more guard
|
|
against a hostile server — but do not let scope creep: the page cap +
|
|
deadline closes the hang, which is the finding. |