Repository navigation
fix(github,gitlab): find gh/glab config on Windows - #11508
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughGitHub and GitLab CLI token configuration lookup now follows ordered, platform-aware candidate paths. Shared path selection and Windows environment fallbacks were added, with documentation covering XDG, native, keyring, and Windows locations. ChangesCLI token resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant gh_hosts_path
participant gh_native_hosts_paths
participant first_existing_file
gh_hosts_path->>gh_native_hosts_paths: Build native platform candidates
gh_hosts_path->>first_existing_file: Pass ordered hosts.yml candidates
first_existing_file-->>gh_hosts_path: Return first existing file or platform default
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR expands GitHub and GitLab CLI config discovery to match their Windows and legacy locations.
Confidence Score: 5/5The PR appears safe to merge. The previously reported glab legacy-path issue is fixed because the current candidate list always checks the home-based legacy config before XDG and platform-native locations, and no blocking failure remains. Important Files Changed
Reviews (7): Last reviewed commit: "fix(github,gitlab): find gh/glab config ..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/dev-tools/github-tokens.md`:
- Around line 198-202: Update the `gh auth token` example in the
credential-command guidance to pass `"$MISE_CREDENTIAL_HOST"` via `--hostname`,
ensuring the token matches mise’s requested host, or explicitly scope the
example to github.com. Keep the surrounding macOS/Windows and `mise token
github` guidance unchanged.
In `@src/gitlab.rs`:
- Around line 365-381: Restore glab_config_path resolution order by adding the
distinct legacy ~/.config/glab-cli/config.yml candidate before the XDG/native
candidates, while retaining GLAB_CONFIG_DIR as highest priority and the existing
fallback behavior. Update the corresponding configuration-resolution list in
docs/dev-tools/backends/gitlab.md to document the same legacy-before-XDG order.
In `@src/tokens.rs`:
- Around line 320-330: Update first_existing_path to select only candidates
where is_file() is true, preserving fallback behavior when no regular file
exists. Add a regression test covering an earlier directory-named candidate
shadowing a later valid hosts.yml or config.yml file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: fe1f876a-35d8-496e-9c46-f9ffe18a6add
📒 Files selected for processing (6)
docs/dev-tools/backends/gitlab.mddocs/dev-tools/github-tokens.mdsrc/env.rssrc/github.rssrc/gitlab.rssrc/tokens.rs
9c1b554 to
2a8c2ee
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/github.rs`:
- Around line 834-854: Update gh_default_hosts_path and gh_native_hosts_paths on
Windows to distinguish an explicitly available APPDATA value from the
synthesized ROAMING_APPDATA path. Only add the native GitHub CLI AppData
candidate when APPDATA exists; when it is unset, preserve the
~/.config/gh/hosts.yml fallback and avoid returning the synthesized AppData path
as the default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e7950a97-0a5f-48b8-b445-e76d7fa1d6dc
📒 Files selected for processing (6)
docs/dev-tools/backends/gitlab.mddocs/dev-tools/github-tokens.mdsrc/env.rssrc/github.rssrc/gitlab.rssrc/tokens.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/dev-tools/backends/gitlab.md
2a8c2ee to
4b6ac43
Compare
4b6ac43 to
f5b75ff
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/env.rs`:
- Around line 93-94: Treat empty environment variables as unset in both
src/env.rs lines 93-94 and src/github.rs lines 825-826: update the LOCALAPPDATA
lookup to filter empty values before falling back to HOME/AppData/Local, and
update the XDG_CONFIG_HOME lookup to filter empty values before constructing
gh/hosts.yml; preserve the existing non-empty path behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e6f8300b-8759-40a6-ac58-0d22dba99d51
📒 Files selected for processing (6)
docs/dev-tools/backends/gitlab.mddocs/dev-tools/github-tokens.mdsrc/env.rssrc/github.rssrc/gitlab.rssrc/tokens.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/dev-tools/backends/gitlab.md
f5b75ff to
86dbd03
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/dev-tools/github-tokens.md`:
- Around line 210-217: Update the TOML example in the documentation so it never
declares the [settings.github] table twice in one snippet. Keep the macOS/Linux
and Windows credential_command examples, but separate the Windows variant into
its own code block or otherwise present it as an alternative configuration.
In `@src/github.rs`:
- Around line 819-820: Update the GH_CONFIG_DIR branch in the GitHub
configuration discovery function and the GLAB_CONFIG_DIR branch in the GitLab
configuration discovery function to ignore empty environment-variable values
before constructing and returning the direct hosts.yml/config.yml candidate.
Preserve direct-path handling for non-empty values and allow empty overrides to
continue into gh/glab fallback discovery.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3322e5ee-63c4-416d-8e95-4c3f50a92b38
📒 Files selected for processing (6)
docs/dev-tools/backends/gitlab.mddocs/dev-tools/github-tokens.mdsrc/env.rssrc/github.rssrc/gitlab.rssrc/tokens.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/dev-tools/backends/gitlab.md
86dbd03 to
0decdd1
Compare
mise reads forge tokens from the `gh` and `glab` CLI config files as a fallback, but on Windows it only ever looked under `~/.config`, so a logged-in user could still go out unauthenticated and hit the 60/hour anonymous GitHub rate limit. `env::XDG_CONFIG_HOME` has no `#[cfg(windows)]` arm (unlike its `XDG_CACHE_HOME` / `XDG_DATA_HOME` neighbours), so on Windows it resolves to `%USERPROFILE%\.config`, and neither `gh_hosts_path` nor `glab_config_path` had a Windows branch -- only a macOS one. The two CLIs do not even agree on the Windows root, so both had to be checked separately: - go-gh's `ConfigDir()` checks `AppData` explicitly, giving `%APPDATA%\GitHub CLI\hosts.yml` - glab goes through `adrg/xdg`, whose Windows `ConfigHome` is `%LOCALAPPDATA%`, giving `%LOCALAPPDATA%\glab-cli\config.yml` `fj` is unaffected: it resolves through `XDG_DATA_HOME`, which already has a Windows arm. Also: - follow go-gh's order for `gh`: `XDG_CONFIG_HOME` is consulted only when actually set, and `~/.config/gh` becomes the explicit final fallback, as in gh itself - extract `tokens::first_existing_path` so the "first candidate that exists, else the conventional location" rule is one implementation with tests, rather than a copy per forge -- the copies are how the Windows location came to be missing from both - document the real Windows locations, and rewrite the keyring note so a Windows user whose `hosts.yml` has no `oauth_token` is pointed at `mise token github` and `use_git_credentials` / `credential_command` instead Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0decdd1 to
be823b9
Compare
|
Three review rounds here all found the same defect at a different call site — an environment variable set to the empty string being treated as set. Rather than wait for the fourth, I went looking for the root, and it is one function.
That is 37 call sites sharing one bug, so
Pinned by a unit test in One case I deliberately did not mirror: |
The bug
mise reads forge tokens from the
ghandglabCLI config files as a fallback. On Windows it only ever looked under~/.config, which is not where either CLI puts them by default — so a logged-in user can still go out unauthenticated and hit GitHub's 60/hour anonymous rate limit.env::XDG_CONFIG_HOMEhas no#[cfg(windows)]arm (unlike itsXDG_CACHE_HOME/XDG_DATA_HOMEneighbours), so on Windows it resolves to%USERPROFILE%\.config, and neithergh_hosts_pathnorglab_config_pathhad a Windows branch — only a macOS one.Measured, same file, only
GH_CONFIG_DIRdiffers:The two CLIs disagree about the Windows root
This is why both had to be checked separately rather than sharing one
%APPDATA%constant.gh— go-ghConfigDir()checksAppDataexplicitly:glab—ConfigDir()isGLAB_CONFIG_DIR→ legacy~/.config/glab-cli(when itsconfig.ymlexists) →xdg.ConfigHome/glab-cli, and adrg/xdg on Windows setsconfigHome = EnvPath(envConfigHome, kf.localAppData)—%LOCALAPPDATA%, not%APPDATA%.gh%APPDATA%\GitHub CLI\hosts.ymlglab%LOCALAPPDATA%\glab-cli\config.ymlfj%LOCALAPPDATA%\forgejo-cli\XDG_DATA_HOME's Windows armNote
.configis not simply wrong on Windows — it is gh's last branch and glab's legacy location, so both are kept as candidates.Also in this PR
ghnow follows go-gh's order:XDG_CONFIG_HOMEis consulted only when actually set (mise's defaulting static cannot express that), and~/.config/ghbecomes the explicit final fallback, matching gh itself. Unix behaviour is unchanged — unsetXDG_CONFIG_HOMEproduces the same path the static did.tokens::first_existing_path: the "first candidate that exists, else the conventional location" rule is now one implementation with unit tests instead of a copy per forge. The copies are precisely how the Windows location came to be missing from both.Scope — what this does not fix
Current
ghstores its token in Windows Credential Manager, so on many setupshosts.ymlexists but has nooauth_tokenkey at all, and no amount of path-fixing helps. Verified here: the realhosts.ymlon this machine holds onlygit_protocol/users/user.Those users need
github.use_git_credentialsorgithub.credential_command, both measured working:The docs previously lumped macOS and Windows together as "stores tokens in the system keyring rather than
hosts.yml", which does not tell a Windows user how to find out which case they are in. That note now points atmise token githuband both settings.Tests
Three unit tests for
first_existing_pathinsrc/tokens.rs— earliest existing candidate wins, a missing earlier candidate is skipped, and the fallback is returned when none exist. Platform-neutral, so they run onwindows-unittoo.Deliberately not attempted: driving
gh_hosts_path()end-to-end.GH_HOSTS,XDG_CONFIG_HOMEand the new statics are allLazyLock,env::HOMEis pinned to$CARGO_MANIFEST_DIR/testundercfg(test), andEnvVarGuardis#[cfg(unix)]so it cannot run onwindows-unit. The extracted helper is the part that can be pinned without that machinery.Note for review
mise's macOS candidate for
gh(~/Library/Application Support/gh/hosts.yml) does not correspond to any go-gh branch — gh uses~/.config/ghon macOS as well. It is.exists()-gated and therefore harmless, so I kept it rather than change behaviour in a PR about Windows; happy to drop it if you'd prefer.Summary by CodeRabbit
%LOCALAPPDATA%-based fallback to improve token resolution reliability.hosts.ymllookup guidance and expanded OS-specific credential helper/keyring scenarios.