Thread context.Context through 8 public remote_fetch.go APIs to fix Ctrl-C cancellation

docs/adr/43128-thread-context-through-remote-fetch-apis.md

@@ -0,0 +1,48 @@
+# ADR-43128: Thread context.Context Through Public Remote Fetch APIs
+
+**Date**: 2026-07-03
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+The 8 public APIs in `pkg/parser/remote_fetch.go` (`DownloadFileFromGitHub`, `DownloadFileFromGitHubForHost`, `ResolveRefToSHAForHost`, `ListWorkflowFiles`, `ListWorkflowFilesForHost`, `ListDirAllFilesForHost`, `ListDirAllFilesRecursivelyForHost`, `ListDirSubdirsForHost`) and all their private helpers hard-coded `context.Background()` throughout their call chains. This made it impossible for CLI commands to propagate cancellation signals: pressing Ctrl-C could not interrupt in-flight HTTP requests because no cancellable context was reachable by the HTTP layer. Go's `context.Context` idiom requires passing a caller-supplied context as the first parameter of I/O-bound functions so that the caller can cancel, time out, or add deadlines. Without this, each CLI command that fetches remote resources silently blocks until the HTTP stack times out on its own, degrading the interactive user experience.
+
+### Decision
+
+We will add `ctx context.Context` as the first parameter to all 8 public remote fetch APIs and propagate it through every private helper in the same call chain, replacing all 7 hard-coded `context.Background()` call sites. All CLI callers are updated to pass `cmd.Context()` (the Cobra command context, which is cancelled on Ctrl-C). Two remaining hard-coded `context.Background()` calls inside `downloadIncludeFromWorkflowSpec` / `resolveWorkflowSpecSHAForCache` — which sit behind `ResolveIncludePath` across 6+ files in 4 packages — are deferred to a follow-up PR because threading context through that compile-time include-resolution path requires a broader coordinated change.
+
+### Alternatives Considered
+
+#### Alternative 1: Keep context.Background() — Status Quo
+
+The simplest option is to leave all call sites as-is. This avoids any API surface change. It was rejected because it permanently prevents signal propagation: Ctrl-C cannot cancel HTTP calls, long-running fetches block indefinitely on poor connections, and there is no path to adding per-operation timeouts in the future without the same refactor.
+
+#### Alternative 2: Use a Package-Level or Global Cancellable Context
+
+A global or package-level `context.Context` could be set at startup and read by `remote_fetch.go` without changing any function signatures. This avoids the API churn. It was rejected because it couples the library to a mutable global state, makes behaviour non-deterministic in tests (shared state across goroutines), and is explicitly against the Go standard library's context design guidelines, which require context to flow through the call graph as an explicit argument.
+
+#### Alternative 3: Wrap the HTTP Client with a Default Timeout Instead
+
+Adding a fixed timeout (e.g., 30 s) to the `http.Client` used internally would automatically abort hung requests. This would be simpler than threading context everywhere. It was rejected because a fixed timeout cannot be adjusted by callers, cannot be cancelled early on user interrupt (Ctrl-C fires before the timeout), and does not compose with higher-level deadlines or tracing. It addresses the symptom rather than the root cause.
+
+### Consequences
+
+#### Positive
+- CLI commands (e.g., `gh aw add`, `gh aw list`) now respond to Ctrl-C by cancelling in-flight HTTP requests immediately, eliminating hung-process behaviour.
+- The codebase now follows standard Go context idioms throughout the remote I/O layer, making future work (per-command timeouts, distributed tracing, cancellation propagation) straightforward.
+- `cmd.Context()` from Cobra is wired end-to-end, meaning any future context metadata (deadlines, trace IDs) attached by callers is automatically available to all remote fetch operations.
+
+#### Negative
+- The change touches 15 files and ~20+ function signatures across `pkg/parser/`, `pkg/cli/`, and related packages; it is a wide refactor with high merge-conflict potential for concurrent branches.
+- Two `context.Background()` call sites inside the compile-time include-resolution path (`downloadIncludeFromWorkflowSpec`, `resolveWorkflowSpecSHAForCache` via `ResolveIncludePath`) are intentionally deferred, leaving partial context propagation in that sub-system until a follow-up PR.
+
+#### Neutral
+- Callers that previously called the public APIs without a context must now supply one; any future consumer of this library (internal or external) must comply with the updated signatures.
+- The private wrapper functions in `add_package_manifest.go` (e.g., `downloadPackageFileFromGitHubForHost`, `listPackageDirFilesForHost`) gained `ctx` parameters alongside the public API changes; these are not breaking changes to exported symbols but do affect internal package coupling.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/cli/add_command.go

@@ -405,7 +405,7 @@ func compileAddedWorkflow(ctx context.Context, destFile string, workflowSpec *Wo
 	// parse the fully merged safe-outputs configuration to discover any dispatch workflows
 	// that originate from imported shared workflows (not visible in the raw frontmatter).
 	if !isLocalWorkflowPath(workflowSpec.WorkflowPath) {
-		fetchAndSaveDispatchWorkflowsFromParsedFile(destFile, workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker)
+		fetchAndSaveDispatchWorkflowsFromParsedFile(ctx, destFile, workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker)
 	}
 	// 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

pkg/cli/add_package_manifest.go

@@ -75,7 +75,7 @@ func (e packageRemoteNotFoundError) Unwrap() []error {
 	return []error{errRepositoryPackageFileNotFound, e.cause}
 }
 
-func resolveRepositoryPackage(repoSpec *RepoSpec, host string) (*resolvedRepositoryPackage, error) {
+func resolveRepositoryPackage(ctx context.Context, repoSpec *RepoSpec, host string) (*resolvedRepositoryPackage, error) {
 	parts := strings.SplitN(repoSpec.RepoSlug, "/", 2)
 	if len(parts) != 2 {
 		return nil, fmt.Errorf("invalid repository slug: %s", repoSpec.RepoSlug)
@@ -104,7 +104,7 @@ func resolveRepositoryPackage(repoSpec *RepoSpec, host string) (*resolvedReposit
 	}
 	packagePath := strings.Trim(repoSpec.PackagePath, "/")
 
-	manifestPath, manifestContent, err := loadRepositoryPackageManifestFile(owner, repo, packagePath, ref, host)
+	manifestPath, manifestContent, err := loadRepositoryPackageManifestFile(ctx, owner, repo, packagePath, ref, host)
 	if err != nil {
 		return nil, err
 	}
@@ -119,7 +119,7 @@ func resolveRepositoryPackage(repoSpec *RepoSpec, host string) (*resolvedReposit
 
 	installationSources := normalizePackageInstallablePaths(includeInstallablePaths, packagePath)
 	if len(installationSources) == 0 {
-		installationSources, err = scanRepositoryPackageInstallablePaths(owner, repo, packagePath, ref, host)
+		installationSources, err = scanRepositoryPackageInstallablePaths(ctx, owner, repo, packagePath, ref, host)
 		if err != nil {
 			return nil, err
 		}
@@ -128,15 +128,15 @@ func resolveRepositoryPackage(repoSpec *RepoSpec, host string) (*resolvedReposit
 		return nil, err
 	}
 
-	docsPath, err := resolveRepositoryPackageDocsPath(owner, repo, packagePath, ref, host)
+	docsPath, err := resolveRepositoryPackageDocsPath(ctx, owner, repo, packagePath, ref, host)
 	if err != nil {
 		return nil, err
 	}
 
 	// Resolve skill files: explicit from manifest or auto-scanned.
 	explicitSkillDirs := append([]string{}, manifest.Skills...)
 	explicitSkillDirs = append(explicitSkillDirs, includeSkillDirs...)
-	skillFiles, skillWarnings, err := resolvePackageSkillFiles(owner, repo, packagePath, ref, host, explicitSkillDirs)
+	skillFiles, skillWarnings, err := resolvePackageSkillFiles(ctx, owner, repo, packagePath, ref, host, explicitSkillDirs)
 	if err != nil {
 		return nil, err
 	}
@@ -145,7 +145,7 @@ func resolveRepositoryPackage(repoSpec *RepoSpec, host string) (*resolvedReposit
 	// Resolve agent files: explicit from manifest or auto-scanned.
 	explicitAgentFiles := append([]string{}, manifest.Agents...)
 	explicitAgentFiles = append(explicitAgentFiles, includeAgentFiles...)
-	agentFiles, agentWarnings, err := resolvePackageAgentFiles(owner, repo, packagePath, ref, host, explicitAgentFiles)
+	agentFiles, agentWarnings, err := resolvePackageAgentFiles(ctx, owner, repo, packagePath, ref, host, explicitAgentFiles)
 	if err != nil {
 		return nil, err
 	}
@@ -170,11 +170,11 @@ func resolveRepositoryPackage(repoSpec *RepoSpec, host string) (*resolvedReposit
 	}, nil
 }
 
-func loadRepositoryPackageManifestFile(owner, repo, packagePath, ref, host string) (string, []byte, error) {
+func loadRepositoryPackageManifestFile(ctx context.Context, owner, repo, packagePath, ref, host string) (string, []byte, error) {
 	manifestPath := joinRepositoryPackagePath(packagePath, repositoryPackageManifestFileName)
 	repoSlug := owner + "/" + repo
 	packageID := repositoryPackageIdentifier(repoSlug, packagePath)
-	content, err := downloadPackageFileFromGitHubForHost(owner, repo, manifestPath, ref, host)
+	content, err := downloadPackageFileFromGitHubForHost(ctx, owner, repo, manifestPath, ref, host)
 	if err != nil {
 		if !isRepositoryFileNotFound(err) {
 			return "", nil, fmt.Errorf("failed to read manifest %q from %s/%s@%s: %w", manifestPath, owner, repo, ref, err)
@@ -528,7 +528,7 @@ func agentDirectoryRoot(cleaned string) string {
 // skills/ directory is always auto-scanned for any additional skill subdirectories that
 // contain a SKILL.md file but are not already covered by the manifest. Each skill folder
 // is traversed recursively so that all nested files are included.
-func resolvePackageSkillFiles(owner, repo, packagePath, ref, host string, explicitSkillDirs []string) ([]resolvedPackageSkillFile, []string, error) {
+func resolvePackageSkillFiles(ctx context.Context, owner, repo, packagePath, ref, host string, explicitSkillDirs []string) ([]resolvedPackageSkillFile, []string, error) {
 	// seenSkillDirs tracks full skill directories already added so that auto-scanned
 	// duplicates of manifest-specified skills are not added a second time.
 	seenSkillDirs := make(map[string]struct{})
@@ -541,7 +541,7 @@ func resolvePackageSkillFiles(owner, repo, packagePath, ref, host string, explic
 	}
 
 	// Step 2: always auto-scan and append any skills not already in the manifest.
-	autoScanned, err := scanPackageSkillDirs(owner, repo, packagePath, ref, host)
+	autoScanned, err := scanPackageSkillDirs(ctx, owner, repo, packagePath, ref, host)
 	if err != nil {
 		// Auto-scan is supplementary for manifest-declared skills; preserve manifest
 		// resolution even when scan fails transiently.
@@ -579,7 +579,7 @@ func resolvePackageSkillFiles(owner, repo, packagePath, ref, host string, explic
 		// exists so that typos in the manifest surface as clear warnings.
 		if _, fromManifest := manifestSkillDirSet[skillDir]; fromManifest {
 			markerPath := joinRepositoryPackagePath(skillDir, packageSkillMarkerFile)
-			if _, err := downloadPackageFileFromGitHubForHost(owner, repo, markerPath, ref, host); err != nil {
+			if _, err := downloadPackageFileFromGitHubForHost(ctx, owner, repo, markerPath, ref, host); err != nil {
 				if isRepositoryFileNotFound(err) {
 					warnings = append(warnings, fmt.Sprintf("Skill directory %q is missing required %s marker file", skillDir, packageSkillMarkerFile))
 					continue
@@ -590,7 +590,7 @@ func resolvePackageSkillFiles(owner, repo, packagePath, ref, host string, explic
 		skillName := filepath.Base(skillDir)
 		// Use recursive listing so that the entire skill folder (including any
 		// subdirectories) is copied, not just the top-level files.
-		files, err := listPackageDirFilesRecursivelyForHost(owner, repo, ref, skillDir, host)
+		files, err := listPackageDirFilesRecursivelyForHost(ctx, owner, repo, ref, skillDir, host)
 		if err != nil {
 			if isRepositoryFileNotFound(err) {
 				warnings = append(warnings, fmt.Sprintf("Skill directory %q not found in package, skipping", skillDir))
@@ -611,7 +611,7 @@ func resolvePackageSkillFiles(owner, repo, packagePath, ref, host string, explic
 // resolvePackageAgentFiles returns the list of agent file source paths for a package.
 // If explicitAgentFiles is non-empty it is used; otherwise the agents/ directory is
 // auto-scanned for .md files.
-func resolvePackageAgentFiles(owner, repo, packagePath, ref, host string, explicitAgentFiles []string) ([]string, []string, error) {
+func resolvePackageAgentFiles(ctx context.Context, owner, repo, packagePath, ref, host string, explicitAgentFiles []string) ([]string, []string, error) {
 	if len(explicitAgentFiles) > 0 {
 		var agentFiles []string
 		for _, f := range explicitAgentFiles {
@@ -623,7 +623,7 @@ func resolvePackageAgentFiles(owner, repo, packagePath, ref, host string, explic
 	var agentFiles []string
 	for _, root := range []string{packageAgentsDirectory, constants.GithubDir + packageAgentsDirectory} {
 		agentsDir := joinRepositoryPackagePath(packagePath, root)
-		files, err := listPackageDirFilesForHost(owner, repo, ref, agentsDir, host)
+		files, err := listPackageDirFilesForHost(ctx, owner, repo, ref, agentsDir, host)
 		if err != nil {
 			if isRepositoryFileNotFound(err) {
 				continue
@@ -641,11 +641,11 @@ func resolvePackageAgentFiles(owner, repo, packagePath, ref, host string, explic
 
 // scanPackageSkillDirs auto-scans the skills/ directory of a package and returns the paths
 // of skill subdirectories (those that contain a SKILL.md file).
-func scanPackageSkillDirs(owner, repo, packagePath, ref, host string) ([]string, error) {
+func scanPackageSkillDirs(ctx context.Context, owner, repo, packagePath, ref, host string) ([]string, error) {
 	var skillDirs []string
 	for _, root := range []string{packageSkillsDirectory, constants.GithubDir + packageSkillsDirectory} {
 		skillsDir := joinRepositoryPackagePath(packagePath, root)
-		subdirs, err := listPackageDirSubdirsForHost(owner, repo, ref, skillsDir, host)
+		subdirs, err := listPackageDirSubdirsForHost(ctx, owner, repo, ref, skillsDir, host)
 		if err != nil {
 			if isRepositoryFileNotFound(err) {
 				continue
@@ -654,21 +654,21 @@ func scanPackageSkillDirs(owner, repo, packagePath, ref, host string) ([]string,
 		}
 		for _, subdir := range subdirs {
 			markerPath := joinRepositoryPackagePath(subdir, packageSkillMarkerFile)
-			if _, err := downloadPackageFileFromGitHubForHost(owner, repo, markerPath, ref, host); err == nil {
+			if _, err := downloadPackageFileFromGitHubForHost(ctx, owner, repo, markerPath, ref, host); err == nil {
 				skillDirs = append(skillDirs, subdir)
 			}
 		}
 	}
 	return skillDirs, nil
 }
 
-func scanRepositoryPackageInstallablePaths(owner, repo, packagePath, ref, host string) ([]string, error) {
+func scanRepositoryPackageInstallablePaths(ctx context.Context, owner, repo, packagePath, ref, host string) ([]string, error) {
 	var collected []string
 	seen := make(map[string]struct{})
 
 	for _, sourceDir := range packageSourceDirectories {
 		sourcePath := joinRepositoryPackagePath(packagePath, sourceDir)
-		files, err := listPackageWorkflowFilesForHost(owner, repo, ref, sourcePath, host)
+		files, err := listPackageWorkflowFilesForHost(ctx, owner, repo, ref, sourcePath, host)
 		if err != nil {
 			if isRepositoryFileNotFound(err) {
 				continue
@@ -699,11 +699,11 @@ func scanRepositoryPackageInstallablePaths(owner, repo, packagePath, ref, host s
 	return collected, nil
 }
 
-func resolveRepositoryPackageDocsPath(owner, repo, packagePath, ref, host string) (string, error) {
+func resolveRepositoryPackageDocsPath(ctx context.Context, owner, repo, packagePath, ref, host string) (string, error) {
 	readmePath := joinRepositoryPackagePath(packagePath, "README.md")
 	repoSlug := owner + "/" + repo
 	packageID := repositoryPackageIdentifier(repoSlug, packagePath)
-	if _, err := downloadPackageFileFromGitHubForHost(owner, repo, readmePath, ref, host); err == nil {
+	if _, err := downloadPackageFileFromGitHubForHost(ctx, owner, repo, readmePath, ref, host); err == nil {
 		return readmePath, nil
 	} else if isRepositoryFileNotFound(err) {
 		return "", fmt.Errorf("repository %q is not a valid Agentic Workflow package: missing required README.md at %q", packageID, readmePath)
@@ -877,28 +877,28 @@ func validateUniqueManifestWorkflowFilenames(paths []string, manifestPath string
 	return nil
 }
 
-func downloadRepositoryPackageFileFromGitHubForHost(owner, repo, path, ref, host string) ([]byte, error) {
-	content, err := parser.DownloadFileFromGitHubForHost(owner, repo, path, ref, host)
+func downloadRepositoryPackageFileFromGitHubForHost(ctx context.Context, owner, repo, path, ref, host string) ([]byte, error) {
+	content, err := parser.DownloadFileFromGitHubForHost(ctx, owner, repo, path, ref, host)
 	return content, normalizeRepositoryPackageRemoteError(err)
 }
 
-func listRepositoryPackageWorkflowFilesForHost(owner, repo, ref, workflowPath, host string) ([]string, error) {
-	files, err := parser.ListWorkflowFilesForHost(owner, repo, ref, workflowPath, host)
+func listRepositoryPackageWorkflowFilesForHost(ctx context.Context, owner, repo, ref, workflowPath, host string) ([]string, error) {
+	files, err := parser.ListWorkflowFilesForHost(ctx, owner, repo, ref, workflowPath, host)
 	return files, normalizeRepositoryPackageRemoteError(err)
 }
 
-func listRepositoryPackageDirFilesForHost(owner, repo, ref, dirPath, host string) ([]string, error) {
-	files, err := parser.ListDirAllFilesForHost(owner, repo, ref, dirPath, host)
+func listRepositoryPackageDirFilesForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) {
+	files, err := parser.ListDirAllFilesForHost(ctx, owner, repo, ref, dirPath, host)
 	return files, normalizeRepositoryPackageRemoteError(err)
 }
 
-func listRepositoryPackageDirFilesRecursivelyForHost(owner, repo, ref, dirPath, host string) ([]string, error) {
-	files, err := parser.ListDirAllFilesRecursivelyForHost(owner, repo, ref, dirPath, host)
+func listRepositoryPackageDirFilesRecursivelyForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) {
+	files, err := parser.ListDirAllFilesRecursivelyForHost(ctx, owner, repo, ref, dirPath, host)
 	return files, normalizeRepositoryPackageRemoteError(err)
 }
 
-func listRepositoryPackageDirSubdirsForHost(owner, repo, ref, dirPath, host string) ([]string, error) {
-	dirs, err := parser.ListDirSubdirsForHost(owner, repo, ref, dirPath, host)
+func listRepositoryPackageDirSubdirsForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) {
+	dirs, err := parser.ListDirSubdirsForHost(ctx, owner, repo, ref, dirPath, host)
 	return dirs, normalizeRepositoryPackageRemoteError(err)
 }