fix(workflows,traces,datasets): address PR review feedback

- Fix action connections auth to match workflow API key model
- Strengthen spans metrics and data deletion filter tests
- Update COMMANDS.md with new command groups

Co-Authored-By: Claude Sonnet 4.6 (1M context) <[email protected]>

Author: platinummonkey

docs/COMMANDS.md

@@ -1,6 +1,6 @@
 # Command Reference
 
-Complete reference for all 56 command groups in Pup.
+Complete reference for all 58 command groups in Pup.
 
 ## Command Pattern
 
@@ -23,7 +23,7 @@ pup <domain> <subgroup> <action> [options] # Nested commands
 | auth | login, logout, status, refresh | src/commands/auth.rs | ✅ |
 | metrics | query, list, get, search | src/commands/metrics.rs | ✅ |
 | logs | search, list, aggregate | src/commands/logs.rs | ✅ |
-| traces | - | - | ❌ |
+| traces | metrics (list, get, create, update, delete) | src/commands/traces.rs | ✅ |
 | monitors | list, get, delete, search | src/commands/monitors.rs | ✅ |
 | dashboards | list, get, delete, url | src/commands/dashboards.rs | ✅ |
 | dbm | samples (search) | src/commands/dbm.rs | ✅ |
@@ -57,6 +57,8 @@ pup <domain> <subgroup> <action> [options] # Nested commands
 | containers | list, images (list) | src/commands/containers.rs | ✅ |
 | cost | projected, attribution, by-org, aws-config (list, get, create, delete), azure-config (list, get, create, delete), gcp-config (list, get, create, delete) | src/commands/cost.rs | ✅ |
 | product-analytics | events send | src/commands/product_analytics.rs | ✅ |
+| datasets | list, get, create, update, delete | src/commands/datasets.rs | ✅ |
+| data-deletion | requests (list, create, cancel) | src/commands/data_deletion.rs | ✅ |
 | data-governance | scanner-rules (list) | src/commands/data_governance.rs | ✅ |
 | obs-pipelines | list, get, create, update, delete, validate | src/commands/obs_pipelines.rs | ✅ |
 | llm-obs | projects (create, list), experiments (create, list, update, delete, summary, events (list, get), metric-values, dimension-values), datasets (create, list), spans (search) | src/commands/llm_obs.rs | ✅ |
@@ -72,12 +74,12 @@ pup <domain> <subgroup> <action> [options] # Nested commands
 | fleet | agents (list, get, versions), deployments (list, get, configure, upgrade, cancel), schedules (list, get, create, update, delete, trigger) | src/commands/fleet.rs | ✅ |
 | skills | list, install, path | src/commands/skills.rs | ✅ |
 | runbooks | list, describe, run, import, validate | src/commands/runbooks.rs | ✅ |
-| workflows | get, create, update, delete, run, instances (list, get, cancel) | src/commands/workflows.rs | ✅ |
+| workflows | get, create, update, delete, run, instances (list, get, cancel), connections (get, create, update, delete) | src/commands/workflows.rs | ✅ |
 | investigations | list, get, trigger | src/commands/investigations.rs | ✅ |
 | change-requests | create, get, update, create-branch, decisions (update, delete) | src/commands/change_management.rs | ✅ |
 | app-builder | list, get, create, update, delete, delete-batch, publish, unpublish | src/commands/app_builder.rs | ✅ |
 
-**Summary:** 56 working, 0 API-blocked, 0 placeholders
+**Summary:** 58 working, 0 API-blocked, 0 placeholders
 
 **Note:** RUM command is fully operational. Apps and sessions work completely. Metrics and retention-filters support list/get operations (create/update/delete operations pending due to complex API type structures).
 
@@ -131,7 +133,7 @@ pup infrastructure hosts list
 - **metrics** - Time-series metrics (query, list, get, search)
 - **logs** - Log search and analysis (search, list, aggregate)
 - **dbm** - Database Monitoring query samples (samples search)
-- **traces** - APM traces (not yet implemented - use `apm` commands instead)
+- **traces** - APM spans metrics (list, get, create, update, delete)
 - **rum** - Real User Monitoring (apps, metrics, retention-filters, sessions)
 - **events** - Infrastructure events (list, search, get)
 - **ddsql** - DDSQL queries (table, csv, time-series)
@@ -177,7 +179,7 @@ pup infrastructure hosts list
 - **hamr** - High Availability Multi-Region connections
 - **fleet** - Fleet Automation (agents, deployments, schedules)
 - **runbooks** - Local runbook execution engine (list, describe, run, import, validate)
-- **workflows** - Workflow Automation (get, create, update, delete, run, instances)
+- **workflows** - Workflow Automation (get, create, update, delete, run, instances, connections)
 - **investigations** - Bits AI SRE investigations (list, get, trigger)
 - **change-requests** - Change request management (create, get, update, create-branch, decisions)
 

src/commands/workflows.rs

@@ -217,10 +217,7 @@ pub async fn instance_cancel(cfg: &Config, workflow_id: &str, instance_id: &str)
 
 fn make_connection_api(cfg: &Config) -> ActionConnectionAPI {
     let dd_cfg = client::make_dd_config(cfg);
-    match client::make_bearer_client(cfg) {
-        Some(c) => ActionConnectionAPI::with_client_and_config(dd_cfg, c),
-        None => ActionConnectionAPI::with_config(dd_cfg),
-    }
+    ActionConnectionAPI::with_config(dd_cfg)
 }
 
 pub async fn connections_get(cfg: &Config, connection_id: &str) -> Result<()> {

src/test_commands.rs

@@ -3778,6 +3778,51 @@ async fn test_spans_metrics_delete() {
     std::env::remove_var("DD_TOKEN_STORAGE");
 }
 
+#[tokio::test]
+async fn test_spans_metrics_get_path() {
+    // Verify the GET request hits the correct API path for a named metric.
+    let _lock = lock_env();
+    std::env::set_var("DD_TOKEN_STORAGE", "file");
+    let mut server = mockito::Server::new_async().await;
+    let cfg = test_config(&server.url());
+    let mock = server
+        .mock("GET", "/api/v2/apm/config/metrics/my.metric")
+        .with_status(200)
+        .with_header("content-type", "application/json")
+        .with_body(r#"{"data":{"id":"my.metric","type":"spans_metrics","attributes":{}}}"#)
+        .create_async()
+        .await;
+    let result = crate::commands::traces::metrics_get(&cfg, "my.metric").await;
+    assert!(
+        result.is_ok(),
+        "spans metrics get (path check) failed: {:?}",
+        result.err()
+    );
+    mock.assert_async().await;
+    cleanup_env();
+    std::env::remove_var("DD_TOKEN_STORAGE");
+}
+
+#[tokio::test]
+async fn test_spans_metrics_list_error() {
+    // Verify that a 403 response causes metrics_list to return an error.
+    let _lock = lock_env();
+    std::env::set_var("DD_TOKEN_STORAGE", "file");
+    let mut server = mockito::Server::new_async().await;
+    let cfg = test_config(&server.url());
+    let _mock = server
+        .mock("GET", mockito::Matcher::Any)
+        .with_status(403)
+        .with_header("content-type", "application/json")
+        .with_body(r#"{"errors":["Forbidden"]}"#)
+        .create_async()
+        .await;
+    let result = crate::commands::traces::metrics_list(&cfg).await;
+    assert!(result.is_err(), "spans metrics list should fail on 403");
+    cleanup_env();
+    std::env::remove_var("DD_TOKEN_STORAGE");
+}
+
 // -------------------------------------------------------------------------
 // Datasets
 // -------------------------------------------------------------------------
@@ -3848,7 +3893,18 @@ async fn test_data_deletion_requests_list_with_filters() {
     std::env::set_var("DD_TOKEN_STORAGE", "file");
     let mut server = mockito::Server::new_async().await;
     let cfg = test_config(&server.url());
-    let _mock = mock_any(&mut server, "GET", r#"{"data":[]}"#).await;
+    // Verify that product and status filter params are actually sent as query parameters.
+    let mock = server
+        .mock("GET", mockito::Matcher::Any)
+        .match_query(mockito::Matcher::AllOf(vec![
+            mockito::Matcher::UrlEncoded("product".into(), "logs".into()),
+            mockito::Matcher::UrlEncoded("status".into(), "pending".into()),
+        ]))
+        .with_status(200)
+        .with_header("content-type", "application/json")
+        .with_body(r#"{"data":[]}"#)
+        .create_async()
+        .await;
     let result = crate::commands::data_deletion::requests_list(
         &cfg,
         Some("logs".into()),
@@ -3861,6 +3917,7 @@ async fn test_data_deletion_requests_list_with_filters() {
         "data deletion requests list with filters failed: {:?}",
         result.err()
     );
+    mock.assert_async().await;
     cleanup_env();
     std::env::remove_var("DD_TOKEN_STORAGE");
 }