feat(cli): verbose auth login <provider> + accept ADR 0008#44
Merged
Conversation
Adds the verbose `sting auth login github` / `auth login gitlab` spelling alongside the short `sting auth <provider>` form (issue #43). Both share the same RunE handlers and flag surface via extracted addAuthGitHub/GitLabFlags helpers, so the two spellings cannot drift apart. Marks ADR 0008 (OAuth App authentication) as Accepted and corrects its token precedence wording to match the code and ADR 0002: most-explicit-wins — dedicated PAT (--token / STING_TOKEN / config) > OAuth (sting auth) > anonymous. Ambient GITHUB_TOKEN / GITLAB_TOKEN and gh/glab stored auth are intentionally never consulted (ADR 0002 remains in force); added guardrail comments in commitclient so no ambient fallback is introduced later. Docs: README and auth help text document both command forms. Refs #42, #43 Signed-off-by: Shawn Stratton <shawn@skaphos.io>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the verbose sting auth login <provider> command spelling to the CLI (sharing handlers and flags with the existing sting auth <provider> form) and updates documentation/ADR status to reflect the now-accepted OAuth design and correct token-precedence wording.
Changes:
- Add
auth logincommand tree (auth login github|gitlab) wired to the sameRunEhandlers and flag sets asauth github|gitlab. - Update CLI help/README to document the additional command spelling.
- Accept ADR 0008 and clarify “most-explicit-wins” token precedence (PAT > stored OAuth > anonymous), explicitly excluding ambient tokens.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the verbose auth login <provider> form alongside existing auth usage. |
| internal/commitclient/commitclient.go | Clarifies token-resolution precedence and explicitly documents the “no ambient token fallback” rule. |
| internal/cli/cli_test.go | Adds a wiring test to ensure verbose auth commands resolve, share handlers, and expose expected flags. |
| internal/cli/auth.go | Registers the new auth login subtree and updates auth help text. |
| internal/cli/auth_login.go | Introduces the new auth login command and provider subcommands. |
| internal/cli/auth_gitlab.go | Refactors GitLab auth flag registration into a shared helper. |
| internal/cli/auth_github.go | Refactors GitHub auth flag registration into a shared helper. |
| docs/adr/0008-oauth-app-authentication.md | Marks ADR 0008 as Accepted and corrects/expands the token-precedence rationale. |
Open
8 tasks
- Reword the shared client-id/-secret flag comment: it no longer claims the GHES guidance is in every command's Long help (the helper is shared with `auth login github`, whose Long points to `auth github --help`). - Guard against a nil RunE before the reflect pointer comparison in the auth login wiring test, so a missing handler fails clearly instead of silently comparing zero pointers. Signed-off-by: Shawn Stratton <shawn@skaphos.io>
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.
Summary
Closes the OAuth-epic work on issues #42 and #43 (parent skaphos-resources#285), and marks the ADR from #41 Accepted.
Implement
sting authandsting initsubcommands #43 —sting auth login <provider>: adds the verbosesting auth login github/sting auth login gitlabspelling alongside the existing shortsting auth <provider>form. Both share the sameRunEhandlers and the identical flag surface via extractedaddAuthGitHubFlags/addAuthGitLabFlagshelpers, so the two spellings cannot drift apart. This was the one unmet acceptance criterion for Implementsting authandsting initsubcommands #43 (everything else —auth github|gitlab|status|logout,init/init github/init gitlab, GHES/self-hosted BYO — was already implemented).Design secure credential storage and GitHub OAuth App registration for Sting #41 — ADR 0008: flipped Status Proposed → Accepted. The design (credential storage, OAuth app registration for both providers, scopes, self-hosted BYO) is fully built and documented.
Update ghclient and providers to consume user OAuth tokens #42 — token precedence: the
commitclientresolution already matches the intended most-explicit-wins order, so no behavior change was needed. Corrected ADR 0008's precedence wording (it previously read "OAuth preferred when both present", which misstated the code) and added guardrail comments:Ambient
GITHUB_TOKEN/GITLAB_TOKENand thegh/glabstored auth are intentionally never consulted, preserving the strict separation of ADR 0002 (which remains in force and is not superseded).Deferred #42 scope → tracked in #45
The remaining items from #42's original scope are carved out into #45 (they don't change the resolved precedence above — they're refactor / observability / lifecycle work):
TokenSource/Authenticatorinterface (currently inline resolvers).Testing
go test ./...— all packages green.scripts/check-coverage.sh) passes;internal/commitclient83.3%,internal/cli64.7% (documented 60% floor).go vet,gofmt -l,golangci-lint run— clean.sting auth --help,auth login --help,auth login github --help(full flag surface present).Closes #42
Closes #43
Refs #41