diff --git a/docs/HELP.md b/docs/HELP.md index 26b91d8..1817767 100644 --- a/docs/HELP.md +++ b/docs/HELP.md @@ -130,7 +130,7 @@ List bugs for a given repository ###### **Options:** -* `--status ` — Status filter +* `--status ` — Status filter — repeat the flag or comma-separate values to combine (e.g. `--status pending,resolved`). Default: pending Default value: `pending` diff --git a/src/commands/bugs.rs b/src/commands/bugs.rs index 9ee29a8..a235e8a 100644 --- a/src/commands/bugs.rs +++ b/src/commands/bugs.rs @@ -118,9 +118,10 @@ pub enum BugCommands { /// If omitted, inferred from the git remote (origin). repo: Option, - /// Status filter - #[arg(long, value_enum, default_value = "pending")] - status: BugReviewState, + /// Status filter — repeat the flag or comma-separate values to + /// combine (e.g. `--status pending,resolved`). Default: pending. + #[arg(long, value_enum, value_delimiter = ',', default_value = "pending")] + status: Vec, /// Only show security vulnerabilities #[arg(long)] @@ -370,6 +371,38 @@ async fn fetch_all_bugs( Ok(all_bugs) } +/// Dedupe a slice of `BugReviewState` while preserving first-seen order. +/// Used to normalize repeated `--status` inputs (e.g. `pending,pending`) so +/// the multi-status helper doesn't fan out duplicate API calls and inflate +/// totals. +fn dedupe_statuses(statuses: &[BugReviewState]) -> Vec { + let mut out: Vec = Vec::with_capacity(statuses.len()); + for s in statuses { + if !out.contains(s) { + out.push(*s); + } + } + out +} + +/// Fetch every bug for each of `statuses`, concatenated in the order given. +/// The bugs API only accepts a single status per request, so multi-status +/// queries fan out into one paginated call per status. Repeated statuses +/// are deduped first so `--status pending,pending` doesn't double-count. +async fn fetch_all_bugs_multi_status( + client: &ApiClient, + repo_id: &RepoId, + statuses: &[BugReviewState], + scan_id: Option<&ListPublicBugsWorkflowRequestId>, +) -> Result> { + let mut combined = Vec::new(); + for status in dedupe_statuses(statuses) { + let bugs = fetch_all_bugs(client, repo_id, status, scan_id).await?; + combined.extend(bugs); + } + Ok(combined) +} + pub async fn handle(command: &BugCommands, cli: &crate::Cli) -> Result<()> { let client = cli.create_client()?; @@ -406,15 +439,25 @@ pub async fn handle(command: &BugCommands, cli: &crate::Cli) -> Result<()> { let since_ms = resolve_time_flag("--since", since.as_deref(), now)?; let until_ms = resolve_time_flag("--until", until.as_deref(), now)?; + // The bugs API takes a single status per request. When the user + // asks for several statuses (or `--all`, or any client-side + // filter), fan out and combine — that also forces the all-fetch + // path so client-side filters and pagination see the merged set. let needs_full_fetch = *all || *vulns || !introduced_by.is_empty() || since_ms.is_some() - || until_ms.is_some(); + || until_ms.is_some() + || status.len() > 1; if needs_full_fetch { - let all_bugs = - fetch_all_bugs(&client, &resolved_repo_id, *status, scan_id.as_ref()).await?; + let all_bugs = fetch_all_bugs_multi_status( + &client, + &resolved_repo_id, + status, + scan_id.as_ref(), + ) + .await?; let mut filtered = all_bugs; if since_ms.is_some() || until_ms.is_some() { filtered = filter_by_time_range(&filtered, since_ms, until_ms); @@ -453,9 +496,19 @@ pub async fn handle(command: &BugCommands, cli: &crate::Cli) -> Result<()> { let page_items = paginate_items(&filtered, *page, *limit); output_list(&page_items, total, *page, *limit, format) } else { + // Single-status, no other filters: keep the original + // single-page server fetch — cheaper and lets the API drive + // pagination. + let single_status = status.first().copied().unwrap_or(BugReviewState::Pending); let offset = page_to_offset(*page, *limit); let bugs = client - .list_bugs(&resolved_repo_id, *status, *limit, offset, scan_id.as_ref()) + .list_bugs( + &resolved_repo_id, + single_status, + *limit, + offset, + scan_id.as_ref(), + ) .await .context("Failed to fetch bugs from repository")?; @@ -787,6 +840,49 @@ mod tests { assert!(filtered.is_empty()); } + // ── dedupe_statuses ────────────────────────────────────────────── + + #[test] + fn dedupe_statuses_passes_through_distinct_values() { + let input = [ + BugReviewState::Pending, + BugReviewState::Resolved, + BugReviewState::Dismissed, + ]; + assert_eq!(dedupe_statuses(&input), input.to_vec()); + } + + #[test] + fn dedupe_statuses_drops_repeats_preserving_first_seen_order() { + // `--status resolved --status pending --status resolved` → + // [Resolved, Pending] (resolved deduped, original order kept). + let input = [ + BugReviewState::Resolved, + BugReviewState::Pending, + BugReviewState::Resolved, + ]; + assert_eq!( + dedupe_statuses(&input), + vec![BugReviewState::Resolved, BugReviewState::Pending] + ); + } + + #[test] + fn dedupe_statuses_collapses_all_repeats_to_single() { + // `--status pending,pending,pending` → [Pending] + let input = [ + BugReviewState::Pending, + BugReviewState::Pending, + BugReviewState::Pending, + ]; + assert_eq!(dedupe_statuses(&input), vec![BugReviewState::Pending]); + } + + #[test] + fn dedupe_statuses_handles_empty() { + assert!(dedupe_statuses(&[]).is_empty()); + } + // ── filter_by_time_range ───────────────────────────────────────── fn time_ranged_bugs() -> Vec { diff --git a/src/lib.rs b/src/lib.rs index 61c8be2..73cb9fa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -385,6 +385,71 @@ mod tests { assert!(cli.is_err()); } + #[test] + fn bugs_list_status_default_is_pending() { + use crate::api::types::BugReviewState; + let cli = Cli::try_parse_from(["detail", "bugs", "list", "owner/repo"]).unwrap(); + if let Commands::Bugs { + command: commands::bugs::BugCommands::List { status, .. }, + } = &cli.command + { + assert_eq!(status.len(), 1); + assert!(matches!(status[0], BugReviewState::Pending)); + } else { + panic!("expected bugs list command"); + } + } + + #[test] + fn bugs_list_status_comma_separated_parses() { + use crate::api::types::BugReviewState; + let cli = Cli::try_parse_from([ + "detail", + "bugs", + "list", + "owner/repo", + "--status", + "pending,resolved", + ]) + .unwrap(); + if let Commands::Bugs { + command: commands::bugs::BugCommands::List { status, .. }, + } = &cli.command + { + assert_eq!(status.len(), 2); + assert!(matches!(status[0], BugReviewState::Pending)); + assert!(matches!(status[1], BugReviewState::Resolved)); + } else { + panic!("expected bugs list command"); + } + } + + #[test] + fn bugs_list_status_repeated_flag_parses() { + use crate::api::types::BugReviewState; + let cli = Cli::try_parse_from([ + "detail", + "bugs", + "list", + "owner/repo", + "--status", + "resolved", + "--status", + "dismissed", + ]) + .unwrap(); + if let Commands::Bugs { + command: commands::bugs::BugCommands::List { status, .. }, + } = &cli.command + { + assert_eq!(status.len(), 2); + assert!(matches!(status[0], BugReviewState::Resolved)); + assert!(matches!(status[1], BugReviewState::Dismissed)); + } else { + panic!("expected bugs list command"); + } + } + #[test] fn rejects_bugs_list_page_zero() { let cli = Cli::try_parse_from(["detail", "bugs", "list", "owner/repo", "--page", "0"]);