[repository-quality] Repository Quality Improvement Report - GH CLI Wrapper Context Propagation Gap (2026-07-31) #49349
Closed
Replies: 1 comment
|
This discussion was automatically closed because it expired on 2026-08-01T13:27:17.699Z.
|
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
🎯 Repository Quality Improvement Report - GH CLI Wrapper Context Propagation Gap
Analysis Date: 2026-07-31
Focus Area: GH CLI Wrapper Context Propagation Gap
Strategy Type: Custom
Custom Area: Yes — a prior run ("Context Propagation & Process Cancellability", 2026-06-03/06-09) tackled bare
exec.Commandcalls broadly, but a distinct, narrower gap remains specifically inpkg/workflow/github_cli.go's publicghCLI wrapper API. Context-aware variants (*Contextsuffix) were added, yet the original non-context wrappers were kept and are still the majority choice at call sites acrosspkg/cli, meaning long-runninggh api/gh pr/gh reposubprocess calls in CLI commands (clone, fork, create PR, merge, secrets) still cannot be cancelled or bounded by a caller-supplied timeout/deadline.Executive Summary
pkg/workflow/github_cli.gooffers two parallel APIs for everyghCLI wrapper: a legacy signature (ExecGH,RunGH,RunGHCombined,RunGHWithHost) that hardcodescontext.Background(), and a context-aware counterpart (ExecGHContext,RunGHContext,RunGHCombinedContext,RunGHContextWithHost) that accepts a caller context. Both are exported and undocumented as to which one is preferred, so new code keeps reaching for the shorter, non-context signature. Grep counts show the non-context variants still outnumber their context-aware siblings at call sites (ExecGH30 vsExecGHContext20,RunGH30 vsRunGHContext17,RunGHCombined17 vsRunGHCombinedContext11,RunGHWithHost8 vsRunGHContextWithHostonly 1), meaning roughly 60% ofghsubprocess invocations acrosspkg/cliare not cancellable or time-boxed.This matters concretely for commands like
pr_command.go's repo clone/fork/PR-create flows,trial_repository.go's repository create/delete, andengine_secrets.go's secret listing — all of which can hang indefinitely on network stalls with no way for an enclosing command (Ctrl-C, CLI-level timeout flag, or test harness deadline) to interrupt them. The fix is mechanical and low-risk: migrate call sites to the*Contextvariants usingcmd.Context()(Cobra) or a boundedcontext.WithTimeout, then deprecate/remove the non-context wrappers to close the gap permanently rather than let it regrow.Full Analysis Report
Focus Area: GH CLI Wrapper Context Propagation Gap
Current State Assessment
pkg/workflow/github_cli.godefines a dual API surface:The same pattern repeats for
RunGH/RunGHContext,RunGHCombined/RunGHCombinedContext, andRunGHWithHost/RunGHContextWithHost. Every non-context wrapper is a one-line pass-through tocontext.Background(), so migrating callers is mechanical (swap function name, thread an existingctxfrom the surrounding Cobra command or caller).Metrics Collected:
ExecGHvsExecGHContextcallsitesRunGHvsRunGHContextcallsitesRunGHCombinedvsRunGHCombinedContextcallsitesRunGHWithHostvsRunGHContextWithHostcallsitescontext.Background()/context.TODO()in prod codectxbackground) exists and registeredpkg/linters/spec_test.goFindings
Strengths
github_cli.golines ~110-232).ctxbackgroundstatic analyzer already flagscontext.Background()inside functions that already receive a context, giving infrastructure to catch regressions.cmd.Context()is readily available in everypkg/clicommand, so migration requires no new plumbing — just passing it through.Areas for Improvement
RunGHWithHost(8 callsites, e.g.pr_command.go:771,793) has almost no context-aware adoption (1/9, 11%) despite being used for cross-host repo/PR operations against GHES/data-residency hosts, which are exactly the operations most likely to need timeouts.pr_command.go,trial_repository.go,add_interactive_git.go) mix context and non-context calls in the same file (e.g.pr_command.gocalls bothRunGHand would benefit from consistently threadingctx), making it unclear which pattern is "current" for new contributors.// Deprecated:) on the non-context wrappers, so nothing signals to authors that the context variant is preferred — the API surface silently invites the legacy call.RunGHInputContexthas no non-context sibling, proving context-first design is already the intended pattern for newer additions — this should be retrofitted onto the older wrapper family.Detailed Analysis
The root cause is API design: adding
*Contextvariants alongside (rather than replacing) the originals avoided a breaking change but left a permanent temptation to use the shorter, no-context signature — which is exactly what happened. Becausepkg/clicommands almost always have acmd *cobra.Commandin scope with a livecmd.Context(), there is no technical barrier to full migration; this is purely an adoption/enforcement gap. Thectxbackgroundlinter, which would catchcontext.Background()calls inside a function already holding a context, doesn't fire ongithub_cli.goitself because the wrapper functions (ExecGH,RunGH, etc.) don't accept a context parameter — the linter is structurally blind to this specific class of API-level, rather than local, hardcoding. Closing this requires both call-site migration and either deprecating or removing the non-context wrappers so the gap cannot silently reopen.🤖 Tasks for Copilot Agent
NOTE TO PLANNER AGENT: Split the following tasks into individual work items.
Improvement Tasks
Task 1: Migrate
pr_command.gogh CLI calls to context-aware wrappersPriority: High
Estimated Effort: Medium
Focus Area: GH CLI Wrapper Context Propagation Gap
Description:
pkg/cli/pr_command.gocontains 15+ call sites usingRunGH,ExecGH, andRunGHWithHost(lines 109, 118, 144, 152, 161, 178, 216, 265, 514, 611, 642, 673, 771, 793) for fork/clone/PR-create/PR-diff operations against GitHub. None of these can be cancelled if the enclosing Cobra command is interrupted. Migrate every call site to its*Contextcounterpart (RunGHContext,ExecGHContext,RunGHContextWithHost), threadingcmd.Context()from the owning CobraRunEfunction down to each call.Acceptance Criteria:
workflow.RunGH(,workflow.ExecGH(,workflow.RunGHWithHost(calls inpkg/cli/pr_command.goreplaced with their*Contextequivalents, passing a realctx(fromcmd.Context()or a threaded parameter)pkg/cli/pr_command_test.gopass unchangedmake fmtrun after editsCode Region:
pkg/cli/pr_command.go(lines 109-793, allworkflow.RunGH*/ExecGH*calls)Task 2: Migrate
trial_repository.goandadd_interactive_git.goto context-aware gh wrappersPriority: High
Estimated Effort: Medium
Focus Area: GH CLI Wrapper Context Propagation Gap
Description:
pkg/cli/trial_repository.go(repo create/delete/discussions-enable, lines 83-210) andpkg/cli/add_interactive_git.go(branch lookup, PR merge/update, lines 207-326) exclusively use non-contextRunGHCombined/ExecGHcalls for potentially slow network operations (repo creation, PR merge). These are prime candidates for user-triggered cancellation (Ctrl-C during interactive trial setup) which currently cannot propagate.Acceptance Criteria:
workflow.RunGHCombined(/workflow.ExecGH(calls in both files replaced withRunGHCombinedContext/ExecGHContext, withctxthreaded from the calling commandghcall if not already coveredCode Region:
pkg/cli/trial_repository.go(lines 83-210),pkg/cli/add_interactive_git.go(lines 207-326)Task 3: Add
// Deprecated:markers to non-context gh wrapper functionsPriority: Medium
Estimated Effort: Small
Focus Area: GH CLI Wrapper Context Propagation Gap
Description: To stop new code from reaching for the legacy signatures, mark
ExecGH,RunGH,RunGHCombined, andRunGHWithHostinpkg/workflow/github_cli.gowith standard Go// Deprecated:doc comments pointing to their*Contextreplacements. This makesstaticcheck/IDE tooling surface a warning at every remaining and future call site without requiring an immediate breaking removal.Acceptance Criteria:
// Deprecated: use <XContext> instead, which accepts a context.Context for cancellation and timeout support.comment linego vet ./.../staticcheck(if configured) shows no new failures beyond the expected deprecation noticesCode Region:
pkg/workflow/github_cli.go(functionsExecGH,RunGH,RunGHCombined,RunGHWithHost)Task 4: Migrate remaining
pkg/cligh wrapper call sites (secrets, checks, enable, automerge, rate-limit)Priority: Medium
Estimated Effort: Large
Focus Area: GH CLI Wrapper Context Propagation Gap
Description: Beyond the two hot files covered in Tasks 1-2, remaining legacy call sites exist in
pkg/cli/add_interactive_secrets.go,pkg/cli/enable.go,pkg/cli/checks_command.go,pkg/cli/pr_automerge.go,pkg/cli/engine_secrets.go,pkg/cli/logs_rate_limit.go, andpkg/cli/outcome_eval_pr.go. These should be migrated in one sweep to fully close the adoption gap for allpkg/cliproduction code, bringing legacy usage to zero.Acceptance Criteria:
workflow.RunGH(/workflow.ExecGH(/workflow.RunGHCombined(/workflow.RunGHWithHost(calls inpkg/cli(excluding_test.gofiles) migrated to*Contextequivalentsgrep -rn "workflow\.\(ExecGH\|RunGH\|RunGHCombined\|RunGHWithHost\)(" pkg/cli --include='*.go' | grep -v _test.goreturns zero results after the changemake agent-report-progress-no-testsucceeds (build + lint pass)Code Region:
pkg/cli/add_interactive_secrets.go,pkg/cli/enable.go,pkg/cli/checks_command.go,pkg/cli/pr_automerge.go,pkg/cli/engine_secrets.go,pkg/cli/logs_rate_limit.go,pkg/cli/outcome_eval_pr.go📊 Historical Context
Previous Focus Areas
fmt.Errorfcalls losing%wwrapping🎯 Recommendations
Immediate Actions (This Week)
pr_command.goandtrial_repository.go/add_interactive_git.gogh wrapper call sites to*Contextvariants (Tasks 1-2) — Priority: HighShort-term Actions (This Month)
// Deprecated:markers to legacy non-context wrappers (Task 3) — Priority: Mediumpkg/clicall sites for full adoption (Task 4) — Priority: MediumLong-term Actions (This Quarter)
ctxbackgroundlinter (or add a new analyzer) to flag exported functions whose only purpose is to call an internal*Contextsibling withcontext.Background(), catching this API-design anti-pattern automatically in future code — Priority: Low📈 Success Metrics
ExecGH/ExecGHContextadoption ratio: 40% context-aware → 100% context-awareRunGHWithHost/RunGHContextWithHostadoption ratio: 11% context-aware → 100% context-awarepkg/cli(non-test): ~65 → 0Next Steps
Generated by Repository Quality Improvement Agent
Next analysis: 2026-08-01 — Focus area selected by diversity algorithm
All reactions