Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 14, 2026

Fix Variable Shadowing in Engine Execution Files

This PR fixes variable shadowing violations in copilot_engine_execution.go and codex_engine.go that caused the CI lint-go job to fail.

Changes Made:

  • Understand the issue and examine affected files
  • Fix variable shadowing in pkg/workflow/copilot_engine_execution.go
    • Declare commandName once before the if sandboxEnabled block
    • Remove duplicate var commandName string declarations
  • Fix variable shadowing in pkg/workflow/codex_engine.go
    • Declare commandName once before the if firewallEnabled block
    • Remove duplicate var commandName string declarations
  • Run linter to verify fixes (✅ PASSED - no shadowing errors)
  • Run tests to ensure no regressions (✅ Unit tests passed, integration test failures are pre-existing and unrelated to these changes)
  • Validate changes are minimal and surgical
  • Merge main branch and resolve conflicts
  • Format code
  • Run linter after merge (✅ PASSED)

Summary:

The variable shadowing issue has been fixed by:

  1. In copilot_engine_execution.go: Moved commandName declaration before the if sandboxEnabled conditional block, eliminating the duplicate declaration in the else branch
  2. In codex_engine.go: Removed duplicate commandName declaration in the non-firewall else block, reusing the variable declared earlier in the function

The linter now passes without shadowing errors. The logic remains identical - only the variable declaration structure changed to avoid shadowing.

Latest update: Successfully merged main branch and resolved conflicts while preserving the variable shadowing fix. Code formatted and linted successfully.

Original prompt

This section details on the original issue you should resolve

<issue_title>[CI Failure Doctor] Variable shadowing in engine execution files causing lint-go failure</issue_title>
<issue_description># 🏥 CI Failure Investigation - Run githubnext/gh-aw#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

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:

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 b...

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits January 14, 2026 23:59
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
…e.go

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix variable shadowing in engine execution files Fix variable shadowing in engine execution command name resolution Jan 15, 2026
Copilot AI requested a review from mnkiefer January 15, 2026 00:16
@pelikhan pelikhan marked this pull request as ready for review January 15, 2026 00:19
@pelikhan
Copy link
Contributor

@copilot merge main, format, lint

Copilot AI and others added 2 commits January 15, 2026 00:36
…shadowing-error

# Conflicts:
#	pkg/workflow/codex_engine.go
#	pkg/workflow/copilot_engine_execution.go
…adowing fix

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 15, 2026

@copilot merge main, format, lint

Done! Successfully merged main branch, formatted code, and ran linter. All checks passed. Commit: 95886f9

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2026

✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2026

📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2026

🎬 THE ENDSmoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2026

🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰

@github-actions
Copy link
Contributor

Smoke Test Results (Run #21022512328)

Last 2 merged PRs:

Test Results:

  • ✅ GitHub MCP: Fetched recent PRs successfully
  • ❌ Serena Go: Tool not available in environment
  • ✅ Playwright: Navigated to github.com (title contains "GitHub")
  • ✅ Tavily: Web search returned 5 results for "GitHub Agentic Workflows"
  • ✅ File Operations: Created and verified test file
  • ✅ Bash Tool: File read successful

Overall Status: PASS (5/6 tests passed)

AI generated by Smoke Claude

@github-actions
Copy link
Contributor

Smoke Test Results (Copilot Engine)

Last 2 Merged PRs:

Test Results:

  • ✅ GitHub MCP: Retrieved merged PRs
  • ❌ Serena Go: Command not available
  • ✅ Playwright: Navigated to GitHub (title: "GitHub · Change is constant. GitHub keeps you ahead. · GitHub")
  • ✅ File Writing: Created test file successfully
  • ✅ Bash Tool: Read file back successfully

Overall Status: PARTIAL PASS (4/5 tests passed)

cc @pelikhan

AI generated by Smoke Copilot

@pelikhan pelikhan merged commit efea251 into main Jan 15, 2026
117 checks passed
@pelikhan pelikhan deleted the copilot/fix-variable-shadowing-error branch January 15, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants