Thread context.Context through all ResolveSHA call sites

cmd/gh-aw/main.go

@@ -205,7 +205,7 @@ Examples:
   ` + string(constants.CLIExtensionPrefix) + ` enable ci-doctor --repo owner/repo  # Enable workflow in specific repository`,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		repoOverride, _ := cmd.Flags().GetString("repo")
-		return cli.EnableWorkflowsByNames(args, repoOverride)
+		return cli.EnableWorkflowsByNames(cmd.Context(), args, repoOverride)
 	},
 }
 
@@ -226,7 +226,7 @@ Examples:
   ` + string(constants.CLIExtensionPrefix) + ` disable ci-doctor --repo owner/repo  # Disable workflow in specific repository`,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		repoOverride, _ := cmd.Flags().GetString("repo")
-		return cli.DisableWorkflowsByNames(args, repoOverride)
+		return cli.DisableWorkflowsByNames(cmd.Context(), args, repoOverride)
 	},
 }
 

docs/adr/32295-thread-context-through-resolvesha-call-chains.md

@@ -0,0 +1,77 @@
+## ADR-32295: Thread `context.Context` Through `ResolveSHA` Call Chains
+
+**Date**: 2026-05-15
+**Status**: Draft
+**Deciders**: Unknown (auto-generated from PR diff)
+
+---
+
+### Part 1 — Narrative (Human-Friendly)
+
+#### Context
+
+`ActionResolver.ResolveSHA` performs `gh api` network calls to resolve action references to pinned commit SHAs and is invoked from compile, lock-file validation, maintenance-workflow generation, and the Copilot setup pipeline. Five production call sites passed a hardcoded `context.Background()`, which meant cobra's signal-aware `cmd.Context()` (Ctrl-C, timeouts) could not propagate into the network layer, and callers had no way to inject deadlines or cancellation. The hardcoded boundaries were spread across `pkg/workflow/` and `pkg/cli/` and reached through long internal call chains, so the fix is not local: every intermediate function must accept and forward `ctx`. The goal of this change is to eliminate all hidden `context.Background()` usage on the resolution path and confine `context.Background()` to top-level CLI entry points and tests.
+
+#### Decision
+
+We will thread `context.Context` end-to-end from cobra `RunE` handlers (via `cmd.Context()`) down to every `ActionResolver.ResolveSHA` invocation, requiring all intermediate functions in `pkg/workflow/` and `pkg/cli/` to take `ctx context.Context` as their first parameter. To support struct-based call paths, the `Compiler` type gains a `ctx` field with `WithContext(ctx)` option and `SetContext(ctx)` mutator, defaulting to `context.Background()` only inside `NewCompiler()` so existing struct constructions remain valid. After this change, `context.Background()` is permitted only at top-level CLI entry points (as a fallback when `cmd.Context()` is unavailable) and in tests; all other call sites **must** propagate `ctx` received from above.
+
+#### Alternatives Considered
+
+##### Alternative 1: Keep `context.Background()` and add timeouts inside `ResolveSHA`
+
+Leave call signatures unchanged and apply a fixed timeout (e.g. `context.WithTimeout(context.Background(), 30s)`) inside `ResolveSHA` itself. Rejected because it solves only the hang-protection symptom and not the cancellation problem: Ctrl-C from cobra still cannot interrupt a hung `gh api` call, tests cannot inject deterministic contexts, and the timeout policy becomes invisible to the caller that actually knows the budget.
+
+##### Alternative 2: Stash context in a package-level or `ActionResolver` field
+
+Set the context once (e.g. on `ActionResolver`) and have `ResolveSHA` read it from `r.ctx` instead of taking a parameter. Rejected because it spreads context lifetime across goroutines and resolver reuse, contradicts the Go convention that `ctx` is the first parameter of any call that may block, and makes per-call deadline injection awkward.
+
+##### Alternative 3: Thread `ctx` only into `Compiler` and a shallow wrapper
+
+Add `ctx` to the `Compiler` struct (the centerpiece of the change) but leave the free functions in `pkg/workflow/` (`CheckActionSHAUpdates`, `ValidateActionSHAsInLockFile`, `ResolveSetupActionReference`, …) and their `pkg/cli/` callers unchanged, having them all read from a single ambient context. Rejected because several of these functions are reachable from CLI entry points that do not construct a `Compiler` (e.g. `update_actions.go`, `enable.go`, `add_command.go`), so a partial threading would leave the original five hardcoded boundaries intact under different names.
+
+#### Consequences
+
+##### Positive
+
+- Cobra's `cmd.Context()` (which is cancelled on SIGINT/SIGTERM) now flows into every `gh api` network call, so `gh aw compile`, `gh aw update-actions`, etc. can be interrupted cleanly while a resolution is in flight.
+- Per-call-site deadlines become possible without touching `ResolveSHA`: callers can wrap with `context.WithTimeout` at the boundary that knows the budget.
+- Tests can inject controlled or already-cancelled contexts to exercise cancellation paths deterministically.
+
+##### Negative
+
+- The change has very broad signature churn — roughly two dozen functions across `pkg/workflow/` and `pkg/cli/` (compile pipeline, `CheckActionSHAUpdates`, `ValidateActionSHAsInLockFile`, `ResolveSetupActionReference`, `GenerateMaintenanceWorkflow`, the entire `copilot_setup.go` chain, `AddResolvedWorkflows`, `EnableWorkflowsByNames`, `InitRepository`, …) take a new first parameter, and any downstream branch or fork must rebase against it.
+- The `Compiler` struct now has two ways to supply context (constructor option `WithContext` and post-hoc `SetContext`), with a `context.Background()` fallback inside `NewCompiler()`; future readers must understand that the fallback exists only for compatibility and is not the intended path.
+- `InitOptions.Ctx` is a nullable field (`InitRepository` falls back to `context.Background()` if nil), which is inconsistent with the "ctx is the first parameter" rule applied everywhere else and may need follow-up to align.
+
+##### Neutral
+
+- `context.Background()` remains in the codebase, but only at top-level CLI entry points (as a fallback when `cmd.Context()` is unavailable) and inside tests; a future lint rule could enforce this boundary.
+- The change is mechanical at most call sites and produces no behavior change when contexts are never cancelled, so it is low-risk to revert if needed.
+
+---
+
+### Part 2 — Normative Specification (RFC 2119)
+
+> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).
+
+#### Context Propagation on the Action-Resolution Path
+
+1. Every call site of `ActionResolver.ResolveSHA` **MUST** pass a `context.Context` obtained from its caller; it **MUST NOT** pass a freshly constructed `context.Background()` or `context.TODO()`.
+2. Every internal function in `pkg/workflow/` and `pkg/cli/` that transitively calls `ActionResolver.ResolveSHA` **MUST** accept a `ctx context.Context` as its first parameter and forward it to its callees.
+3. `context.Background()` **MUST NOT** appear on the resolution call chain except at top-level CLI entry points (cobra `RunE` handlers, `main`) as a fallback when `cmd.Context()` is unavailable, and in test files.
+
+#### Compiler and Init Boundaries
+
+1. The `Compiler` type **MUST** expose a way to associate a `context.Context` with the compiler instance (e.g. `WithContext(ctx)` constructor option and/or `SetContext(ctx)` method).
+2. `NewCompiler()` **MAY** default the embedded context to `context.Background()` when no context is supplied, but compile entry points reachable from a cobra command **SHOULD** call `SetContext(cmd.Context())` (or equivalent) before invoking compile.
+3. CLI entry-point handlers (cobra `RunE`) **MUST** pass `cmd.Context()` into the first downstream function call rather than `context.Background()`.
+4. `InitOptions.Ctx` **MAY** be nil; `InitRepository` **MUST** fall back to `context.Background()` only when `Ctx` is nil.
+
+#### Conformance
+
+An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/25916052628) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/cli/add_command.go

@@ -188,13 +188,13 @@ func AddWorkflows(ctx context.Context, workflows []string, opts AddOptions) (*Ad
 		return nil, err
 	}
 
-	return AddResolvedWorkflows(workflows, resolved, opts)
+	return AddResolvedWorkflows(ctx, workflows, resolved, opts)
 }
 
 // AddResolvedWorkflows adds workflows using pre-resolved workflow data.
 // This allows callers to resolve workflows early (e.g., to show descriptions) and then add them later.
 // The opts.Quiet parameter suppresses detailed output (useful for interactive mode where output is already shown).
-func AddResolvedWorkflows(workflowStrings []string, resolved *ResolvedWorkflows, opts AddOptions) (*AddWorkflowsResult, error) {
+func AddResolvedWorkflows(ctx context.Context, workflowStrings []string, resolved *ResolvedWorkflows, opts AddOptions) (*AddWorkflowsResult, error) {
 	addLog.Printf("Adding workflows: count=%d, engineOverride=%s, createPR=%v, noGitattributes=%v, opts.WorkflowDir=%s, noStopAfter=%v, stopAfter=%s", len(workflowStrings), opts.EngineOverride, opts.CreatePR, opts.NoGitattributes, opts.WorkflowDir, opts.NoStopAfter, opts.StopAfter)
 
 	result := &AddWorkflowsResult{}
@@ -230,7 +230,7 @@ func AddResolvedWorkflows(workflowStrings []string, resolved *ResolvedWorkflows,
 	// Handle PR creation workflow
 	if opts.CreatePR {
 		addLog.Print("Creating workflow with PR")
-		prNumber, prURL, err := addWorkflowsWithPR(resolved.Workflows, opts)
+		prNumber, prURL, err := addWorkflowsWithPR(ctx, resolved.Workflows, opts)
 		if err != nil {
 			return nil, err
 		}
@@ -241,19 +241,19 @@ func AddResolvedWorkflows(workflowStrings []string, resolved *ResolvedWorkflows,
 
 	// Handle normal workflow addition - pass resolved workflows with content
 	addLog.Print("Adding workflows normally without PR")
-	return result, addWorkflows(resolved.Workflows, opts)
+	return result, addWorkflows(ctx, resolved.Workflows, opts)
 }
 
 // addWorkflows handles workflow addition using pre-fetched content
-func addWorkflows(workflows []*ResolvedWorkflow, opts AddOptions) error {
+func addWorkflows(ctx context.Context, workflows []*ResolvedWorkflow, opts AddOptions) error {
 	addLog.Printf("Adding %d workflow(s) to repository", len(workflows))
 	// Create file tracker for all operations
 	tracker := NewFileTracker()
-	return addWorkflowsWithTracking(workflows, tracker, opts)
+	return addWorkflowsWithTracking(ctx, workflows, tracker, opts)
 }
 
 // addWorkflows handles workflow addition using pre-fetched content
-func addWorkflowsWithTracking(workflows []*ResolvedWorkflow, tracker *FileTracker, opts AddOptions) error {
+func addWorkflowsWithTracking(ctx context.Context, workflows []*ResolvedWorkflow, tracker *FileTracker, opts AddOptions) error {
 	addLog.Printf("Adding %d workflow(s) with tracking: force=%v, disableSecurityScanner=%v", len(workflows), opts.Force, opts.DisableSecurityScanner)
 	// Ensure .gitattributes is configured unless flag is set
 	if !opts.NoGitattributes {
@@ -279,7 +279,7 @@ func addWorkflowsWithTracking(workflows []*ResolvedWorkflow, tracker *FileTracke
 			fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Adding workflow %d/%d: %s", i+1, len(workflows), resolved.Spec.WorkflowName)))
 		}
 
-		if err := addWorkflowWithTracking(resolved, tracker, opts); err != nil {
+		if err := addWorkflowWithTracking(ctx, resolved, tracker, opts); err != nil {
 			return fmt.Errorf("failed to add workflow '%s': %w", resolved.Spec.String(), err)
 		}
 	}
@@ -292,7 +292,7 @@ func addWorkflowsWithTracking(workflows []*ResolvedWorkflow, tracker *FileTracke
 }
 
 // addWorkflowWithTracking adds a workflow using pre-fetched content with file tracking
-func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, opts AddOptions) error {
+func addWorkflowWithTracking(ctx context.Context, resolved *ResolvedWorkflow, tracker *FileTracker, opts AddOptions) error {
 	workflowSpec := resolved.Spec
 	sourceContent := resolved.Content
 	sourceInfo := resolved.SourceInfo
@@ -364,7 +364,7 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o
 
 	// For remote workflows, fetch and save all dependencies (includes, imports, dispatch workflows, resources)
 	if !isLocalWorkflowPath(workflowSpec.WorkflowPath) {
-		if err := fetchAllRemoteDependencies(context.Background(), string(sourceContent), workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker); err != nil {
+		if err := fetchAllRemoteDependencies(ctx, string(sourceContent), workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker); err != nil {
 			return err
 		}
 	} else if sourceInfo != nil && sourceInfo.IsLocal {
@@ -537,15 +537,15 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o
 	// Compile any dispatch-workflow .md dependencies that were just fetched and lack a
 	// .lock.yml. The dispatch-workflow validator requires every .md dispatch target to be
 	// compiled before the main workflow can be validated.
-	compileDispatchWorkflowDependencies(destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, tracker)
+	compileDispatchWorkflowDependencies(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, tracker)
 
 	// Compile the workflow
 	if tracker != nil {
-		if err := compileWorkflowWithTracking(destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, tracker); err != nil {
+		if err := compileWorkflowWithTracking(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, tracker); err != nil {
 			printCompilationError(err, opts.Quiet)
 		}
 	} else {
-		if err := compileWorkflow(destFile, opts.Verbose, opts.Quiet, opts.EngineOverride); err != nil {
+		if err := compileWorkflow(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride); err != nil {
 			printCompilationError(err, opts.Quiet)
 		}
 	}

pkg/cli/add_command_test.go

@@ -151,6 +151,7 @@ func TestAddResolvedWorkflows(t *testing.T) {
 
 			opts := AddOptions{}
 			_, err := AddResolvedWorkflows(
+				context.Background(),
 				[]string{"test/repo/test-workflow"},
 				resolved,
 				opts,
@@ -461,7 +462,7 @@ func TestAddWorkflowWithTracking_SourceFieldVariants(t *testing.T) {
 			}
 			opts := AddOptions{DisableSecurityScanner: true}
 
-			err := addWorkflowWithTracking(resolved, nil, opts)
+			err := addWorkflowWithTracking(context.Background(), resolved, nil, opts)
 			require.NoError(t, err, "addWorkflowWithTracking should succeed")
 
 			written, err := os.ReadFile(filepath.Join(workflowsDir, tt.spec.WorkflowName+".md"))
@@ -515,7 +516,7 @@ func TestAddWorkflowWithTracking_UsesActualFetchedPath(t *testing.T) {
 	opts := AddOptions{
 		DisableSecurityScanner: true,
 	}
-	err := addWorkflowWithTracking(resolved, nil, opts)
+	err := addWorkflowWithTracking(context.Background(), resolved, nil, opts)
 	require.NoError(t, err, "addWorkflowWithTracking should succeed")
 
 	// Read the written file

pkg/cli/add_gitattributes_test.go

@@ -3,6 +3,7 @@
 package cli
 
 import (
+	"context"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -89,7 +90,7 @@ This is a test workflow.`
 
 		// Call addWorkflows with noGitattributes=false
 		opts := AddOptions{}
-		err := addWorkflows([]*ResolvedWorkflow{resolved}, opts)
+		err := addWorkflows(context.Background(), []*ResolvedWorkflow{resolved}, opts)
 		if err != nil {
 			// Log any error but don't fail - we're testing gitattributes behavior
 			t.Logf("Note: workflow addition returned: %v", err)
@@ -119,7 +120,7 @@ This is a test workflow.`
 
 		opts := AddOptions{NoGitattributes: true}
 		// Call addWorkflows with noGitattributes=true
-		err := addWorkflows([]*ResolvedWorkflow{resolved}, opts)
+		err := addWorkflows(context.Background(), []*ResolvedWorkflow{resolved}, opts)
 		if err != nil {
 			// Log any error but don't fail - we're testing gitattributes behavior
 			t.Logf("Note: workflow addition returned: %v", err)
@@ -142,7 +143,7 @@ This is a test workflow.`
 
 		opts := AddOptions{NoGitattributes: true}
 		// Call addWorkflows with noGitattributes=true
-		err := addWorkflows([]*ResolvedWorkflow{resolved}, opts)
+		err := addWorkflows(context.Background(), []*ResolvedWorkflow{resolved}, opts)
 		if err != nil {
 			// Log any error but don't fail - we're testing gitattributes behavior
 			t.Logf("Note: workflow addition returned: %v", err)

pkg/cli/add_interactive_git.go

@@ -39,7 +39,7 @@ func (c *AddInteractiveConfig) createWorkflowPRAndConfigureSecret(ctx context.Co
 		StopAfter:              c.StopAfter,
 		DisableSecurityScanner: false,
 	}
-	result, err := AddResolvedWorkflows(c.WorkflowSpecs, c.resolvedWorkflows, opts)
+	result, err := AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, opts)
 	if err != nil {
 		return fmt.Errorf("failed to add workflow: %w", err)
 	}