diff --git a/pkg/cli/docker_images.go b/pkg/cli/docker_images.go index de268308b7c..e44ba8aa0c9 100644 --- a/pkg/cli/docker_images.go +++ b/pkg/cli/docker_images.go @@ -9,6 +9,7 @@ import ( "sync" "time" + "github.com/github/gh-aw/pkg/ctxutil" "github.com/github/gh-aw/pkg/logger" ) @@ -67,17 +68,10 @@ var pullState = &dockerPullState{ mockDockerAvailable: true, } -func normalizeDockerContext(ctx context.Context) context.Context { - if ctx == nil { - return context.TODO() - } - return ctx -} - // isDockerImageAvailableUnlocked checks if a Docker image is available locally // This function must be called with pullState.mu held (either RLock or Lock) func isDockerImageAvailableUnlocked(ctx context.Context, image string) bool { - ctx = normalizeDockerContext(ctx) + ctx = ctxutil.OrBackground(ctx) // Check if we're in mock mode (for testing) if pullState.mockAvailableInUse { @@ -100,7 +94,7 @@ func isDockerImageAvailableUnlocked(ctx context.Context, image string) bool { // IsDockerImageAvailable checks if a Docker image is available locally func IsDockerImageAvailable(ctx context.Context, image string) bool { - ctx = normalizeDockerContext(ctx) + ctx = ctxutil.OrBackground(ctx) pullState.mu.RLock() defer pullState.mu.RUnlock() @@ -116,7 +110,7 @@ func IsDockerImageDownloading(image string) bool { // IsDockerAvailable checks if the Docker daemon is running and accessible func IsDockerAvailable(ctx context.Context) bool { - ctx = normalizeDockerContext(ctx) + ctx = ctxutil.OrBackground(ctx) mockEnabled, mockAvailable := func() (bool, bool) { pullState.mu.RLock() @@ -146,7 +140,7 @@ func IsDockerAvailable(ctx context.Context) bool { // The returned join function blocks until the download goroutine exits and returns // any error that occurred (nil on success or context cancellation). func StartDockerImageDownload(ctx context.Context, image string) (bool, func() error) { - ctx = normalizeDockerContext(ctx) + ctx = ctxutil.OrBackground(ctx) // Check availability and downloading status atomically under lock pullState.mu.Lock() diff --git a/pkg/cli/docker_images_test.go b/pkg/cli/docker_images_test.go index 44dd71c8705..980be8a35a3 100644 --- a/pkg/cli/docker_images_test.go +++ b/pkg/cli/docker_images_test.go @@ -286,28 +286,6 @@ func TestMockImageAvailability(t *testing.T) { ResetDockerPullState() } -func TestNormalizeDockerContext_NilContextReturnsTODO(t *testing.T) { - //nolint:staticcheck // Intentionally validating nil context normalization behavior. - ctx := normalizeDockerContext(nil) - - if ctx == nil { - t.Fatal("Expected nil context to be replaced") - } - - if err := ctx.Err(); err != nil { - t.Fatalf("Expected replacement context to be active, got err: %v", err) - } -} - -func TestNormalizeDockerContext_PreservesNonNilContext(t *testing.T) { - ctx, cancel := context.WithCancel(t.Context()) - defer cancel() - - if normalizeDockerContext(ctx) != ctx { - t.Fatal("Expected non-nil context to be preserved") - } -} - func TestIsDockerAvailable_NilContext(t *testing.T) { ResetDockerPullState() SetMockDockerAvailable(true) diff --git a/pkg/workflow/github_cli.go b/pkg/workflow/github_cli.go index 706c3879c0d..eadc27cc7a6 100644 --- a/pkg/workflow/github_cli.go +++ b/pkg/workflow/github_cli.go @@ -13,6 +13,7 @@ import ( "sync" "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/ctxutil" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/tty" ) @@ -39,16 +40,14 @@ func getDefaultGHHost() string { // setupGHCommand creates an exec.Cmd for gh CLI with proper token configuration. // This is the core implementation shared by ExecGH and ExecGHContext. -// When ctx is nil, it falls back to context.TODO(). +// When ctx is nil, it falls back to context.Background(). func setupGHCommand(ctx context.Context, args ...string) *exec.Cmd { // Check if GH_TOKEN or GITHUB_TOKEN is available ghToken := lookupProcessEnv("GH_TOKEN") githubToken := lookupProcessEnv("GITHUB_TOKEN") ghHost := lookupProcessEnv("GH_HOST") - if ctx == nil { - ctx = context.TODO() - } + ctx = ctxutil.OrBackground(ctx) cmd := exec.CommandContext(ctx, "gh", args...) if ghToken != "" || githubToken != "" { diff --git a/pkg/workflow/skills_ref_resolution.go b/pkg/workflow/skills_ref_resolution.go index 3c6aa627426..640e399805a 100644 --- a/pkg/workflow/skills_ref_resolution.go +++ b/pkg/workflow/skills_ref_resolution.go @@ -1,9 +1,10 @@ package workflow import ( - "context" "fmt" "os" + + "github.com/github/gh-aw/pkg/ctxutil" ) // resolveFrontmatterSkillRefs pins non-SHA remote skill refs (owner/repo[/path]@ref) to @@ -67,12 +68,7 @@ func (c *Compiler) resolveSkillRefSpec(data *WorkflowData, markdownPath, spec st return spec } - ctx := data.Ctx - if ctx == nil { - ctx = context.Background() - } - - sha, err := data.ActionResolver.ResolveSHA(ctx, parsed.repoPath, parsed.ref) + sha, err := data.ActionResolver.ResolveSHA(ctxutil.OrBackground(data.Ctx), parsed.repoPath, parsed.ref) if err != nil { skillsFrontmatterLog.Printf("skills[%d]: failed to resolve ref %q for %q to a SHA: %v", idx, parsed.ref, parsed.repoPath, err) fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", diff --git a/pkg/workflow/skills_ref_resolution_test.go b/pkg/workflow/skills_ref_resolution_test.go index 962a496f9f1..6dba992da1e 100644 --- a/pkg/workflow/skills_ref_resolution_test.go +++ b/pkg/workflow/skills_ref_resolution_test.go @@ -65,6 +65,27 @@ func TestResolveFrontmatterSkillRefs_PinsNonSHARefUsingCache(t *testing.T) { assert.Empty(t, strings.TrimSpace(output), "no warning expected when resolution succeeds") } +func TestResolveFrontmatterSkillRefs_PinsNonSHARefWithNilContext(t *testing.T) { + tmpDir := testutil.TempDir(t, "skill-ref-cache-nil-context") + cache := NewActionCache(tmpDir) + resolver := NewActionResolver(cache) + const sha = "1f181b37d3fe5862ab590648f25a292e345b5de6" + cache.Set("githubnext/skills", "main", sha) + + compiler := NewCompiler(WithVersion("dev")) + data := newTestSkillWorkflowData([]string{"githubnext/skills@main"}) + data.Ctx = nil + data.ActionResolver = resolver + + output := withCapturedStderr(t, func() { + compiler.resolveFrontmatterSkillRefs(data, "workflow.md") + }) + + assert.Equal(t, "githubnext/skills@"+sha, data.Skills[0]) + assert.Equal(t, "githubnext/skills@"+sha, data.SkillReferences[0].Skill) + assert.Empty(t, strings.TrimSpace(output), "no warning expected when resolution succeeds") +} + func TestResolveFrontmatterSkillRefs_LeavesFullSHAUnchanged(t *testing.T) { compiler := NewCompiler(WithVersion("dev")) const sha = "1f181b37d3fe5862ab590648f25a292e345b5de6"