[CI Failure Doctor] Variable shadowing in engine execution files causing lint-go failure

[github-actions]

🏥 CI Failure Investigation - Run #21012377690

Summary

The CI workflow failed due to variable shadowing violations detected by the Go linter in engine execution code. The variable commandName is declared multiple times in different scopes within the same function, causing lint-go job to fail.

Failure Details

• Run: #21012377690
• Commit: 283ab7c9
• PR: Add engine.command field to use custom CLI executable #9987 - "Add engine.command field to use custom CLI executable"
• Trigger: push to main branch
• Failed Job: lint-go (failed after 13 seconds)
• Impact: 27 other jobs cancelled due to early failure

Root Cause Analysis

PR #9987 introduced a new engine.command field to support custom CLI executables. The implementation added logic to determine which command name to use based on whether a custom command was specified. However, the code introduced variable shadowing by declaring var commandName string multiple times in different conditional blocks within the same function scope.

Variable Shadowing Locations

File: pkg/workflow/copilot_engine_execution.go

// Line 156 - Inside sandboxEnabled block
var commandName string
if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" {
    commandName = workflowData.EngineConfig.Command
    ...
}

// Line 185 - Inside else block (sandboxEnabled == false)
var commandName string  // ❌ SHADOWING - redeclared in different scope
if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" {
    commandName = workflowData.EngineConfig.Command
    ...
}

File: pkg/workflow/codex_engine.go

// Line 168 - Inside sandboxEnabled block
var commandName string
if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" {
    commandName = workflowData.EngineConfig.Command
    ...
}

// Line 312 - Inside non-AWF block
var commandName string  // ❌ SHADOWING - redeclared in different scope
if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" {
    commandName = workflowData.EngineConfig.Command
    ...
}

Why This Is a Problem

1. Lint Failure: golangci-lint's shadow checker detects this as a potential bug pattern
2. Code Smell: Variable redeclaration in different scopes indicates duplicated logic
3. Maintenance Risk: Future changes might only update one scope, leading to inconsistent behavior

Failed Jobs and Errors

• lint-go: ❌ failure (13s) - Variable shadowing violations
• 27 other jobs: ⚠️ cancelled (all integration tests, builds, security scans)

The early failure of lint-go triggered GitHub Actions' fail-fast behavior, cancelling all parallel jobs.

Investigation Findings

Historical Context

This is not the first time variable shadowing has caused lint failures:

• Issue Fix variable shadowing and linting violations in security fix #8657: "Fix variable shadowing and linting violations in security fix" (closed)
• Issue Remove unused logger variables causing lint failures #8514: "Remove unused logger variables causing lint failures" (closed)

Pattern Analysis

The codebase has recurring issues with:

1. Variable shadowing in conditional blocks
2. Duplicated logic across multiple scopes
3. Missing pre-commit lint checks

Recommended Actions

Immediate Fix (Priority: HIGH)

• Refactor copilot_engine_execution.go: Declare commandName once before the if sandboxEnabled check
• Refactor codex_engine.go: Declare commandName once before the conditional blocks
• Extract common logic: Create helper function getCommandName(workflowData *WorkflowData, defaultCmd string) string
• Verify fix: Run make lint locally before committing

Refactoring Approach

// ✅ GOOD - Single declaration before conditional blocks
func (e *CopilotEngine) GetExecutionSteps(...) {
    // Determine command name once at function start
    commandName := getCommandName(workflowData, "copilot")
    
    if sandboxEnabled {
        // Use commandName without redeclaring
        baseCommand = fmt.Sprintf("%s %s", commandName, shellJoinArgs(copilotArgs))
    } else {
        // Use commandName without redeclaring
        baseCommand = fmt.Sprintf("%s %s", commandName, shellJoinArgs(copilotArgs))
    }
}

// Helper function to reduce duplication
func getCommandName(workflowData *WorkflowData, defaultCmd string) string {
    if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" {
        return workflowData.EngineConfig.Command
    }
    return defaultCmd
}

Prevention Strategies (Priority: MEDIUM)

• Enable pre-commit hooks: Run make lint before allowing commits
• Update CI/CD: Add lint check as required status check (don't allow merge without passing)
• Documentation: Add to AGENTS.md section on common lint errors and how to fix them
• Code Review: Require review from someone who has fixed similar issues before

AI Team Self-Improvement

Copy-paste these additional instructions into AGENTS.md or developer instructions:

### Variable Shadowing Prevention

**ALWAYS avoid variable shadowing when declaring variables in conditional blocks:**

❌ **WRONG - Variable shadowing:**
```go
if condition {
    var commandName string
    commandName = "value1"
} else {
    var commandName string  // ❌ Shadows previous declaration
    commandName = "value2"
}

✅ CORRECT - Single declaration:

var commandName string
if condition {
    commandName = "value1"
} else {
    commandName = "value2"
}

✅ BETTER - Extract to function:

commandName := determineCommandName(config, defaultValue)
// Use commandName in conditional blocks without redeclaring

Pre-commit Checklist:

• Run make lint to catch shadowing violations
• Check for duplicate logic across conditional blocks
• Consider extracting repeated patterns into helper functions

## Historical Context

This failure pattern is similar to:
- **#8657**: Variable shadowing in security fix (fixed by extracting helper functions)
- **#8514**: Unused variables causing lint failures (fixed by removing declarations)

The codebase has shown a pattern of lint failures being introduced during feature development, particularly when adding new conditional logic paths.

## Additional Notes

- The PR #9987 itself is a valuable feature (custom CLI executable paths)
- The logic is correct, just needs refactoring to avoid shadowing
- All tests in the PR passed, suggesting test coverage is good
- The feature should be retained, just the variable declarations need fixing

---

**Investigation completed**: 2026-01-14T22:39:22Z  
**Investigator**: CI Failure Doctor  
**Confidence**: HIGH (root cause confirmed by code analysis)




> AI generated by [CI Failure Doctor](https://github.com/githubnext/gh-aw/actions/runs/21012396640)
>
> To add this workflow in your repository, run `gh aw add githubnext/agentics/workflows/ci-doctor.md@ea350161ad5dcc9624cf510f134c6a9e39a6f94d`. See [usage guide](https://githubnext.github.io/gh-aw/tools/cli/).

<!-- agentic-workflow: CI Failure Doctor, engine: copilot, run: https://github.com/githubnext/gh-aw/actions/runs/21012396640 -->

[github-actions]

🍪 Issue Monster has assigned this to Copilot!

I've identified this issue as a good candidate for automated resolution and assigned it to the Copilot agent.

The Copilot agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

AI generated by Issue Monster