Skip to content

fix: exclude terminal-status oauth configs from expiring token refresh query - #4754

Merged
Pratham-Mishra04 merged 1 commit into
devfrom
06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants
Jun 30, 2026
Merged

fix: exclude terminal-status oauth configs from expiring token refresh query#4754
Pratham-Mishra04 merged 1 commit into
devfrom
06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants

Conversation

@Pratham-Mishra04

Copy link
Copy Markdown
Collaborator

Summary

The OAuth token refresh worker was repeatedly selecting permanently-dead tokens on every tick because their expires_at timestamp remains in the past after a terminal failure (e.g., invalid_grant / "Grant not found"). This caused the same error to be logged indefinitely. Tokens whose owning oauth_config has reached a terminal state (expired or revoked) no longer need to be refreshed — they require re-authorization instead.

Changes

  • GetExpiringOauthTokens now excludes tokens whose associated oauth_config has a status of expired or revoked, using a NOT EXISTS subquery. This prevents the refresh worker from endlessly retrying permanently-dead grants.
  • A new test, TestGetExpiringOauthTokens_ExcludesTerminalConfigs, validates that tokens linked to terminal configs are excluded from the result, while tokens with an active (authorized) config or no config at all are still returned.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./framework/configstore/... -run TestGetExpiringOauthTokens_ExcludesTerminalConfigs -v

Expected: the test passes, confirming that tokens with expired or revoked configs are excluded, while tokens with authorized configs or no config are included.

Breaking changes

  • Yes
  • No

Related issues

Security considerations

This change reduces unnecessary outbound refresh requests for revoked or expired OAuth grants, which could otherwise leak information about token state to external authorization servers on every refresh tick.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8c1e8ded-32ab-4982-8510-1c4a2efb750c

📥 Commits

Reviewing files that changed from the base of the PR and between e424bd0 and aa1ca02.

📒 Files selected for processing (2)
  • framework/configstore/rdb.go
  • framework/configstore/rdb_oauth2_test.go
📝 Walkthrough

Walkthrough

GetExpiringOauthTokens in rdb.go gains a NOT EXISTS subquery that excludes tokens whose associated oauth_configs row has status IN ('expired', 'revoked'). A new test TestGetExpiringOauthTokens_ExcludesTerminalConfigs validates all four cases: expired config, revoked config, live config, and orphan token.

Changes

Terminal OAuth config exclusion

Layer / File(s) Summary
Query filter and test coverage
framework/configstore/rdb.go, framework/configstore/rdb_oauth2_test.go
GetExpiringOauthTokens adds a correlated NOT EXISTS subquery against oauth_configs to drop tokens with status IN ('expired','revoked'); the new test covers live, expired, revoked, and orphan token/config combinations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A bunny checks the token queue,
"Expired or revoked? Skip those two!"
NOT EXISTS hops into place,
Keeping stale configs out of the race.
🐇 Only the fresh ones get to renew!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: excluding terminal-status OAuth configs from the expiring-token query.
Description check ✅ Passed The description follows the template well, covering summary, changes, testing, change type, impact, security, and checklist items.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants

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

Pratham-Mishra04 commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps

greptile-apps Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
framework/configstore/rdb.go Adds a terminal-status filter to the expiring OAuth token query.
framework/configstore/rdb_oauth2_test.go Adds test coverage for terminal, active, and orphan OAuth token selection.

Reviews (7): Last reviewed commit: "fix: stop token refresh worker from retr..." | Re-trigger Greptile

Comment thread framework/configstore/rdb.go
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 29, 2026
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode branch from 92bc69c to e1cdee5 Compare June 29, 2026 10:59
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants branch from 6f31c89 to e424bd0 Compare June 29, 2026 10:59
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode branch from e1cdee5 to ad3df93 Compare June 29, 2026 18:18
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants branch from e424bd0 to 5888e03 Compare June 29, 2026 18:18
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode branch from ad3df93 to a9fef25 Compare June 30, 2026 07:53
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants branch from 5888e03 to 9b9c2d9 Compare June 30, 2026 07:53
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode branch from a9fef25 to 6686182 Compare June 30, 2026 11:43
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants branch from 9b9c2d9 to 500e369 Compare June 30, 2026 11:43
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode branch from 6686182 to 6673c1a Compare June 30, 2026 13:44
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants branch from 500e369 to 36bd514 Compare June 30, 2026 13:44

Pratham-Mishra04 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Merge activity

  • Jun 30, 1:53 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 30, 2:35 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jun 30, 2:36 PM UTC: @Pratham-Mishra04 merged this pull request with Graphite.

@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode to graphite-base/4754 June 30, 2026 14:31
@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from graphite-base/4754 to dev June 30, 2026 14:34
@Pratham-Mishra04
Pratham-Mishra04 dismissed coderabbitai[bot]’s stale review June 30, 2026 14:34

The base branch was changed.

@Pratham-Mishra04
Pratham-Mishra04 requested a review from a team as a code owner June 30, 2026 14:34
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants branch from 36bd514 to aa1ca02 Compare June 30, 2026 14:34
@Pratham-Mishra04
Pratham-Mishra04 merged commit da99b8c into dev Jun 30, 2026
14 of 16 checks passed
@Pratham-Mishra04
Pratham-Mishra04 deleted the 06-28-fix_stop_token_refresh_worker_from_retrying_permanently-failed_oauth_grants branch June 30, 2026 14:36
akshaydeo pushed a commit that referenced this pull request Jul 1, 2026
…h query (#4754)

## Summary

The OAuth token refresh worker was repeatedly selecting permanently-dead tokens on every tick because their `expires_at` timestamp remains in the past after a terminal failure (e.g., `invalid_grant` / "Grant not found"). This caused the same error to be logged indefinitely. Tokens whose owning `oauth_config` has reached a terminal state (`expired` or `revoked`) no longer need to be refreshed — they require re-authorization instead.

## Changes

- `GetExpiringOauthTokens` now excludes tokens whose associated `oauth_config` has a status of `expired` or `revoked`, using a `NOT EXISTS` subquery. This prevents the refresh worker from endlessly retrying permanently-dead grants.
- A new test, `TestGetExpiringOauthTokens_ExcludesTerminalConfigs`, validates that tokens linked to terminal configs are excluded from the result, while tokens with an active (`authorized`) config or no config at all are still returned.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./framework/configstore/... -run TestGetExpiringOauthTokens_ExcludesTerminalConfigs -v
```

Expected: the test passes, confirming that tokens with `expired` or `revoked` configs are excluded, while tokens with `authorized` configs or no config are included.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

This change reduces unnecessary outbound refresh requests for revoked or expired OAuth grants, which could otherwise leak information about token state to external authorization servers on every refresh tick.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
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.

2 participants