Skip to content
Merged
37 changes: 35 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,39 @@ Snyk (security scan):
- `snyk test` command scans your project, tests dependencies for vulnerabilities, and reports how many vulnerabilities are found.
- `snyk code test` analyzes source code for security issues, often referred to as Static Application Security Testing (SAST).

### GitHub Sub-Issues REST API Reference
**Documentation**: https://docs.github.com/en/rest/issues/sub-issues?apiVersion=2022-11-28

**CRITICAL**: Use **issue ID** (not issue number) in API requests!

**Working Commands**:
```bash
# List sub-issues
curl -L -H "Accept: application/vnd.github+json" \
-H "Authorization: Bearer $(gh auth token)" \
-H "X-GitHub-Api-Version: 2022-11-28" \
https://api.github.com/repos/{owner}/{repo}/issues/{issue_number}/sub_issues

# Add sub-issue (use issue ID, not number!)
curl -L -X POST -H "Accept: application/vnd.github+json" \
-H "Authorization: Bearer $(gh auth token)" \
-H "X-GitHub-Api-Version: 2022-11-28" \
https://api.github.com/repos/{owner}/{repo}/issues/{issue_number}/sub_issues \
-d '{"sub_issue_id": {ISSUE_ID}}'

# Remove sub-issue
curl -L -X DELETE -H "Accept: application/vnd.github+json" \
-H "Authorization: Bearer $(gh auth token)" \
-H "X-GitHub-Api-Version: 2022-11-28" \
https://api.github.com/repos/{owner}/{repo}/issues/{issue_number}/sub_issue \
-d '{"sub_issue_id": {ISSUE_ID}}'

# Get issue ID from issue number
gh api repos/{owner}/{repo}/issues/{number} --jq '.id'
```

**Tested & Verified**: 2025-06-01 - All endpoints work correctly


## Development Commands

Expand Down Expand Up @@ -141,7 +174,7 @@ For detailed technical architecture, design patterns, and implementation guideli

**Quick Reference:**
- **Core Interfaces**: `PackageManager` and `SysPkg` (interface.go)
- **CommandBuilder Pattern**: Target architecture for Issue #20
- **CommandRunner Pattern**: Unified architecture for all package managers (Issue #20)
- **Package Structure**: `/cmd`, `/manager`, `/osinfo`, `/testing`
- **Testing Strategy**: Three-layer approach (unit, integration, mock)
- **Exit Code Complexity**: Each PM has unique behaviors (see docs/EXIT_CODES.md)
Expand Down Expand Up @@ -180,7 +213,7 @@ For detailed technical architecture, design patterns, and implementation guideli
7. **GitHub workflow compatibility fixes** βœ… - Go 1.23.4, Docker multi-OS testing
8. **Fix APT exit code bug** - Remove incorrect handling of exit code 100 as "no packages found" (it means error!)
9. **Fix Snap exit code bug** - Remove incorrect handling of exit code 64 as "no packages found" (it means usage error!)
10. **Implement CommandBuilder interface (Issue #20)** - Replace direct exec.Command calls with testable CommandBuilder pattern
10. **Migrate to CommandRunner interface (Issue #20)** - Achieve architectural consistency across all package managers
11. **Add exit code documentation** βœ… - Created comprehensive exit code docs for all package managers

### βœ… COMPLETED INVESTIGATIONS (Collapsed)
Expand Down
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,8 @@ SysPkg follows modern testing best practices with a three-layer approach that al
```go
func TestParseFindOutput_BehaviorWithFixtures(t *testing.T) {
fixture := loadFixture(t, "search-vim-rocky8.txt")
packages := yum.ParseFindOutput(fixture, &manager.Options{})
pm := yum.NewPackageManager()
packages := pm.ParseFindOutput(fixture, &manager.Options{})
// Test parser behavior with real output
}
```
Expand Down
64 changes: 50 additions & 14 deletions docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,27 +38,63 @@ type SysPkg interface {

## Command Execution Architecture

### Current State (Issue #20)
Mixed patterns across package managers:
- **YUM**: Uses CommandRunner for some operations (Find), direct exec.Command for others
- **APT/Snap/Flatpak**: All use direct exec.Command calls
### CommandRunner Pattern (Issue #20) βœ… IMPLEMENTED

### Target Architecture: CommandBuilder Pattern (Option C)
All package managers now use the unified CommandRunner interface for consistent, testable command execution:

**Current State**: APT βœ… Complete, YUM βœ… Complete, Snap ❌ No DI, Flatpak ❌ No DI

#### executeCommand Pattern βœ… COMPLETED (2025-06-02)

Both APT and YUM now implement centralized command execution through the `executeCommand()` helper method:

```go
type CommandBuilder interface {
CommandContext(ctx context.Context, name string, args ...string) *exec.Cmd
// Centralized command execution for both interactive and non-interactive modes
func (a *PackageManager) executeCommand(ctx context.Context, args []string, opts *manager.Options) ([]byte, error) {
if opts != nil && opts.Interactive {
// Interactive mode uses RunInteractive for stdin/stdout/stderr handling
err := a.getRunner().RunInteractive(ctx, pm, args, aptNonInteractiveEnv...)
return nil, err
}
// Use RunContext for non-interactive execution (automatically includes LC_ALL=C)
return a.getRunner().RunContext(ctx, pm, args, aptNonInteractiveEnv...)
}
```

**Why CommandBuilder (not Extended CommandRunner)**:
- **Exit code complexity**: Each PM has unique exit code behaviors (APT 100=error, YUM 100=success)
- **Maximum flexibility**: Full access to exec.Cmd features (env, stdin/stdout, working dir)
- **Simple interface**: Only 1 method vs multiple in extended interface
- **Go idiomatic**: Returns standard `*exec.Cmd` type
- **No generic helpers needed**: Each PM handles its own exit codes
**Benefits**:
- **DRY Principle**: Eliminated repeated interactive/non-interactive logic
- **Maintainability**: Command execution changes in one place
- **Consistency**: APT and YUM follow identical patterns
- **Code Reduction**: APT reduced from 17 to 7 direct `getRunner()` calls

```go
type CommandRunner interface {
// Run executes a command with automatic 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
}
```

**Critical Discovery**: Package managers have wildly inconsistent exit codes:
**Why CommandRunner Pattern**:
- **Automatic LC_ALL=C**: Consistent English output across all package managers
- **Built-in interactive support**: Dedicated `RunInteractive()` method
- **Simplified testing**: Map-based mocking vs complex shell script generation
- **DRY principle**: Eliminates repetitive environment variable setup
- **Proven success**: YUM migration demonstrated robustness and maintainability

**Benefits Achieved**:
- βœ… **Consistent architecture** across APT and YUM package managers
- βœ… **Better encapsulation** - utility functions converted to methods
- βœ… **Simplified signatures** - eliminated parameter explosion through function chains
- βœ… **Easy mocking** for comprehensive test coverage
- βœ… **Constructor standardization** - clear production vs testing patterns

**Exit Code Handling**: Each package manager still handles its own exit codes appropriately:
- APT: Exit code 100 = any error
- YUM: Exit code 100 = updates available (success!)
- Snap: Exit code 64 = usage error (not "no packages found")
Expand Down
Loading