client: silent OAuth refresh-and-retry on 401 for Fjord-Account hosts #228

Merged

stephen merged 1 commit from fix/client-silent-refresh into main

2026-07-22 19:40:53 +00:00

stephen commented

2026-07-22 18:34:00 +00:00

Owner

Bug

fj's silent OAuth refresh was wired into only fj instances and fj auth. The shared Client (src/client/mod.rs) that backs fj api, fj pr, fj issue, fj repo, fj branch, etc. resolved a static bearer at construction and had no 401/refresh/retry logic. So on a Fjord-Account (SSO / FjordBearer) host, once the short-lived access token expired, every command except instances/auth 401'd with the server's "Token expired; sign in again" despite a valid stored refresh token.

Fix

Centralize refresh-and-retry in Client's core request path (request_with_headers, split into an outer refresh wrapper + send_once for the transient-failure retry loop). On a 401 for a host we actually sent a Fjord bearer to:

fjord::oidc::valid_refresh_token gates on an identity-bound stored refresh token,
fjord::oidc::refresh_session refreshes + persists the rotated token and returns the fresh access token,
the in-memory bearer is swapped and the request is retried exactly once.

PAT hosts, off-host (fj api <url>) requests, and the no-refresh-token case surface the original 401 unchanged. Bounded to a single refresh+retry, so a persistently-401 endpoint can't loop.

`&self` token mutability

request_with_headers takes &self, so the bearer moved behind an Arc<RwLock<String>> (shared across clones, so a refresh on one is visible to later requests and every clone). The lock guard is never held across an await: the header is built from a cloned snapshot, and the post-refresh swap is a short write. Client stays cheap to clone (Arc).

`fj api` path

Confirmed: the raw fj api path (src/cli/api.rs) surfaces the response straight from request_with_headers, and both the single-request and --paginate follow-up requests funnel through it, so they all get the refresh. Not just the typed Client::json/send helpers.

instances.rs call-site

Kept, not removed. The brief expected fetch_instances to become redundant, but fj instances talks to the platform's /api/v1/me/forge-instances via fjord::list_instances, which bypasses Client entirely (Client is scoped to a single forge gateway). Removing its refresh would regress fj instances. Added a comment explaining the two refresh paths are independent.

Tests

Four wiremock regressions (a #[cfg(test)] refresh hook seams the keychain-backed fjord::oidc flow so the wiring is testable without a keychain):

FjordBearer 401 + valid refresh -> refreshes once, retries, succeeds, and the rotated token is what the retry sends.
FjordBearer 401 + no valid refresh -> original 401 surfaces, no retry.
PAT host 401 -> unchanged, refresh never attempted.
Persistently-401 endpoint -> refreshes at most once (no loop).

Verify

cargo build, cargo test (682 pass, 2 pre-existing ignored), cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --check all green. No em-dashes in comments.

Do not merge. No tag pushed.

## Bug `fj`'s silent OAuth refresh was wired into only `fj instances` and `fj auth`. The shared `Client` (`src/client/mod.rs`) that backs `fj api`, `fj pr`, `fj issue`, `fj repo`, `fj branch`, etc. resolved a **static bearer at construction and had no 401/refresh/retry logic**. So on a Fjord-Account (SSO / `FjordBearer`) host, once the short-lived access token expired, every command except instances/auth 401'd with the server's "Token expired; sign in again" despite a valid stored refresh token. ## Fix Centralize refresh-and-retry in `Client`'s core request path (`request_with_headers`, split into an outer refresh wrapper + `send_once` for the transient-failure retry loop). On a 401 for a host we actually sent a Fjord bearer to: 1. `fjord::oidc::valid_refresh_token` gates on an identity-bound stored refresh token, 2. `fjord::oidc::refresh_session` refreshes + persists the rotated token and returns the fresh access token, 3. the in-memory bearer is swapped and the request is retried **exactly once**. PAT hosts, off-host (`fj api <url>`) requests, and the no-refresh-token case surface the original 401 unchanged. Bounded to a single refresh+retry, so a persistently-401 endpoint can't loop. ## `&self` token mutability `request_with_headers` takes `&self`, so the bearer moved behind an `Arc<RwLock<String>>` (shared across clones, so a refresh on one is visible to later requests and every clone). The lock guard is never held across an await: the header is built from a cloned snapshot, and the post-refresh swap is a short write. `Client` stays cheap to clone (`Arc`). ## `fj api` path Confirmed: the raw `fj api` path (`src/cli/api.rs`) surfaces the response straight from `request_with_headers`, and both the single-request and `--paginate` follow-up requests funnel through it, so they all get the refresh. Not just the typed `Client::json`/`send` helpers. ## instances.rs call-site **Kept, not removed.** The brief expected `fetch_instances` to become redundant, but `fj instances` talks to the platform's `/api/v1/me/forge-instances` via `fjord::list_instances`, which bypasses `Client` entirely (`Client` is scoped to a single forge gateway). Removing its refresh would regress `fj instances`. Added a comment explaining the two refresh paths are independent. ## Tests Four wiremock regressions (a `#[cfg(test)]` refresh hook seams the keychain-backed `fjord::oidc` flow so the wiring is testable without a keychain): - FjordBearer 401 + valid refresh -> refreshes once, retries, succeeds, and the **rotated** token is what the retry sends. - FjordBearer 401 + no valid refresh -> original 401 surfaces, no retry. - PAT host 401 -> unchanged, refresh never attempted. - Persistently-401 endpoint -> refreshes at most once (no loop). ## Verify `cargo build`, `cargo test` (682 pass, 2 pre-existing ignored), `cargo clippy --all-targets --all-features -- -D warnings`, `cargo fmt --check` all green. No em-dashes in comments. Do not merge. No tag pushed.

stephen added 1 commit

2026-07-22 18:34:00 +00:00

client: silent OAuth refresh-and-retry on 401 for Fjord-Account hosts

Forseti review / forseti review (advisory) (pull_request_target) Failing after 10s

Details

ci / check (pull_request) Successful in 10m37s

Details

ci / coverage (pull_request) Successful in 1m53s

Details

ci / live-e2e (pull_request) Successful in 2m7s

Details

a462a129d9

The shared Client resolved a static bearer at construction and never
refreshed it, so once a Fjord-Account (FjordBearer) access token expired
every command backed by Client (fj api/pr/issue/repo/branch/...) 401'd
with 'Token expired; sign in again' despite a valid stored refresh token.
Only fj instances and fj auth had their own refresh.

Centralize it in Client's core request path: on a 401 for a host we sent
a Fjord bearer to, silently call the OIDC refresh, swap the in-memory
bearer, and retry the request exactly once. PAT hosts, off-host requests,
and the no-refresh-token case surface the original 401 unchanged. Bounded
to one refresh+retry, so a persistently-401 endpoint can't loop. Because
the refresh lives in request_with_headers, the raw fj api path and every
paginated follow-up benefit, not just the typed helpers.

Token mutability: the bearer moves behind an Arc<RwLock<String>> so the
&self request path can swap it and every later request (and clone) picks
it up; the guard is never held across an await. A cfg(test) refresh hook
seams the keychain-backed fjord::oidc flow so the wiring is testable
against a mock server.

fetch_instances keeps its own refresh: fj instances talks to the platform
via fjord::list_instances, which bypasses Client entirely, so the two
refresh paths are independent (comment added, not removed).

stephen merged commit 895fe904cd into main

2026-07-22 19:40:53 +00:00

stephen deleted branch fix/client-silent-refresh

2026-07-22 19:40:53 +00:00

stephen referenced this pull request from a commit

2026-07-22 19:40:53 +00:00

client: silent OAuth refresh-and-retry on 401 for Fjord-Account hosts (#228)

stephen referenced this pull request

2026-07-22 19:54:01 +00:00

Release v0.4.1 #229