Skip to content

Conversation

@danielfbm
Copy link
Contributor

@danielfbm danielfbm commented Nov 12, 2025

🎯 Overview

This PR introduces a comprehensive refactoring of the PR-CLI command execution architecture by extracting and unifying command execution logic into a dedicated executor package. This change improves code maintainability, testability, and provides a consistent execution flow across CLI and webhook modes.

📋 Changes

New Architecture

  • New pkg/executor package: Centralized command execution logic with clear separation of concerns
  • Unified execution flow: Both CLI and webhook modes now use the same underlying executor
  • Configuration-driven behavior: Execution behavior is now controlled via ExecutionConfig instead of scattered conditionals

Key Components

1. CommandExecutor (executor.go)

  • Main orchestrator for command execution
  • Handles three command types: SingleCommand, BuiltInCommand, MultiCommand
  • Coordinates validation, execution, error handling, and metrics recording
  • 194 lines with comprehensive logic

2. Validator (validator.go)

  • Centralized validation logic for command execution
  • Validates PR status and comment sender based on configuration
  • Different validation rules for single vs multi-commands
  • 160 lines with smart validation rules

3. ResultHandler (result_handler.go)

  • Handles execution results and error formatting
  • Posts error comments to PRs when configured
  • Generates multi-command execution summaries
  • Detects and handles CommentedError to avoid duplicate comments

4. ExecutionConfig (config.go)

  • Configuration structure controlling execution behavior
  • Two preset configurations:
    • NewCLIExecutionConfig: Posts errors as PR comments, validates comment sender
    • NewWebhookExecutionConfig: Logs errors only, trusts webhook validation
  • 74 lines defining execution modes

5. ExecutionResult (result.go)

  • Structured result objects for command execution
  • Different result types for single, built-in, and multi-commands
  • 67 lines of result structures

6. MetricsRecorder (metrics.go)

  • Interface for recording execution metrics
  • Decouples metric recording from execution logic
  • Webhook implementation in pkg/webhook/metrics_recorder.go

Refactored Components

webhook/worker.go (108 → 50 lines, 54% reduction)

  • Removed duplicate command execution logic (~90 lines)
  • Now uses unified CommandExecutor
  • Simplified from 3 methods to 1 method (executeWithUnifiedExecutor)

cmd/options.go (minor updates)

  • Updated to use new executor package
  • Passes execution config to executor

Test Coverage

  • 1,072 lines of tests across 6 test files
  • Comprehensive unit tests for all new components:
    • executor_test.go (278 lines)
    • validator_test.go (175 lines)
    • result_test.go (174 lines)
    • result_handler_test.go (136 lines)
    • metrics_test.go (49 lines)

🎁 Benefits

1. Code Reusability

  • Eliminates ~100 lines of duplicated execution logic between CLI and webhook modes
  • Single source of truth for command execution behavior

2. Improved Testability

  • Each component has focused responsibilities and can be tested independently
  • Comprehensive test coverage with 1,072 lines of unit tests
  • Mock-friendly interfaces (MetricsRecorder)

3. Better Maintainability

  • Clear separation of concerns: validation, execution, result handling
  • Configuration-driven behavior reduces conditional complexity
  • Easier to add new command types or execution modes

4. Consistent Behavior

  • CLI and webhook modes now behave consistently
  • Unified error handling and metrics recording
  • Same validation logic applied across all entry points

5. Enhanced Debugging

  • Structured logging throughout execution flow
  • Clear execution context passed through all components
  • Metrics recorded at key execution points

📊 Statistics

  • New files: 14 (7 implementation + 7 tests)
  • Modified files: 5
  • Total additions: ~1,919 lines
  • Total deletions: ~114 lines
  • Net change: +1,805 lines (mostly new tests and extracted logic)
  • Code reduction in worker.go: 54% (108 → 50 lines)

🔍 Technical Details

Execution Flow

  1. Parse command (existing parser)
  2. Create execution context with config
  3. Validate command based on type and config
  4. Execute command via PRHandler
  5. Handle results (post comments, record metrics)
  6. Return structured result

Configuration Modes

  • CLI Mode: Validates sender, posts errors as comments, interactive behavior
  • Webhook Mode: Trusts webhook validation, logs errors, returns error status

Error Handling

  • Detects CommentedError to avoid duplicate error comments
  • Configuration controls whether errors are posted to PR or returned
  • Multi-command execution can stop on first error or continue

✅ Testing

All existing tests pass. New comprehensive test coverage includes:

  • Command execution scenarios (single, built-in, multi)
  • Validation logic (comment sender, PR status)
  • Error handling (commented errors, posting failures)
  • Result formatting and handling
  • Metrics recording

🔄 Backward Compatibility

This refactoring maintains full backward compatibility:

  • No changes to public APIs
  • Same command behavior from user perspective
  • No configuration changes required
  • Existing metrics remain unchanged

📚 Related Documentation

  • CLAUDE.md updated with repository overview and development guidance

@alaudabot
Copy link

🚨 Stale Pull Request Warning

This pull request has been inactive for 32 days.

Automated Actions Schedule:

  • ⚠️ Warning: After 30 days (now)
  • 🔒 Auto-close: After 60 days
  • 🗑️ Branch deletion: After 90 days (if not protected)

To keep this PR active:

  • Add new commits
  • Reply to this comment
  • Request reviews

Protected branches (won't be deleted): main,release-*,alauda-*

This is an automated message. Reply to this comment to reset the inactivity timer.

@danielfbm danielfbm closed this Jan 6, 2026
@danielfbm danielfbm deleted the refactor/pr-cli-command-executor branch January 6, 2026 10:38
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.

3 participants