Skip to content

feat: configure transparent OAuth token refresh via auth.oauth - #1323

Open
joshuaellis wants to merge 7 commits into
mainfrom
feat/SDK-2421
Open

joshuaellis wants to merge 7 commits into
mainfrom
feat/SDK-2421

Conversation

@joshuaellis

@joshuaellis joshuaellis commented Sep 3, 2026 •

Copy link
Copy Markdown
Member

Description

Apps authenticating with short-lived OAuth access tokens would have to catch 401s, refresh, and re-create clients themselves which is annoying for everyone involved.

This lets the client own that instead: configure auth.oauth with an OAuthTokenSetup and requests, live streams, and uploads stay authenticated across token rotations without the app noticing. When a token genuinely can't be refreshed because the session is over then the app gets a single onAuthError signal to send the user back through login, instead of scattered 401s.


Note

High Risk
Touches authentication across HTTP, uploads, and long-lived SSE connections, including refresh deduplication and 401/reconnect semantics that affect session security.

Overview
Adds auth.oauth (OAuthTokenSetup with getToken, refresh, optional getExpiresAt / onAuthError) as an alternative to a static token, with config validation that the two cannot be combined.

A new oauthRefreshHandler wraps the normal request pipeline: proactive refresh before expiry, single-flight refresh() across concurrent callers, one automatic retry on 401 for fetch-based APIs, and onAuthError when refresh fails. The same setup is wired into paths that skip the default handler—asset uploads (fetch and XHR), listen / live EventSource (Bearer resolved per fetch, including reconnects), plus 401-driven reconnect with token refresh and Last-Event-ID resume so SSE can continue after auth recovery.

Uploads do not auto-retry after 401 (body may be consumed); they still refresh so a caller retry gets a new token.

Reviewed by Cursor Bugbot for commit 371fd57. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel

vercel Bot commented Sep 3, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
tsdocs-client Ignored Ignored Sep 21, 2026 2:21pm UTC

Request Review

squiggler-app Bot added a commit that referenced this pull request Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

📚 TypeDoc Generation Result

✅ TypeDoc generated successfully!

  • File size: 4.1M
  • Total exports: 254
  • Artifact: typedoc-78bf7fe61da273b9b0f53124a2fd72bfb61afd18
  • HTML docs preview: Download artifact

The TypeDoc JSON file has been generated and validated. All documentation scripts completed successfully.

@github-actions

github-actions Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 84.16% 1998 / 2374
🔵 Statements 83.81% 2076 / 2477
🔵 Functions 78.92% 543 / 688
🔵 Branches 83.12% 1463 / 1760
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/config.ts 92.1% 91.75% 100% 92% 27, 78-79, 105, 108, 115-117
src/types.ts 100% 100% 100% 100%
src/assets/AssetsClient.ts 63.93% 50.87% 73.33% 66.1% 232-234, 259-283, 292-303, 322-324, 334, 347-353
src/data/dataMethods.ts 86.89% 92.96% 74.76% 87% 290, 353-354, 448-481, 517-578, 667, 708-709, 1268-1292, 1298, 1306, 1324
src/data/listen.ts 100% 100% 100% 100%
src/data/live.ts 95.65% 93.9% 94.73% 98.82% 175, 260, 294
src/data/reconnectOnConnectionFailure.ts 87.5% 100% 75% 92.3% 72
src/data/resolveEventSourceFetch.ts 100% 100% 100% 100%
src/http/oauthRefreshHandler.ts 87.5% 75% 86.66% 97.87% 34, 145, 171, 177, 193
Generated in workflow #5140 for commit 371fd57 by the Vitest Coverage Report Action

@joshuaellis
joshuaellis force-pushed the feat/SDK-2421 branch 3 times, most recently from 8ee8ab0 to d3395f1 Compare September 4, 2026 08:56
@joshuaellis
joshuaellis marked this pull request as ready for review September 4, 2026 09:15

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread src/data/reconnectOnConnectionFailure.ts
Comment thread src/data/listen.ts

return connectEventSource(initEventSource, listenFor).pipe(
reconnectOnConnectionFailure(),
reconnectOnConnectionFailure(tokenSetup && getOAuthRefresher(tokenSetup)),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will break resumability as it will trigger a new listener request, which instantiates a new EventSource rather than reusing the existing one, bypassing the existing EventSource's built-in error/retry mechanism (which sends the Last-Event-ID header on reconnect, telling the server where to resume from).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had a look at producing tests which hopefully disproves your concern - let me know if i've missed a case worth having!

@joshuaellis

This comment was marked as outdated.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread src/http/oauthRefreshHandler.ts
@joshuaellis
joshuaellis force-pushed the feat/SDK-2421 branch 2 times, most recently from c2da3af to 07be524 Compare September 7, 2026 15:32
@joshuaellis
joshuaellis requested a review from bjoerge September 7, 2026 15:34

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread src/http/requestOptions.ts Outdated
Comment thread test/resumability.test.ts Outdated
ryanbonial
ryanbonial previously approved these changes Sep 10, 2026
Comment thread src/types.ts Outdated
/** @defaultValue true */
useCdn?: boolean
token?: string
token?: string | OAuthTokenSetup

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is potentially breaking, no? a consumer doing e.g. Bearer ${client.config().token}. Should we consider a different field for OauthTokenSetup?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it would break TS because it could be an object instead of a string. I've updated to a separate field – lmk what you think.

lastAuthRetryAt = Date.now()
return concat(
of({type: 'reconnect' as const}),
from(refreshAuth()).pipe(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a huge deal I think, but this calls refreshAuth() eagerly. Would be better to use defer(refreshAuth) here to make it call refreshAuth at subscription time.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL! Thanks ✅

for (const [key, value] of Object.entries(extraHeaders)) {
headers.set(key, value)
if (lastEventId && !headers.has('last-event-id')) {
headers.set('Last-Event-ID', lastEventId)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EventSource instances also tracks Last-Event-ID internally, how will these interact? We need to be extra careful here so we don't lose events or get duplicated delivery. Find it a bit hard to judge whether that's a real concern though 😅

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wrapper only sets the header when the package hasn't (!headers.has('last-event-id')), and both values come from the same source (#lastEventId, surfaced as message.lastEventId). So on a live instance's own reconnects the package's header wins; the client-tracked value only seeds a fresh instance. Added a test in 0be6f0e for the case that would go wrong: fresh instance seeded with evt-1, receives evt-2, drops, and its own reconnect sends evt-2.

I've (claude) tried to add tests to cover this. Beyond that i'm not sure how else we can test this – open to ideas.

Comment thread src/data/resolveEventSourceFetch.ts Outdated
// unhandled-rejection tracker flags a rejected promise in that gap.
// Attach a no-op handler synchronously; the rejection still propagates
// through the async return to the `eventsource` package's catch.
response.catch(() => {})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a hack to me. Any way we can avoid this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, reworked.

@joshuaellis joshuaellis changed the title feat: accept an OAuth token setup as token for transparent refresh feat: configure transparent OAuth token refresh via auth.oauth Sep 21, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1e9840c. Configure here.

Comment thread src/data/live.ts Outdated
@pkg-pr-new

pkg-pr-new Bot commented Sep 22, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/@sanity/client@1323

commit: 371fd57

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants