Skip to content

fix: propagate context through all CLI subprocess calls for Ctrl-C cancellation#26372

Merged
dsyme merged 7 commits intomainfrom
fix/ctx-propagation-ctrl-c
Apr 15, 2026
Merged

fix: propagate context through all CLI subprocess calls for Ctrl-C cancellation#26372
dsyme merged 7 commits intomainfrom
fix/ctx-propagation-ctrl-c

Conversation

@dsyme
Copy link
Copy Markdown
Collaborator

@dsyme dsyme commented Apr 15, 2026

Problem

Pressing Ctrl-C during long-running gh aw commands (e.g. audit, logs, update) would register the interrupt but background operations — artifact downloads, log fetches, workflow file downloads, git operations — would continue printing output long after the user had returned to the prompt.

The root cause: signal.NotifyContext in main.go sets up context cancellation correctly, but many functions deep in pkg/cli/ called workflow.RunGH / RunGHCombined (non-context variants) or bare exec.Command (no context), so the cancellation signal never reached those subprocesses.

Changes

Thread context.Context through the full call chain for all gh CLI subprocess invocations and git operations:

  • pkg/workflow/github_cli.go — add RunGHCombinedContext
  • pkg/cli/download_workflow.go — add ctx to downloadWorkflowContent, downloadWorkflowContentViaGit, downloadWorkflowContentViaGitClone; all exec.Commandexec.CommandContext
  • pkg/cli/update_workflows.go — pass ctx to all downloadWorkflowContent calls
  • pkg/cli/update_actions.goexec.Command("git", "ls-remote", ...)exec.CommandContext
  • pkg/cli/audit.go — add ctx to fetchWorkflowRunMetadata, resolveWorkflowDisplayName, renderAuditReport
  • pkg/cli/audit_comparison.go — add ctx to findPreviousSuccessfulWorkflowRuns, buildAuditComparisonForRun
  • pkg/cli/audit_diff.go — add ctx to loadRunSummaryForDiff
  • pkg/cli/audit_diff_command.go — pass ctx to loadRunSummaryForDiff calls
  • pkg/cli/logs_download.go — add ctx to downloadWorkflowRunLogs, listRunArtifactNames, downloadArtifactsByName, retryCriticalArtifacts, downloadRunArtifacts; use ExecGHContext / RunGHContext
  • pkg/cli/logs_orchestrator.go — pass ctx to downloadRunArtifacts in the concurrent pool goroutine
  • pkg/cli/trial_runner.go — add ctx to getCurrentGitHubUsername
  • All affected test files updated accordingly

This is a continuation of the earlier fix that threaded context through the update command pipeline (update_actions.go, update_workflows.go, update_command.go, upgrade_command.go).

…ncellation

Thread context.Context through the full call chain in pkg/cli/ so that
Ctrl-C (via signal.NotifyContext in main.go) correctly terminates all
background gh CLI subprocesses and git operations.

Previously, many functions called workflow.RunGH / RunGHCombined (no
context) or exec.Command (no context), so Ctrl-C had no effect on
in-flight network operations like artifact downloads, log fetches, and
workflow file downloads.

Changes:
- pkg/workflow/github_cli.go: add RunGHCombinedContext
- pkg/cli/download_workflow.go: add ctx to downloadWorkflowContent,
  downloadWorkflowContentViaGit, downloadWorkflowContentViaGitClone;
  all exec.Command -> exec.CommandContext
- pkg/cli/update_workflows.go: pass ctx to all downloadWorkflowContent calls
- pkg/cli/update_actions.go: exec.Command(git ls-remote) -> exec.CommandContext
- pkg/cli/audit.go: add ctx to fetchWorkflowRunMetadata,
  resolveWorkflowDisplayName, renderAuditReport
- pkg/cli/audit_comparison.go: add ctx to findPreviousSuccessfulWorkflowRuns,
  buildAuditComparisonForRun
- pkg/cli/audit_diff.go: add ctx to loadRunSummaryForDiff
- pkg/cli/audit_diff_command.go: pass ctx to loadRunSummaryForDiff calls
- pkg/cli/logs_download.go: add ctx to downloadWorkflowRunLogs,
  listRunArtifactNames, downloadArtifactsByName, retryCriticalArtifacts,
  downloadRunArtifacts; use ExecGHContext / RunGHContext
- pkg/cli/logs_orchestrator.go: pass ctx to downloadRunArtifacts in pool
- pkg/cli/trial_runner.go: add ctx to getCurrentGitHubUsername
- All affected test files updated accordingly
Copilot AI review requested due to automatic review settings April 15, 2026 05:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR ensures Ctrl-C cancellation propagates through the full CLI call chain by threading context.Context into gh/git subprocess execution paths, preventing background operations from continuing after an interrupt.

Changes:

  • Added context-aware gh helpers (e.g., RunGHCombinedContext) and updated call sites to use *Context variants.
  • Threaded context.Context through workflow update, audit, logs download, and trial execution code paths.
  • Updated unit tests to match the new context-aware function signatures.
Show a summary per file
File Description
pkg/workflow/github_cli.go Adds RunGHCombinedContext to support cancelable combined-output gh calls with spinner.
pkg/cli/upgrade_command.go Passes ctx into action update pipeline during upgrade.
pkg/cli/update_workflows.go Threads ctx through workflow update resolution and gh api calls.
pkg/cli/download_workflow.go Converts git subprocesses to exec.CommandContext and threads ctx through download helpers.
pkg/cli/update_command.go Passes ctx into workflow/action update steps.
pkg/cli/update_actions.go Adds ctx to action update + action-ref rewriting pipelines; uses context-aware gh/git subprocess calls.
pkg/cli/trial_runner.go Threads ctx into gh api user lookup for default trial repo selection.
pkg/cli/logs_orchestrator.go Passes ctx into concurrent artifact download tasks.
pkg/cli/logs_download.go Threads ctx through log/artifact download helpers; uses ExecGHContext / RunGHContext.
pkg/cli/audit.go Threads ctx through metadata fetch, display-name resolution, and report rendering.
pkg/cli/audit_comparison.go Adds ctx to comparison discovery and baseline processing paths.
pkg/cli/audit_diff.go Threads ctx through run-summary loading (artifact download path).
pkg/cli/audit_diff_command.go Passes ctx into diff summary loading calls.
pkg/cli/update_workflows_test.go Updates tests for context-aware releases API stub + resolver signatures.
pkg/cli/update_actions_test.go Updates tests for context-aware action-release/tag resolution stubs and call sites.
pkg/cli/update_command_test.go Updates tests for context-aware action/workflow resolution helpers.
pkg/cli/logs_download_test.go Updates tests for context-aware retry helper signature.
pkg/cli/audit_test.go Updates tests for context-aware audit helpers.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 19/19 changed files
  • Comments generated: 2

Comment thread pkg/cli/audit_comparison.go
Comment thread pkg/cli/update_actions.go Outdated
@github-actions
Copy link
Copy Markdown
Contributor

Hey @dsyme 👋 — great work threading context.Context through the full CLI subprocess call chain to fix Ctrl-C cancellation! This is exactly the kind of focused, well-scoped fix the project needs, and the comprehensive coverage across audit, logs, update, and download_workflow code paths is thorough.

The PR looks solid — it's well-described, includes test updates across all affected files, and doesn't introduce any new dependencies. This looks ready for maintainer review! 🎉

Generated by Contribution Check · ● 1.5M ·

@dsyme dsyme merged commit e44f4e7 into main Apr 15, 2026
57 checks passed
@dsyme dsyme deleted the fix/ctx-propagation-ctrl-c branch April 15, 2026 06:04
Copilot AI added a commit that referenced this pull request Apr 15, 2026
…files

- Export GH_AW_NODE_BIN from install_awf_binary.sh when bundle installed
- Use ${GH_AW_NODE_BIN:-node} in copilot AWF execution to fix node not found
  on aw-gpu-runner-T4 where sudo resets PATH
- Regenerate all 191 stale lock files from PR #26372 regression
- Update golden test files for new command format

Fixes: node: command not found in Daily Issues Report (#26393)
Fixes: 18 stale lock files from context propagation refactor (#26372)

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d5f4442b-0f77-44b6-87f9-be3cba9165a0

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan added a commit that referenced this pull request Apr 15, 2026
…found` on GPU runners + regenerate stale lock files (#26427)

* Initial plan

* fix: use absolute node path in AWF command and regenerate stale lock files

- Export GH_AW_NODE_BIN from install_awf_binary.sh when bundle installed
- Use ${GH_AW_NODE_BIN:-node} in copilot AWF execution to fix node not found
  on aw-gpu-runner-T4 where sudo resets PATH
- Regenerate all 191 stale lock files from PR #26372 regression
- Update golden test files for new command format

Fixes: node: command not found in Daily Issues Report (#26393)
Fixes: 18 stale lock files from context propagation refactor (#26372)

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d5f4442b-0f77-44b6-87f9-be3cba9165a0

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* fix: resolve node path inline in PathSetup before sudo launches AWF

Instead of writing GH_AW_NODE_BIN to GITHUB_ENV in install_awf_binary.sh,
capture the absolute node path inline in the AWF execution step's PathSetup
(before sudo resets PATH). sudo -E preserves the exported var, and AWF's
--env-all forwards it into the container where ${GH_AW_NODE_BIN:-node}
resolves to the correct binary.

- Revert install_awf_binary.sh GITHUB_ENV export
- Add GH_AW_NODE_BIN resolution to PathSetup in copilot_engine_execution.go
- Update golden test fixtures and regenerate all 191 lock files

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/54d6db18-494d-4caf-97f8-b62b0157dfef

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* Add changeset

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants