Skip to content

[WIP] Fix lint-go failure after PR #10756 merge#10777

Merged
pelikhan merged 2 commits intomainfrom
copilot/fix-lint-go-failure
Jan 20, 2026
Merged

[WIP] Fix lint-go failure after PR #10756 merge#10777
pelikhan merged 2 commits intomainfrom
copilot/fix-lint-go-failure

Conversation

Copy link
Contributor

Copilot AI commented Jan 20, 2026

Fix lint-go Failure from PR #10756

Investigation Summary

  • Investigated CI failure from workflow run 21156741258
  • Identified root cause: unused function hasInternalPrefix in pkg/workflow/compiler_orchestrator_test.go
  • Confirmed fix already merged in PR Remove unused hasInternalPrefix function causing lint failure #10774
  • Verified linter now passes on main branch
  • Add documentation to AGENTS.md for test file linting best practices

Root Cause

The lint-go failure was caused by an unused helper function hasInternalPrefix in the new test file pkg/workflow/compiler_orchestrator_test.go. The unused linter detected this function was defined but never called.

Resolution Status

Already Fixed - PR #10774 removed the unused function and the linter now passes.

Documentation Added

Added comprehensive "Test File Linting Requirements" section to AGENTS.md with:

  • Guidance on running linters after adding test files
  • Common linting issues (unused functions, testifylint best practices)
  • Examples of correct and incorrect test code
  • Emphasis on the specific hasInternalPrefix issue as a cautionary example

This documentation will help prevent similar issues in the future by making AI agents aware of:

  1. The need to run make lint after creating test files
  2. Common pitfalls with unused helper functions
  3. testifylint best practices for assertions
Original prompt

This section details on the original issue you should resolve

<issue_title>[CI Failure Doctor] lint-go failure after merge of PR #10756 (compiler test files)</issue_title>
<issue_description>## Summary
The lint-go CI job failed on main branch after merging PR #10756 which added comprehensive test coverage for compiler core files.

Failure Details

  • Run: 21156741258
  • Commit: 7a21eaf
  • Trigger: push to main
  • Failed Job: lint-go
  • Failed Step: Run golangci-lint
  • Started: 2026-01-20T01:51:39Z
  • Completed: 2026-01-20T01:52:22Z (43 seconds)

Root Cause Analysis

What Changed

PR #10756 added 3 new test files with 41 test functions totaling 1,278 lines:

  • pkg/workflow/compiler_test.go (328 lines, 12 tests)
  • pkg/workflow/compiler_orchestrator_test.go (489 lines, 12 tests)
  • pkg/workflow/compiler_activation_jobs_test.go (461 lines, 17 tests)

What Failed

The lint-go job runs these steps in sequence:

  1. ✅ Check Go formatting (go fmt ./...) - PASSED
  2. ✅ Install golangci-lint - PASSED
  3. ❌ Run golangci-lint (make golint) - FAILED
  4. ⏭️ Lint error messages - SKIPPED

Enabled Linters

The golangci-lint configuration (.golangci.yml) enables:

  • testifylint - Enforce testify best practices (most likely culprit for test files)
  • misspell - Catch common misspellings
  • unconvert - Remove unnecessary type conversions

Most Likely Cause

Since go fmt passed but golangci-lint failed, and the changes are all test files, the issue is most likely testifylint violations. Common testifylint issues include:

  • Missing assertion messages
  • Incorrect use of assert vs require
  • Using assert.NotNil/assert.Nil instead of assert.NoError/assert.Error
  • Not using testify helper functions properly

Investigation Findings

Timeline

  1. 01:50:25Z - PR Add test coverage for compiler core files (2,291 untested lines) #10756 merged to main by @pelikhan
  2. 01:50:28Z - CI workflow triggered (3 seconds after merge)
  3. 01:50:46Z - lint-go job started
  4. 01:51:39Z - golangci-lint step started
  5. 01:52:22Z - golangci-lint step failed (43 seconds duration)

Pre-merge Status

  • All CI checks on the PR branch passed before merge
  • The failure only occurred on the main branch after merge
  • This suggests the lint-go checks may differ between PR and main branch runs

Historical Context

Recent PR #10742 ("chore: fix lint errors from go fmt") indicates recurring linting issues in the codebase. The AGENTS.md file has mandatory pre-commit validation requirements but they may not have been followed.

Reproduction Steps

To reproduce this failure locally:

# Clone and checkout the failing commit
git clone https://github.com/githubnext/gh-aw.git
cd gh-aw
git checkout 7a21eaf2ea0f8fc6377186a3dbd97963aae89495

# Install dependencies
make deps-dev

# Run the linter (this should fail)
make golint

# Or run full lint suite
make lint

Recommended Actions

Immediate Fix

  1. Run make golint locally to see specific linting errors
  2. Fix testifylint violations in the three new test files
  3. Run make agent-finish to verify all checks pass
  4. Commit fix with message: "fix: resolve golangci-lint errors in compiler test files"

Specific Files to Check

  • pkg/workflow/compiler_test.go
  • pkg/workflow/compiler_orchestrator_test.go
  • pkg/workflow/compiler_activation_jobs_test.go

Common testifylint Fixes

// ❌ BAD - No message
assert.NoError(t, err)

// ✅ GOOD - With descriptive message
assert.NoError(t, err, "Failed to compile workflow")

// ❌ BAD - Using NotNil for error
assert.NotNil(t, err)

// ✅ GOOD - Using Error for error
assert.Error(t, err, "Should return error for invalid input")

Prevention Strategies

For Contributors

The AGENTS.md file already has mandatory pre-commit requirements, but they need to be reinforced:

  1. Always run before committing:

    make agent-finish  # Runs build, test, recompile, fmt, lint
  2. Minimum for Go code changes:

    make fmt      # Format Go code
    make lint     # Run all linters including golangci-lint

Process Improvements

  1. Add pre-commit hook: Install git pre-commit hook that runs make fmt && make lint
  2. Enhanced PR checks: Ensure lint-go runs on PR branches with same configuration as main
  3. Clearer error messages: When lint-go fails, the CI should show specific linting errors in the summary

AI Team Self-Improvement

Add to AGENTS.md under "Critical Requirements":

### Test File Linting Requirements

**When adding new test files (*.test.go, *_test.go):**

1. **Run linters after creating tests**:
   ```bash
   make lint        # Catches testifylint, misspell, unconvert issues
   ```

2. **testifylint best practices**:
   - Always provide descriptive assert...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes githubnext/gh-aw#10772

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
@pelikhan pelikhan closed this Jan 20, 2026
@pelikhan pelikhan reopened this Jan 20, 2026
Copilot AI requested a review from mnkiefer January 20, 2026 02:46
Copilot stopped work on behalf of mnkiefer due to an error January 20, 2026 02:46
@pelikhan pelikhan marked this pull request as ready for review January 20, 2026 02:48
@pelikhan pelikhan merged commit 1972429 into main Jan 20, 2026
34 checks passed
@pelikhan pelikhan deleted the copilot/fix-lint-go-failure branch January 20, 2026 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants