[code-simplifier] refactor: extract awfVersionAtLeast helper to deduplicate version-check functions

[github-actions]

Code Simplification - 2026-05-12

This PR simplifies recently introduced code to improve clarity and maintainability while preserving all functionality.

Files Simplified

• pkg/workflow/awf_helpers.go — extracted shared awfVersionAtLeast helper, replaced four near-identical version-check functions with one-liner wrappers

Improvements Made

Eliminated ~60 lines of duplicate logic

The four awfSupports* functions (ExcludeEnv, CliProxy, AllowHostPorts, DockerHostPathPrefix) were structurally identical:

// Before (×4 with different constants)
func awfSupportsX(firewallConfig *FirewallConfig) bool {
    var versionStr string
    if firewallConfig != nil && firewallConfig.Version != "" {
        versionStr = firewallConfig.Version
    } else {
        versionStr = string(constants.DefaultFirewallVersion)
    }
    if strings.EqualFold(versionStr, "latest") {
        return true
    }
    return semverutil.Compare(versionStr, string(constants.AWFXMinVersion)) >= 0
}

Replaced with a shared helper:

// After: one shared helper + four one-liners
func awfVersionAtLeast(firewallConfig *FirewallConfig, minVersion constants.Version) bool { ... }

func awfSupportsCliProxy(firewallConfig *FirewallConfig) bool {
    return awfVersionAtLeast(firewallConfig, constants.AWFCliProxyMinVersion)
}
// ... etc.

Changes Based On

Recent changes from:

• Auto-detect ARC/DinD and emit AWF --docker-host-path-prefix in generated workflows #31614 — Auto-detect ARC/DinD and emit AWF --docker-host-path-prefix in generated workflows (added awfSupportsDockerHostPathPrefix, making the 4th near-identical function)

Testing

• ✅ TestAWFSupportsExcludeEnv — all 8 cases pass
• ✅ TestAWFSupportsCliProxy — all cases pass
• ✅ TestAWFSupportsAllowHostPorts — all 7 cases pass
• ✅ TestAWFSupportsDockerHostPathPrefix — all 5 cases pass
• ✅ Build succeeds (make build)
• ✅ No functional changes — behavior is identical

Review Focus

Please verify:

• awfVersionAtLeast correctly captures the shared behavior of all four original functions
• The awfSupportsExcludeEnv simplification: the old code returned true immediately for nil/empty config (instead of using DefaultFirewallVersion). This is semantically equivalent since DefaultFirewallVersion >= AWFExcludeEnvMinVersion, as confirmed by all existing tests passing.

---

Automated by Code Simplifier Agent - analyzing code from the last 24 hours

Generated by Code Simplifier · ● 21.3M · ◷

• expires on May 13, 2026, 4:38 AM UTC

[Copilot]

Pull request overview

This PR refactors AWF version feature-gating by extracting a shared awfVersionAtLeast helper and converting multiple near-identical awfSupports* functions into one-line wrappers.

Changes:

• Added awfVersionAtLeast helper to centralize AWF version comparison logic (incl. default version + "latest" handling).
• Simplified awfSupportsExcludeEnv, awfSupportsCliProxy, awfSupportsAllowHostPorts, and awfSupportsDockerHostPathPrefix to delegate to the helper.
• Minor formatting cleanup in NewCompiler struct initialization.

Show a summary per file

File	Description
pkg/workflow/awf_helpers.go	Extracts shared version-check helper and refactors AWF feature support checks to use it
pkg/workflow/compiler_types.go	Formatting-only adjustment in compiler initialization

Copilot's findings

Tip

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

• Files reviewed: 1/2 changed files
• Comments generated: 1

[github-actions]

Skills-Based Review 🧠

Applied /zoom-out and /improve-codebase-architecture — appropriate for this refactor that consolidates four near-identical version-check functions into a shared helper.

Key Themes

• Solid deduplication — awfVersionAtLeast is a genuinely deep module: a small interface hiding the fallback-to-default / "latest" / semver-compare logic that was previously copy-pasted four times.
• One implicit invariant — the behavioral equivalence for the nil-config path in awfSupportsExcludeEnv depends on DefaultFirewallVersion >= AWFExcludeEnvMinVersion, which is not tested (see inline comment).
• Helper declaration order — minor readability nit: awfVersionAtLeast is currently declared after its first caller (see inline comment).

Positive Highlights

• ✅ Deletion test passes: deleting the four old bodies would have scattered the latest/semver/fallback logic back across four callers — the new helper clearly earns its keep.
• ✅ All existing tests pass and cover the four public wrappers thoroughly.
• ✅ The PR description honestly flags the one subtle equivalence assumption and explains why it holds.
• ✅ Clean commit — no unrelated changes.

Verdict

Approving. The two inline observations are non-blocking suggestions; the refactor is correct and improves locality.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 4.3M

[github-actions]

[/improve-codebase-architecture] The helper is declared after its first caller (awfSupportsExcludeEnv at line 638). In Go the compiler does not care about declaration order within a package, but by convention helper functions are defined before the functions that use them (or at least grouped together). Moving awfVersionAtLeast above awfSupportsExcludeEnv would make the file easier to read top-to-bottom without having to scroll down to find what a one-liner delegates to.

[github-actions]

[/zoom-out] The behavioral equivalence for awfSupportsExcludeEnv with a nil/empty config relies on the invariant DefaultFirewallVersion >= AWFExcludeEnvMinVersion (currently v0.25.43 >= v0.25.3 ✅). This invariant is implicit — nothing enforces it at compile-time or in tests.

If DefaultFirewallVersion were ever rolled back below v0.25.3, this function would silently return false for unconfigured workflows instead of true, a subtle behavioral regression. Consider asserting it in the constants spec test:

// DefaultFirewallVersion must be >= all feature-flag minimum versions
assert.GreaterOrEqual(t, semverutil.Compare(string(DefaultFirewallVersion), string(AWFExcludeEnvMinVersion)), 0)

This makes the invariant visible and caught at test time rather than discovered in production.