[fp-enhancer] Improve pkg/cli - Apply functional/immutability patterns

[github-actions]

Functional/Immutability Enhancements - Package: pkg/cli

This PR applies moderate, tasteful functional/immutability techniques to the pkg/cli package to improve code clarity, safety, testability, and maintainability.

Round-Robin Progress: This is part of a systematic package-by-package refactoring. Next package to process: pkg/cli/fileutil

---

Summary of Changes

This PR focuses on two key improvements:

1. Transformative Data Operations - Replace imperative loops with declarative sliceutil.Map calls
2. Immutable Initialization - Remove unnecessary empty slice literals in struct initialization

Files Changed: 4 files
Lines Changed: +10 / -28 (net -18 lines)
Test Coverage: ✅ All tests pass (5.156s)
Build Status: ✅ Clean compilation

---

Detailed Changes

1. Transformative Data Operations

Replaced imperative map extraction loops with declarative sliceutil.Map calls for clearer, more functional data transformations.

Files affected:

• pkg/cli/add_workflow_pr.go - Extract workflow names (lines 119-122)
• pkg/cli/audit_report_analysis.go - Extract MCP server names (lines 93-96)

Before (imperative):

workflowNames := make([]string, len(workflows))
for i, wf := range workflows {
    workflowNames[i] = wf.Spec.WorkflowName
}

After (functional):

workflowNames := sliceutil.Map(workflows, func(wf *ResolvedWorkflow) string {
    return wf.Spec.WorkflowName
})

Benefits:

• ✅ More declarative - expresses "what" not "how"
• ✅ Eliminates index variable and mutation
• ✅ Leverages existing pkg/sliceutil utilities
• ✅ Clearer intent - transformation is explicit

2. Immutable Initialization

Removed unnecessary empty slice initialization in struct literals. Go's zero values for slices are already nil, which works correctly with JSON omitempty tags.

Files affected:

• pkg/cli/access_log.go - Simplified DomainAnalysis initialization (2 locations)
• pkg/cli/firewall_log.go - Simplified FirewallAnalysis initialization (2 locations)

Before (verbose):

analysis := &DomainAnalysis{
    DomainBuckets: DomainBuckets{
        AllowedDomains: []string{},  // Unnecessary
        BlockedDomains: []string{},  // Unnecessary
    },
}

After (concise):

analysis := &DomainAnalysis{}

Why this works:

• DomainBuckets has json:"allowed_domains,omitempty" tags
• Go zero values for slices (nil) work correctly with omitempty
• Slices are later populated via SetAllowedDomains()/SetBlockedDomains()
• No behavior change - JSON serialization is identical

Benefits:

• ✅ Less verbose - removes 8 lines of boilerplate
• ✅ Clearer intent - only specify non-default values
• ✅ Leverages Go zero values idiomatically
• ✅ Easier to maintain - fewer lines to update

---

Testing

• ✅ All tests pass - go test ./pkg/cli/ (5.156s)
• ✅ Test coverage verified - Multiple test files exist:
  • pkg/cli/add_workflow_pr_test.go
  • pkg/cli/audit_report_helpers_test.go
  • pkg/cli/audit_test.go
  • pkg/cli/domain_buckets_test.go
• ✅ Specific test runs:
  • TestSanitizeBranchName - PASS
  • TestAudit* - PASS (all audit tests)
  • TestBuildAccessLogSummary* - PASS
  • TestBuildFirewallLogSummary* - PASS
• ✅ Code formatted - make fmt
• ✅ Clean compilation - go build ./pkg/cli/...
• ✅ No behavioral changes - Pure refactoring

---

Principles Applied

1. Declarative Over Imperative - sliceutil.Map expresses transformation intent clearly
2. Immutability First - Eliminate unnecessary slice mutations during initialization
3. Leverage Existing Utilities - Use pkg/sliceutil instead of reinventing patterns
4. Zero Value Semantics - Rely on Go's zero values where appropriate
5. Pragmatic Balance - Changes improve clarity without dogmatic adherence

---

Review Focus

Please verify:

• ✅ Functional transformations maintain behavior
• ✅ Empty slice removal doesn't affect JSON serialization
• ✅ No unintended side effects or behavior changes
• ✅ Changes improve code clarity

---

Examples

Example 1: Workflow Name Extraction

Before:

workflowNames := make([]string, len(workflows))
for i, wf := range workflows {
    workflowNames[i] = wf.Spec.WorkflowName
}
joinedNames = strings.Join(workflowNames, ", ")

After:

workflowNames := sliceutil.Map(workflows, func(wf *ResolvedWorkflow) string {
    return wf.Spec.WorkflowName
})
joinedNames = strings.Join(workflowNames, ", ")

Impact: 4 lines → 3 lines, clearer transformation intent

---

Example 2: MCP Server Name Extraction

Before:

serverNames := make([]string, len(processedRun.MCPFailures))
for i, failure := range processedRun.MCPFailures {
    serverNames[i] = failure.ServerName
}
findings = append(findings, Finding{
    Description: fmt.Sprintf("Failed MCP servers: %s", strings.Join(serverNames, ", ")),
    // ...
})

After:

serverNames := sliceutil.Map(processedRun.MCPFailures, func(failure MCPFailureReport) string {
    return failure.ServerName
})
findings = append(findings, Finding{
    Description: fmt.Sprintf("Failed MCP servers: %s", strings.Join(serverNames, ", ")),
    // ...
})

Impact: 4 lines → 3 lines, functional transformation

---

Example 3: Struct Initialization

Before:

analysis := &DomainAnalysis{
    DomainBuckets: DomainBuckets{
        AllowedDomains: []string{},
        BlockedDomains: []string{},
    },
}

After:

analysis := &DomainAnalysis{}

Impact: 6 lines → 1 line, leverages zero values

---

Benefits Summary

Metric	Impact
Lines of Code	-18 lines (net reduction)
Cyclomatic Complexity	Reduced (fewer loops)
Readability	Improved (declarative style)
Maintainability	Improved (less boilerplate)
Type Safety	Maintained (generic sliceutil.Map)
Test Coverage	Maintained (all tests pass)

---

Next Steps

Cache Updated: pkg/cli marked as processed
Next Package: pkg/cli/fileutil will be analyzed in the next workflow run

This systematic approach ensures:

• ✅ One package at a time for focused, high-quality improvements
• ✅ Comprehensive testing before moving forward
• ✅ Gradual, incremental refactoring without overwhelming changes
• ✅ Full coverage of all packages over multiple runs

---

Automated by Functional Pragmatist - applying moderate functional/immutability techniques to pkg/cli

AI generated by Functional Pragmatist

• expires on Feb 18, 2026, 9:28 AM UTC

[pelikhan]

@copilot format, lint go

[Copilot]

@pelikhan I've opened a new pull request, #16339, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

This PR applies functional programming and immutability patterns to the pkg/cli package, focusing on two key improvements: replacing imperative loops with declarative transformations using sliceutil.Map, and removing unnecessary empty slice initialization in favor of Go's zero values.

Changes:

• Replace imperative for-loops with declarative sliceutil.Map calls for extracting fields from slices (2 locations)
• Simplify struct initialization by removing unnecessary empty slice literals (4 locations)

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
pkg/cli/add_workflow_pr.go	Replace imperative loop with sliceutil.Map for extracting workflow names
pkg/cli/audit_report_analysis.go	Replace imperative loop with sliceutil.Map for extracting MCP server names
pkg/cli/firewall_log.go	Remove unnecessary empty slice initialization in FirewallAnalysis structs (2 locations)
pkg/cli/access_log.go	Remove unnecessary empty slice initialization in DomainAnalysis structs (2 locations)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.