Skip to content

fix: refresh the wrangler OAuth token and retry once on 401 - #9

Merged
matej21 merged 2 commits into
mainfrom
fix/oauth-refresh-8
Nov 2, 2026
Merged

matej21 merged 2 commits into
mainfrom
fix/oauth-refresh-8

Conversation

@matej21

@matej21 matej21 commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Closes #8

Problem

With wrangler login credentials, oblaka --remote failed hard with 401 Authentication error (code 10000). The token provider only refreshed when the stored expiration_time was within the expiry buffer, and CloudflareClient.fetch threw on any non-OK response. A token the API rejected while its stored expiry still looked valid could not be recovered; wrangler whoami (which rewrites the stored token) was the workaround.

The exact cause of the original failure is not proven: wrangler itself refreshes only after the stored expiry has passed and has no 401 retry. This PR adds the refresh-and-retry safeguard the issue asks for, so a rejected OAuth token is recovered either way.

Change

  • createWranglerTokenProvider now returns { getToken, refreshToken }.
    • getToken() keeps the expiry-based refresh (same flow and endpoint as wrangler).
    • refreshToken(rejected) returns a replacement for a token the API rejected. It first re-reads wrangler's config: if the file changed since oblaka last read or wrote it (wrangler refreshed, possibly rotating the refresh token), it adopts that instead of spending a stale refresh token. Otherwise it refreshes and persists, as before.
    • The newest credentials, including a rotated refresh token, are kept in memory, so --no-persist-token or a failed write-back doesn't lead to reusing a rejected token or a spent refresh token.
    • Concurrent refreshes share one in-flight token request, so parallel callers don't each spend a refresh token that Cloudflare may rotate.
  • CloudflareClient takes an optional refreshToken hook. On a 401 it asks for a replacement and retries the request once. If the retry is rejected too, the error carries a wrangler login hint. If the refresh itself fails, the error keeps the request URL and the original 401 body, and the refresh error includes the token endpoint's response. With CLOUDFLARE_API_TOKEN there is no hook, so nothing changes.

Tests

  • tests/auth.test.ts: refresh of a rejected token whose stored expiry is still valid (persisted), adopting a token wrangler wrote meanwhile, concurrent refresh dedupe (expiry path and concurrent 401s), rotated refresh token reused without persisting, refresh failure propagating the hint and endpoint body.
  • tests/client.test.ts: 401, then refresh and a successful retry; POST retried with the same body; a second 401 fails with the hint; a failed refresh keeps the request context; no retry without a hook.

Ran locally: bun test (129 pass), bun run typecheck, bun run lint, bun run format:check.

Status

  • Review round 1 (e677e55): approve, 4 findings (1 low-medium, 1 low, 1 nit, 1 info); all addressed in fe6c425
  • CI: pass on fe6c425
  • Merged in 0a6edb8 (merge commit)

🤖 Generated with Claude Code

The token provider refreshed only when the stored expiration_time was near,
so a token the API rejected while its stored expiry still looked valid caused
a hard 401 (code 10000). The client now asks the provider for a replacement
on a 401 and retries the request once.

The provider re-reads wrangler's config before refreshing, so a token wrangler
already refreshed (and a rotated refresh token) is adopted instead of spending
a stale refresh token. Concurrent refreshes share one token request.

Closes #8

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@matej21

matej21 commented Nov 2, 2026

Copy link
Copy Markdown
Member Author

Review round 1

  • Reviewer: Claude Code subagent (general-purpose, Claude Opus 5.5), independent read-only review of the diff
  • Commit: e677e55
  • Verdict: approve (finding 1 recommended before merge)
  • Checks re-run by reviewer: bun test (125 pass), typecheck, lint, format:check

Verified correct: the shared refresh promise always clears and can't run twice at once; concurrent 401s share one refresh; the retry sends the same request; API-token users don't change (no hook, so no retry); no new type escapes.

Findings

  1. low-medium: src/auth.ts, refreshToken. With --no-persist-token (or a failed write-back), the provider keeps only the new access token in memory. The rotated refresh token is lost, and the file still holds the old access token. A second 401 in the same process re-adopts the already rejected file token (it differs from the newly rejected one and looks unexpired), and later refreshes spend the old refresh token. Reproduced with a probe script.
  2. low: src/client.ts. When the refresh itself fails, the error loses the request URL and the original 401. The refresh error also drops the token endpoint's response body.
  3. nit: tests. There is no client test where the refresh rejects and no POST-with-body retry test. The concurrency test starts the refresh through getToken with an expired token, not through several refreshToken calls on a still-valid token, which is the issue's scenario.
  4. info: wrangler has no 401 retry. It refreshes only after the stored expiry has passed, so the exact root cause of --remote fails with 401 Authentication error instead of refreshing the wrangler OAuth token #8 is not proven. The PR description shouldn't claim more than adding the 401 refresh-and-retry safeguard the issue asks for.

…h failure

Without persisting (or when the write-back fails), a second 401 re-adopted
the already rejected token from the file and refreshed with a spent refresh
token. The provider now keeps the newest credentials in memory, adopts the
file only when it changed since the last read or write, and never reuses a
rejected token.

When the refresh itself fails, the client error now keeps the request URL
and the original 401 body, and the refresh error includes the token
endpoint's response.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@matej21

matej21 commented Nov 2, 2026

Copy link
Copy Markdown
Member Author

Response to review round 1

  1. low-medium, rotated refresh token lost / rejected token re-adopted: fixed in fe6c425. The provider keeps the newest credentials (including a rotated refresh token) in memory. It adopts the file only when the file changed since the last read or write, and it never reuses a rejected token. Regression test: "without persisting, a second 401 refreshes with the rotated refresh token" (fails against e677e55).
  2. low, error context lost on refresh failure: fixed in fe6c425. The client error keeps the URL and the original 401 body and appends the refresh failure (with cause). The refresh error now includes the token endpoint's response body.
  3. nit, test coverage: fixed in fe6c425. Added tests for a rejected refresh in the client, a POST retry with the same body, concurrent refreshToken calls on a still-valid token, and a late 401 after the refresh finished.
  4. info, root cause of --remote fails with 401 Authentication error instead of refreshing the wrangler OAuth token #8 unproven: the PR description now says so. The change adds the refresh-and-retry safeguard the issue asks for; it doesn't claim to explain the original failure.

@matej21
matej21 merged commit 0a6edb8 into main Nov 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--remote fails with 401 Authentication error instead of refreshing the wrangler OAuth token

1 participant