Skip to content

refactor: reduce complexity of execute_impl in create_pull_request.rs#1609

Merged
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-create-pr-execute-impl-698d5d15357d37f5
Jul 22, 2026
Merged

refactor: reduce complexity of execute_impl in create_pull_request.rs#1609
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-create-pr-execute-impl-698d5d15357d37f5

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

What was complex

execute_impl in src/safe_outputs/create_pull_request.rs had grown to 883 lines — a massive single method handling every phase of PR creation inline:

  • Patch validation (file existence, size, SHA-256 check, path security, protected files)
  • No-changes handling with IfNoChanges dispatch
  • Agent label validation against allowlist + merge with operator labels
  • ADO branch push with TOCTOU collision retry (using duplicated serde_json::json! block)
  • Branch renaming logic inlined in two separate places

The function was readable locally but too long to hold in working memory, and the duplicated branch-suffix logic was a DRY violation.

What was changed

Six focused helpers extracted from execute_impl:

Helper Lines extracted Responsibility
handle_no_changes ~54 Builds symlink suffix, dispatches on IfNoChanges variant
validate_and_build_labels ~44 Validates agent labels against allowlist, merges with operator labels
push_new_branch ~70 ADO REST push + single TOCTOU-collision retry
build_push_payload ~15 Pure: builds the ADO push JSON body
random_branch_suffix ~5 Encapsulates rand::rng().random()
rename_branch_suffix ~7 Replaces duplicated rfind('-') / format! pattern

PushBranchParams struct introduced to satisfy Clippy's too_many_arguments lint on push_new_branch.

Before / After

Lines
Before 883
After (execute_impl only) 718
Total extracted 165 (into 6 helpers)

Verification

  • cargo build — clean
  • cargo test — all tests pass
  • cargo clippy --all-targets --all-features — 1 pre-existing warning in common.rs (unrelated), no new warnings

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 · 174 AIC · ⌖ 11.4 AIC · ⊞ 7.2K ·

Extract six focused helpers from the 883-line execute_impl method:
- handle_no_changes: no-changes branch with symlink suffix logic
- validate_and_build_labels: agent label validation + merge with operator labels
- push_new_branch: ADO push + TOCTOU branch-collision retry
- build_push_payload: pure function building the ADO push JSON body
- random_branch_suffix: encapsulates rand usage
- rename_branch_suffix: replaces inline rfind/format pattern used in two places

execute_impl shrinks from 883 lines to 718 lines. Each extracted function has
a single clear responsibility and is individually testable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesadevine
jamesadevine marked this pull request as ready for review July 22, 2026 20:54
@jamesadevine
jamesadevine merged commit e221af7 into main Jul 22, 2026
6 of 8 checks passed
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