Fix Go version ordering in GetNpmBinPathSetup#14237
Conversation
The find command in GetNpmBinPathSetup returns hostedtoolcache bin directories alphabetically, causing go/1.23.12 to shadow go/1.25.0 when both exist. This breaks `make build` in smoke-codex because go.mod requires >= 1.25.0 but the older version is found first. After the find prepends all bin dirs, re-prepend $GOROOT/bin so the version set by actions/setup-go takes precedence. Use `|| true` to ensure the command chain continues when GOROOT is not set. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes PATH ordering for Go in GetNpmBinPathSetup() so the Go version selected by actions/setup-go (via $GOROOT) takes precedence over alphabetically earlier toolcache entries, addressing make build failures in smoke-codex.
Changes:
- Update
GetNpmBinPathSetup()to re-prepend$GOROOT/binafter the hostedtoolcachefindPATH population and add|| trueto avoid&&chain short-circuiting whenGOROOTis unset. - Add unit + Linux shell-based tests covering ordering and chaining behavior.
- Regenerate multiple workflow
.lock.ymlfiles to embed the updated PATH setup snippet.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/engine_helpers.go | Adjust GetNpmBinPathSetup() PATH logic to ensure $GOROOT/bin wins over alphabetically ordered toolcache bins. |
| pkg/workflow/engine_helpers_test.go | Add tests for GetNpmBinPathSetup() string contents, Go precedence behavior, and &&-chain resilience when GOROOT is unset. |
| .github/workflows/unbloat-docs.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/typist.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/test-create-pr-error-handling.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/step-name-alignment.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/static-analysis-report.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/smoke-project.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/smoke-codex.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/smoke-claude.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/sergo.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/semantic-function-refactor.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/scout.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/schema-consistency-checker.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/safe-output-health.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/prompt-clustering-analysis.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/lockfile-stats.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/issue-arborist.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/instructions-janitor.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/go-pattern-detector.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/go-logger.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/go-fan.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/github-mcp-tools-report.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/github-mcp-structural-analysis.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/example-workflow-analyzer.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/duplicate-code-detector.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/developer-docs-consolidator.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/deep-report.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-team-evolution-insights.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-safe-output-optimizer.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-performance-summary.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-observability-report.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-multi-device-docs-tester.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-issues-report.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-fact.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-doc-updater.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-code-metrics.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/daily-choice-test.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/copilot-session-insights.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/copilot-agent-analysis.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/commit-changes-analyzer.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/codex-github-remote-mcp-test.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/cloclo.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/cli-version-checker.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/claude-code-user-docs-review.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/changeset.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/blog-auditor.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
| .github/workflows/audit-workflows.lock.yml | Regenerated lockfile to include updated PATH setup snippet. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| os.MkdirAll(goOld, 0o755) | ||
| os.MkdirAll(goNew, 0o755) | ||
|
|
||
| // Write fake go scripts: old reports 1.23, new reports 1.25 | ||
| os.WriteFile(filepath.Join(goOld, "go"), []byte("#!/bin/bash\necho 'go version go1.23.12 linux/amd64'\n"), 0o755) | ||
| os.WriteFile(filepath.Join(goNew, "go"), []byte("#!/bin/bash\necho 'go version go1.25.0 linux/amd64'\n"), 0o755) |
There was a problem hiding this comment.
The test ignores errors from os.MkdirAll/os.WriteFile. If creating dirs/files fails, the test may still proceed and accidentally invoke the system go, leading to misleading passes/failures. Please check and fail on these errors (e.g., if err != nil { t.Fatal(...) }).
| // Simulate the PATH setup with GOROOT pointing to the newer version | ||
| shellCmd := fmt.Sprintf( | ||
| `export GOROOT=%q; export PATH="$(find %q -maxdepth 4 -type d -name bin 2>/dev/null | tr '\n' ':')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true; go version`, | ||
| filepath.Join(tmpDir, "go", "1.25.0", "x64"), | ||
| tmpDir, | ||
| ) | ||
|
|
||
| cmd := exec.Command("bash", "-c", shellCmd) | ||
| output, err := cmd.Output() | ||
| if err != nil { | ||
| t.Fatalf("Failed to execute shell command: %v", err) | ||
| } | ||
|
|
||
| result := strings.TrimSpace(string(output)) | ||
| if !strings.Contains(result, "go1.25.0") { | ||
| t.Errorf("Expected go1.25.0 to take precedence, but got: %s", result) | ||
| } |
There was a problem hiding this comment.
This test only asserts go version output contains go1.25.0, but it doesn’t verify that the executed go binary is the fake script created under tmpDir. If temp dir setup fails (or PATH isn’t updated as expected), the system go (which may also be 1.25.x) could make the test pass. Please assert command -v go/which go points into tmpDir (or add a unique marker to the fake script output and assert on it).
| // GOROOT re-prepend should come AFTER the find command | ||
| findIdx := strings.Index(pathSetup, "find /opt/hostedtoolcache") | ||
| gorootIdx := strings.Index(pathSetup, "$GOROOT") | ||
| if gorootIdx < findIdx { | ||
| t.Errorf("GOROOT re-prepend should come after find command, got: %s", pathSetup) | ||
| } |
There was a problem hiding this comment.
The ordering assertion uses Index() but doesn’t guard against findIdx == -1. If the find substring ever changes (or is absent), gorootIdx < findIdx won’t trigger and the test can pass incorrectly. Please assert findIdx >= 0 (and gorootIdx >= 0) before comparing their relative order.
|
✅ Changeset Generator completed successfully! |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
🧪 Agent Container Tool Check
Result: 10/12 tools fully working ✅ | 2/12 tools have runtime issues Notes:
|
|
🤖 Beep boop! The smoke test agent just passed through here on run §21768305194! All systems nominal, tests are green ✅, and the robots are happy! 🎉 P.S. If you see any smoke, it's probably just our testing equipment working overtime. Totally normal. Nothing to worry about. Carry on! 😄
|
Smoke Test ResultsPRs Tested:
Test Results:
Overall Status: PASS ✅ @Mossaka - All tests passed successfully!
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Smoke test results
|
# Conflicts: # .github/workflows/lockfile-stats.lock.yml
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✅ Changeset Generator completed successfully! |
Agent Container Tool Check
Result: 10/12 tools available Issues Found:
This indicates a potential issue with the container image or tool installation process.
|
Smoke Test Results ✅PR Titles:
Tests:
Overall: PASS ✅
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Merged PR titles: Remove anonymous bash tool syntax, require explicit configuration | Fix plugin command syntax:
|
Summary
make buildfailure in smoke-codex caused by Go 1.23.12 shadowing Go 1.25.0 in PATH$GOROOT/binafter thefindcommand inGetNpmBinPathSetup()to ensure the version set byactions/setup-gotakes precedence|| trueto prevent&&chain short-circuit when GOROOT is not setRoot Cause
GetNpmBinPathSetup()usesfind /opt/hostedtoolcache -maxdepth 4 -type d -name binto locate all bin directories.findreturns results alphabetically, sogo/1.23.12/x64/bincomes beforego/1.25.0/x64/bin. This prepends the older Go version first in PATH, shadowing the correct version installed byactions/setup-go.The copilot engine doesn't use
GetNpmBinPathSetup()(it invokes copilot directly), which is why smoke-copilot doesn't have this issue.Fix
After the find command prepends all hostedtoolcache bin directories, conditionally re-prepend
$GOROOT/bin:AWF's entrypoint.sh exports
GOROOTbefore the user command runs, so the inner bash (codex/claude wrapper) inherits the correct value.Test plan
TestGetNpmBinPathSetup— validates GOROOT re-prepend is presentTestGetNpmBinPathSetup_GorootOrdering— simulates two Go versions, verifies 1.25 winsTestGetNpmBinPathSetup_NoGorootDoesNotBreakChain— verifies command chain continues when GOROOT is emptymake buildsucceeds with Go 1.25🤖 Generated with Claude Code
Changeset
$GOROOT/binafter enumerating hostedtoolcache bins so Go versions set by actions/setup-go stay first and add regression coverage for the ordering and the empty-GOROOT command chain.