Merge pull request #708 from DataDog/dd/workflow-diff-issue-634-20260804-a7c9

Add reusable workflow diff

Author: platinummonkey

docs/COMMANDS.md

@@ -79,7 +79,7 @@ pup <domain> <subgroup> <action> [options] # Nested commands
 | fleet | agents (list, get, versions, tracers), deployments (list, get, configure, upgrade, cancel), schedules (list, get, create, update, delete, trigger), tracers (list), clusters (list), instrumented-pods (list) | src/commands/fleet.rs | ✅ |
 | skills | list, install, path (positional `<platform>`: claude/cursor/codex/opencode/windsurf/gemini/pi/devin/all; `--name`, `--type`, `--project` for project-local scope) | src/commands/skills.rs | ✅ |
 | runbooks | list, describe, run, import, validate | src/commands/runbooks.rs | ✅ |
-| workflows | get, create, update, delete, run, instances (list, get, cancel), connections (get, create, update, delete) | src/commands/workflows.rs | ✅ |
+| workflows | get, create, update, diff, 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 | ✅ |
 | change-stories | list | src/commands/change_stories.rs | ✅ |
@@ -193,7 +193,7 @@ pup infrastructure hosts list
 - **hamr** - High Availability Multi-Region connections
 - **fleet** - Fleet Automation (agents, deployments, schedules, tracers, clusters, instrumented-pods)
 - **runbooks** - Local runbook execution engine (list, describe, run, import, validate)
-- **workflows** - Workflow Automation (get, create, update, delete, run, instances, connections)
+- **workflows** - Workflow Automation (get, create, update, diff, delete, run, instances, connections)
 - **investigations** - Bits AI SRE investigations (list, get, trigger)
 - **change-requests** - Change request management (create, get, update, create-branch, decisions)
 - **change-stories** - Change events for a service (deployments, feature flags, config, k8s, watchdog) over time window

docs/EXAMPLES.md

@@ -773,6 +773,16 @@ pup workflows create --file=workflow.json
 pup workflows update <workflow-id> --file=workflow.json
 ```
 
+### Diff a Workflow
+```bash
+# Compare a candidate JSON file against the live workflow
+pup workflows diff <workflow-id> workflow.json
+
+# Scope or suppress specific field paths
+pup workflows diff <workflow-id> workflow.json --only data.attributes.spec
+pup workflows diff <workflow-id> workflow.json --ignore data.attributes.updatedAt
+```
+
 ### Delete a Workflow
 ```bash
 pup workflows delete <workflow-id>

src/commands/monitors.rs

@@ -114,65 +114,39 @@ pub async fn diff(
     // Trade-off: field-name typos in the candidate file won't be caught here;
     // they will appear as added/removed pairs in the diff output, which is still
     // actionable. `pup monitors update` will fail or no-op on unknown fields.
-    let mut candidate: serde_json::Value = util::read_json_file(file)?;
+    let candidate: serde_json::Value = util::read_json_file(file)?;
 
     // Fetch the live monitor
     let api = crate::make_api!(MonitorsAPI, cfg);
     let live = api
         .get_monitor(monitor_id, GetMonitorOptionalParams::default())
         .await
         .map_err(|e| anyhow::anyhow!("failed to get monitor: {:?}", e))?;
-    let mut live = serde_json::to_value(&live)
+    let live = serde_json::to_value(&live)
         .map_err(|e| anyhow::anyhow!("failed to serialize monitor {monitor_id} for diff: {e:?}"))?;
 
-    // Normalize both sides: strip server-managed read-only fields and drop nulls
-    // so absent and explicit-null compare equal.
-    // Note: the API may populate concrete option defaults (e.g. new_host_delay,
-    // notify_no_data) in the live response that the candidate omits. Those appear
-    // as "removed" because the candidate is treated as the complete desired state.
-    // Use --ignore to suppress specific option fields if the noise is unwanted.
-    util_ext::normalize_for_diff(&mut live, util_ext::READONLY_MONITOR_FIELDS);
-    util_ext::normalize_for_diff(&mut candidate, util_ext::READONLY_MONITOR_FIELDS);
-
-    let entries = util_ext::scope_diff(util_ext::diff_json(&live, &candidate), only, ignore);
-
     // `update` (PUT) is a partial/merge update: fields absent from the candidate
     // file are left unchanged on the live monitor, not deleted. "removed" entries
     // in this diff show what the candidate does not specify — they will NOT be
     // removed by `pup monitors update`. Run `update` only for "added"/"modified"
     // changes; "removed" entries require no action unless you want to add those
     // fields to the candidate explicitly.
-    let has_removed = entries
-        .iter()
-        .any(|e| e.change == util_ext::ChangeKind::Removed);
-    let next_action = if entries.is_empty() {
-        None
-    } else if has_removed {
-        Some(
-            "review changes — note: 'removed' entries will NOT be deleted by \
-             `pup monitors update` (partial update)"
-                .to_string(),
-        )
-    } else {
-        Some("review changes, then run `pup monitors update`".to_string())
-    };
-    let meta = Metadata {
-        count: Some(entries.len()),
-        truncated: false,
-        command: Some("monitors diff".to_string()),
-        next_action,
-    };
-    formatter::format_and_print(
-        &entries,
-        &cfg.output_format,
-        cfg.agent_mode,
-        Some(&meta),
-        cfg.jq.as_deref(),
-    )?;
-    if entries.is_empty() && !cfg.agent_mode {
-        eprintln!("No changes — monitor {monitor_id} is in sync.");
-    }
-    Ok(())
+    let resource_id = monitor_id.to_string();
+    let mut options = util_ext::ResourceDiffOptions::new(
+        "monitors diff",
+        "pup monitors update",
+        "monitor",
+        &resource_id,
+    );
+    options.readonly_paths = util_ext::READONLY_MONITOR_FIELDS;
+    options.only = only;
+    options.ignore = ignore;
+    options.removed_entries_next_action = Some(
+        "review changes — note: 'removed' entries will NOT be deleted by \
+         `pup monitors update` (partial update)",
+    );
+    options.no_changes_message = Some(format!("No changes — monitor {monitor_id} is in sync."));
+    util_ext::format_resource_diff(cfg, &live, &candidate, &options)
 }
 
 pub async fn search(

src/commands/workflows.rs

@@ -8,6 +8,7 @@ use datadog_api_client::datadogV2::api_workflow_automation::{
 
 use crate::config::Config;
 use crate::formatter::{self, Metadata};
+use crate::raw_client;
 use crate::util;
 use crate::util_ext;
 
@@ -54,6 +55,31 @@ pub async fn update(cfg: &Config, workflow_id: &str, file: &str) -> Result<()> {
     formatter::output(cfg, &resp)
 }
 
+pub async fn diff(
+    cfg: &Config,
+    workflow_id: &str,
+    file: &str,
+    only: &[String],
+    ignore: &[String],
+) -> Result<()> {
+    let candidate: serde_json::Value = util::read_json_file(file)?;
+    let live = raw_client::raw_get(cfg, &format!("/api/v2/workflows/{workflow_id}"), &[])
+        .await
+        .map_err(|e| anyhow::anyhow!("failed to get workflow: {e:?}"))?;
+
+    let mut options = util_ext::ResourceDiffOptions::new(
+        "workflows diff",
+        "pup workflows update",
+        "workflow",
+        workflow_id,
+    );
+    options.readonly_paths = util_ext::READONLY_WORKFLOW_FIELDS;
+    options.only = only;
+    options.ignore = ignore;
+    options.no_changes_message = Some(format!("No changes - workflow {workflow_id} is in sync."));
+    util_ext::format_resource_diff(cfg, &live, &candidate, &options)
+}
+
 pub async fn delete(cfg: &Config, workflow_id: &str) -> Result<()> {
     let api = make_api(cfg);
     api.delete_workflow(workflow_id.to_string())
@@ -270,6 +296,69 @@ mod tests {
 
     use crate::test_support::*;
 
+    #[tokio::test]
+    async fn test_workflows_diff_detects_changes() {
+        let _lock = lock_env().await;
+        let mut server = mockito::Server::new_async().await;
+        let cfg = test_config(&server.url());
+
+        let live_body = r#"{
+            "data": {
+                "id": "wf-123",
+                "type": "workflows",
+                "attributes": {
+                    "action_id": "wf-123",
+                    "name": "Old workflow",
+                    "description": "Deploy service",
+                    "spec": {"steps": [{"name": "deploy", "timeout": 60}]},
+                    "updatedAt": "2024-01-01T00:00:00Z"
+                }
+            }
+        }"#;
+        let _mock = mock_any(&mut server, "GET", live_body).await;
+
+        let candidate = r#"{
+            "data": {
+                "type": "workflows",
+                "attributes": {
+                    "name": "New workflow",
+                    "description": "Deploy service",
+                    "spec": {"steps": [{"name": "deploy", "timeout": 90}]}
+                }
+            }
+        }"#;
+        let path = write_temp_json("pup_workflows_diff_detects_changes.json", candidate);
+
+        let result = super::diff(&cfg, "wf-123", path.to_str().unwrap(), &[], &[]).await;
+        let _ = std::fs::remove_file(path);
+        assert!(result.is_ok(), "workflows diff failed: {:?}", result.err());
+        cleanup_env();
+    }
+
+    #[tokio::test]
+    async fn test_workflows_diff_file_not_found() {
+        let cfg = test_config("http://unused.local");
+        let result = super::diff(&cfg, "wf-123", "/nonexistent/path.json", &[], &[]).await;
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("failed to read file"));
+    }
+
+    #[tokio::test]
+    async fn test_workflows_diff_invalid_json() {
+        let path = write_temp_json("pup_workflows_diff_invalid_json.json", "not valid json {{{");
+        let cfg = test_config("http://unused.local");
+        let result = super::diff(&cfg, "wf-123", path.to_str().unwrap(), &[], &[]).await;
+        let _ = std::fs::remove_file(path);
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("failed to parse JSON"));
+    }
+
     #[tokio::test]
     async fn test_connections_get() {
         let _lock = lock_env().await;

src/main.rs

@@ -4744,6 +4744,23 @@ enum WorkflowActions {
         #[arg(long)]
         file: String,
     },
+    /// Diff a candidate JSON definition against the live workflow
+    Diff {
+        workflow_id: String,
+        file: String,
+        #[arg(
+            long,
+            value_delimiter = ',',
+            help = "Restrict the diff to these field paths (dot-notation, comma-separated or repeated)"
+        )]
+        only: Vec<String>,
+        #[arg(
+            long,
+            value_delimiter = ',',
+            help = "Exclude these field paths from the diff (dot-notation, comma-separated or repeated)"
+        )]
+        ignore: Vec<String>,
+    },
     /// Delete a workflow
     Delete { workflow_id: String },
     /// Execute a workflow via API trigger
@@ -16560,6 +16577,14 @@ async fn main_inner() -> anyhow::Result<()> {
             WorkflowActions::Update { workflow_id, file } => {
                 commands::workflows::update(&cfg, &workflow_id, &file).await?;
             }
+            WorkflowActions::Diff {
+                workflow_id,
+                file,
+                only,
+                ignore,
+            } => {
+                commands::workflows::diff(&cfg, &workflow_id, &file, &only, &ignore).await?;
+            }
             WorkflowActions::Delete { workflow_id } => {
                 commands::workflows::delete(&cfg, &workflow_id).await?;
             }