[WIP] Add helper to wrap 'gh' calls using Exec #3952
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Created pkg/workflow/gh_helper.go with ExecGH function - ExecGH wraps gh CLI calls and uses GITHUB_TOKEN as fallback when GH_TOKEN is not set - Added comprehensive tests in gh_helper_test.go - Updated action_resolver.go to use new ExecGH helper - All tests pass, code formatted and linted Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new ExecGH() helper function to wrap gh CLI command execution with automatic token fallback support. The implementation ensures that GitHub CLI commands work in environments where GITHUB_TOKEN is set but GH_TOKEN is not, by setting GH_TOKEN from GITHUB_TOKEN when needed.
Key changes:
- New
ExecGH()helper function inpkg/workflow/gh_helper.gowith token precedence logic - Comprehensive test coverage in
pkg/workflow/gh_helper_test.go - Updated
action_resolver.goto use the new helper instead of directexec.Command("gh", ...)calls
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/gh_helper.go | New helper function that wraps gh CLI calls with GH_TOKEN/GITHUB_TOKEN fallback logic |
| pkg/workflow/gh_helper_test.go | Comprehensive tests covering token precedence scenarios and multi-argument handling |
| pkg/workflow/action_resolver.go | Updated to use ExecGH() helper instead of exec.Command("gh", ...) |
| pkg/cli/.github/aw/actions-lock.json | New empty auto-generated lock file (possibly unrelated to PR purpose) |
| .github/workflows/daily-team-status.lock.yml | Timestamp update in auto-generated lock file (unrelated to PR purpose) |
| .github/workflows/ci-doctor.lock.yml | Timestamp update in auto-generated lock file (unrelated to PR purpose) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Source: githubnext/agentics/workflows/ci-doctor.md@09e77ed2e49f0612e258db12839e86e8e2a6c692 | ||
| # | ||
| # Effective stop-time: 2025-11-16 12:42:25 | ||
| # Effective stop-time: 2025-11-16 13:10:44 |
There was a problem hiding this comment.
[nitpick] This file appears to be an auto-generated lock file with only timestamp updates. These changes are unrelated to the PR's purpose of adding a gh CLI helper function.
Recommendation: These lock files should likely be regenerated in a separate commit or excluded from this PR to keep the changes focused on the gh helper implementation. The timestamp changes will be regenerated the next time the workflows are compiled anyway.
| # Effective stop-time: 2025-11-16 13:10:44 | |
| # Effective stop-time: 2025-11-06 13:10:44 |
| { | ||
| "entries": {} | ||
| } |
There was a problem hiding this comment.
[nitpick] This appears to be a new empty actions lock file that was auto-generated. It's unclear why this file is being created in the pkg/cli/.github/aw/ directory.
Questions:
- Is this file expected to be in the
pkg/cli/directory, or should it be at the repository root? - Is this related to the gh helper implementation, or an unrelated side effect?
Recommendation: If this file is unrelated to the PR's purpose, consider excluding it from this PR and investigating why it was generated.
| { | |
| "entries": {} | |
| } |
| // Check if GH_TOKEN is already set | ||
| ghToken := os.Getenv("GH_TOKEN") | ||
| if ghToken != "" { | ||
| ghHelperLog.Printf("GH_TOKEN is set, using it for gh CLI") | ||
| return cmd | ||
| } | ||
|
|
||
| // Fall back to GITHUB_TOKEN if available | ||
| githubToken := os.Getenv("GITHUB_TOKEN") | ||
| if githubToken != "" { | ||
| ghHelperLog.Printf("GH_TOKEN not set, using GITHUB_TOKEN as fallback for gh CLI") | ||
| // Set GH_TOKEN in the command's environment | ||
| cmd.Env = append(os.Environ(), "GH_TOKEN="+githubToken) | ||
| } else { | ||
| ghHelperLog.Printf("Neither GH_TOKEN nor GITHUB_TOKEN is set, gh CLI will use default authentication") | ||
| } | ||
|
|
There was a problem hiding this comment.
The token precedence order is inconsistent with the existing GetGitHubToken() function in pkg/parser/github.go.
Current implementation (this PR):
- GH_TOKEN
- GITHUB_TOKEN
- Default gh CLI auth
Existing pattern in pkg/parser/github.go:
- GITHUB_TOKEN
- GH_TOKEN
gh auth tokencommand
The PR description states "make sure it uses GITHUB_TOKEN if GH_TOKEN is not available" and mentions following existing code patterns. However, this implementation prioritizes GH_TOKEN first, which contradicts the stated goal.
Recommendation: Either align with the existing GetGitHubToken() precedence (prefer GITHUB_TOKEN first), or document why this helper should use a different order. The inconsistency could lead to confusion and unexpected behavior across the codebase.
| // Check if GH_TOKEN is already set | |
| ghToken := os.Getenv("GH_TOKEN") | |
| if ghToken != "" { | |
| ghHelperLog.Printf("GH_TOKEN is set, using it for gh CLI") | |
| return cmd | |
| } | |
| // Fall back to GITHUB_TOKEN if available | |
| githubToken := os.Getenv("GITHUB_TOKEN") | |
| if githubToken != "" { | |
| ghHelperLog.Printf("GH_TOKEN not set, using GITHUB_TOKEN as fallback for gh CLI") | |
| // Set GH_TOKEN in the command's environment | |
| cmd.Env = append(os.Environ(), "GH_TOKEN="+githubToken) | |
| } else { | |
| ghHelperLog.Printf("Neither GH_TOKEN nor GITHUB_TOKEN is set, gh CLI will use default authentication") | |
| } | |
| // Prefer GITHUB_TOKEN over GH_TOKEN for consistency with GetGitHubToken() | |
| githubToken := os.Getenv("GITHUB_TOKEN") | |
| if githubToken != "" { | |
| ghHelperLog.Printf("GITHUB_TOKEN is set, using it for gh CLI (sets GH_TOKEN in env)") | |
| cmd.Env = append(os.Environ(), "GH_TOKEN="+githubToken) | |
| return cmd | |
| } | |
| // Fall back to GH_TOKEN if available | |
| ghToken := os.Getenv("GH_TOKEN") | |
| if ghToken != "" { | |
| ghHelperLog.Printf("GITHUB_TOKEN not set, using GH_TOKEN for gh CLI") | |
| // No need to set env, inherited from parent | |
| return cmd | |
| } | |
| ghHelperLog.Printf("Neither GITHUB_TOKEN nor GH_TOKEN is set, gh CLI will use default authentication") |
| originalGHToken := os.Getenv("GH_TOKEN") | ||
| originalGitHubToken := os.Getenv("GITHUB_TOKEN") | ||
| defer func() { | ||
| os.Setenv("GH_TOKEN", originalGHToken) | ||
| os.Setenv("GITHUB_TOKEN", originalGitHubToken) |
There was a problem hiding this comment.
The environment restoration logic has a bug. If the original environment variables were unset (empty string returned by os.Getenv), calling os.Setenv with an empty string will set them to an empty value instead of unsetting them.
Issue:
originalGHToken := os.Getenv("GH_TOKEN") // Returns "" if unset
defer func() {
os.Setenv("GH_TOKEN", originalGHToken) // Sets to "" instead of unsetting
}()Fix:
originalGHToken, hadGHToken := os.LookupEnv("GH_TOKEN")
originalGitHubToken, hadGitHubToken := os.LookupEnv("GITHUB_TOKEN")
defer func() {
if hadGHToken {
os.Setenv("GH_TOKEN", originalGHToken)
} else {
os.Unsetenv("GH_TOKEN")
}
if hadGitHubToken {
os.Setenv("GITHUB_TOKEN", originalGitHubToken)
} else {
os.Unsetenv("GITHUB_TOKEN")
}
}()This ensures proper cleanup regardless of the original environment state.
| originalGHToken := os.Getenv("GH_TOKEN") | |
| originalGitHubToken := os.Getenv("GITHUB_TOKEN") | |
| defer func() { | |
| os.Setenv("GH_TOKEN", originalGHToken) | |
| os.Setenv("GITHUB_TOKEN", originalGitHubToken) | |
| originalGHToken, hadGHToken := os.LookupEnv("GH_TOKEN") | |
| originalGitHubToken, hadGitHubToken := os.LookupEnv("GITHUB_TOKEN") | |
| defer func() { | |
| if hadGHToken { | |
| os.Setenv("GH_TOKEN", originalGHToken) | |
| } else { | |
| os.Unsetenv("GH_TOKEN") | |
| } | |
| if hadGitHubToken { | |
| os.Setenv("GITHUB_TOKEN", originalGitHubToken) | |
| } else { | |
| os.Unsetenv("GITHUB_TOKEN") | |
| } |
| originalGHToken := os.Getenv("GH_TOKEN") | ||
| originalGitHubToken := os.Getenv("GITHUB_TOKEN") | ||
| defer func() { | ||
| os.Setenv("GH_TOKEN", originalGHToken) | ||
| os.Setenv("GITHUB_TOKEN", originalGitHubToken) |
There was a problem hiding this comment.
The environment restoration logic has a bug. If the original environment variables were unset (empty string returned by os.Getenv), calling os.Setenv with an empty string will set them to an empty value instead of unsetting them.
Issue:
originalGHToken := os.Getenv("GH_TOKEN") // Returns "" if unset
defer func() {
os.Setenv("GH_TOKEN", originalGHToken) // Sets to "" instead of unsetting
}()Fix:
originalGHToken, hadGHToken := os.LookupEnv("GH_TOKEN")
originalGitHubToken, hadGitHubToken := os.LookupEnv("GITHUB_TOKEN")
defer func() {
if hadGHToken {
os.Setenv("GH_TOKEN", originalGHToken)
} else {
os.Unsetenv("GH_TOKEN")
}
if hadGitHubToken {
os.Setenv("GITHUB_TOKEN", originalGitHubToken)
} else {
os.Unsetenv("GITHUB_TOKEN")
}
}()This ensures proper cleanup regardless of the original environment state.
| originalGHToken := os.Getenv("GH_TOKEN") | |
| originalGitHubToken := os.Getenv("GITHUB_TOKEN") | |
| defer func() { | |
| os.Setenv("GH_TOKEN", originalGHToken) | |
| os.Setenv("GITHUB_TOKEN", originalGitHubToken) | |
| originalGHToken, hadGHToken := os.LookupEnv("GH_TOKEN") | |
| originalGitHubToken, hadGitHubToken := os.LookupEnv("GITHUB_TOKEN") | |
| defer func() { | |
| if hadGHToken { | |
| os.Setenv("GH_TOKEN", originalGHToken) | |
| } else { | |
| os.Unsetenv("GH_TOKEN") | |
| } | |
| if hadGitHubToken { | |
| os.Setenv("GITHUB_TOKEN", originalGitHubToken) | |
| } else { | |
| os.Unsetenv("GITHUB_TOKEN") | |
| } |
| # Source: githubnext/agentics/workflows/daily-team-status.md@1e366aa4518cf83d25defd84e454b9a41e87cf7c | ||
| # | ||
| # Effective stop-time: 2025-12-14 12:42:33 | ||
| # Effective stop-time: 2025-12-14 13:10:46 |
There was a problem hiding this comment.
[nitpick] This file appears to be an auto-generated lock file with only timestamp updates. These changes are unrelated to the PR's purpose of adding a gh CLI helper function.
Recommendation: These lock files should likely be regenerated in a separate commit or excluded from this PR to keep the changes focused on the gh helper implementation. The timestamp changes will be regenerated the next time the workflows are compiled anyway.
| # Effective stop-time: 2025-12-14 13:10:46 | |
| # Effective stop-time: <PREVIOUS_TIMESTAMP> |
Plan: Add helper for gh CLI execution with token fallback ✅
Analysis
pkg/cli/exec.goandpkg/parser/github.gopkg/parser/github.gohasGetGitHubToken()that checks GITHUB_TOKEN → GH_TOKEN → gh auth tokenpkg/cli/exec.gohasghExecOrFallback()but it only checks GH_TOKENMinimal Changes Plan
pkg/workflowpackage to wrap gh CLI callsaction_resolver.goto use the new helperImplementation Summary
Created
pkg/workflow/gh_helper.go:ExecGH(args ...string) *exec.Cmdfunction wrapsexec.Command("gh", ...)callsGH_TOKENis set, uses it if available (no environment modification needed)GITHUB_TOKENifGH_TOKENis not setGH_TOKENenvironment variable in the command's environment when using fallbackCreated
pkg/workflow/gh_helper_test.go:Updated
pkg/workflow/action_resolver.go:os/execimport (no longer needed)exec.Command("gh", "api", apiPath, "--jq", ".object.sha")toExecGH("api", apiPath, "--jq", ".object.sha")Test Results
Changes Made
How It Works
When
ExecGH()is called:ghcommand with the provided argumentsGH_TOKENenvironment variable is setGH_TOKENexists, returns command as-is (uses parent environment)GH_TOKENdoesn't exist butGITHUB_TOKENdoes, setsGH_TOKENin command's environmentThis ensures that gh CLI commands work correctly in environments like GitHub Actions where
GITHUB_TOKENis available butGH_TOKENmay not be set.Original prompt
💡 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.