Refreshable tokens (1/7): foundation credential model, storage migration, and file lock by babakks · Pull Request #14450 · cli/cli

babakks · 2026-09-15T01:28:41Z

Part of #14449.

Description

Foundation for refreshable (short-lived) OAuth tokens. This PR carries no user-facing behavior; it prepares the ground so the later PRs can add the refresh machinery without re-threading call sites or churning unrelated code.

It does three things:

Introduces a single gh.Credential type and migrates the auth storage interface (and its callers) to speak that type instead of a bare token string.
Bumps cli/go-gh and cli/oauth to pick up the refreshable-credential support this feature builds on.
Adds a blocking, timeout-bounded file-lock acquisition to internal/flock, used later to serialize single-use refresh-token exchanges across processes.

How did you test this change?

This feature is verified end to end as a whole rather than per PR. End-to-end tests should pass.

Key points

Why a gh.Credential type now. Until now the auth storage layer spoke bare token strings, which is all a non-expiring token needs. A refreshable credential is more than a string: it carries a refresh token and expiry timestamps that must travel together with the access token through every layer that reads or writes it. Introducing the struct and migrating the interface in one mechanical, behavior-preserving step here means the later PRs only add fields and behavior, rather than re-plumbing every call site while also introducing new logic. It keeps the risky, reviewable logic changes out of a large mechanical diff.
Why the blocking flock. The refresh path needs cross-process mutual exclusion so two gh processes cannot spend the same single-use refresh token at once. The existing lock primitive was a non-blocking try-lock, which would force a refresh to fail immediately if a peer held the lock. This PR adds a blocking acquire with a timeout: a refresh waits briefly for a peer to finish (and then observes the peer's rotated credential on reload) instead of failing, while the timeout guarantees it can never hang forever. The timeout value and the refresh-specific naming land with the refresh path in PR 2.
The dependency bump is an interim pin. Before final merge we will tag and release cli/go-gh and cli/oauth, repin to the released versions, and re-run go mod tidy. This is tracked in the envelope issue.

Notes for reviewers

Start with the gh.Credential type and the storage-interface change, then the caller migration (mechanical), then the flock timeout addition. The migration should not change any observable behavior; if you spot a behavioral difference, that is a bug.

Commits (review in order):

build(deps): bump go-gh and oauth for refreshable credentials
fix(gh): introduce gh.Credential and switch auth storage interface to it
fix(config): migrate auth storage layer and callers to gh.Credential
fix(flock): add blocking lock acquisition with timeout

Authorship and follow-up

Who wrote this:

A human wrote it.
An agent wrote it under close human direction.
An agent wrote it independently, and no human has guided the implementation beyond the initial prompt.

Who answers review comments:

@babakks will read and reply directly.
An agent will draft replies and @username will read them before they are posted.
Nobody has explicitly committed to replying.

Bump github.com/cli/go-gh/v2 and github.com/cli/oauth to the pseudo-versions that expose the refreshable-credential and OAuth refresh APIs used by the token refresh support that follows. Co-authored-by: Copilot <[email protected]> Copilot-Session: 2dc3f23c-61a6-43a7-85cd-90aa872a7fc6

Add the gh.Credential type, its IsRefreshable helper, and the token source constants, and change the AuthConfig interface so ActiveToken and TokenForUser return a Credential instead of a (token, source) pair. This is the foundational type the refreshable-credential work builds on. Also un-ignore the internal/gh directory by anchoring the gh ignore rule to the repo root (/gh), so the built binary is still ignored but package files are not. Co-authored-by: Copilot <[email protected]> Copilot-Session: 2dc3f23c-61a6-43a7-85cd-90aa872a7fc6

Update the config AuthConfig implementation (ActiveTokenType, ActiveToken, HasActiveToken, TokenForUser, SwitchUser) and every caller and test double to the Credential-returning API introduced in the previous commit. This is a mechanical, behavior-preserving migration with no refreshable logic yet. Co-authored-by: Copilot <[email protected]> Copilot-Session: 2dc3f23c-61a6-43a7-85cd-90aa872a7fc6

Extend the file lock with a blocking acquire that retries until it succeeds or a timeout elapses, used to serialize concurrent token-refresh attempts. Co-authored-by: Copilot <[email protected]> Copilot-Session: 2dc3f23c-61a6-43a7-85cd-90aa872a7fc6

Copilot

Copilot review overview

🟡 Changes recommended

The exported HTTP client contract becomes unusable externally, and lock acquisition can succeed after context cancellation.

Get a fresh assessment by requesting another Copilot review.

Review tier: Balanced
Findings: 1 · 1

Open (2)

Internal return type breaks the exported HTTP client API · New
Check cancellation before attempting the lock · New

What changed in this PR

Introduces the credential and locking foundations needed for refreshable OAuth tokens without intentionally changing user-facing behavior.

Changes:

Adds gh.Credential and migrates authentication storage consumers.
Adds timeout-bounded file-lock acquisition and tests.
Updates OAuth dependencies and related fixtures.

File	Description
`.gitignore`	Limits the `gh` ignore rule to the repository root.
`api/http_client.go`	Migrates HTTP authentication to credentials.
`api/http_client_test.go`	Updates HTTP client test configuration.
`go.mod`	Pins refresh-support dependency revisions.
`go.sum`	Updates dependency checksums.
`internal/attachments/client_test.go`	Updates attachment test authentication.
`internal/authflow/flow.go`	Adapts OAuth flow configuration.
`internal/config/auth_config_test.go`	Migrates authentication storage assertions.
`internal/config/config.go`	Returns credentials from authentication storage.
`internal/flock/flock.go`	Adds blocking lock acquisition.
`internal/flock/flock_test.go`	Tests blocking, timeout, and error behavior.
`internal/gh/gh.go`	Defines the credential model and interface.
`internal/ghcmd/cmd.go`	Adapts authentication recovery logic.
`pkg/cmd/agent-task/agent_task.go`	Uses credential fields and source constants.
`pkg/cmd/agent-task/shared/capi.go`	Extracts the CAPI access token.
`pkg/cmd/auth/gitcredential/helper.go`	Migrates Git credential lookup.
`pkg/cmd/auth/gitcredential/helper_test.go`	Updates the credential-helper fake.
`pkg/cmd/auth/logout/logout_test.go`	Updates logout token assertions.
`pkg/cmd/auth/refresh/refresh.go`	Migrates refresh token lookups.
`pkg/cmd/auth/refresh/refresh_test.go`	Updates refresh assertions.
`pkg/cmd/auth/shared/writeable.go`	Uses credential source metadata.
`pkg/cmd/auth/status/status.go`	Migrates status credential lookup.
`pkg/cmd/auth/token/token.go`	Migrates token output lookup.
`pkg/cmd/config/get/get.go`	Extracts tokens from credentials.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

williammartin · 2026-10-02T10:58:06Z


 type config interface {
-	ActiveToken(string) (string, string)
+	ActiveToken(string) gh.Credential


Some packages this will be a problem for:

https://github.com/KEINOS/go-gisty

https://github.com/kurtosis-tech/kurtosis

williammartin · 2026-10-02T12:54:17Z

+	}
+
+	if user, err := c.ActiveUser(hostname); err == nil {
+		if credential, err := c.TokenForUser(hostname, user); err == nil {


This isn't quite behaviour-preserving. Previously the active-user lookup only checked the per-user keyring slot (TokenFromKeyringForUser). Going through TokenForUser also checks hosts.<host>.users.<user>.oauth_token in the config file, ahead of the unkeyed keyring fallback.

I checked it with a scratch test: active user u, users.u.oauth_token: per-user-config, an unkeyed keyring token legacy-keyring, and no host-level oauth_token:

trunk: token="legacy-keyring" source="keyring" PR: token="per-user-config" source="oauth_token"

It should be rare in practice because activateUser always mirrors the per-user insecure token into the host-level slot, and the new result is arguably more correct. But the description says any behavioural difference is a bug, so can we either call this out as intentional (I assume it's groundwork for the refreshable slot in #14451) with a test pinning it, or keep the old lookup order here?

williammartin · 2026-10-02T12:54:17Z

+	// ActiveToken retrieves the active credential for the given hostname, searching environment variables, general
+	// configuration, and finally encrypted storage. It returns the last stored token as-is, including the current
+	// access token of a refreshable credential, and never contacts the token endpoint to refresh it. A caller that
+	// needs a token valid for API calls should use ActiveTokenWithRefresh instead.


ActiveTokenWithRefresh doesn't exist at this point in the stack. Same for TokenForUserWithRefresh below at L246, and the matching comments in internal/config/config.go (L257, L569). Fine if this only ever merges with #14451, but if the PRs land separately these docs point at nothing.

williammartin · 2026-10-02T12:54:17Z

+	TokenSourceOAuthToken = "oauth_token"
+	// TokenSourceRefreshableOAuthToken indicates the token was read from the refreshable_oauth_token entry in the
+	// config file, which holds a refreshable credential and is kept distinct from the non-expiring oauth_token entry.
+	TokenSourceRefreshableOAuthToken = "refreshable_oauth_token"


Nothing in this PR uses TokenSourceRefreshableOAuthToken.

Resolve go.mod/go.sum conflicts by keeping trunk's dependency versions and re-pinning go-gh to v2.16.2-0.20260914233729-2ce792192e43. The old pseudo-version was v2.16.1-0.*, which sorts below the v2.16.1 release. The cli/oauth pin is unchanged. Co-authored-by: Copilot App <[email protected]>

babakks and others added 4 commits September 15, 2026 00:55

Copilot AI balanced review requested due to automatic review settings September 15, 2026 01:28

babakks requested a review from a team as a code owner September 15, 2026 01:28

babakks requested a review from niik September 15, 2026 01:28

Copilot started reviewing on behalf of babakks September 15, 2026 01:29 View session

This was referenced Sep 15, 2026

Refreshable tokens (2/7): refresher domain, OAuth refresher, storage, and wiring #14451

Open

Refreshable (short-lived) OAuth token support #14449

Open

babakks added this pull request to stack #14457 September 15, 2026 01:31

Copilot AI reviewed Sep 15, 2026

View reviewed changes

suzuki-shunsuke mentioned this pull request Sep 24, 2026

ghtkn's advantages over GitHub CLI's Refreshable (short-lived) OAuth token support suzuki-shunsuke/ghtkn#651

Open

This was referenced Oct 2, 2026

Replace cli/cli dependency with go-gh mdb/gh-release-report#18

Open

Replace cli/cli api dependency with go-gh mdb/gh-dispatch#72

Open

refactor: replace cli/cli dependency with go-gh kurtosis-tech/kurtosis#3235

Merged

williammartin reviewed Oct 2, 2026

View reviewed changes

Navigation Menu

Repository navigation

Refreshable tokens (1/7): foundation credential model, storage migration, and file lock - #14450

Refreshable tokens (1/7): foundation credential model, storage migration, and file lock#14450
babakks wants to merge 5 commits into
trunkfrom
babakks/refresh-token-a-foundation

babakks commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

williammartin Oct 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

williammartin Oct 2, 2026

Uh oh!

williammartin Oct 2, 2026 •

edited

Loading

Uh oh!

williammartin Oct 2, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

babakks commented Sep 15, 2026

Description

How did you test this change?

Key points

Notes for reviewers

Authorship and follow-up

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

williammartin Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

williammartin Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

williammartin Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

williammartin Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

williammartin Oct 2, 2026 •

edited

Loading

williammartin Oct 2, 2026 •

edited

Loading

williammartin Oct 2, 2026 •

edited

Loading