Skip to content

fix(oauth): keep refresh tokens alive when Kody refreshes concurrently - #123

Merged
kody-bot merged 2 commits into
mainfrom
cursor/refresh-token-reuse-grace-f42b
Sep 24, 2026
Merged

kody-bot merged 2 commits into
mainfrom
cursor/refresh-token-reuse-grace-f42b

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

MediRSS was forcing a weekly MediRSS re-auth in Kody because overlapping refresh-token rotations were treated as theft and the whole token family was revoked.

Root cause

Access tokens last 1 hour. Refresh tokens rotate on every use, and any reuse revoked the family. That is correct for a stolen token replayed later. It is not correct for the client Kody actually is.

Kody's MCP OAuth client is @modelcontextprotocol/sdk 1.30.0 auth() (packages/worker depends on agents 0.22.0, which uses that SDK). auth() has no single-flight lock around refresh. When the access token expires, two in-flight calls both POST the same refresh token. The loser receives invalid_grant. The SDK then calls invalidateCredentials('tokens'), which deletes the stored refresh token. If the family was already revoked, the winner's replacement is dead too, so the next refresh fails the same way.

The account card is written after that wipe, so storage no longer has a refresh token:

This MCP server's stored access token is no longer usable and Kody has no refresh token to renew it (phase token exchange, mcp https://mediarss.doddsfamily.us/mcp, id 7b628aeb-def2-45b6-9c48-e42585e84026).

That id is Kody's recovery attempt id, not the saved server id. formatMcpOAuthSettleDetails appends id ${attemptId} (packages/worker/src/mcp-client/oauth-token-recovery.ts stamps a new attempt id when it parks the connection). The saved server is 293a8499-2649-43b6-bd09-ac72ba674274. mcpServerList shows hasRefreshToken: false for that server.

Checked against the live request and ruled out as the cause of the missing refresh token:

  • Client id https://kody.codes/oauth/client-metadata.json includes grant_types: ["authorization_code", "refresh_token"]. Redirect is https://kody.codes/account/mcp-servers/oauth/callback.
  • Live authorization-server metadata advertises refresh_token.
  • The live authorize URL sends scope=mcp:read mcp:write and resource=https://mediarss.doddsfamily.us/mcp. The token endpoint ignores unknown form fields, including resource. A code exchange and a refresh that both send that resource still return a refresh_token (new test).
  • A grant that never included a refresh token would die about an hour after authorize (JWT exp is 3600s, and every MCP request re-checks it), not after a week of use.
  • home (https://kody-home.doddsfamily.us/mcp) stays ready with hasRefreshToken: true. The same Kody client survives there. That server also advertises refresh_token, but it is a different authorization server. The failure is specific to this server's rotate-and-revoke-on-any-reuse policy. A sibling agent is checking Kody's client; this PR fixes the server behavior that turns that client's overlap into a logged-out connection.

Fix

Reuse of a refresh token within 60 seconds returns the family's current live refresh token (HTTP 200, same refresh_token the first caller received) and does not revoke the family. Requests for that token are serialized per family in-process, with atomic used_at / replaced_by updates if two processes share the database file.

Reuse after 60 seconds still revokes the family, including when the presented token's own expires_at has already passed. Revoked tokens are stamped outside the grace window so presenting one cannot mint a replacement.

replaced_by is added in 20260923235900_refresh_token_reuse_grace. Public clients are still required to rotate refresh tokens (MCP authorization spec); the grace window is only for the overlap.

Test Plan

  • npm run validate (format, lint, typecheck, 271 tests)
  • Authorize and refresh with scope=mcp:read mcp:write and resource=https://mediarss.doddsfamily.us/mcp still return a refresh token
  • Immediate replay of a just-rotated refresh token returns 200 and the same successor; a later refresh of that successor still works
  • Two overlapping refreshes of the same token both return 200 with one shared successor, and that successor still refreshes
  • Replay after the grace window, and replay of an expired used token, still return invalid_grant and revoke the descendant

After deploy, Kent should:

  1. Open the authorization link once from /account/mcp-servers for the saved server 293a8499-2649-43b6-bd09-ac72ba674274 (https://mediarss.doddsfamily.us/mcp).
  2. Confirm mcpServerList shows mediarss with hasRefreshToken: true and connected.
  3. Use the server across an access-token expiry (1 hour) or several refreshes. hasRefreshToken should stay true without another browser login.
  4. Optional: [oauth.token] refresh_reused_within_grace in the logs means an overlap was absorbed. refresh_replay_revoked still means a real replay outside the window.

Checklist

  • Tests updated
  • Docs updated (no user-facing docs; this is token-endpoint behavior)

Screenshots

Open in Web Open in Cursor 

Summary by CodeRabbit

  • Bug Fixes
    • Refresh-token requests now retain the refresh token when using a resource indicator.
    • Briefly reusing a recently rotated refresh token returns the current token instead of failing. Concurrent refresh requests also receive a consistent result.
    • Reuse after the grace period continues to invalidate the token family. Requested scopes that exceed the original grant are rejected.

Concurrent MCP clients present the same rotating refresh token when an
access token expires. Treating that overlap as theft revoked the family
and forced a manual login. Reuse within 60 seconds now returns the live
refresh token; later reuse still revokes the family.

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 81a44ab0-3cc3-4ff1-b58b-649ff921827d

📥 Commits

Reviewing files that changed from the base of the PR and between af2bf0d and 893ff10.

📒 Files selected for processing (7)
  • app/db/migrate.test.ts
  • app/db/migrations/20260923235900_refresh_token_reuse_grace/down.sql
  • app/db/migrations/20260923235900_refresh_token_reuse_grace/up.sql
  • app/db/schema.ts
  • app/oauth/oauth.test.ts
  • app/oauth/refresh-tokens.ts
  • app/routes/oauth/token.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Refresh-token redemption now records successor tokens and permits reuse within a 60-second grace window. Reuse after that window revokes the token family. The OAuth token route uses the redemption result, and the migration and OAuth tests cover the updated behavior.

Changes

Refresh token redemption

Layer / File(s) Summary
Persist refresh-token successors
app/db/schema.ts, app/db/migrations/20260923235900_refresh_token_reuse_grace/*.sql, app/db/migrate.test.ts
The schema and migration add the nullable replaced_by column. Migration tests cover applying, adopting, and reverting the migration, and assert the column list.
Redeem and rotate refresh tokens
app/oauth/refresh-tokens.ts
redeemRefreshToken validates tokens and scopes, serializes in-process refreshes by family, and rotates unused tokens. Reuse within 60 seconds returns the live family token; later reuse revokes the family.
Connect redemption to the OAuth route
app/routes/oauth/token.ts, app/oauth/oauth.test.ts
The route handles rejected and reused redemption results. Tests cover resource indicators, grace-window reuse, concurrent refreshes, and replay after the grace window.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant OAuthTokenRoute
  participant redeemRefreshToken
  participant RefreshTokenStore
  Client->>OAuthTokenRoute: Submit refresh-token grant
  OAuthTokenRoute->>redeemRefreshToken: Pass token, clientId, and scope
  redeemRefreshToken->>RefreshTokenStore: Claim token and find live family token
  redeemRefreshToken->>RefreshTokenStore: Create successor and link replaced_by
  redeemRefreshToken-->>OAuthTokenRoute: Return rotated, reused, or rejected result
  OAuthTokenRoute-->>Client: Return token response or error
Loading

Merge Risk: ⚪ Minimal · up to 893ff

Overlapping refresh requests from the same OAuth client now receive the same rotated refresh token instead of revoking the session. Replay after 60 seconds still revokes the token family. The schema change is additive and reversible, and the new tests cover the intended timing behavior. No concrete merge-blocking issue was found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 5 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main OAuth change: preserving refresh tokens during concurrent Kody refresh requests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 5 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

The live authorize request includes scope mcp:read mcp:write and
resource=https://mediarss.doddsfamily.us/mcp. That parameter is ignored
by the token endpoint and must not be what drops the refresh token.

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
@kody-bot
kody-bot marked this pull request as ready for review September 24, 2026 00:12
@kody-bot
kody-bot merged commit f0b04ab into main Sep 24, 2026
6 checks passed
@cursor

cursor Bot commented Sep 24, 2026

Copy link
Copy Markdown

Squash-merged and the Docker image is published (f0b04ab).

One-time re-auth, after the host is running that commit (GET /health → commit.sha):

  1. From https://kody.codes/account/mcp-servers, authorize saved server 293a8499-2649-43b6-bd09-ac72ba674274 once.
  2. After the access token expires (about 1 hour), confirm hasRefreshToken stays true.

Live /health is still 394bfed (process started 2026-08-31), so re-auth before the new image is up will not stick.

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.

3 participants