Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/HELP.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ List bugs for a given repository

###### **Options:**

* `--status <STATUS>` — Status filter
* `--status <STATUS>` — Status filter — repeat the flag or comma-separate values to combine (e.g. `--status pending,resolved`). Default: pending

Default value: `pending`

Expand Down
110 changes: 103 additions & 7 deletions src/commands/bugs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ pub enum BugCommands {
/// If omitted, inferred from the git remote (origin).
repo: Option<String>,

/// 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<BugReviewState>,

/// Only show security vulnerabilities
#[arg(long)]
Expand Down Expand Up @@ -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<BugReviewState> {
let mut out: Vec<BugReviewState> = 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<Vec<Bug>> {
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()?;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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")?;

Expand Down Expand Up @@ -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<Bug> {
Expand Down
65 changes: 65 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"]);
Expand Down
Loading