fix(pagination): address pagination audit — wire dropped flags, validate limits, fix panics

Addresses all issues identified in the endpoint pagination audit:

**Silently dropped CLI flags (.. destructure pattern)**
- cases: wire --page-number into SearchCasesOptionalParams
- on-call: wire --page-number and --sort into GetTeamMembershipsOptionalParams
- security: wire --sort into SecurityMonitoringSignalsSort (with bail! for invalid values)
- cicd pipelines: wire --branch and --pipeline-name as quoted query filters
- cicd events: wire --sort with explicit bail! (no silent fallback)
- error-tracking: wire --from, --to, --order-by; rename _limit → limit
- investigations: wire --monitor-id as filter_monitor_id when non-zero
- metrics: wire --tag-filter into ListActiveMetricsOptionalParams

**Silent limit clamping replaced with bail!**
- monitors list: replace clamp(1,1000) with !(1..=1000).contains bail!; add page param
- monitors search: wire --page, --per-page, --sort
- traces: replace limit.min(1000) with bail! for out-of-range values
- workflows: replace limit.clamp(1,100) with bail! for out-of-range values

**Panic risk (.unwrap()) replaced with ok_or_else**
- cicd: DateTime::from_timestamp_millis in pipelines_list and events_search
- rum: DateTime::from_timestamp_millis in sessions_search and sessions_list

**Additional fixes from review**
- on-call: move sort/role validation before resolve_team_id (fail-fast, no wasted API call)
- on-call: replace dead `_ => ADMIN` catch-all in memberships_add/update with bail!
- on-call: extract parse_team_role helper to avoid duplication
- on-call: resolve_team_id uses ok_or_else instead of unwrap_or_default on resp.data
- on-call: case-insensitive handle comparison in resolve_team_id
- error-tracking: println! → eprintln! for empty-result message
- cicd: quote branch/pipeline-name values in query to handle spaces; escape internal quotes
- security/cicd: remove silent sort fallback catch-all, add explicit asc/desc aliases

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: platinummonkey

src/commands/cases.rs

@@ -29,9 +29,16 @@ fn make_api(cfg: &Config) -> CaseManagementAPI {
 // Core case operations
 // ---------------------------------------------------------------------------
 
-pub async fn search(cfg: &Config, query: Option<String>, page_size: i64) -> Result<()> {
+pub async fn search(
+    cfg: &Config,
+    query: Option<String>,
+    page_size: i64,
+    page_number: i64,
+) -> Result<()> {
     let api = make_api(cfg);
-    let mut params = SearchCasesOptionalParams::default().page_size(page_size);
+    let mut params = SearchCasesOptionalParams::default()
+        .page_size(page_size)
+        .page_number(page_number);
     if let Some(q) = query {
         params = params.filter(q);
     }
@@ -372,7 +379,7 @@ mod tests {
         let mut s = mockito::Server::new_async().await;
         let cfg = test_config(&s.url());
         mock_all(&mut s, r#"{"data": []}"#).await;
-        let _ = super::search(&cfg, None, 10).await;
+        let _ = super::search(&cfg, None, 10, 0).await;
         cleanup_env();
     }
 

src/commands/cicd.rs

@@ -25,21 +25,37 @@ pub async fn pipelines_list(
     from: String,
     to: String,
     limit: i32,
+    branch: Option<String>,
+    pipeline_name: Option<String>,
 ) -> Result<()> {
     let api = crate::make_api!(CIVisibilityPipelinesAPI, cfg);
 
     let from_ms = util::parse_time_to_unix_millis(&from)?;
     let to_ms = util::parse_time_to_unix_millis(&to)?;
     let from_str = chrono::DateTime::from_timestamp_millis(from_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--from value {from_ms}ms is out of representable range"))?
         .to_rfc3339();
     let to_str = chrono::DateTime::from_timestamp_millis(to_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--to value {to_ms}ms is out of representable range"))?
         .to_rfc3339();
 
-    let mut filter = CIAppPipelinesQueryFilter::new().from(from_str).to(to_str);
+    let mut query_parts: Vec<String> = Vec::new();
     if let Some(q) = query {
-        filter = filter.query(q);
+        query_parts.push(q);
+    }
+    if let Some(b) = branch {
+        query_parts.push(format!("@git.branch:\"{}\"", b.replace('"', "\\\"")));
+    }
+    if let Some(p) = pipeline_name {
+        query_parts.push(format!(
+            "@ci.pipeline.name:\"{}\"",
+            p.replace('"', "\\\"")
+        ));
+    }
+
+    let mut filter = CIAppPipelinesQueryFilter::new().from(from_str).to(to_str);
+    if !query_parts.is_empty() {
+        filter = filter.query(query_parts.join(" "));
     }
 
     let body = CIAppPipelineEventsRequest::new()
@@ -88,18 +104,27 @@ pub async fn events_search(
     from: String,
     to: String,
     limit: i32,
+    sort: String,
 ) -> Result<()> {
     let api = crate::make_api!(CIVisibilityPipelinesAPI, cfg);
 
     let from_ms = util::parse_time_to_unix_millis(&from)?;
     let to_ms = util::parse_time_to_unix_millis(&to)?;
     let from_str = chrono::DateTime::from_timestamp_millis(from_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--from value {from_ms}ms is out of representable range"))?
         .to_rfc3339();
     let to_str = chrono::DateTime::from_timestamp_millis(to_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--to value {to_ms}ms is out of representable range"))?
         .to_rfc3339();
 
+    let sort_val = match sort.as_str() {
+        "asc" | "timestamp" => CIAppSort::TIMESTAMP_ASCENDING,
+        "desc" | "-timestamp" => CIAppSort::TIMESTAMP_DESCENDING,
+        other => anyhow::bail!(
+            "invalid --sort value: {other:?}\nExpected: asc (ascending) or desc (descending)"
+        ),
+    };
+
     let filter = CIAppPipelinesQueryFilter::new()
         .from(from_str)
         .to(to_str)
@@ -108,7 +133,7 @@ pub async fn events_search(
     let body = CIAppPipelineEventsRequest::new()
         .filter(filter)
         .page(CIAppQueryPageOptions::new().limit(limit))
-        .sort(CIAppSort::TIMESTAMP_DESCENDING);
+        .sort(sort_val);
 
     let params = SearchCIAppPipelineEventsOptionalParams::default().body(body);
     let resp = api
@@ -124,10 +149,10 @@ pub async fn events_aggregate(cfg: &Config, query: String, from: String, to: Str
     let from_ms = util::parse_time_to_unix_millis(&from)?;
     let to_ms = util::parse_time_to_unix_millis(&to)?;
     let from_str = chrono::DateTime::from_timestamp_millis(from_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--from value {from_ms}ms is out of representable range"))?
         .to_rfc3339();
     let to_str = chrono::DateTime::from_timestamp_millis(to_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--to value {to_ms}ms is out of representable range"))?
         .to_rfc3339();
 
     let filter = CIAppPipelinesQueryFilter::new()
@@ -157,10 +182,10 @@ pub async fn tests_search(
     let from_ms = util::parse_time_to_unix_millis(&from)?;
     let to_ms = util::parse_time_to_unix_millis(&to)?;
     let from_str = chrono::DateTime::from_timestamp_millis(from_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--from value {from_ms}ms is out of representable range"))?
         .to_rfc3339();
     let to_str = chrono::DateTime::from_timestamp_millis(to_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--to value {to_ms}ms is out of representable range"))?
         .to_rfc3339();
 
     let filter = CIAppTestsQueryFilter::new()
@@ -187,10 +212,10 @@ pub async fn tests_aggregate(cfg: &Config, query: String, from: String, to: Stri
     let from_ms = util::parse_time_to_unix_millis(&from)?;
     let to_ms = util::parse_time_to_unix_millis(&to)?;
     let from_str = chrono::DateTime::from_timestamp_millis(from_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--from value {from_ms}ms is out of representable range"))?
         .to_rfc3339();
     let to_str = chrono::DateTime::from_timestamp_millis(to_ms)
-        .unwrap()
+        .ok_or_else(|| anyhow::anyhow!("--to value {to_ms}ms is out of representable range"))?
         .to_rfc3339();
 
     let filter = CIAppTestsQueryFilter::new()
@@ -314,7 +339,7 @@ mod tests {
         let mut s = mockito::Server::new_async().await;
         let cfg = test_config(&s.url());
         mock_all(&mut s, r#"{"data": []}"#).await;
-        let _ = super::pipelines_list(&cfg, None, "1h".into(), "now".into(), 10).await;
+        let _ = super::pipelines_list(&cfg, None, "1h".into(), "now".into(), 10, None, None).await;
         cleanup_env();
     }
 
@@ -358,4 +383,17 @@ mod tests {
             .contains("invalid sort value"));
         cleanup_env();
     }
+
+    #[tokio::test]
+    async fn test_cicd_events_search_invalid_sort() {
+        let cfg = test_config("http://unused.local");
+        let result =
+            super::events_search(&cfg, "*".into(), "1h".into(), "now".into(), 10, "bogus".into())
+                .await;
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("invalid --sort value"));
+    }
 }

src/commands/error_tracking.rs

@@ -1,31 +1,46 @@
 use anyhow::Result;
-use chrono::Utc;
 use datadog_api_client::datadogV2::api_error_tracking::{
     ErrorTrackingAPI, GetIssueOptionalParams, SearchIssuesOptionalParams,
 };
 use datadog_api_client::datadogV2::model::{
     IssuesSearchRequest, IssuesSearchRequestData, IssuesSearchRequestDataAttributes,
-    IssuesSearchRequestDataAttributesPersona, IssuesSearchRequestDataAttributesTrack,
-    IssuesSearchRequestDataType,
+    IssuesSearchRequestDataAttributesOrderBy, IssuesSearchRequestDataAttributesPersona,
+    IssuesSearchRequestDataAttributesTrack, IssuesSearchRequestDataType,
 };
 
 use crate::config::Config;
 use crate::formatter;
+use crate::util;
 
+#[allow(clippy::too_many_arguments)]
 pub async fn issues_search(
     cfg: &Config,
     query: Option<String>,
-    _limit: i32,
+    limit: i32,
+    from: String,
+    to: String,
+    order_by: String,
     track: Option<String>,
     persona: Option<String>,
 ) -> Result<()> {
     let api = crate::make_api!(ErrorTrackingAPI, cfg);
 
-    let now = Utc::now().timestamp_millis();
-    let one_day_ago = now - 86_400_000; // 24 hours in millis
+    let from_ms = util::parse_time_to_unix_millis(&from)?;
+    let to_ms = util::parse_time_to_unix_millis(&to)?;
+
+    let order_by_val = match order_by.to_uppercase().as_str() {
+        "TOTAL_COUNT" => IssuesSearchRequestDataAttributesOrderBy::TOTAL_COUNT,
+        "FIRST_SEEN" => IssuesSearchRequestDataAttributesOrderBy::FIRST_SEEN,
+        "IMPACTED_SESSIONS" => IssuesSearchRequestDataAttributesOrderBy::IMPACTED_SESSIONS,
+        "PRIORITY" => IssuesSearchRequestDataAttributesOrderBy::PRIORITY,
+        other => anyhow::bail!(
+            "invalid --order-by value: {other:?}\nExpected: TOTAL_COUNT, FIRST_SEEN, IMPACTED_SESSIONS, PRIORITY"
+        ),
+    };
 
     let query_str = query.unwrap_or_else(|| "*".to_string());
-    let mut attrs = IssuesSearchRequestDataAttributes::new(one_day_ago, query_str, now);
+    let mut attrs =
+        IssuesSearchRequestDataAttributes::new(from_ms, query_str, to_ms).order_by(order_by_val);
     if let Some(ref t) = track {
         let track_value = match t.to_lowercase().as_str() {
             "trace" => IssuesSearchRequestDataAttributesTrack::TRACE,
@@ -55,17 +70,22 @@ pub async fn issues_search(
     let body = IssuesSearchRequest::new(data);
     let params = SearchIssuesOptionalParams::default();
 
-    let resp = api
+    let mut resp = api
         .search_issues(body, params)
         .await
         .map_err(|e| anyhow::anyhow!("failed to search issues: {e:?}"))?;
-    let val = serde_json::to_value(&resp)?;
-    if let Some(data) = val.get("data") {
-        if data.as_array().is_some_and(|a| a.is_empty()) {
-            println!("No error tracking issues found matching the specified criteria.");
-            return Ok(());
+
+    if resp.data.as_ref().is_some_and(|d| d.is_empty()) {
+        eprintln!("No error tracking issues found matching the specified criteria.");
+        return Ok(());
+    }
+
+    if limit > 0 {
+        if let Some(data) = resp.data.as_mut() {
+            data.truncate(limit as usize);
         }
     }
+
     formatter::output(cfg, &resp)
 }
 
@@ -90,7 +110,17 @@ mod tests {
         let mut s = mockito::Server::new_async().await;
         let cfg = test_config(&s.url());
         mock_all(&mut s, r#"{"data": []}"#).await;
-        let _ = super::issues_search(&cfg, None, 10, Some("trace".into()), None).await;
+        let _ = super::issues_search(
+            &cfg,
+            None,
+            10,
+            "1d".into(),
+            "now".into(),
+            "TOTAL_COUNT".into(),
+            Some("trace".into()),
+            None,
+        )
+        .await;
         cleanup_env();
     }
 
@@ -100,7 +130,17 @@ mod tests {
         let mut s = mockito::Server::new_async().await;
         let cfg = test_config(&s.url());
         mock_all(&mut s, r#"{"data": []}"#).await;
-        let _ = super::issues_search(&cfg, None, 10, None, Some("BROWSER".into())).await;
+        let _ = super::issues_search(
+            &cfg,
+            None,
+            10,
+            "1d".into(),
+            "now".into(),
+            "TOTAL_COUNT".into(),
+            None,
+            Some("BROWSER".into()),
+        )
+        .await;
         cleanup_env();
     }
 
@@ -110,10 +150,41 @@ mod tests {
         let mut s = mockito::Server::new_async().await;
         let cfg = test_config(&s.url());
         mock_all(&mut s, r#"{"data": []}"#).await;
-        let _ = super::issues_search(&cfg, None, 10, Some("RUM".into()), None).await;
+        let _ = super::issues_search(
+            &cfg,
+            None,
+            10,
+            "1d".into(),
+            "now".into(),
+            "TOTAL_COUNT".into(),
+            Some("RUM".into()),
+            None,
+        )
+        .await;
         cleanup_env();
     }
 
+    #[tokio::test]
+    async fn test_error_tracking_issues_search_invalid_order_by() {
+        let cfg = test_config("http://unused.local");
+        let result = super::issues_search(
+            &cfg,
+            None,
+            10,
+            "1d".into(),
+            "now".into(),
+            "INVALID".into(),
+            Some("trace".into()),
+            None,
+        )
+        .await;
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("invalid --order-by value"));
+    }
+
     #[test]
     fn test_error_tracking_clap_mutual_exclusivity() {
         let result = crate::Cli::command().try_get_matches_from([

src/commands/investigations.rs

@@ -9,11 +9,14 @@ fn make_api(cfg: &Config) -> BitsAIAPI {
     crate::make_api!(BitsAIAPI, cfg)
 }
 
-pub async fn list(cfg: &Config, page_limit: i64, page_offset: i64) -> Result<()> {
+pub async fn list(cfg: &Config, page_limit: i64, page_offset: i64, monitor_id: i64) -> Result<()> {
     let api = make_api(cfg);
-    let params = ListInvestigationsOptionalParams::default()
+    let mut params = ListInvestigationsOptionalParams::default()
         .page_limit(page_limit)
         .page_offset(page_offset);
+    if monitor_id != 0 {
+        params = params.filter_monitor_id(monitor_id);
+    }
     let resp = api
         .list_investigations(params)
         .await
@@ -52,7 +55,7 @@ mod tests {
         let mut s = mockito::Server::new_async().await;
         let cfg = test_config(&s.url());
         mock_all(&mut s, r#"{"data": []}"#).await;
-        let _ = super::list(&cfg, 10, 0).await;
+        let _ = super::list(&cfg, 10, 0, 0).await;
         cleanup_env();
     }
 

src/commands/metrics.rs

@@ -40,11 +40,19 @@ use crate::config::Config;
 use crate::formatter;
 use crate::util;
 
-pub async fn list(cfg: &Config, filter: Option<String>, from: String) -> Result<()> {
+pub async fn list(
+    cfg: &Config,
+    filter: Option<String>,
+    from: String,
+    tag_filter: Option<String>,
+) -> Result<()> {
     let api = crate::make_api!(MetricsV1API, cfg);
 
     let from_ts = util::parse_time_to_unix(&from)?;
-    let params = ListActiveMetricsOptionalParams::default();
+    let mut params = ListActiveMetricsOptionalParams::default();
+    if let Some(tf) = tag_filter {
+        params = params.tag_filter(tf);
+    }
 
     let resp = api
         .list_active_metrics(from_ts, params)
@@ -199,7 +207,7 @@ mod tests {
         )
         .await;
 
-        let result = super::list(&cfg, None, "1h".into()).await;
+        let result = super::list(&cfg, None, "1h".into(), None).await;
         assert!(result.is_ok(), "metrics list failed: {:?}", result.err());
         cleanup_env();
     }