alert for unsupported auth tokens#113
Conversation
28d4490 to
51dfa0e
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds token-type detection to prevent misleading error messages when users authenticate with a Personal Access Token (PAT) instead of OAuth. Previously, PAT-authenticated requests to cli_internal endpoints returned 404, which was displayed as "Stacked PRs are not enabled for this repository" — now the CLI detects the token type upfront and shows a clear message directing users to authenticate with OAuth.
Changes:
- Adds
internal/config/auth.gowith PAT detection methods (IsPersonalAccessToken,WarnIfPAT,RepoHost) plus test-injection hooks (TokenForHostFn,RepoOverride) on the Config struct. - Adds a pre-flight PAT check in
cmd/submit.goand replaces all 404 warning messages in submit/link/checkout withwarnStacksUnavailableOrPAT()which conditionally shows the PAT-specific or generic message. - Adds comprehensive tests for the new auth detection logic and the pre-flight check path.
Show a summary per file
| File | Description |
|---|---|
internal/config/config.go |
Adds TokenForHostFn and RepoOverride fields to Config; Repo() respects RepoOverride |
internal/config/auth.go |
New file with PAT detection logic and WarnIfPAT helper |
internal/config/auth_test.go |
Unit tests for token prefix detection and WarnIfPAT output |
cmd/utils.go |
Adds warnStacksUnavailableOrPAT helper that delegates to WarnIfPAT or generic message |
cmd/utils_test.go |
Tests for the new helper and introduces setTestTokenForHost test utility |
cmd/submit.go |
Adds pre-flight PAT check; replaces direct warning with warnStacksUnavailableOrPAT in 404 handlers |
cmd/submit_test.go |
Integration tests for PAT pre-flight, plus existing 404 tests updated with OAuth token override |
cmd/link.go |
Replaces direct warning with warnStacksUnavailableOrPAT in two 404 handlers |
cmd/checkout.go |
Replaces direct error message with warnStacksUnavailableOrPAT in 404 handler |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 9/9 changed files
- Comments generated: 3
When users authenticate the GitHub CLI with a personal access token
(PAT) instead of OAuth (`gh auth login`), the `cli_internal` stacks
API endpoints return 404. The CLI previously interpreted this as
"Stacked PRs are not enabled for this repository," which is misleading
— the feature may be enabled, but the token type simply cannot access
the internal endpoints.
This is a recurring source of user confusion. The docs already note
that PATs are not supported, but users don't always read them before
hitting the error.
This change adds token-type detection by inspecting the `gh` auth
token prefix:
- `gho_` → OAuth (supported)
- `ghs_` → GitHub App installation token (supported)
- `ghp_` → Classic PAT (NOT supported)
- `github_pat_` → Fine-grained PAT (NOT supported)
When a PAT is detected, the CLI now shows:
⚠ Personal access tokens are not supported by gh stack
Run `gh auth login` to authenticate with OAuth instead.
Instead of the misleading:
⚠ Stacked PRs are not enabled for this repository
Changes:
- Add `internal/config/auth.go` with auth detection methods on Config:
`IsPersonalAccessToken()`, `WarnIfPAT()`, and `RepoHost()`. Uses a
`TokenForHostFn` field on Config for test overrides, following the
same pattern as `GitHubClientOverride`.
- Add a pre-flight PAT check in `cmd/submit.go` before the
`ListStacks` call. If a PAT is detected, the command aborts early
with a clear error instead of making a doomed API call.
- Update all 404 handlers for `cli_internal` endpoints to check the
token type and show the appropriate message:
- `cmd/submit.go` (createNewStack)
- `cmd/link.go` (listStacksSafe, createLink)
- `cmd/checkout.go` (checkoutRemoteStack)
- Add `warnStacksUnavailableOrPAT()` helper in `cmd/utils.go` that
shows the PAT-specific warning when applicable, falling back to the
generic "not enabled" message for non-PAT tokens.
- Add unit tests in `internal/config/auth_test.go` for token prefix
detection and warning output.
- Add integration tests in `cmd/submit_test.go` verifying that both
classic PATs (`ghp_`) and fine-grained PATs (`github_pat_`) trigger
the pre-flight check and abort before any API calls.
- Add `warnStacksUnavailableOrPAT` tests in `cmd/utils_test.go`
verifying correct message selection based on token type.
- Update existing 404 tests to explicitly set an OAuth token so they
continue exercising the ListStacks 404 path.
51dfa0e to
7fc120f
Compare
| var httpErr *api.HTTPError | ||
| if errors.As(err, &httpErr) && httpErr.StatusCode == 404 { | ||
| cfg.Errorf("Stacked PRs are not enabled for this repository") | ||
| warnStacksUnavailableOrPAT(cfg) |
| if cfg.WarnIfPAT() { | ||
| return ErrStacksUnavailable | ||
| } |
Lukeghenco
left a comment
There was a problem hiding this comment.
Solution is clean and clear. I see no issues with this going out as is. I also appreciate the failure warning clean up to prevent the need of passing the same string around throughout the codebase with the new warnStacksUnavailableOrPAT func.
When users authenticate the GitHub CLI with a personal access token (PAT) instead of OAuth (
gh auth login), thecli_internalstacks API endpoints return 404. The CLI previously interpreted this as "Stacked PRs are not enabled for this repository," which is misleading — the feature may be enabled, but the token type simply cannot access the internal endpoints.This is a recurring source of user confusion. The docs already note that PATs are not supported, but users don't always read them before hitting the error.
This change adds token-type detection by inspecting the
ghauth token prefix:gho_→ OAuth (supported)ghs_→ GitHub App installation token (supported)ghp_→ Classic PAT (NOT supported)github_pat_→ Fine-grained PAT (NOT supported)When a PAT is detected, the CLI now shows:
⚠ Personal access tokens are not supported by gh stack
Run
gh auth loginto authenticate with OAuth instead.Instead of the misleading:
⚠ Stacked PRs are not enabled for this repository
Changes:
Add
internal/config/auth.gowith auth detection methods on Config:IsPersonalAccessToken(),WarnIfPAT(), andRepoHost(). Uses aTokenForHostFnfield on Config for test overrides, following the same pattern asGitHubClientOverride.Add a pre-flight PAT check in
cmd/submit.gobefore theListStackscall. If a PAT is detected, the command aborts early with a clear error instead of making a doomed API call.Update all 404 handlers for
cli_internalendpoints to check the token type and show the appropriate message:cmd/submit.go(createNewStack)cmd/link.go(listStacksSafe, createLink)cmd/checkout.go(checkoutRemoteStack)Add
warnStacksUnavailableOrPAT()helper incmd/utils.gothat shows the PAT-specific warning when applicable, falling back to the generic "not enabled" message for non-PAT tokens.Add unit tests in
internal/config/auth_test.gofor token prefix detection and warning output.Add integration tests in
cmd/submit_test.goverifying that both classic PATs (ghp_) and fine-grained PATs (github_pat_) trigger the pre-flight check and abort before any API calls.Add
warnStacksUnavailableOrPATtests incmd/utils_test.goverifying correct message selection based on token type.Update existing 404 tests to explicitly set an OAuth token so they continue exercising the ListStacks 404 path.