From bfd2a0c8e0d4558fedece25b88fb93e5e44b72d7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 10:42:05 +0000 Subject: [PATCH 01/13] Initial plan From 6032bce6aef3afad2264a8b9bcb19deb32f55f5f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 10:46:31 +0000 Subject: [PATCH 02/13] Implement background token refresh for old tokens - Add issued_at field to TokenData to track token creation time - Add is_old() method to check if token should be refreshed (< 50% lifetime) - Trigger non-blocking background refresh when token is old but valid - Keep blocking refresh for expired tokens - Add comprehensive tests for background refresh behavior Co-authored-by: iwillspeak <1004401+iwillspeak@users.noreply.github.com> --- src/resolver_athena_client/client/channel.py | 82 ++++++- tests/client/test_channel.py | 219 +++++++++++++++++++ 2 files changed, 297 insertions(+), 4 deletions(-) diff --git a/src/resolver_athena_client/client/channel.py b/src/resolver_athena_client/client/channel.py index 00131a0..099db8d 100644 --- a/src/resolver_athena_client/client/channel.py +++ b/src/resolver_athena_client/client/channel.py @@ -28,11 +28,29 @@ class TokenData(NamedTuple): access_token: str expires_at: float scheme: str + issued_at: float = 0.0 def is_valid(self) -> bool: """Check if this token is still valid (with a 30-second buffer).""" return time.time() < (self.expires_at - 30) + def is_old(self) -> bool: + """Check if this token should be proactively refreshed. + + A token is considered "old" if less than 50% of its lifetime remains. + This allows background refresh to happen before expiry while the token + is still usable. + """ + if self.issued_at == 0.0: + # Fallback for tokens created before issued_at was tracked + # Consider old if less than 30% of expiry buffer remains + return time.time() >= (self.expires_at - 90) + + current_time = time.time() + total_lifetime = self.expires_at - self.issued_at + time_remaining = self.expires_at - current_time + return time_remaining < (total_lifetime * 0.5) + class CredentialHelper: """OAuth credential helper for managing authentication tokens.""" @@ -67,13 +85,16 @@ def __init__( self._audience: str = audience self._token_data: TokenData | None = None self._lock: threading.Lock = threading.Lock() + self._refresh_thread: threading.Thread | None = None def get_token(self) -> TokenData: """Get valid token data, refreshing if necessary. - Uses double-checked locking: the happy path (token is valid) - avoids acquiring the lock entirely. The lock is only taken - when the token needs to be refreshed. + Uses double-checked locking with background refresh optimization: + - If token is valid and fresh, returns immediately (no lock) + - If token is valid but old, triggers background refresh and + returns current token + - If token is expired, blocks until new token is acquired Returns ------- @@ -86,9 +107,15 @@ def get_token(self) -> TokenData: """ token_data = self._token_data + + # Fast path: token is valid and fresh if token_data is not None and token_data.is_valid(): + # If token is old, trigger background refresh + if token_data.is_old(): + self._start_background_refresh() return token_data + # Slow path: token is expired or missing, must block with self._lock: token_data = self._token_data if token_data is not None and token_data.is_valid(): @@ -102,6 +129,51 @@ def get_token(self) -> TokenData: raise RuntimeError(msg) return token_data + def _start_background_refresh(self) -> None: + """Start a background thread to refresh the token. + + Only starts a new thread if one isn't already running. + + This method is safe to call multiple times - it only starts a new + thread if no refresh is currently in progress. + """ + # Quick check without lock - if refresh thread exists and is + # alive, skip + if self._refresh_thread is not None and self._refresh_thread.is_alive(): + return + + # Try to acquire lock and start refresh + if self._lock.acquire(blocking=False): + try: + # Double-check: another thread might have started refresh + refresh_needed = ( + self._refresh_thread is None + or not self._refresh_thread.is_alive() + ) + if refresh_needed: + self._refresh_thread = threading.Thread( + target=self._background_refresh, + daemon=True, + ) + self._refresh_thread.start() + finally: + self._lock.release() + + def _background_refresh(self) -> None: + """Background thread target for token refresh. + + Acquires the lock and refreshes the token. Errors are silently + ignored since the next foreground request will retry if needed. + """ + with self._lock: + # ruff: noqa: SIM105 + try: + self._refresh_token() + except Exception: # noqa: BLE001, S110 + # Silently ignore errors in background refresh + # Next get_token() call will retry if needed + pass + def _refresh_token(self) -> None: """Refresh the authentication token by making an OAuth request. @@ -138,10 +210,12 @@ def _refresh_token(self) -> None: token_type = raw.get("token_type", "Bearer") # Preserve server-provided casing, only strip whitespace scheme: str = token_type.strip() if token_type else "Bearer" + current_time = time.time() self._token_data = TokenData( access_token=access_token, - expires_at=time.time() + expires_in, + expires_at=current_time + expires_in, scheme=scheme, + issued_at=current_time, ) except httpx.HTTPStatusError as e: diff --git a/tests/client/test_channel.py b/tests/client/test_channel.py index 221907d..4ffe002 100644 --- a/tests/client/test_channel.py +++ b/tests/client/test_channel.py @@ -426,3 +426,222 @@ def test_plugin_catches_unexpected_exceptions(self) -> None: plugin(mock_context, mock_callback) mock_callback.assert_called_once_with((), runtime_error) + + +class TestBackgroundTokenRefresh: + """Tests for background token refresh functionality.""" + + def test_token_is_old_when_past_halfway_lifetime(self) -> None: + """Test that a token is considered old when past 50% of its lifetime.""" + current_time = time.time() + # Token with 1 hour lifetime, 20 minutes remaining (33%) + token = TokenData( + access_token="test_token", + expires_at=current_time + 1200, # 20 minutes from now + scheme="Bearer", + issued_at=current_time - 2400, # 40 minutes ago + ) + # Total lifetime = 3600s, remaining = 1200s (33%), so it's old + assert token.is_old() + + def test_token_is_not_old_when_fresh(self) -> None: + """Test that a token is not old when more than 50% lifetime remains.""" + current_time = time.time() + # Token with 1 hour lifetime, 40 minutes remaining (67%) + token = TokenData( + access_token="test_token", + expires_at=current_time + 2400, # 40 minutes from now + scheme="Bearer", + issued_at=current_time - 1200, # 20 minutes ago + ) + # Total lifetime = 3600s, remaining = 2400s (67%), so it's fresh + assert not token.is_old() + + def test_token_is_old_fallback_for_legacy_tokens(self) -> None: + """Test fallback logic for tokens without issued_at.""" + current_time = time.time() + # Legacy token without issued_at (defaults to 0.0) + token = TokenData( + access_token="test_token", + expires_at=current_time + 60, # 1 minute from now + scheme="Bearer", + ) + # Should be considered old if less than 90s remain + assert token.is_old() + + def test_token_is_not_old_fallback_for_fresh_legacy_tokens(self) -> None: + """Test fallback logic for fresh legacy tokens.""" + current_time = time.time() + # Legacy token with plenty of time remaining + token = TokenData( + access_token="test_token", + expires_at=current_time + 200, # 200 seconds from now + scheme="Bearer", + ) + # Should not be considered old if more than 90s remain + assert not token.is_old() + + def test_get_token_triggers_background_refresh_for_old_token(self) -> None: + """Test that get_token triggers background refresh for old tokens.""" + helper = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + ) + + current_time = time.time() + # Set up an old but valid token + helper._token_data = TokenData( + access_token="old_token", + expires_at=current_time + 1200, # 20 minutes remaining + scheme="Bearer", + issued_at=current_time - 2400, # 40 minutes ago, so it's old + ) + + with mock.patch.object( + helper, "_start_background_refresh" + ) as mock_start: + token_data = helper.get_token() + + # Should return current token immediately + assert token_data.access_token == "old_token" + # Should have triggered background refresh + mock_start.assert_called_once() + + def test_get_token_does_not_trigger_refresh_for_fresh_token(self) -> None: + """Test that get_token does not trigger refresh for fresh tokens.""" + helper = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + ) + + current_time = time.time() + # Set up a fresh, valid token + helper._token_data = TokenData( + access_token="fresh_token", + expires_at=current_time + 2400, # 40 minutes remaining + scheme="Bearer", + issued_at=current_time - 1200, # 20 minutes ago, so it's fresh + ) + + with mock.patch.object( + helper, "_start_background_refresh" + ) as mock_start: + token_data = helper.get_token() + + # Should return current token + assert token_data.access_token == "fresh_token" + # Should NOT have triggered background refresh + mock_start.assert_not_called() + + def test_background_refresh_does_not_start_if_already_running(self) -> None: + """Test that background refresh doesn't start duplicate threads.""" + helper = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + ) + + # Mock a running refresh thread + mock_thread = mock.Mock() + mock_thread.is_alive.return_value = True + helper._refresh_thread = mock_thread + + with mock.patch("threading.Thread") as mock_thread_class: + helper._start_background_refresh() + + # Should not create a new thread + mock_thread_class.assert_not_called() + + def test_background_refresh_starts_new_thread_if_none_exists(self) -> None: + """Test that background refresh starts a thread when none exists.""" + helper = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + ) + + mock_thread = mock.Mock() + with mock.patch("threading.Thread", return_value=mock_thread): + helper._start_background_refresh() + + # Should have started the thread + mock_thread.start.assert_called_once() + + def test_background_refresh_silently_handles_errors(self) -> None: + """Test that background refresh silently ignores errors.""" + helper = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + ) + + # Mock refresh to raise an error + with mock.patch.object( + helper, "_refresh_token", side_effect=OAuthError("Test error") + ): + # Should not raise an exception + helper._background_refresh() + + def test_get_token_blocks_for_expired_token(self) -> None: + """Test that get_token blocks and refreshes when token is expired.""" + helper = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + ) + + # Set up an expired token + helper._token_data = TokenData( + access_token="expired_token", + expires_at=time.time() - 100, # Expired + scheme="Bearer", + issued_at=time.time() - 3700, + ) + + mock_response = mock.Mock() + mock_response.json.return_value = { + "access_token": "new_token", + "expires_in": 3600, + "token_type": "bearer", + } + mock_response.raise_for_status.return_value = None + + with mock.patch("httpx.Client") as mock_client: + mock_response_obj = mock_client.return_value.__enter__.return_value + mock_response_obj.post.return_value = mock_response + + token_data = helper.get_token() + + # Should have refreshed and returned new token + assert token_data.access_token == "new_token" + # Should have called the OAuth endpoint + mock_response_obj.post.assert_called_once() + + def test_refresh_token_sets_issued_at(self) -> None: + """Test that _refresh_token sets the issued_at timestamp.""" + helper = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + ) + + mock_response = mock.Mock() + mock_response.json.return_value = { + "access_token": "new_token", + "expires_in": 3600, + "token_type": "bearer", + } + mock_response.raise_for_status.return_value = None + + before_time = time.time() + with mock.patch("httpx.Client") as mock_client: + mock_response_obj = mock_client.return_value.__enter__.return_value + mock_response_obj.post.return_value = mock_response + + _ = helper.get_token() + + after_time = time.time() + + # Check that issued_at was set to a reasonable value + assert helper._token_data is not None + assert before_time <= helper._token_data.issued_at <= after_time + # Check that expires_at is approximately issued_at + 3600 + assert ( + helper._token_data.expires_at - helper._token_data.issued_at - 3600 + < 1 + ) From c50327de35fbf3ed6808f81479bc76a5d7d98c59 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 10:48:11 +0000 Subject: [PATCH 03/13] fix: update tests to match preserved token scheme casing Tests were expecting capitalized schemes but implementation was changed in 4b7e73a to preserve server-provided casing. Co-authored-by: iwillspeak <1004401+iwillspeak@users.noreply.github.com> --- tests/client/test_channel.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/client/test_channel.py b/tests/client/test_channel.py index 4ffe002..5fe30c0 100644 --- a/tests/client/test_channel.py +++ b/tests/client/test_channel.py @@ -173,7 +173,7 @@ def test_get_token_success(self) -> None: token_data = helper.get_token() assert token_data.access_token == "new_access_token" - assert token_data.scheme == "Bearer" + assert token_data.scheme == "bearer" assert helper._token_data is not None assert helper._token_data.expires_at is not None @@ -198,7 +198,7 @@ def test_get_token_respects_token_type(self) -> None: token_data = helper.get_token() - assert token_data.scheme == "Dpop" + assert token_data.scheme == "DPoP" def test_get_token_defaults_to_bearer(self) -> None: """Test that scheme defaults to Bearer when token_type is absent.""" From b8e9fa3f3071be640e5d4ac6284714ed045042d2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 10:51:20 +0000 Subject: [PATCH 04/13] Address code review feedback - Add logging for background refresh failures - Improve comments explaining magic numbers - Add test for server casing preservation - Add comment explaining test tolerance Co-authored-by: iwillspeak <1004401+iwillspeak@users.noreply.github.com> --- src/resolver_athena_client/client/channel.py | 26 ++++++++----- tests/client/test_channel.py | 39 ++++++++++++++++++++ 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/src/resolver_athena_client/client/channel.py b/src/resolver_athena_client/client/channel.py index 099db8d..48c4b30 100644 --- a/src/resolver_athena_client/client/channel.py +++ b/src/resolver_athena_client/client/channel.py @@ -1,6 +1,7 @@ """Channel creation utilities for the Athena client.""" import json +import logging import threading import time from typing import NamedTuple, override @@ -15,6 +16,8 @@ OAuthError, ) +logger = logging.getLogger(__name__) + class TokenData(NamedTuple): """Immutable snapshot of token state. @@ -42,8 +45,9 @@ def is_old(self) -> bool: is still usable. """ if self.issued_at == 0.0: - # Fallback for tokens created before issued_at was tracked - # Consider old if less than 30% of expiry buffer remains + # Fallback for tokens created before issued_at was tracked. + # Use 90s threshold (3x the 30s validity buffer) to ensure + # old tokens get refreshed proactively. return time.time() >= (self.expires_at - 90) current_time = time.time() @@ -162,17 +166,21 @@ def _start_background_refresh(self) -> None: def _background_refresh(self) -> None: """Background thread target for token refresh. - Acquires the lock and refreshes the token. Errors are silently - ignored since the next foreground request will retry if needed. + Acquires the lock and refreshes the token. Errors are logged + but silently ignored since the next foreground request will + retry if needed. """ with self._lock: - # ruff: noqa: SIM105 try: self._refresh_token() - except Exception: # noqa: BLE001, S110 - # Silently ignore errors in background refresh - # Next get_token() call will retry if needed - pass + except Exception as e: # noqa: BLE001 + # Log but don't raise - background refresh failures + # are recoverable (next get_token() will retry) + logger.debug( + "Background token refresh failed, " + "will retry on next request: %s", + e, + ) def _refresh_token(self) -> None: """Refresh the authentication token by making an OAuth request. diff --git a/tests/client/test_channel.py b/tests/client/test_channel.py index 5fe30c0..99bc852 100644 --- a/tests/client/test_channel.py +++ b/tests/client/test_channel.py @@ -222,6 +222,44 @@ def test_get_token_defaults_to_bearer(self) -> None: assert token_data.scheme == "Bearer" + def test_get_token_preserves_server_casing(self) -> None: + """Test that server-provided token_type casing is preserved.""" + test_cases = [ + ("Bearer", "Bearer"), + ("bearer", "bearer"), + ("BEARER", "BEARER"), + ("DPoP", "DPoP"), + ("dpop", "dpop"), + (" Bearer ", "Bearer"), # Whitespace is stripped + ] + + for server_type, expected_scheme in test_cases: + helper = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + ) + + mock_response = mock.Mock() + mock_response.json.return_value = { + "access_token": "test_token", + "expires_in": 3600, + "token_type": server_type, + } + mock_response.raise_for_status.return_value = None + + with mock.patch("httpx.Client") as mock_client: + mock_response_obj = ( + mock_client.return_value.__enter__.return_value + ) + mock_response_obj.post.return_value = mock_response + + token_data = helper.get_token() + + assert token_data.scheme == expected_scheme, ( + f"Expected {expected_scheme} for {server_type}, " + f"got {token_data.scheme}" + ) + def test_get_token_cached(self) -> None: """Test that cached token is returned when valid.""" helper = CredentialHelper( @@ -641,6 +679,7 @@ def test_refresh_token_sets_issued_at(self) -> None: assert helper._token_data is not None assert before_time <= helper._token_data.issued_at <= after_time # Check that expires_at is approximately issued_at + 3600 + # Allow 1 second tolerance for test execution time assert ( helper._token_data.expires_at - helper._token_data.issued_at - 3600 < 1 From dd00a1c0b4bbbab2a694731f1e10b2ca537b4785 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 11:28:03 +0000 Subject: [PATCH 05/13] Address PR review comments - Make issued_at a required field (remove default value) - Remove fallback logic for legacy tokens in is_old() - Add stampede prevention in _background_refresh - Remove obsolete tests for fallback logic - Add test for stampede prevention Co-authored-by: iwillspeak <1004401+iwillspeak@users.noreply.github.com> --- src/resolver_athena_client/client/channel.py | 14 ++--- tests/client/test_channel.py | 55 +++++++++++--------- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/resolver_athena_client/client/channel.py b/src/resolver_athena_client/client/channel.py index 48c4b30..f86ec5d 100644 --- a/src/resolver_athena_client/client/channel.py +++ b/src/resolver_athena_client/client/channel.py @@ -31,7 +31,7 @@ class TokenData(NamedTuple): access_token: str expires_at: float scheme: str - issued_at: float = 0.0 + issued_at: float def is_valid(self) -> bool: """Check if this token is still valid (with a 30-second buffer).""" @@ -44,12 +44,6 @@ def is_old(self) -> bool: This allows background refresh to happen before expiry while the token is still usable. """ - if self.issued_at == 0.0: - # Fallback for tokens created before issued_at was tracked. - # Use 90s threshold (3x the 30s validity buffer) to ensure - # old tokens get refreshed proactively. - return time.time() >= (self.expires_at - 90) - current_time = time.time() total_lifetime = self.expires_at - self.issued_at time_remaining = self.expires_at - current_time @@ -171,6 +165,12 @@ def _background_refresh(self) -> None: retry if needed. """ with self._lock: + # Check if token still needs refresh (prevent stampede) + token_data = self._token_data + if token_data is not None and not token_data.is_old(): + # Token was already refreshed by another thread + return + try: self._refresh_token() except Exception as e: # noqa: BLE001 diff --git a/tests/client/test_channel.py b/tests/client/test_channel.py index 99bc852..d4b107a 100644 --- a/tests/client/test_channel.py +++ b/tests/client/test_channel.py @@ -117,6 +117,7 @@ def test_is_token_valid_with_expired_token(self) -> None: access_token="test_token", expires_at=time.time() - 100, scheme="Bearer", + issued_at=time.time() - 3700, ) assert not helper._token_data.is_valid() @@ -132,6 +133,7 @@ def test_is_token_valid_with_valid_token(self) -> None: access_token="test_token", expires_at=time.time() + 3600, scheme="Bearer", + issued_at=time.time(), ) assert helper._token_data.is_valid() @@ -147,6 +149,7 @@ def test_is_token_valid_with_soon_expiring_token(self) -> None: access_token="test_token", expires_at=time.time() + 20, scheme="Bearer", + issued_at=time.time() - 3580, ) assert not helper._token_data.is_valid() @@ -272,6 +275,7 @@ def test_get_token_cached(self) -> None: access_token="cached_token", expires_at=time.time() + 3600, scheme="Bearer", + issued_at=time.time(), ) token_data = helper.get_token() @@ -359,6 +363,7 @@ def test_invalidate_token(self) -> None: access_token="valid_token", expires_at=time.time() + 3600, scheme="Bearer", + issued_at=time.time(), ) helper.invalidate_token() @@ -377,6 +382,7 @@ def test_get_token_refreshes_after_invalidation(self) -> None: access_token="old_token", expires_at=time.time() + 3600, scheme="Bearer", + issued_at=time.time(), ) helper.invalidate_token() @@ -407,6 +413,7 @@ def test_plugin_passes_bearer_token_to_callback(self) -> None: access_token="test-bearer-token", expires_at=time.time() + 3600, scheme="Bearer", + issued_at=time.time(), ) plugin = _AutoRefreshTokenAuthMetadataPlugin(mock_helper) @@ -426,6 +433,7 @@ def test_plugin_respects_token_scheme(self) -> None: access_token="dpop-token", expires_at=time.time() + 3600, scheme="Dpop", + issued_at=time.time(), ) plugin = _AutoRefreshTokenAuthMetadataPlugin(mock_helper) @@ -495,30 +503,6 @@ def test_token_is_not_old_when_fresh(self) -> None: # Total lifetime = 3600s, remaining = 2400s (67%), so it's fresh assert not token.is_old() - def test_token_is_old_fallback_for_legacy_tokens(self) -> None: - """Test fallback logic for tokens without issued_at.""" - current_time = time.time() - # Legacy token without issued_at (defaults to 0.0) - token = TokenData( - access_token="test_token", - expires_at=current_time + 60, # 1 minute from now - scheme="Bearer", - ) - # Should be considered old if less than 90s remain - assert token.is_old() - - def test_token_is_not_old_fallback_for_fresh_legacy_tokens(self) -> None: - """Test fallback logic for fresh legacy tokens.""" - current_time = time.time() - # Legacy token with plenty of time remaining - token = TokenData( - access_token="test_token", - expires_at=current_time + 200, # 200 seconds from now - scheme="Bearer", - ) - # Should not be considered old if more than 90s remain - assert not token.is_old() - def test_get_token_triggers_background_refresh_for_old_token(self) -> None: """Test that get_token triggers background refresh for old tokens.""" helper = CredentialHelper( @@ -617,6 +601,29 @@ def test_background_refresh_silently_handles_errors(self) -> None: # Should not raise an exception helper._background_refresh() + def test_background_refresh_prevents_stampede(self) -> None: + """Test background refresh skips refresh if token is fresh.""" + helper = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + ) + + current_time = time.time() + # Set up a fresh token (already refreshed by another thread) + helper._token_data = TokenData( + access_token="fresh_token", + expires_at=current_time + 2400, # 40 minutes remaining + scheme="Bearer", + issued_at=current_time - 1200, # 20 minutes ago, so it's fresh + ) + + # Mock refresh to track if it's called + with mock.patch.object(helper, "_refresh_token") as mock_refresh: + helper._background_refresh() + + # Should NOT have called refresh since token is fresh + mock_refresh.assert_not_called() + def test_get_token_blocks_for_expired_token(self) -> None: """Test that get_token blocks and refreshes when token is expired.""" helper = CredentialHelper( From dfac34180b0f853aa58dabdd48e555678cde7d7a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:56:45 +0000 Subject: [PATCH 06/13] Revert Bearer/DPoP test assertion changes from c50327d The target branch now properly fixes the test by providing "Bearer" in the mock data. Reverting my previous fix that changed the assertion to match the old mock data. Co-authored-by: anna-singleton-resolver <199753965+anna-singleton-resolver@users.noreply.github.com> --- tests/client/test_channel.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/client/test_channel.py b/tests/client/test_channel.py index d4b107a..1c025ab 100644 --- a/tests/client/test_channel.py +++ b/tests/client/test_channel.py @@ -165,7 +165,7 @@ def test_get_token_success(self) -> None: mock_response.json.return_value = { "access_token": "new_access_token", "expires_in": 3600, - "token_type": "bearer", + "token_type": "Bearer", } mock_response.raise_for_status.return_value = None @@ -176,7 +176,7 @@ def test_get_token_success(self) -> None: token_data = helper.get_token() assert token_data.access_token == "new_access_token" - assert token_data.scheme == "bearer" + assert token_data.scheme == "Bearer" assert helper._token_data is not None assert helper._token_data.expires_at is not None From 694cf1b4206bbe87b7e8a4f86185b447ae3bebb0 Mon Sep 17 00:00:00 2001 From: anna-singleton-resolver Date: Fri, 20 Feb 2026 16:09:45 +0000 Subject: [PATCH 07/13] fix: start refresh thread checks for already refreshed token --- src/resolver_athena_client/client/channel.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/resolver_athena_client/client/channel.py b/src/resolver_athena_client/client/channel.py index f86ec5d..64c0fac 100644 --- a/src/resolver_athena_client/client/channel.py +++ b/src/resolver_athena_client/client/channel.py @@ -143,10 +143,13 @@ def _start_background_refresh(self) -> None: # Try to acquire lock and start refresh if self._lock.acquire(blocking=False): try: - # Double-check: another thread might have started refresh + # Double-check: another thread might have started refresh, + # or the token may have been refreshed. refresh_needed = ( self._refresh_thread is None or not self._refresh_thread.is_alive() + or self._token_data is None + or not self._token_data.is_old() ) if refresh_needed: self._refresh_thread = threading.Thread( From 28372db1f8095d585059a55db09a9e20fc4d5295 Mon Sep 17 00:00:00 2001 From: anna-singleton-resolver Date: Fri, 20 Feb 2026 16:13:32 +0000 Subject: [PATCH 08/13] chore: remove extraneous comment --- src/resolver_athena_client/client/channel.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/resolver_athena_client/client/channel.py b/src/resolver_athena_client/client/channel.py index 64c0fac..aa5006b 100644 --- a/src/resolver_athena_client/client/channel.py +++ b/src/resolver_athena_client/client/channel.py @@ -88,12 +88,6 @@ def __init__( def get_token(self) -> TokenData: """Get valid token data, refreshing if necessary. - Uses double-checked locking with background refresh optimization: - - If token is valid and fresh, returns immediately (no lock) - - If token is valid but old, triggers background refresh and - returns current token - - If token is expired, blocks until new token is acquired - Returns ------- A valid ``TokenData`` containing access token, expiry, and scheme From 02b15874945d9c3f900e998e51d7a6cd5d9c411a Mon Sep 17 00:00:00 2001 From: anna-singleton-resolver Date: Fri, 20 Feb 2026 16:14:14 +0000 Subject: [PATCH 09/13] feat: reduce eagerness of background refresh from at 50% remaining to 25% remaining --- src/resolver_athena_client/client/channel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolver_athena_client/client/channel.py b/src/resolver_athena_client/client/channel.py index aa5006b..3c9c6dd 100644 --- a/src/resolver_athena_client/client/channel.py +++ b/src/resolver_athena_client/client/channel.py @@ -47,7 +47,7 @@ def is_old(self) -> bool: current_time = time.time() total_lifetime = self.expires_at - self.issued_at time_remaining = self.expires_at - current_time - return time_remaining < (total_lifetime * 0.5) + return time_remaining < (total_lifetime * 0.25) class CredentialHelper: From 809af3840440440d6a3c2dee9cb55ec620088b05 Mon Sep 17 00:00:00 2001 From: anna-singleton-resolver Date: Fri, 20 Feb 2026 16:36:31 +0000 Subject: [PATCH 10/13] doc: update is_old docstring --- src/resolver_athena_client/client/channel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolver_athena_client/client/channel.py b/src/resolver_athena_client/client/channel.py index 3c9c6dd..da4d044 100644 --- a/src/resolver_athena_client/client/channel.py +++ b/src/resolver_athena_client/client/channel.py @@ -40,7 +40,7 @@ def is_valid(self) -> bool: def is_old(self) -> bool: """Check if this token should be proactively refreshed. - A token is considered "old" if less than 50% of its lifetime remains. + A token is considered "old" if less than 25% of its lifetime remains. This allows background refresh to happen before expiry while the token is still usable. """ From 5ceec3b83a56523e1115db4e9fe9cabd0a85a629 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Feb 2026 09:46:04 +0000 Subject: [PATCH 11/13] fix: correct boolean logic in _start_background_refresh Changed from OR to AND logic: refresh should start if (refresh_not_active AND token_needs_refresh), not if any condition is true. This prevents unnecessary refresh attempts. Co-authored-by: iwillspeak <1004401+iwillspeak@users.noreply.github.com> --- src/resolver_athena_client/client/channel.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/resolver_athena_client/client/channel.py b/src/resolver_athena_client/client/channel.py index da4d044..6187abb 100644 --- a/src/resolver_athena_client/client/channel.py +++ b/src/resolver_athena_client/client/channel.py @@ -139,12 +139,14 @@ def _start_background_refresh(self) -> None: try: # Double-check: another thread might have started refresh, # or the token may have been refreshed. - refresh_needed = ( + refresh_not_active = ( self._refresh_thread is None or not self._refresh_thread.is_alive() - or self._token_data is None - or not self._token_data.is_old() ) + token_needs_refresh = ( + self._token_data is None or self._token_data.is_old() + ) + refresh_needed = refresh_not_active and token_needs_refresh if refresh_needed: self._refresh_thread = threading.Thread( target=self._background_refresh, From 1e59cce0f3dc9de31f3fc9b2b3eda604cd378c5e Mon Sep 17 00:00:00 2001 From: anna-singleton-resolver Date: Mon, 23 Feb 2026 10:31:51 +0000 Subject: [PATCH 12/13] test: fix test assertions to be on 25% instead of 50% --- tests/client/test_channel.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/client/test_channel.py b/tests/client/test_channel.py index 1c025ab..de20356 100644 --- a/tests/client/test_channel.py +++ b/tests/client/test_channel.py @@ -478,20 +478,20 @@ class TestBackgroundTokenRefresh: """Tests for background token refresh functionality.""" def test_token_is_old_when_past_halfway_lifetime(self) -> None: - """Test that a token is considered old when past 50% of its lifetime.""" + """Test that a token is considered old when past 25% of its lifetime.""" current_time = time.time() # Token with 1 hour lifetime, 20 minutes remaining (33%) token = TokenData( access_token="test_token", - expires_at=current_time + 1200, # 20 minutes from now + expires_at=current_time + 600, # 10 minutes from now scheme="Bearer", - issued_at=current_time - 2400, # 40 minutes ago + issued_at=current_time - 3_000, # 50 minutes ago ) # Total lifetime = 3600s, remaining = 1200s (33%), so it's old assert token.is_old() def test_token_is_not_old_when_fresh(self) -> None: - """Test that a token is not old when more than 50% lifetime remains.""" + """Test that a token is not old when more than 25% lifetime remains.""" current_time = time.time() # Token with 1 hour lifetime, 40 minutes remaining (67%) token = TokenData( @@ -514,9 +514,9 @@ def test_get_token_triggers_background_refresh_for_old_token(self) -> None: # Set up an old but valid token helper._token_data = TokenData( access_token="old_token", - expires_at=current_time + 1200, # 20 minutes remaining + expires_at=current_time + 600, # 10 minutes from now scheme="Bearer", - issued_at=current_time - 2400, # 40 minutes ago, so it's old + issued_at=current_time - 3_000, # 50 minutes ago ) with mock.patch.object( From 55aad5a6d94fa8a8481fa598cbdedbfa1c960390 Mon Sep 17 00:00:00 2001 From: anna-singleton-resolver Date: Mon, 23 Feb 2026 10:39:45 +0000 Subject: [PATCH 13/13] feat: allow configurable proactive_refresh_threshold --- src/resolver_athena_client/client/channel.py | 40 ++++++++++++++++---- tests/client/test_channel.py | 23 +++++++++-- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/resolver_athena_client/client/channel.py b/src/resolver_athena_client/client/channel.py index 6187abb..e5aec66 100644 --- a/src/resolver_athena_client/client/channel.py +++ b/src/resolver_athena_client/client/channel.py @@ -37,17 +37,27 @@ def is_valid(self) -> bool: """Check if this token is still valid (with a 30-second buffer).""" return time.time() < (self.expires_at - 30) - def is_old(self) -> bool: + def is_old(self, proactive_refresh_threshold: float) -> bool: """Check if this token should be proactively refreshed. - A token is considered "old" if less than 25% of its lifetime remains. - This allows background refresh to happen before expiry while the token - is still usable. + A token is considered "old" if less than the + proactive_refresh_threshold of its lifetime remains. This allows + background refresh to happen before expiry while the token is still + usable. + + Args: + ---- + proactive_refresh_threshold: Fraction of token lifetime past which + to trigger proactive refresh (e.g. 0.25 for 25%) + """ + if proactive_refresh_threshold <= 0 or proactive_refresh_threshold >= 1: + msg = "proactive_refresh_threshold must be between 0 and 1" + raise ValueError(msg) current_time = time.time() total_lifetime = self.expires_at - self.issued_at time_remaining = self.expires_at - current_time - return time_remaining < (total_lifetime * 0.25) + return time_remaining < (total_lifetime * proactive_refresh_threshold) class CredentialHelper: @@ -59,6 +69,7 @@ def __init__( client_secret: str, auth_url: str = "https://crispthinking.auth0.com/oauth/token", audience: str = "crisp-athena-live", + proactive_refresh_threshold: float = 0.25, ) -> None: """Initialize the credential helper. @@ -68,6 +79,8 @@ def __init__( client_secret: OAuth client secret auth_url: OAuth token endpoint URL audience: OAuth audience + proactive_refresh_threshold: Fraction of token lifetime to trigger + proactive refresh (default 0.25 for 25%) """ if not client_id: @@ -85,6 +98,12 @@ def __init__( self._lock: threading.Lock = threading.Lock() self._refresh_thread: threading.Thread | None = None + if proactive_refresh_threshold <= 0 or proactive_refresh_threshold >= 1: + msg = "proactive_refresh_threshold must be a float between 0 and 1" + raise ValueError(msg) + + self._proactive_refresh_threshold: float = proactive_refresh_threshold + def get_token(self) -> TokenData: """Get valid token data, refreshing if necessary. @@ -103,7 +122,7 @@ def get_token(self) -> TokenData: # Fast path: token is valid and fresh if token_data is not None and token_data.is_valid(): # If token is old, trigger background refresh - if token_data.is_old(): + if token_data.is_old(self._proactive_refresh_threshold): self._start_background_refresh() return token_data @@ -144,7 +163,10 @@ def _start_background_refresh(self) -> None: or not self._refresh_thread.is_alive() ) token_needs_refresh = ( - self._token_data is None or self._token_data.is_old() + self._token_data is None + or self._token_data.is_old( + self._proactive_refresh_threshold + ) ) refresh_needed = refresh_not_active and token_needs_refresh if refresh_needed: @@ -166,7 +188,9 @@ def _background_refresh(self) -> None: with self._lock: # Check if token still needs refresh (prevent stampede) token_data = self._token_data - if token_data is not None and not token_data.is_old(): + if token_data is not None and not token_data.is_old( + self._proactive_refresh_threshold + ): # Token was already refreshed by another thread return diff --git a/tests/client/test_channel.py b/tests/client/test_channel.py index de20356..33f8d85 100644 --- a/tests/client/test_channel.py +++ b/tests/client/test_channel.py @@ -97,6 +97,23 @@ def test_init_with_empty_client_secret(self) -> None: client_secret="", ) + @pytest.mark.parametrize( + "invalid", + [-0.1, 1.1, -0.5, 2.0], + ) + def test_init_with_invalid_proactive_refresh_threshold( + self, invalid: float + ) -> None: + with pytest.raises( + ValueError, + match="proactive_refresh_threshold must be a float between 0 and 1", + ): + _ = CredentialHelper( + client_id="test_client_id", + client_secret="test_client_secret", + proactive_refresh_threshold=invalid, + ) + def test_is_token_valid_with_no_token(self) -> None: """Test token is not valid when no token data is set.""" helper = CredentialHelper( @@ -487,8 +504,8 @@ def test_token_is_old_when_past_halfway_lifetime(self) -> None: scheme="Bearer", issued_at=current_time - 3_000, # 50 minutes ago ) - # Total lifetime = 3600s, remaining = 1200s (33%), so it's old - assert token.is_old() + # Total lifetime = 3600s, remaining = 600s (1/6th), so it's old + assert token.is_old(0.25) def test_token_is_not_old_when_fresh(self) -> None: """Test that a token is not old when more than 25% lifetime remains.""" @@ -501,7 +518,7 @@ def test_token_is_not_old_when_fresh(self) -> None: issued_at=current_time - 1200, # 20 minutes ago ) # Total lifetime = 3600s, remaining = 2400s (67%), so it's fresh - assert not token.is_old() + assert not token.is_old(0.25) def test_get_token_triggers_background_refresh_for_old_token(self) -> None: """Test that get_token triggers background refresh for old tokens."""