Repository navigation
Replace cli/cli api dependency with go-gh - #72
Open
williammartin wants to merge 1 commit into
Open
williammartin wants to merge 1 commit into
williammartin wants to merge 1 commit into
Conversation
Replace cli/cli's api.Client and the run/workflow shared packages with local types and helpers built on go-gh's RESTClient. Only cli/cli's iostreams and cmdutil are still used. Tests run against an httptest server instead of httpmock. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
williammartin
marked this pull request as ready for review
October 2, 2026 12:33
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Hihi, thanks for creating an extension for the GitHub CLI. I'm a maintainer for
gh. We were doing some work on short lived tokens and I noticed you are depending on theapipackage from thecli/climodule. Historically, the maintainers did a bad job of setting expectations around importing packages from this module. I'm strongly considering biting the bullet and moving packages intointernalto reduce our ongoing maintenance burden, but I'd prefer not to just break everyone.This PR removes the dependency on
github.com/cli/cli/v2/api(andpkg/httpmock) in favour ofgithub.com/cli/go-gh/v2'sapi.RESTClient, which we do expect to follow semantic versioning.pkg/iostreamsandpkg/cmdutilare still imported fromcli/cli. Replacingiostreamsmeans copying in a lot of terminal handling code, so I've left it out to keep this PR small. If anything, it indicates an opportunity to create new abstractions ingo-gh.Run/workflow types
The upstream
run/sharedfetch helpers (GetRun,GetJobs,GetAnnotations, etc.) all take thecli/cliapi.Client, so they're replaced with small go-gh equivalents inruns.go.I also redefined the handful of types and render helpers this extension uses instead of importing them from
run/sharedandworkflow/shared:Run,Job,Step,Annotation,RenderJobs,RenderAnnotationsandSymbol. Upstreamshared.Runkeeps the workflow name in an unexported field that onlycli/cli's ownGetRuncan populate. Reusing it would render an empty workflow name in the run header.Behaviour change: per-host auth
Previously the HTTP client was built for the default host (
GH_HOST, the sole logged-in host, orgithub.com), while the request URL was derived from the--repohost. go-gh only attaches theAuthorizationheader when the request host matches the client's host, so-R ghes.example.com/o/rwas sent unauthenticated.The client is now built with
Host: repo.RepoHost(), so the target host's token is used. I haven't exercised this against a GHES instance.-R o/r/-R github.com/o/r-R ghes.example.com/o/r, logged in to GHES-R ghes.example.com/o/r, not logged in to GHESauthentication token not found for host ghes.example.comRelatedly, the current user's login (used to find the dispatched run) now comes from REST
GET /userinstead of GraphQL.Test Change
Since I wanted to drop
httpmockusage, I swapped it for thehttptestpackage. A smaller change would have been a simple replacement transport stubber but honestly, I thinkhttptestis just a better black box approach. Can back it out if you feel differently! The expected output in the tests is unchanged.Caveat
Copilot did most of the work here so I can't say I've given it extreme scrutiny but I did guide it along the way and it looks generally correct to me.
Demo
These GIFs were recorded on github.com with a build from this PR, against a scratch repo containing copies of this repo's
HelloandGoodbyeworkflows. I also recorded the failure scenario with a build ofmain, and its output matched, including exit code1.gh dispatch workflow, success:gh dispatch repository, success:gh dispatch workflow, failure:Two things in these recordings are pre-existing on
mainand unchanged here:Error: SilentError.