Comparing changes

api.Client.REST and RESTWithNext only cover sending JSON and decoding JSON, so call sites needing anything else have stayed on the raw *http.Client. This adds the surface those sites need. Request and RequestWithContext return the response for the caller to consume rather than decoding into a receiver, which covers streaming bodies, non-JSON bodies, response headers and the status code of a successful response. Neither special-cases 204/205 nor decodes on success, so bodiless and non-JSON successes both work. Endpoint resolution is delegated to go-gh rather than reimplemented here, so that per-host API endpoint routing is inherited once it lands upstream rather than having to be rebuilt. This is why the surface does not hand back a caller-built *http.Request: building one requires resolving the URL first. RequestOption carries per-request headers, which are the single largest blocker across the remaining call sites, and go-gh has no equivalent since its headers are client-level only. api.Client already builds a fresh go-gh client per call, so client-level headers are effectively per-request. WithEndpointScopes preserves the EndpointNeedsScopes hook for call sites that can no longer reach the response before it becomes an error. DoRequest exists for the few sites that must set request fields which are neither method, path, body nor header, such as ContentLength and GetBody when uploading a release asset. Those sites all use absolute URLs supplied by the API, where there is no endpoint resolution to delegate, so it gives up nothing. Header precedence is verified against the transport built by NewHTTPClient rather than a test tripper, since the value of a per-request header depends entirely on it winning against the real transport defaults. Refs #13991 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

The two release lookup sites in pkg/cmd/release/shared built an absolute URL with JoinPathWithHostPrefix and sent it through the raw http.Client. Both are plain GETs whose only reason for bypassing api.Client was that they need to treat a 404 as a not-found sentinel rather than an error. They now build a relative path and go through Request, which resolves the endpoint in go-gh. That is what lets these sites pick up api_host config later; lifting the absolute URL across would have moved the code without delivering the benefit. The 404 sentinel moves from a status check on the response to errors.As on the returned HTTPError. RequestWithContext is used rather than REST so that the cancellation FetchRelease relies on to drop the losing lookup is preserved. TestFetchRefSHA already pinned the sentinel for one of the two sites. Tests are added for the other, which was unpinned, covering the sentinel both directly through FetchLatestRelease and through the dual lookup in FetchRelease. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Both Contents API sites bypassed api.Client only because they need a custom Accept media type: the object+json type that makes the API return a single object for files and directories alike, and the raw type used for files above the inline content limit. They now go through Request with WithHeader, and contentsAPIPath returns a relative path so the endpoint is resolved by go-gh rather than baked in here. The existing tests already match on the Accept header, so they confirm the per-request header survives the trip through the client and the transport. Test_contentsAPIPath is updated for the relative path it now builds. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Three sites remained on the raw client. All now go through Request. repoExists and fetchCommitSHA are body-blind or read a non-JSON body, so Request is the right fit rather than REST: go-gh's Do unmarshals the body even when the response argument is nil, which would reject the empty and non-JSON bodies that TestRepoExists already covers. repoExists keeps its exact-status contract. Only 200 counts as existence, and any other success status is still reported as an error, which the existing tests require. fetchCommitSHA and downloadAsset need custom Accept media types, and neither was covered by a test, so a missing header would have gone unnoticed. Tests are added that match on the media type, along with the 422 commit-not-found sentinel that also moves to errors.As. downloadAsset takes a hostname now because it is given an absolute asset URL by the API, which is requested as given rather than resolved against the host. Two asset fixtures in manager_test.go used a host-relative URL, which only worked because the mock transport never validated it. They are now absolute like every other asset fixture in that file. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Four sites, three of which stream a response body the client must not decode. ListArtifacts had its own apiGet helper that duplicated RESTWithNext, including a copy of the Link header regex. It is deleted in favour of RESTWithNext, which already does exactly this and keeps the absolute next-page URLs working. The two log fetchers and the artifact download hand their response body to the caller, so they use Request. Their not-found sentinels move to errors.As, and both log fetchers keep their existing contract of accepting only 200. The artifact download URL comes from the API and stays absolute, so that site takes a hostname to identify the client rather than to build the URL. Test_Download constructed an apiPlatform with no repo, which is not a state the command can produce; it now sets one like the other tests in the file. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Eight sites across delete, delete-asset, edit, create, download and upload. The three deletes and the create existence check are body-blind or have no body at all, so they use Request rather than REST, which would try to decode one. editRelease also uses Request: REST returns early on a 204 without decoding, which would turn today's error into a zero valued release. uploadAsset is the one site that needs DoRequest. It sets ContentLength and GetBody on the request, neither of which is a header, and without them a file body would be sent chunked. Its retry behaviour is preserved by wrapping only transport failures as errNetwork and leaving a response-carrying failure to be judged by status, which Test_uploadWithDelete_retry covers. The archive download overrides CheckRedirect to avoid legacy Codeload resources. That is client level rather than request level, so it keeps copying the client and hands the copy to NewClientFromHTTP. No new API is needed. httpDoer is gone. It existed only because these sites took a narrow interface, and api.Client needs a concrete *http.Client. The upload retry test drives the client through a RoundTripper instead. Asset, release and upload URLs all come from the API and stay absolute. Sites that build their own URL now build a relative path instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Migrates the four remaining raw httpClient.Do call sites under pkg/cmd/repo: delete, garden, and the two topics calls in edit. Constructed URLs become relative paths so they will pick up api_host resolution from go-gh. Two behaviour notes: garden previously returned a bare errors.New("api call failed") on any non-2xx. It now returns api.HTTPError, so users see the API's own message. This is a user-visible message change and is pinned by a new test. Its explicit Content-Type: application/json; charset=utf-8 is dropped because the client already sends exactly that header; verified against the real transport rather than assumed. gh repo edit's Content-Type differs (no charset) so it stays. delete keeps its CheckRedirect client copy and declares the delete_repo scope through api.WithEndpointScopes. That suggestion had no coverage, so a test now pins it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Migrates the three remaining raw httpClient.Do call sites in pkg/cmd/gist and pkg/cmd/pr/diff. gist create and pr diff construct their URLs, so those become relative paths and will pick up api_host resolution from go-gh. gist create declares its scope through api.WithEndpointScopes, and its explicit Content-Type is dropped because the client already sends exactly that header. The scope suggestion had no coverage, so a test now pins it. GetRawGistFile keeps its absolute URL: raw gists are served from gist.githubusercontent.com rather than the API host, so rewriting them to a gateway would be wrong. It gains a hostname parameter purely to configure the client, which both callers already have to hand. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Migrates the last four in-scope raw httpClient.Do call sites. auth's GetScopes and GetCurrentLogin run before the token is stored in config, so they pass it explicitly. The transport only sets Authorization when it is absent, so the header set at the call site wins and no new concept is needed. GetCurrentLogin's hand-rolled GraphQL POST becomes an api.Client GraphQL call. Both functions took a narrow one-method httpClient interface, which is dropped in favour of *http.Client so the api client can be constructed from it; the existing tests already passed a real client, so they are unchanged. search keeps its own error type, because it formats invalid queries differently from the rest of the CLI. api.HTTPError carries the same fields, so the generic error is translated back rather than surfaced directly, including the status line fallback for non-JSON responses. update keeps its absolute URL deliberately: CLI releases live on github.com regardless of the host the user has configured, so it must not be rewritten to their API host. This completes the rollout. The census command from the design now returns only the ten genuinely out-of-scope sites plus DoRequest's own implementation, and no "deferred from moving to api.Client" comments remain. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Request delegates to go-gh, which builds an http.Client of its own from the transport alone. A CheckRedirect set on the client passed to NewClientFromHTTP is therefore silently discarded, which broke both call sites that rely on one. gh release download --archive stopped rewriting "legacy" Codeload paths, so the archive was saved under the wrong name. Acceptance caught this. gh repo delete stopped surfacing the 3xx for a renamed or transferred repo. Instead the redirect was followed, turning DELETE into GET against the new location, so the command reported success while deleting nothing. Nothing caught this: the existing unit test stubs a 307 with no Location header, and Go cannot follow a redirect without one, so the test passed whatever the redirect policy was. Both now send through DoRequest, which uses the client it was given. DoRequest gains RequestOption support so gh repo delete keeps declaring delete_repo. The cost is that gh repo delete keeps an absolute URL and so will not pick up api_host resolution until go-gh can carry a redirect policy. Release assets are unaffected, since their URLs come from the API and were always absolute. Tests are added at both sites and in the api package. They use real servers, because redirects are followed by the client, above the transport, so a stubbed RoundTripper cannot exercise this at all. Each was confirmed to fail against the broken version. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

The "repo transferred ownership" case stubbed a 307 with no Location header. Go declines to follow a redirect without one, so the redirect policy under test was never consulted and the case passed whether or not deleteRepo suppressed redirects. Removing the CheckRedirect from deleteRepo left the whole Test_deleteRun suite green. Add the Location header so the stub is a redirect Go will actually follow, and assert wantStderr on the error path, which the early return had been skipping. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

RepoExists called client.HTTP().Head directly, bypassing api.Client entirely, and RenameRepo passed REST an absolute RESTPrefix URL, which go-gh requests as given rather than resolving against the host endpoint. Neither would pick up api_host. Both now build relative paths. RepoExists uses Request, since a HEAD response has no body to decode, and maps a 404 HTTPError back to a false result to preserve its existing contract. Add a case pinning that only 200 counts as existence, which the previous switch expressed through its default arm. This removes the last ghinstance.RESTPrefix use from the api package. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

gh gist had no acceptance coverage, which left the gist call sites migrated to api.Client resting on unit tests alone. Two scripts cover the create, view, edit, rename, list and delete flows, and between them exercise three of the four migrated sites: the create POST, the single gist GET, and the GraphQL list. The fourth, fetching a truncated file's full content from a raw URL, needs a file over the API's 1MB inline limit to trigger. That is too large to carry in a txtar, so it stays with the unit tests and the gap is noted in the script. Deletion is tested without a deferred cleanup, following repo-delete and ssh-key, because a deferred delete would fail on the second attempt. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

The script asserted that a file was absent immediately after removing it, which reads the gist back before the write has necessarily settled and fails when it has not. Removing a file issues the same update request as adding one, so the step covered no further ground to pay for that. Drop the --remove step, and drop the assertion that the old filename is gone after a rename, which is redundant because the new name being present already proves the rename happened. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Reads of a gist that has just been updated serve the previous state often enough to make the script unreliable, measured at 2 failures in 10 attempts against the real API. Creating and deleting a gist read back consistently over 15 attempts, so the remaining script stays. Removing rather than tolerating the race, because the script covered no code this rollout changed. Fetching, listing and updating a gist already went through the api client before this work; only creation and the raw file fetch moved. Creation is still covered, and the raw fetch needs a file larger than a txtar can hold. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Updates to an existing gist are visible to a following read around eight times in ten, settling within about a second and a half, so a sleep after each update is enough. This matches how the pr and search scripts already wait on writes. The failures were not only late assertions. Rename reads the gist to build its update, so a stale read there renames nothing, exits 0, and leaves the gist permanently wrong. Two runs in fifteen never converged however long they were polled. Sleeping before the rename rather than before the assertion removed those, fifteen runs in fifteen. Creating a gist reads back consistently over fifteen attempts and needs no sleep. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 144144f1-73c0-46dd-a4f2-5a1b3d0c2f83

Navigation Menu

Uh oh!

Comparing changes

Open a pull request

Uh oh!

Commits on Aug 6, 2026

This comparison is taking too long to generate.

Uh oh!