Route extension requests through api.Client - #14059
Merged
Merged
Conversation
Co-authored-by: Copilot App <[email protected]> Copilot-Session: 598c3576-22dc-4d7f-a72d-4f84922174a1
Co-authored-by: Copilot App <[email protected]> Copilot-Session: d5151527-3286-43fd-a420-38f496a076fa
Co-authored-by: Copilot App <[email protected]> Copilot-Session: 598c3576-22dc-4d7f-a72d-4f84922174a1
Co-authored-by: Copilot App <[email protected]> Copilot-Session: a237433f-9657-4224-bad4-c4dc1c0fdc31
williammartin
force-pushed
the
williammartin-route-extension-http
branch
from
August 4, 2026 09:11
38bd255 to
73eb0a7
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the effort to consolidate GitHub API interactions behind api.Client by migrating the gh extension command’s repository and release lookup helpers to use api.NewClientFromHTTP(...).REST(...) instead of manually constructing URLs and calling http.Client directly. The intent is to keep user-visible behavior the same while enabling cross-cutting API request behavior in one place.
Changes:
- Migrates extension lookup helpers (
repoExists,hasScript,fetchLatestRelease,fetchReleaseFromTag) to route REST requests throughapi.Client. - Updates existing extension install tests to return JSON for the
/contents/...endpoint, matching the real API shape. - Adds a new
http_test.gofile to cover the migrated helpers’ success and error paths.
Show a summary per file
| File | Description |
|---|---|
| pkg/cmd/extension/manager_test.go | Updates /contents/... stubs to return JSON objects instead of a plain string body. |
| pkg/cmd/extension/http.go | Switches selected helper functions from raw http.Client usage to api.Client.REST, including 404 handling via api.HTTPError. |
| pkg/cmd/extension/http_test.go | Adds unit tests for the migrated helpers’ primary and error behaviors. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
Co-authored-by: Copilot App <[email protected]> Copilot-Session: a237433f-9657-4224-bad4-c4dc1c0fdc31
Co-authored-by: Copilot App <[email protected]> Copilot-Session: a237433f-9657-4224-bad4-c4dc1c0fdc31
Co-authored-by: Copilot App <[email protected]> Copilot-Session: a237433f-9657-4224-bad4-c4dc1c0fdc31
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (3)
pkg/cmd/extension/http.go:130
- The TODO follow-up comment is incomplete (it ends with "constructed" and lacks a period). Finishing the sentence makes the deferred-work rationale clearer.
// TODO(api-client-rollout)
// This line of code is part of a mechanical roll out of the api client.
// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
pkg/cmd/extension/http.go:158
- The TODO follow-up comment is incomplete (it ends with "constructed" and lacks a period). Please finish the sentence for clarity and consistency.
// TODO(api-client-rollout)
// This line of code is part of a mechanical roll out of the api client.
// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
pkg/cmd/extension/http.go:57
- The TODO follow-up comment is incomplete (it ends with "constructed" and lacks a period), which makes the intent harder to read later. Please finish the sentence and add consistent punctuation/capitalization.
This issue also appears in the following locations of the same file:
- line 128
- line 156
// TODO(api-client-rollout)
// This line of code is part of a mechanical roll out of the api client.
// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
williammartin
force-pushed
the
williammartin-route-extension-http
branch
from
August 4, 2026 11:32
9289b7f to
420a538
Compare
williammartin
marked this pull request as ready for review
August 4, 2026 11:33
tidy-dev
approved these changes
Aug 4, 2026
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Aug 21, 2026
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.97.0` → `v2.98.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.98.0`](https://github.com/cli/cli/releases/tag/v2.98.0): GitHub CLI 2.98.0 [Compare Source](cli/cli@v2.97.0...v2.98.0) #### Security A security vulnerability has been identified, and fixed, that binds the local forwarded port to all available network interfaces by default. Users of `gh codespace ports forward` are advised to update `gh` to version `v2.98.0` as soon as possible. For more information see: <GHSA-vfhh-p7hm-pxfh> #### Support worktrees in `pr checkout` Users can now checkout a pull request into a git worktree by using the new `--worktree PATH` flag in `gh pr checkout`: ```shell gh pr checkout 12 --worktree ../wt-feature ``` #### Add semantic search to `search issues` The `gh search issues` command now supports semantic search for issues. Users can select the search type by passing the `--search-type` flag: ```shell gh search issues --search-type semantic ... gh search issues --search-type hybrid ... ``` For more information about semantic search see: ["Improved Search for github issues is now generally available"](https://github.blog/changelog/2026-04-02-improved-search-for-github-issues-is-now-generally-available/). #### What's Changed ##### ✨ Features - Add --worktree flag to gh pr checkout by [@​tidy-dev](https://github.com/tidy-dev) in [#​13946](cli/cli#13946) - Set GH\_EXTENSION=1 when gh invokes an extension by [@​williammartin](https://github.com/williammartin) in [#​14072](cli/cli#14072) - Add --search-type flag for semantic and hybrid issue search by [@​michaeljacholke](https://github.com/michaeljacholke) in [#​14006](cli/cli#14006) ##### 🐛 Fixes - Fix `RESTWithNext` error type, repairing `gh status` and attestation retries by [@​williammartin](https://github.com/williammartin) in [#​13988](cli/cli#13988) - Trim spaces when parsing X-Oauth-Scopes in `gh release create` by [@​williammartin](https://github.com/williammartin) in [#​14065](cli/cli#14065) - Fix project item-add output for non-TTY by [@​zwick](https://github.com/zwick) in [#​14056](cli/cli#14056) ##### 📚 Docs & Chores - Slim down dependabot triage comments by [@​williammartin](https://github.com/williammartin) in [#​14019](cli/cli#14019) - Require explicit MR review ownership by [@​williammartin](https://github.com/williammartin) in [#​14028](cli/cli#14028) - Collapse spam triage into the agentic issue-triage workflow by [@​williammartin](https://github.com/williammartin) in [#​14027](cli/cli#14027) - Run Dependabot triage every hour by [@​sergiou87](https://github.com/sergiou87) in [#​14030](cli/cli#14030) - Route deploy key requests through api.Client by [@​williammartin](https://github.com/williammartin) in [#​13989](cli/cli#13989) - Route ssh key requests through api.Client by [@​williammartin](https://github.com/williammartin) in [#​13994](cli/cli#13994) - Route gpg key requests through api.Client by [@​williammartin](https://github.com/williammartin) in [#​13997](cli/cli#13997) - Route autolink requests through api.Client by [@​williammartin](https://github.com/williammartin) in [#​14013](cli/cli#14013) - Route extension requests through api.Client by [@​williammartin](https://github.com/williammartin) in [#​14059](cli/cli#14059) - Route release creation through api.Client by [@​williammartin](https://github.com/williammartin) in [#​14062](cli/cli#14062) - Tell agents to use the MR template in AGENTS.md by [@​williammartin](https://github.com/williammartin) in [#​14074](cli/cli#14074) - Make Dependabot triage cheaper and more decisive by [@​williammartin](https://github.com/williammartin) in [#​14079](cli/cli#14079) - Route release deletions through api.Client by [@​williammartin](https://github.com/williammartin) in [#​14077](cli/cli#14077) - Give Dependabot triage a real reachability check by [@​williammartin](https://github.com/williammartin) in [#​14087](cli/cli#14087) - Restore automatic spam issue closure by [@​williammartin](https://github.com/williammartin) in [#​14088](cli/cli#14088) - Add a scheduled tech debt burndown skill by [@​williammartin](https://github.com/williammartin) in [#​14095](cli/cli#14095) - Use reflect.Pointer instead of deprecated reflect.Ptr by [@​williammartin](https://github.com/williammartin) in [#​14098](cli/cli#14098) - Clarify what belongs in the MR template's testing section by [@​williammartin](https://github.com/williammartin) in [#​14103](cli/cli#14103) - Rename cli-code-reviewer skill to code-review by [@​BagToad](https://github.com/BagToad) in [#​14116](cli/cli#14116) - Add aw-actions group to dependabot configuration by [@​babakks](https://github.com/babakks) in [#​14123](cli/cli#14123) - Isolate tests from local machine's auth and git configuration by [@​BagToad](https://github.com/BagToad) in [#​14128](cli/cli#14128) - Don't ask for feature detection cleanup comments when not needed by [@​babakks](https://github.com/babakks) in [#​14139](cli/cli#14139) - Accept pre-release tags in deployment validation by [@​BagToad](https://github.com/BagToad) in [#​14193](cli/cli#14193) - ci: add temporary step to verify Linux repo signing keys by [@​babakks](https://github.com/babakks) in [#​14202](cli/cli#14202) - Revert "ci: add temporary step to verify Linux repo signing keys" by [@​babakks](https://github.com/babakks) in [#​14203](cli/cli#14203) - Fix issue triage action compatibility \[skip changelog] by [@​tidy-dev](https://github.com/tidy-dev) in [#​14207](cli/cli#14207) #####Dependencies - chore(deps): bump github.com/sigstore/sigstore-go from 1.2.2 to 1.3.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14047](cli/cli#14047) - chore(deps): bump the codeql-actions group across 1 directory with 3 updates by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14049](cli/cli#14049) - chore(deps): bump google.golang.org/grpc from 1.82.1 to 1.83.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14048](cli/cli#14048) - chore(deps): bump github.com/google/go-containerregistry from 0.21.7 to 0.21.8 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14066](cli/cli#14066) - chore(deps): bump actions/attest from 4.2.1 to 4.2.2 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14100](cli/cli#14100) - chore(deps): bump azure/login from 3.0.0 to 3.0.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14101](cli/cli#14101) - chore(deps): bump the codeql-actions group across 1 directory with 3 updates by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14091](cli/cli#14091) - chore(deps): bump github/gh-aw-actions/setup-cli from 0.83.4 to 0.85.4 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14068](cli/cli#14068) - chore(deps): bump github.com/google/go-containerregistry from 0.21.8 to 0.21.9 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14119](cli/cli#14119) - chore(deps): bump github.com/klauspost/compress from 1.19.1 to 1.19.2 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14120](cli/cli#14120) - chore(deps): bump the aw-actions group with 2 updates by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14147](cli/cli#14147) - chore: sign APT repository with both keys by [@​babakks](https://github.com/babakks) in [#​13271](cli/cli#13271) - chore(deps): bump github.com/yuin/goldmark from 1.8.4 to 1.8.5 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14029](cli/cli#14029) - chore(deps): bump actions/attest from 4.2.0 to 4.2.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14050](cli/cli#14050) - Bump golangci-lint in CI to v2.12.2 by [@​williammartin](https://github.com/williammartin) in [#​14102](cli/cli#14102) - chore(deps): bump the aw-actions group with 2 updates by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14124](cli/cli#14124) - chore(deps): bump google.golang.org/protobuf from 1.36.11 to 1.36.12 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14140](cli/cli#14140) - Upgrade gh-aw workflows to v0.85.4 by [@​tidy-dev](https://github.com/tidy-dev) in [#​14141](cli/cli#14141) - Bump Go to 1.26.6 by [@​github-actions](https://github.com/github-actions)\[bot] in [#​14143](cli/cli#14143) - chore: bump go to 1.26.7 by [@​babakks](https://github.com/babakks) in [#​14205](cli/cli#14205) - chore(deps): bump github.com/stretchr/testify from 1.11.1 to 1.12.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14204](cli/cli#14204) - chore(deps): bump the codeql-actions group across 1 directory with 3 updates by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14169](cli/cli#14169) - chore(deps): bump golang.org/x/crypto from 0.54.0 to 0.55.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14164](cli/cli#14164) - chore(deps): bump charm.land/lipgloss/v2 from 2.0.5 to 2.0.6 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14166](cli/cli#14166) - Bump gh-aw-actions to v0.87.1 and recompile agentic workflows by [@​BagToad](https://github.com/BagToad) in [#​14210](cli/cli#14210) #### New Contributors - [@​sergiou87](https://github.com/sergiou87) made their first contribution in [#​14030](cli/cli#14030) - [@​michaeljacholke](https://github.com/michaeljacholke) made their first contribution in [#​14006](cli/cli#14006) **Full Changelog**: <cli/cli@v2.97.0...v2.98.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yODguMCIsInVwZGF0ZWRJblZlciI6IjQzLjI4OC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
1 task
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
Part of #13991
This PR migrates the
gh extensionrepository and release lookups to use theapi.Clientfor API interactions. There should be no user visible change.Three call sites move across:
hasScript,fetchLatestReleaseandfetchReleaseFromTag. Three deliberately stay on the rawhttp.Client:repoExistsdistinguishes exactly200(exists) from exactly404(does not exist) and treats every other status as an error, while ignoring the response body entirely.downloadAssetstreams the extension binary to disk and sendsAccept: application/octet-stream.fetchCommitSHAsendsAccept: application/vnd.github.v3.shaand reads the body as a bare SHA.api.Client.RESTunmarshals every 2xx other than204and205as JSON, does not surface the response status on success, and offers no way to set a per-request header or to reach the raw response body. So none of those three can go through it as it stands:repoExistswould start accepting unexpected 2xx statuses as "exists" and would reject a bodyless or non-JSON200, and the other two have no body it can decode. All three keepsafeurl.JoinPathWithHostPrefix, since they still build the full URL themselves.They are deferred rather than excluded. All three are GitHub API requests that should route through
api.Clientonce it exposes a lower-level surface carrying request headers, response status and a streamable body without duplicating endpoint derivation. Each carries aTODO(api-client-rollout)comment recording its specific reason.hasScripttreats a 404 as a value rather than an error. Becauseapi.Client.RESTturns a 404 into anapi.HTTPError, that is now expressed witherrors.Asinstead of a status switch.One detail changed.
hasScriptpreviously returnedtruewithout ever reading the body, whereasapi.Client.RESTreads it. The call passesnilas the response destination, so the body is not decoded into anything and its shape is irrelevant: the object returned for a file, the array returned for a directory listing, and the objects returned for symlinks and submodules all still yieldtrue. What remains is that the body must be valid JSON at all, which required updating two stubs inmanager_test.gothat returned the bare stringscript. The real endpoint always returns JSON.pkg/cmd/extension/http.gohad no test file at all, so this PR adds one. It covers the three migrated functions and pins the behaviour ofrepoExiststhat kept it on raw HTTP: a200succeeds with a JSON, empty or non-JSON body, a404is(false, nil), and201and204remain errors.Acceptance Test
The existing
extension.txtarscript covers create, install, exec, upgrade and remove for a script extension, which exercises both of the functions whose behaviour changed.Notes
This is a stacked PR. It targets #14013.