{{ message }}
Expose api_host through gh config - #14332
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A new error assertion violates the repository’s required fatal-assertion convention.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
What changed in this PR
Exposes per-host api_host through gh config.
Changes:
- Registers and documents
api_host. - Requires
--hostwhen setting it. - Adds set and list coverage.
| File | Description |
|---|---|
internal/config/config.go |
Defines option metadata and value lookup. |
pkg/cmd/config/set/set.go |
Enforces per-host configuration. |
pkg/cmd/config/set/set_test.go |
Tests validation and setting behavior. |
pkg/cmd/config/list/list_test.go |
Tests listing the configured API host. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
williammartin
force-pushed
the
williammartin-config-api-host
branch
from
September 3, 2026 12:09
56221e5 to
bd58a4b
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The required config get validation is missing, and unrelated extension behavior is bundled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
 |
pkg/cmd/config/set/set.go — 🛑 Requirement: Enforce the per-host-only metadata in config get as well. This helper is… |
 |
pkg/extensions/official.go — 💭 Commentary: Split this extension-registry update from the api_host configuration change.… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
pkg/cmd/config/set/set_test.go — 🛑 Requirement: Use a fatal assertion for this error check AGENTS.md:112 requires require, rather… View resolved comment |
williammartin
marked this pull request as ready for review
September 3, 2026 12:48
williammartin
force-pushed
the
williammartin-config-api-host
branch
from
September 3, 2026 12:52
bd58a4b to
57d0072
Compare
babakks
approved these changes
Sep 3, 2026
| var Options = []ConfigOption{ | ||
| { | ||
| Key: apiHostKey, | ||
| Description: "experimental: the hostname to use when making API requests for a GitHub host. Note: this is not a security boundary and requests to the canonical host will remain authenticated", |
Member
There was a problem hiding this comment.
nitpick: maybe also indicate that it shouldn't have the scheme or port in it.
Suggested change
williammartin
force-pushed
the
williammartin-config-api-host
branch
from
September 3, 2026 13:03
57d0072 to
bd58a4b
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
williammartin
force-pushed
the
williammartin-config-api-host
branch
from
September 3, 2026 13:05
bd58a4b to
bf19e9c
Compare
williammartin
enabled auto-merge
September 3, 2026 13:06
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.
Relates to #13717
Description
The recently added per-host
api_hostsetting could only be configured by editinghosts.ymldirectly. This adds it to the supportedgh configoptions, so it can be set and read withgh config setandgh config get.api_hostonly makes sense for a GitHub host, so setting or getting it without--hostnow returns a flag error. The option metadata marks this constraint asPerHostOnly, keeping the command validation generic.The option description feeds the generated
gh confighelp and manual. It identifiesapi_hostas experimental, explains that it redirects API requests for a GitHub host, and warns that it is not a security boundary because requests may still reach the original host.How did you test this change?
1. Set and get
I built this branch and used an isolated
GH_CONFIG_DIRto set and then read anapi_hostfor a synthetic GitHub hostname.2. Set and inspect
hosts.ymlI set
api_host, then displayed the isolatedhosts.ymland saw the value stored under the requested GitHub hostname.3. Set without
--hostI attempted to set
api_hostwithout--hostand saw the new error and usage guidance.4. Get without
--hostI attempted to read
api_hostwithout--hostand saw the matching error and usage guidance.Authorship and follow-up
Who wrote this:
Who answers review comments: