Implement CommandRunner pattern for unified testable package manager operations

[bluet]

Problem Statement

Currently, package managers use inconsistent command execution patterns, creating architectural and testing issues:

Current State Analysis

• YUM: ✅ Uses CommandRunner (recently migrated, fully tested)
• APT: Uses CommandBuilder ✅ MIGRATED (Issue APT CommandRunner Migration (Issue #20 Part 1) #27 - PR feat: implement CommandRunner architecture for APT and YUM package managers (Issue #20) #26)
• Snap: Uses direct exec.Command calls (Issue Snap CommandRunner Migration (Issue #20 Part 2) #28)
• Flatpak: Uses direct exec.Command calls (Issue Flatpak CommandRunner Migration (Issue #20 Part 3) #29)

Issues with Mixed Approaches

1. Architectural inconsistency - Different patterns across package managers
2. Complex testing - CommandBuilder requires shell script mocking vs simple map-based mocking
3. Manual LC_ALL=C setup - Repetitive environment configuration in APT/Snap/Flatpak
4. Inconsistent interactive mode - Manual stdin/stdout/stderr handling
5. New developer complexity - CommandBuilder requires deep exec.Cmd knowledge

Decision: Standardize on CommandRunner

Based on comprehensive analysis, CommandRunner is superior to CommandBuilder for this project:

Why CommandRunner Wins

1. Automatic LC_ALL=C Handling

CommandRunner automatically prepends LC_ALL=C for consistent English output across all package managers, with user override capability:

// CommandRunner: Automatic
output, err := runner.RunContext(ctx, "yum", []string{"info", "vim"})

// CommandBuilder: Manual setup required everywhere
cmd := builder.CommandContext(ctx, "yum", "info", "vim")
cmd.Env = append(os.Environ(), "LC_ALL=C", "DEBIAN_FRONTEND=noninteractive")

2. Simplified Testing

// CommandRunner: Simple map-based mocking
mock.AddCommand("yum", []string{"info", "vim"}, []byte("output"), nil)

// CommandBuilder: Complex shell script generation
mock.AddMockResult("yum", []string{"info", "vim"}, &MockResult{
    Stdout: []byte("output"), ExitCode: 0,
})

3. Built-in Interactive Support

CommandRunner has dedicated RunInteractive() method that properly handles stdin/stdout/stderr without LC_ALL=C interference.

4. Consistency with Project Goals

From CLAUDE.md: "Use KISS (Keep It Simple and Stupid) and DRY (Don't Repeat Yourself)"

CommandRunner eliminates repetitive LC_ALL=C setup and interactive mode handling across all package managers.

5. Proven Success

YUM's recent migration to CommandRunner shows:

• 100% test coverage maintained
• Cleaner, more readable code
• Robust environment variable handling
• Successful interactive mode support

Solution: Migrate All Package Managers to CommandRunner

CommandRunner Interface

type CommandRunner interface {
    // Run executes a command with LC_ALL=C for consistent English output
    Run(name string, args ...string) ([]byte, error)
    
    // RunContext executes with context support and LC_ALL=C, plus optional extra env
    RunContext(ctx context.Context, name string, args []string, env ...string) ([]byte, error)
    
    // RunInteractive executes in interactive mode with stdin/stdout/stderr passthrough
    RunInteractive(ctx context.Context, name string, args []string, env ...string) error
}

Implementation Plan - UPDATED

Migration Progress:

1. APT ✅ COMPLETED (Issue APT CommandRunner Migration (Issue #20 Part 1) #27 - PR feat: implement CommandRunner architecture for APT and YUM package managers (Issue #20) #26) - Replace CommandBuilder with CommandRunner
2. Snap 🔄 IN PROGRESS (Issue Snap CommandRunner Migration (Issue #20 Part 2) #28) - Replace direct exec.Command with CommandRunner
3. Flatpak ⏳ PLANNED (Issue Flatpak CommandRunner Migration (Issue #20 Part 3) #29) - Replace direct exec.Command with CommandRunner

Benefits

Testing Benefits

• ✅ Simple mocking - Map-based command mocking vs complex shell scripts
• ✅ Environment testing - Built-in environment variable tracking
• ✅ Interactive testing - Dedicated test methods for interactive mode
• ✅ 100% test coverage - All commands can be easily mocked

Architecture Benefits

• ✅ Consistent interface - Same pattern across all package managers
• ✅ Automatic LC_ALL=C - No manual environment setup needed
• ✅ Built-in interactive support - Proper stdin/stdout/stderr handling
• ✅ Simple for new developers - Easy to understand and implement

Code Quality Benefits

• ✅ DRY principle - Eliminates repetitive environment setup
• ✅ KISS principle - Simple interface vs complex CommandBuilder
• ✅ Maintainability - Consistent patterns across codebase

Acceptance Criteria - UPDATED

• APT migrated from CommandBuilder to CommandRunner ✅ (Issue APT CommandRunner Migration (Issue #20 Part 1) #27 - PR feat: implement CommandRunner architecture for APT and YUM package managers (Issue #20) #26)
• Snap migrated from direct exec.Command to CommandRunner (Issue Snap CommandRunner Migration (Issue #20 Part 2) #28)
• Flatpak migrated from direct exec.Command to CommandRunner (Issue Flatpak CommandRunner Migration (Issue #20 Part 3) #29)
• LC_ALL=C automatically handled across all package managers ✅ (APT+YUM complete)
• Interactive mode works consistently across all package managers ✅ (APT+YUM complete)
• Comprehensive test coverage maintained for all migrations ✅ (APT complete)
• All existing functionality preserved ✅ (APT verified)
• Documentation updated to reflect architectural consistency ✅ (Updated for APT)

Completion Status: ✅ 1/3 package managers completed (APT ✅, Snap ⏳, Flatpak ⏳)

Priority

High Priority - This achieves architectural consistency and leverages the proven CommandRunner interface that's already successful with YUM.

Related Work

• ✅ CommandRunner interface implemented and proven with YUM
• ✅ YUM migration completed successfully (100% test coverage)
• ✅ APT migration completed successfully (Issue APT CommandRunner Migration (Issue #20 Part 1) #27 - PR feat: implement CommandRunner architecture for APT and YUM package managers (Issue #20) #26)
• ✅ MockCommandRunner supports environment variable tracking
• ✅ Interactive mode support built-in and tested

Sub-Issues

This large architectural change has been broken down into manageable sub-issues:

• Issue APT CommandRunner Migration (Issue #20 Part 1) #27: ✅ APT CommandRunner Migration (COMPLETED by PR feat: implement CommandRunner architecture for APT and YUM package managers (Issue #20) #26)
• Issue Snap CommandRunner Migration (Issue #20 Part 2) #28: 🔄 Snap CommandRunner Migration (In Progress)
• Issue Flatpak CommandRunner Migration (Issue #20 Part 3) #29: ⏳ Flatpak CommandRunner Migration (Planned)

Progress: 1/3 package managers completed. APT migration successful with full test coverage and architectural improvements.

[bluet]

🔄 Architecture Decision Update

CLARIFICATION: CommandBuilder Pattern is the chosen solution

The issue description correctly identifies CommandBuilder as Option C and the preferred solution. However, some references in documentation mentioned using "YUM's CommandRunner as reference" which was inconsistent.

✅ Confirmed Architecture Decision:

Interface:

type CommandBuilder interface {
    CommandContext(ctx context.Context, name string, args ...string) *exec.Cmd
}

🎯 Implementation Plan Clarified:

1. Create CommandBuilder interface (single method, maximum flexibility)
2. YUM Migration: Replace existing CommandRunner → CommandBuilder
3. APT/Snap/Flatpak: Replace direct exec.Command → CommandBuilder
4. Unified Architecture: All package managers use the same pattern

📋 Why CommandBuilder (Not CommandRunner):

• ✅ Exit Code Flexibility: Each PM handles unique exit codes (APT 100=error, YUM 100=success!)
• ✅ Simple Interface: Only 1 method vs multiple
• ✅ Standard Go Patterns: Returns *exec.Cmd
• ✅ Future Proof: Handles any exec.Cmd feature

This ensures consistent, testable architecture across all package managers while respecting each PM's unique behaviors.

[bluet]

📝 Issue Title Updated

Old Title: "Refactor all package manager operations to use testable CommandRunner interface"
New Title: "Implement CommandBuilder pattern for testable cross-platform package manager operations"

🎯 Key Cross-Platform Testing Benefit

The primary motivation for CommandBuilder is enabling cross-platform testing:

🌍 Cross-Platform Testing Goal:

• ✅ Test YUM operations on Ubuntu (using fixtures + mocked CommandBuilder)
• ✅ Test APT operations on Rocky Linux (using fixtures + mocked CommandBuilder)
• ✅ Test any PM on any OS (that's what fixtures are for!)

🚧 Current Limitation:

• ❌ Can only test YUM on RHEL-based systems
• ❌ Can only test APT on Debian-based systems
• ❌ Direct exec.Command calls depend on actual system state

✅ CommandBuilder Solution:

{ 是 shell 關鍵字

This architectural change is essential for comprehensive testing across all development environments.

[bluet]

📝 Issue Title Updated

Old Title: "Refactor all package manager operations to use testable CommandRunner interface"
New Title: "Implement CommandBuilder pattern for testable cross-platform package manager operations"

🎯 Key Cross-Platform Testing Benefit

The primary motivation for CommandBuilder is enabling cross-platform testing:

🌍 Cross-Platform Testing Goal:

• ✅ Test YUM operations on Ubuntu (using fixtures + mocked CommandBuilder)
• ✅ Test APT operations on Rocky Linux (using fixtures + mocked CommandBuilder)
• ✅ Test any PM on any OS (that's what fixtures are for!)

🚧 Current Limitation:

• ❌ Can only test YUM on RHEL-based systems
• ❌ Can only test APT on Debian-based systems
• ❌ Direct exec.Command calls depend on actual system state

✅ CommandBuilder Solution:

With CommandBuilder, we can mock command execution and use authentic fixtures to test any package manager on any operating system. This is essential for comprehensive testing across all development environments.