Conversation
Instead of immediately forcing re-authentication when the local clock says the token is expired, probe GET /api/0/users/me/ to verify the token is actually invalid. This avoids unnecessary logouts when the stored expiry is stale or the clock is skewed. Restores the env parameter to tokenExchangeCallback for SENTRY_HOST access. Adds a new "success_probed" metric outcome to distinguish probe-verified tokens from clock-verified ones. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace raw fetch with SentryApiService.getAuthenticatedUser() for the upstream token validity check. Merge probed-success into the regular success metric outcome, and extend the probe-issued TTL to 1 hour since a 401 from Sentry will trigger re-auth anyway. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Log unexpected errors (network failures, server errors) when probing upstream token validity, while silently handling expected 401s. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add end-to-end tests that exercise the full refresh token flow: issue tokens → refresh → use refreshed token for MCP request. Tests both paths: clock-valid refresh (fast path) and clock-expired refresh with upstream probe (slow path). Both verify the refreshed token actually works for subsequent MCP requests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the fast-path and probe-success unit tests from helpers.test.ts since the integration tests in mcp.test.ts now cover these flows end-to-end. Keep the edge case unit tests (no refresh token, network error, safety window, missing expiry) that integration tests don't exercise. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use outcome "success_probed" for tokens verified via upstream probe, keeping "success" for the clock-verified fast path. This makes the two paths distinguishable in metrics dashboards. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Add refreshToken to DEFAULT_WORKER_PROPS so tokenExchangeCallback doesn't early-return before reaching the probe logic. Add assertion that the upstream API probe was actually called during the expired-clock refresh test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of immediately forcing re-authentication when the local clock
says the OAuth token is expired, probe
GET /api/0/users/me/to verifythe token is actually invalid. This avoids unnecessary logouts when the
stored expiry timestamp is stale or the clock is skewed.
Restores the
envparameter totokenExchangeCallback(removed inb2d46e2 when it became unused) so the callback can read
SENTRY_HOST.Adds a
success_probedmetric outcome to distinguish probe-verifiedtokens from clock-verified ones. Network errors during the probe fail
closed (force re-auth).