diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index 2c363bb0..8c6f9bbc 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -17,6 +17,13 @@ const MAX_GROOM_LIMIT: i64 = 200; /// holding the backend DB lock. const MAX_WINDOW_WEEKS: i64 = 52; +/// Default/max page size for `list` — omitting `limit` used to return every +/// matching item unbounded, which can blow past the MCP response token cap +/// on large projects (155 items / 52k chars observed). Mirrors the +/// `unwrap_or`+`clamp` pattern already used by `search`/`groom` below. +const DEFAULT_LIST_LIMIT: i64 = 50; +const MAX_LIST_LIMIT: i64 = 500; + fn priority_rank(p: &str) -> u8 { match p { "urgent" => 5, @@ -258,14 +265,15 @@ impl AgentflareMcp { }); } + let total = items.len(); let offset = req.offset.unwrap_or(0) as usize; - let items = items.into_iter().skip(offset); - let items: Vec<_> = match req.limit { - Some(limit) => items.take(limit as usize).collect(), - None => items.collect(), - }; + let limit = req + .limit + .unwrap_or(DEFAULT_LIST_LIMIT) + .clamp(0, MAX_LIST_LIMIT) as usize; + let page: Vec<_> = items.into_iter().skip(offset).take(limit).collect(); - let summaries: Vec = items + let summaries: Vec = page .into_iter() .map(|i| { let state = state_by_id.get(i.state_id.as_str()); @@ -282,7 +290,21 @@ impl AgentflareMcp { } }) .collect(); - Ok(serde_json::to_string_pretty(&summaries).unwrap_or_default()) + + let next_offset = (offset.saturating_add(summaries.len()) < total && limit > 0) + .then_some(offset.saturating_add(limit)); + let prev_offset = (limit > 0 && offset > 0 && total > 0) + .then_some(offset.min(total).saturating_sub(limit)); + + let page = ItemListPage { + items: summaries, + total, + offset, + limit, + next_offset, + prev_offset, + }; + Ok(serde_json::to_string_pretty(&page).unwrap_or_default()) })? } diff --git a/src/mcp_server/tests/item_tests.rs b/src/mcp_server/tests/item_tests.rs index 56e2b09f..848f4833 100644 --- a/src/mcp_server/tests/item_tests.rs +++ b/src/mcp_server/tests/item_tests.rs @@ -331,7 +331,7 @@ fn item_list_filters_by_assignee_or_unassigned_and_sorts_open_first() { .unwrap(), ) .unwrap(); - let names: Vec<&str> = listed + let names: Vec<&str> = listed["items"] .as_array() .unwrap() .iter() @@ -386,7 +386,7 @@ fn item_list_defaults_assignee_filter_to_server_identity() { .unwrap(), ) .unwrap(); - let mut names: Vec<&str> = defaulted + let mut names: Vec<&str> = defaulted["items"] .as_array() .unwrap() .iter() @@ -405,7 +405,7 @@ fn item_list_defaults_assignee_filter_to_server_identity() { .unwrap(), ) .unwrap(); - let mut names2: Vec<&str> = explicit + let mut names2: Vec<&str> = explicit["items"] .as_array() .unwrap() .iter() @@ -466,7 +466,7 @@ fn item_list_state_group_filter_accepts_comma_separated_groups() { .unwrap(), ) .unwrap(); - let names: Vec<&str> = listed + let names: Vec<&str> = listed["items"] .as_array() .unwrap() .iter() @@ -1206,7 +1206,7 @@ fn item_groom_benchmark() { .unwrap(), ) .unwrap(); - let shortlist_ids: Vec = listed + let shortlist_ids: Vec = listed["items"] .as_array() .unwrap() .iter() @@ -1247,13 +1247,54 @@ fn item_list_respects_limit_and_offset() { .unwrap(), ) .unwrap(); - let names: Vec<&str> = listed + let names: Vec<&str> = listed["items"] .as_array() .unwrap() .iter() .map(|i| i["name"].as_str().unwrap()) .collect(); assert_eq!(names, vec!["B"]); + assert_eq!(listed["total"], 3); + assert_eq!(listed["offset"], 1); + assert_eq!(listed["limit"], 1); + assert_eq!(listed["next_offset"], 2); + assert_eq!(listed["prev_offset"], 0); +} + +#[test] +fn item_list_pagination_edges_out_of_range_offset_and_zero_limit() { + let (_tmp, s) = harness(); + for name in ["A", "B", "C"] { + s.item(Parameters(empty_item_create(name))).unwrap(); + } + + let past_end: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "list".into(), + limit: Some(1), + offset: Some(100), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(past_end["items"].as_array().unwrap().len(), 0); + assert_eq!(past_end["next_offset"], serde_json::Value::Null); + assert_eq!(past_end["prev_offset"], 2); + + let zero_limit: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "list".into(), + limit: Some(0), + offset: Some(1), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(zero_limit["items"].as_array().unwrap().len(), 0); + assert_eq!(zero_limit["next_offset"], serde_json::Value::Null); + assert_eq!(zero_limit["prev_offset"], serde_json::Value::Null); } #[test] @@ -1268,11 +1309,13 @@ fn item_list_returns_lean_projection_with_readable_state() { .unwrap(), ) .unwrap(); - let first = &listed.as_array().unwrap()[0]; + let first = &listed["items"].as_array().unwrap()[0]; assert_eq!(first["state"], "Backlog"); assert_eq!(first["state_group"], "backlog"); assert!(first.get("description").is_none()); assert!(first.get("metadata").is_none()); + assert_eq!(listed["next_offset"], serde_json::Value::Null); + assert_eq!(listed["prev_offset"], serde_json::Value::Null); } #[test] diff --git a/src/mcp_server/types.rs b/src/mcp_server/types.rs index 34477a8f..3d111ab7 100644 --- a/src/mcp_server/types.rs +++ b/src/mcp_server/types.rs @@ -727,11 +727,13 @@ pub(crate) struct ItemRequest { #[serde(default)] pub(crate) state_group: Option, #[schemars( - description = "Max items to return (list: omit for no limit; search: omit for 20, capped at 1000; groom: omit for 15, capped at 200)" + description = "Max items to return (list: omit for 50, capped at 500; search: omit for 20, capped at 1000; groom: omit for 15, capped at 200)" )] #[serde(default)] pub(crate) limit: Option, - #[schemars(description = "Items to skip before applying limit (list); default 0")] + #[schemars( + description = "Items to skip before applying limit (list); default 0. `list`'s response includes `next_offset`/`prev_offset` — pass those back here to page forward/backward" + )] #[serde(default)] pub(crate) offset: Option, #[schemars(description = "FTS5 search query (search)")] @@ -773,6 +775,22 @@ pub(crate) struct ItemSummary { pub(crate) updated_at: i64, } +/// `item(list)`'s response envelope — carries `next_offset`/`prev_offset` so +/// a caller paging through a large project can navigate by re-sending the +/// given offset as-is, rather than re-deriving it from `total`/`limit`. +#[derive(Debug, serde::Serialize)] +pub(crate) struct ItemListPage { + pub(crate) items: Vec, + /// Count matching the filters, before this page's offset/limit were applied. + pub(crate) total: usize, + pub(crate) offset: usize, + pub(crate) limit: usize, + /// Pass as `offset` on the next call for the next page; `null` on the last page. + pub(crate) next_offset: Option, + /// Pass as `offset` on the next call for the previous page; `null` on the first page. + pub(crate) prev_offset: Option, +} + /// One shortlisted item plus the decision-support signals `groom` computes /// server-side (staleness, blocking, fan-in, near-duplicates) so the caller /// doesn't have to re-derive them by eyeballing timestamps and free text.