Skip to content

refactor: reduce complexity of execute_impl in comment_on_work_item.rs#1521

Merged
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-comment-on-work-item-6b080c884b5604e7
Jul 15, 2026
Merged

refactor: reduce complexity of execute_impl in comment_on_work_item.rs#1521
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-comment-on-work-item-6b080c884b5604e7

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Summary

execute_impl in src/safe_outputs/comment_on_work_item.rs contained a deeply nested 3-arm match block (nesting depth 6, ~50 lines) that mixed three different concerns:

  1. Fast ID-based allow/deny (allows_id)
  2. Area-path prefix extraction (area_path_prefix)
  3. Async ADO API fetch + case-insensitive path-boundary comparison (get_work_item_area_path)

All of this was inline inside execute_impl, making it hard to read and test independently.

Change

Extracted a new validate_target_policy async helper:

async fn validate_target_policy(
    target: &CommentTarget,
    client: &reqwest::Client,
    org_url: &str,
    project: &str,
    token: &str,
    work_item_id: i64,
) -> anyhow::Result<Option<String>>
  • Returns Ok(None) → work item is allowed, proceed.
  • Returns Ok(Some(msg)) → policy rejects this work item; caller returns ExecutionResult::failure(msg).
  • Returns Err(...) → unexpected infrastructure failure (bubbled up with ?).

execute_impl is reduced to a single guard:

if let Some(rejection_msg) =
    validate_target_policy(target, &client, org_url, project, token, self.work_item_id).await?
{
    return Ok(ExecutionResult::failure(rejection_msg));
}

Before / After

Metric Before After
Max nesting depth 6 3
execute_impl lines ~160 ~110
validate_target_policy lines ~55 (new, focused)

Verification

  • All tests pass (cargo test)
  • Clippy clean (cargo clippy --all-targets --all-features)
  • No behaviour changes — same logic, same error messages

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • spsprodeus21.vssps.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "spsprodeus21.vssps.visualstudio.com"

See Network Configuration for more information.

Generated by Cyclomatic Complexity Reducer · 53.3 AIC · ⌖ 12.3 AIC · ⊞ 7.2K ·

Extract validate_target_policy helper to flatten the 3-arm match/nested-match
block that combined allows_id, area_path_prefix, and get_work_item_area_path
into a single nesting depth 6 block. The new helper returns Ok(None) for
"allowed", Ok(Some(msg)) for "rejected", keeping execute_impl to a single
early-return guard.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesadevine jamesadevine force-pushed the refactor/reduce-complexity-comment-on-work-item-6b080c884b5604e7 branch from 7a20ed0 to ccad08a Compare July 15, 2026 10:03
@jamesadevine jamesadevine marked this pull request as ready for review July 15, 2026 10:03
@jamesadevine jamesadevine merged commit 4f3b748 into main Jul 15, 2026
9 checks passed
@jamesadevine jamesadevine deleted the refactor/reduce-complexity-comment-on-work-item-6b080c884b5604e7 branch July 15, 2026 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant