refactor: reduce complexity of execute_safe_output in src/execute.rs (#307)

Author: github-actions[bot]

src/execute.rs

@@ -5,6 +5,7 @@
 
 use anyhow::{Result, bail};
 use log::{debug, error, info, warn};
+use serde::de::DeserializeOwned;
 use serde_json::Value;
 use std::collections::HashMap;
 use std::path::Path;
@@ -213,6 +214,35 @@ pub async fn execute_safe_outputs(
     Ok(results)
 }
 
+/// Parse a JSON entry as `T` and run it through `execute_sanitized`.
+///
+/// This is the common path for all tools that implement `Executor`. The tool name
+/// is used only for the error message so callers don't have to repeat it.
+async fn dispatch_tool<T>(
+    tool_name: &str,
+    entry: &Value,
+    ctx: &ExecutionContext,
+) -> Result<ExecutionResult>
+where
+    T: DeserializeOwned + Executor,
+{
+    debug!("Parsing {} payload", tool_name);
+    let mut output: T = serde_json::from_value(entry.clone())
+        .map_err(|e| anyhow::anyhow!("Failed to parse {}: {}", tool_name, e))?;
+    output.execute_sanitized(ctx).await
+}
+
+macro_rules! dispatch_executor_tools {
+    ($tool_name:expr, $entry:expr, $ctx:expr, { $($name:literal => $ty:ty),+ $(,)? }) => {
+        match $tool_name {
+            $(
+                $name => dispatch_tool::<$ty>($tool_name, $entry, $ctx).await.map(Some),
+            )+
+            _ => Ok(None),
+        }
+    };
+}
+
 /// Execute a single safe output entry, returning the tool name and result
 pub async fn execute_safe_output(
     entry: &Value,
@@ -226,191 +256,49 @@ pub async fn execute_safe_output(
 
     debug!("Dispatching tool: {}", tool_name);
 
-    // Dispatch based on tool name
-    let result = match tool_name {
-        "create-work-item" => {
-            debug!("Parsing create-work-item payload");
-            let mut output: CreateWorkItemResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse create-work-item: {}", e))?;
-            debug!(
-                "create-work-item: title='{}', description length={}",
-                output.title,
-                output.description.len()
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "comment-on-work-item" => {
-            debug!("Parsing comment-on-work-item payload");
-            let mut output: CommentOnWorkItemResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse comment-on-work-item: {}", e))?;
-            debug!(
-                "comment-on-work-item: work_item_id={}, body length={}",
-                output.work_item_id,
-                output.body.len()
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "update-work-item" => {
-            debug!("Parsing update-work-item payload");
-            let mut output: UpdateWorkItemResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse update-work-item: {}", e))?;
-            debug!("update-work-item: id={}", output.id);
-            output.execute_sanitized(ctx).await?
-        }
-        "create-pull-request" => {
-            debug!("Parsing create-pull-request payload");
-            let mut output: CreatePrResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse create-pull-request: {}", e))?;
-            debug!(
-                "create-pull-request: title='{}', repo='{}', branch='{}', patch='{}'",
-                output.title, output.repository, output.source_branch, output.patch_file
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "update-wiki-page" => {
-            debug!("Parsing update-wiki-page payload");
-            let mut output: UpdateWikiPageResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse update-wiki-page: {}", e))?;
-            debug!(
-                "update-wiki-page: path='{}', content length={}",
-                output.path,
-                output.content.len()
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "create-wiki-page" => {
-            debug!("Parsing create-wiki-page payload");
-            let mut output: CreateWikiPageResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse create-wiki-page: {}", e))?;
-            debug!(
-                "create-wiki-page: path='{}', content length={}",
-                output.path,
-                output.content.len()
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "add-pr-comment" => {
-            debug!("Parsing add-pr-comment payload");
-            let mut output: AddPrCommentResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse add-pr-comment: {}", e))?;
-            debug!(
-                "add-pr-comment: pr_id={}, content length={}",
-                output.pull_request_id,
-                output.content.len()
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "link-work-items" => {
-            debug!("Parsing link-work-items payload");
-            let mut output: LinkWorkItemsResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse link-work-items: {}", e))?;
-            debug!(
-                "link-work-items: source={}, target={}, type={}",
-                output.source_id, output.target_id, output.link_type
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "queue-build" => {
-            debug!("Parsing queue-build payload");
-            let mut output: QueueBuildResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse queue-build: {}", e))?;
-            debug!("queue-build: pipeline_id={}", output.pipeline_id);
-            output.execute_sanitized(ctx).await?
-        }
-        "create-git-tag" => {
-            debug!("Parsing create-git-tag payload");
-            let mut output: CreateGitTagResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse create-git-tag: {}", e))?;
-            debug!("create-git-tag: tag_name='{}'", output.tag_name);
-            output.execute_sanitized(ctx).await?
-        }
-        "add-build-tag" => {
-            debug!("Parsing add-build-tag payload");
-            let mut output: AddBuildTagResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse add-build-tag: {}", e))?;
-            debug!("add-build-tag: build_id={}, tag='{}'", output.build_id, output.tag);
-            output.execute_sanitized(ctx).await?
-        }
-        "create-branch" => {
-            debug!("Parsing create-branch payload");
-            let mut output: CreateBranchResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse create-branch: {}", e))?;
-            debug!("create-branch: branch_name='{}'", output.branch_name);
-            output.execute_sanitized(ctx).await?
-        }
-        "update-pr" => {
-            debug!("Parsing update-pr payload");
-            let mut output: UpdatePrResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse update-pr: {}", e))?;
-            debug!(
-                "update-pr: pr_id={}, operation='{}'",
-                output.pull_request_id, output.operation
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "upload-attachment" => {
-            debug!("Parsing upload-attachment payload");
-            let mut output: UploadAttachmentResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse upload-attachment: {}", e))?;
-            debug!(
-                "upload-attachment: work_item_id={}, file_path='{}'",
-                output.work_item_id, output.file_path
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "submit-pr-review" => {
-            debug!("Parsing submit-pr-review payload");
-            let mut output: SubmitPrReviewResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse submit-pr-review: {}", e))?;
-            debug!(
-                "submit-pr-review: pr_id={}, event='{}'",
-                output.pull_request_id, output.event
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "reply-to-pr-review-comment" => {
-            debug!("Parsing reply-to-pr-review-comment payload");
-            let mut output: ReplyToPrCommentResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse reply-to-pr-review-comment: {}", e))?;
-            debug!(
-                "reply-to-pr-review-comment: pr_id={}, thread_id={}",
-                output.pull_request_id, output.thread_id
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "resolve-pr-thread" => {
-            debug!("Parsing resolve-pr-thread payload");
-            let mut output: ResolvePrThreadResult = serde_json::from_value(entry.clone())
-                .map_err(|e| anyhow::anyhow!("Failed to parse resolve-pr-thread: {}", e))?;
-            debug!(
-                "resolve-pr-thread: pr_id={}, thread_id={}, status='{}'",
-                output.pull_request_id, output.thread_id, output.status
-            );
-            output.execute_sanitized(ctx).await?
-        }
-        "noop" => {
-            debug!("Skipping noop entry");
-            ExecutionResult::success("Skipped informational output: noop")
+    // Dispatch based on tool name. All standard tools go through `dispatch_tool` which
+    // handles deserialization and sanitized execution uniformly. Special cases (informational
+    // outputs and report-incomplete) are handled inline.
+    let result = if let Some(dispatched_result) = dispatch_executor_tools!(tool_name, entry, ctx, {
+        "create-work-item" => CreateWorkItemResult,
+        "comment-on-work-item" => CommentOnWorkItemResult,
+        "update-work-item" => UpdateWorkItemResult,
+        "create-pull-request" => CreatePrResult,
+        "update-wiki-page" => UpdateWikiPageResult,
+        "create-wiki-page" => CreateWikiPageResult,
+        "add-pr-comment" => AddPrCommentResult,
+        "link-work-items" => LinkWorkItemsResult,
+        "queue-build" => QueueBuildResult,
+        "create-git-tag" => CreateGitTagResult,
+        "add-build-tag" => AddBuildTagResult,
+        "create-branch" => CreateBranchResult,
+        "update-pr" => UpdatePrResult,
+        "upload-attachment" => UploadAttachmentResult,
+        "submit-pr-review" => SubmitPrReviewResult,
+        "reply-to-pr-review-comment" => ReplyToPrCommentResult,
+        "resolve-pr-thread" => ResolvePrThreadResult,
+    })? {
+        dispatched_result
+    } else {
+        match tool_name {
+        // Informational outputs — no side effects, always succeed
+        "noop" | "missing-tool" | "missing-data" => {
+            debug!("Skipping informational entry: {}", tool_name);
+            ExecutionResult::success(format!("Skipped informational output: {}", tool_name))
         }
+        // report-incomplete does not implement Executor; Stage 3 surfaces its reason as a failure
         "report-incomplete" => {
             let mut output: ReportIncompleteResult = serde_json::from_value(entry.clone())
                 .map_err(|e| anyhow::anyhow!("Failed to parse report-incomplete: {}", e))?;
             output.sanitize_content_fields();
             debug!("report-incomplete: {}", output.reason);
             ExecutionResult::failure(format!("Agent reported task incomplete: {}", output.reason))
         }
-        "missing-tool" => {
-            debug!("Skipping missing-tool entry");
-            ExecutionResult::success("Skipped informational output: missing-tool")
-        }
-        "missing-data" => {
-            debug!("Skipping missing-data entry");
-            ExecutionResult::success("Skipped informational output: missing-data")
-        }
         other => {
             error!("Unknown tool type: {}", other);
             bail!("Unknown tool type: {}. No executor registered.", other)
         }
+    }
     };
 
     Ok((tool_name.to_string(), result))

src/safeoutputs/add_build_tag.rs

@@ -112,6 +112,7 @@ impl Executor for AddBuildTagResult {
 
     async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result<ExecutionResult> {
         info!("Adding tag '{}' to build #{}", self.tag, self.build_id);
+        debug!("add-build-tag: build_id={}, tag='{}'", self.build_id, self.tag);
 
         // 1. Extract required context
         let org_url = ctx

src/safeoutputs/add_pr_comment.rs

@@ -208,6 +208,11 @@ impl Executor for AddPrCommentResult {
             self.pull_request_id,
             self.content.len()
         );
+        debug!(
+            "add-pr-comment: pr_id={}, content length={}",
+            self.pull_request_id,
+            self.content.len()
+        );
 
         let org_url = ctx
             .ado_org_url
@@ -625,4 +630,3 @@ allowed-statuses:
         );
     }
 }
-

src/safeoutputs/comment_on_work_item.rs

@@ -170,14 +170,19 @@ impl Executor for CommentOnWorkItemResult {
         format!("comment on work item #{}", self.work_item_id)
     }
 
-    async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result<ExecutionResult> {
-        info!(
-            "Commenting on work item #{}: {} chars",
-            self.work_item_id,
-            self.body.len()
-        );
-
-        let org_url = ctx
+    async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result<ExecutionResult> {
+        info!(
+            "Commenting on work item #{}: {} chars",
+            self.work_item_id,
+            self.body.len()
+        );
+        debug!(
+            "comment-on-work-item: work_item_id={}, body length={}",
+            self.work_item_id,
+            self.body.len()
+        );
+
+        let org_url = ctx
             .ado_org_url
             .as_ref()
             .context("AZURE_DEVOPS_ORG_URL not set")?;

src/safeoutputs/create_branch.rs

@@ -201,6 +201,7 @@ impl Executor for CreateBranchResult {
 
     async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result<ExecutionResult> {
         info!("Creating branch: '{}'", self.branch_name);
+        debug!("create-branch: branch_name='{}'", self.branch_name);
 
         let org_url = ctx
             .ado_org_url

src/safeoutputs/create_git_tag.rs

@@ -244,6 +244,7 @@ impl Executor for CreateGitTagResult {
 
     async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result<ExecutionResult> {
         info!("Creating git tag: '{}'", self.tag_name);
+        debug!("create-git-tag: tag_name='{}'", self.tag_name);
 
         let org_url = ctx
             .ado_org_url

src/safeoutputs/create_pr.rs

@@ -541,6 +541,10 @@ impl Executor for CreatePrResult {
             "Creating PR: '{}' in repository '{}'",
             self.title, self.repository
         );
+        debug!(
+            "create-pull-request: title='{}', repo='{}', branch='{}', patch='{}'",
+            self.title, self.repository, self.source_branch, self.patch_file
+        );
         debug!("PR description length: {} chars", self.description.len());
         debug!("Source branch: {}", self.source_branch);
         debug!("Patch file: {}", self.patch_file);

src/safeoutputs/create_wiki_page.rs

@@ -192,6 +192,11 @@ impl Executor for CreateWikiPageResult {
 
     async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result<ExecutionResult> {
         info!("Creating wiki page: '{}'", self.path);
+        debug!(
+            "create-wiki-page: path='{}', content length={}",
+            self.path,
+            self.content.len()
+        );
         debug!("Content length: {} chars", self.content.len());
 
         let org_url = ctx
@@ -922,4 +927,3 @@ wiki-name: "MyProject.wiki"
         assert_eq!(encoded, "MyProject.wiki");
     }
 }
-

src/safeoutputs/create_work_item.rs

@@ -243,6 +243,11 @@ impl Executor for CreateWorkItemResult {
 
     async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result<ExecutionResult> {
         info!("Creating work item: '{}'", self.title);
+        debug!(
+            "create-work-item: title='{}', description length={}",
+            self.title,
+            self.description.len()
+        );
         debug!("Description length: {} chars", self.description.len());
 
         let org_url = ctx
@@ -538,4 +543,3 @@ tags:
         assert_eq!(config.tags, vec!["my-tag"]);
     }
 }
-

src/safeoutputs/link_work_items.rs

@@ -145,6 +145,10 @@ impl Executor for LinkWorkItemsResult {
             "Linking work item #{} -> #{} ({})",
             self.source_id, self.target_id, self.link_type
         );
+        debug!(
+            "link-work-items: source={}, target={}, type={}",
+            self.source_id, self.target_id, self.link_type
+        );
 
         let org_url = ctx
             .ado_org_url