From bf2f13f53a7c76709d86af6d930a04ad66ca0143 Mon Sep 17 00:00:00 2001 From: Bionic711 Date: Wed, 9 Sep 2026 09:24:56 -0500 Subject: [PATCH 1/3] Fix cross-worker admin settings consistency and Redis Explorer Fixes #1477 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- application/single_app/app.py | 1 - application/single_app/app_settings_cache.py | 348 +++------- application/single_app/app_settings_store.py | 207 ++++++ application/single_app/background_tasks.py | 32 +- application/single_app/config.py | 2 +- .../single_app/functions_appinsights.py | 5 +- .../single_app/functions_control_center.py | 25 +- .../single_app/functions_redis_monitoring.py | 24 +- .../single_app/functions_retention_policy.py | 11 +- .../single_app/functions_service_health.py | 10 +- application/single_app/functions_settings.py | 241 ++----- .../single_app/plugin_validation_endpoint.py | 53 +- .../single_app/route_backend_agents.py | 51 +- .../route_backend_control_center.py | 29 +- .../route_backend_retention_policy.py | 18 +- .../single_app/route_backend_settings.py | 18 +- application/single_app/route_custom_pages.py | 7 +- .../route_frontend_admin_settings.py | 23 +- .../single_app/simplechat_scheduler.py | 1 - .../single_app/templates/admin_settings.html | 1 + docs/admin/scale.md | 40 ++ docs/explanation/release_notes.md | 22 + .../test_app_settings_auxiliary_writers.py | 625 ++++++++++++++++++ .../test_app_settings_cache_versioning.py | 37 +- .../test_app_settings_store_consistency.py | 470 +++++++++++++ ...content_understanding_extraction_engine.py | 6 +- .../test_cosmos_wave1_cache_fallback.py | 10 +- .../test_cosmos_wave5a3_redis_monitoring.py | 174 ++++- ...test_get_settings_merge_bool_regression.py | 11 +- ...est_settings_deep_merge_persistence_fix.py | 283 ++------ ...tabular_parity_stale_settings_migration.py | 13 +- .../test_admin_settings_save_consistency.py | 61 ++ 32 files changed, 2020 insertions(+), 839 deletions(-) create mode 100644 application/single_app/app_settings_store.py create mode 100644 functional_tests/test_app_settings_auxiliary_writers.py create mode 100644 functional_tests/test_app_settings_store_consistency.py create mode 100644 ui_tests/test_admin_settings_save_consistency.py diff --git a/application/single_app/app.py b/application/single_app/app.py index 8865a9382..45af06ef4 100644 --- a/application/single_app/app.py +++ b/application/single_app/app.py @@ -289,7 +289,6 @@ def initialize_application(force=False): settings, get_redis_cache_infrastructure_endpoint(redis_hostname) ) - app_settings_cache.update_settings_cache(settings) sanitized_settings = sanitize_settings_for_logging(settings) debug_print(f"DEBUG:Application settings: {sanitized_settings}") sanitized_settings_cache = sanitize_settings_for_logging(app_settings_cache.get_settings_cache()) diff --git a/application/single_app/app_settings_cache.py b/application/single_app/app_settings_cache.py index f271690b9..6c3ad0716 100644 --- a/application/single_app/app_settings_cache.py +++ b/application/single_app/app_settings_cache.py @@ -2,7 +2,8 @@ """ WARNING: NEVER 'from app_settings_cache import' settings or any other module that imports settings. ALWAYS import app_settings_cache and use app_settings_cache.get_settings_cache() to get settings. -This supports the dynamic selection of redis or in-memory caching of settings. +App settings are read from Redis or Cosmos, never from a worker-local snapshot. +Other cache families in this module retain their own fallback policies. """ import json import logging @@ -10,6 +11,11 @@ import threading import time from datetime import datetime, timedelta +from azure.core.exceptions import AzureError +from azure.cosmos.exceptions import CosmosResourceNotFoundError +from redis.exceptions import RedisError + +from app_settings_store import AppSettingsStore, SETTINGS_REVISION_FIELD # Redis client construction lives in functions_redis_client so session, cache, and admin # diagnostics code paths share one place that resolves service type, port, and credentials. @@ -19,24 +25,16 @@ create_redis_client, ) -# NOTE: functions_keyvault is imported locally inside configure_app_cache to avoid a circular -# import (functions_keyvault -> app_settings_cache -> functions_keyvault). -# functions_appinsights is also imported locally for the same reason. +# Logging/configuration imports are deferred to avoid startup dependency cycles. -_settings = None _logger = logging.getLogger(__name__) -APP_SETTINGS_CACHE = {} +APP_SETTINGS_STORE = None APP_USER_UI_SETTINGS_CACHE = {} APP_STREAM_SESSION_METADATA = {} APP_STREAM_SESSION_EVENTS = {} -APP_SETTINGS_CACHE_VERSION = 0 APP_GOVERNANCE_CACHE_VERSION = 0 -APP_SETTINGS_SHARED_VERSION_CACHE = {'value': 0, 'expires_at': 0} APP_GOVERNANCE_SHARED_VERSION_CACHE = {'value': 0, 'expires_at': 0} APP_REDIS_CLIENT = None -APP_SETTINGS_CACHE_KEY = 'APP_SETTINGS_CACHE' -APP_SETTINGS_CACHE_VERSION_KEY = 'APP_SETTINGS_CACHE_VERSION' -APP_SETTINGS_CACHE_VERSION_DOC_ID = 'app_settings_cache_version' USER_UI_SETTINGS_CACHE_KEY_PREFIX = 'USER_UI_SETTINGS' USER_UI_SETTINGS_CACHE_TTL_SECONDS = 120 GOVERNANCE_CACHE_VERSION_KEY = 'GOVERNANCE_CACHE_VERSION' @@ -48,7 +46,6 @@ update_settings_cache = None get_settings_cache = None get_app_settings_cache_version = None -bump_app_settings_cache_version = None initialize_stream_session_cache = None set_stream_session_meta = None get_stream_session_meta = None @@ -148,6 +145,63 @@ def _set_ttl_cached_version(version_cache, version): version_cache['expires_at'] = time.time() + CACHE_VERSION_READ_TTL_SECONDS +def _log_settings_fallback(error): + # Logging reads settings itself; the logging entrypoint guards re-entrancy. + from functions_appinsights import log_event + + log_event( + "[ASC] Shared settings unavailable; reading Cosmos without a worker snapshot.", + extra={'error_type': type(error).__name__}, + level=logging.WARNING, + ) + + +def get_settings_store(): + """Initialize connections lazily, without retaining any settings payload.""" + global APP_SETTINGS_STORE, get_settings_cache, update_settings_cache + global get_app_settings_cache_version + + if APP_SETTINGS_STORE is None: + # config imports logging/cache during startup; defer until it is initialized. + from config import cosmos_settings_container + + try: + initial = cosmos_settings_container.read_item(item='app_settings', partition_key='app_settings') + except CosmosResourceNotFoundError: + initial = {} + required = bool(initial.get('enable_redis_cache', False)) + redis_client = None + if required: + try: + redis_client = create_redis_client( + settings=initial, + credential_purpose=CREDENTIAL_PURPOSE_APP_CACHE, + socket_connect_timeout=5, + socket_timeout=5, + ) + except (RedisError, AzureError, ValueError) as error: + _log_settings_fallback(error) + APP_SETTINGS_STORE = AppSettingsStore( + cosmos_settings_container, + redis_client, + redis_required=required, + on_fallback=_log_settings_fallback, + ) + get_settings_cache = APP_SETTINGS_STORE.read + update_settings_cache = _refresh_authoritative_settings + get_app_settings_cache_version = _get_settings_revision + return APP_SETTINGS_STORE + + +def _refresh_authoritative_settings(_obsolete_snapshot=None): + """Compatibility entrypoint: never publish a caller's potentially stale snapshot.""" + return get_settings_store().write(lambda settings: settings) + + +def _get_settings_revision(): + return int(get_settings_store().read().get(SETTINGS_REVISION_FIELD, 0)) + + def _log_cache_fallback(operation, exception, log_event_func=None): message = f"[ASC] Redis cache operation failed; using Cosmos/local fallback for {operation}." _logger.warning("%s Error: %s", message, exception) @@ -248,94 +302,8 @@ def _delete_cosmos_cache_entry(cache_key, log_event_func=None): return False -def _get_app_settings_cache_version_fallback(log_event_func=None): - global APP_SETTINGS_CACHE_VERSION - try: - from config import cosmos_settings_container - return _get_ttl_cached_cosmos_version( - APP_SETTINGS_SHARED_VERSION_CACHE, - cosmos_settings_container, - APP_SETTINGS_CACHE_VERSION_DOC_ID, - APP_SETTINGS_CACHE_VERSION, - log_event_func=log_event_func, - ) - except Exception as ex: - _logger.warning("[ASC] Shared cache version read failed; using local version fallback: %s", ex) - if callable(log_event_func): - log_event_func( - "[ASC] Shared cache version read failed; using local version fallback.", - extra={'version_doc_id': APP_SETTINGS_CACHE_VERSION_DOC_ID, 'error': str(ex)}, - level=logging.WARNING, - ) - with _app_cache_lock: - return APP_SETTINGS_CACHE_VERSION - - -def _bump_app_settings_cache_version_fallback(log_event_func=None): - global APP_SETTINGS_CACHE_VERSION - try: - from config import cosmos_settings_container - bumped_version = _bump_cosmos_cache_version( - cosmos_settings_container, - APP_SETTINGS_CACHE_VERSION_DOC_ID, - log_event_func=log_event_func, - ) - if bumped_version is not None: - with _app_cache_lock: - APP_SETTINGS_CACHE_VERSION = bumped_version - _set_ttl_cached_version(APP_SETTINGS_SHARED_VERSION_CACHE, bumped_version) - return bumped_version - except Exception as ex: - _logger.warning("[ASC] Shared cache version bump failed; using local version fallback: %s", ex) - if callable(log_event_func): - log_event_func( - "[ASC] Shared cache version bump failed; using local version fallback.", - extra={'version_doc_id': APP_SETTINGS_CACHE_VERSION_DOC_ID, 'error': str(ex)}, - level=logging.WARNING, - ) - - with _app_cache_lock: - APP_SETTINGS_CACHE_VERSION += 1 - fallback_version = APP_SETTINGS_CACHE_VERSION - _set_ttl_cached_version(APP_SETTINGS_SHARED_VERSION_CACHE, fallback_version) - return fallback_version - - -def _update_settings_cache_fallback(new_settings, log_event_func=None): - global APP_SETTINGS_CACHE, APP_SETTINGS_CACHE_VERSION - shared_version = _get_app_settings_cache_version_fallback(log_event_func=log_event_func) - with _app_cache_lock: - APP_SETTINGS_CACHE = copy.deepcopy(new_settings or {}) - APP_SETTINGS_CACHE_VERSION = shared_version - - def _get_settings_cache_fallback(log_event_func=None): - global APP_SETTINGS_CACHE, APP_SETTINGS_CACHE_VERSION - shared_version = _get_app_settings_cache_version_fallback(log_event_func=log_event_func) - with _app_cache_lock: - if APP_SETTINGS_CACHE and APP_SETTINGS_CACHE_VERSION == shared_version: - return copy.deepcopy(APP_SETTINGS_CACHE) - - try: - from config import cosmos_settings_container - loaded_settings = cosmos_settings_container.read_item( - item='app_settings', - partition_key='app_settings', - ) - with _app_cache_lock: - APP_SETTINGS_CACHE = copy.deepcopy(loaded_settings or {}) - APP_SETTINGS_CACHE_VERSION = shared_version - return copy.deepcopy(loaded_settings or {}) - except Exception as ex: - _logger.warning("[ASC] Failed to refresh app settings cache from Cosmos; using local cache fallback: %s", ex) - if callable(log_event_func): - log_event_func( - "[ASC] Failed to refresh app settings cache from Cosmos; using local cache fallback.", - extra={'error': str(ex)}, - level=logging.WARNING, - ) - with _app_cache_lock: - return copy.deepcopy(APP_SETTINGS_CACHE) + return get_settings_store().read(use_cosmos=True) def _get_governance_cache_version_fallback(log_event_func=None): @@ -552,24 +520,14 @@ def _assign_fallback_cache_functions(log_event_func=None): global initialize_stream_session_cache, set_stream_session_meta, get_stream_session_meta global append_stream_session_event, get_stream_session_events, delete_stream_session_cache global get_user_ui_settings_cache, set_user_ui_settings_cache, delete_user_ui_settings_cache - global get_app_settings_cache_version, bump_app_settings_cache_version + global get_app_settings_cache_version global get_governance_cache_version, bump_governance_cache_version global app_cache_is_using_redis global APP_REDIS_CLIENT app_cache_is_using_redis = False APP_REDIS_CLIENT = None - update_settings_cache = lambda new_settings: _update_settings_cache_fallback( - new_settings, - log_event_func=log_event_func, - ) - get_settings_cache = lambda: _get_settings_cache_fallback(log_event_func=log_event_func) - get_app_settings_cache_version = lambda: _get_app_settings_cache_version_fallback( - log_event_func=log_event_func, - ) - bump_app_settings_cache_version = lambda: _bump_app_settings_cache_version_fallback( - log_event_func=log_event_func, - ) + get_settings_store() initialize_stream_session_cache = lambda cache_key, metadata, ttl_seconds=None: ( _initialize_stream_session_cache_fallback( cache_key, @@ -637,21 +595,27 @@ def get_app_cache_redis_client(): def configure_app_cache(settings, redis_cache_endpoint=None): - global _settings, update_settings_cache, get_settings_cache, APP_SETTINGS_CACHE + global update_settings_cache, get_settings_cache, APP_SETTINGS_STORE global APP_USER_UI_SETTINGS_CACHE, APP_STREAM_SESSION_METADATA, APP_STREAM_SESSION_EVENTS - global APP_SETTINGS_CACHE_VERSION, APP_GOVERNANCE_CACHE_VERSION - global APP_SETTINGS_SHARED_VERSION_CACHE, APP_GOVERNANCE_SHARED_VERSION_CACHE + global APP_GOVERNANCE_CACHE_VERSION, APP_GOVERNANCE_SHARED_VERSION_CACHE global initialize_stream_session_cache, set_stream_session_meta, get_stream_session_meta global append_stream_session_event, get_stream_session_events, delete_stream_session_cache global get_user_ui_settings_cache, set_user_ui_settings_cache, delete_user_ui_settings_cache - global get_app_settings_cache_version, bump_app_settings_cache_version + global get_app_settings_cache_version global get_governance_cache_version, bump_governance_cache_version global app_cache_is_using_redis global APP_REDIS_CLIENT # Local import to avoid circular dependency: functions_keyvault imports app_settings_cache. from functions_appinsights import log_event - _settings = settings - use_redis = _settings.get('enable_redis_cache', False) + from config import cosmos_settings_container + + use_redis = settings.get('enable_redis_cache', False) + APP_SETTINGS_STORE = AppSettingsStore( + cosmos_settings_container, + redis_required=use_redis, + on_fallback=_log_settings_fallback, + ) + get_settings_store() app_cache_is_using_redis = False APP_REDIS_CLIENT = None @@ -673,75 +637,17 @@ def configure_app_cache(settings, redis_cache_endpoint=None): redis_client = create_redis_client( settings=settings, credential_purpose=CREDENTIAL_PURPOSE_APP_CACHE, + socket_connect_timeout=5, + socket_timeout=5, ) app_cache_is_using_redis = True APP_REDIS_CLIENT = redis_client + APP_SETTINGS_STORE.redis = redis_client except Exception as redis_init_error: _log_cache_fallback('redis_initialization', redis_init_error, log_event_func=log_event) _assign_fallback_cache_functions(log_event_func=log_event) return - def get_app_settings_cache_version_redis(): - try: - cached = redis_client.get(APP_SETTINGS_CACHE_VERSION_KEY) - if cached is None: - redis_client.setnx(APP_SETTINGS_CACHE_VERSION_KEY, 0) - return 0 - return _normalize_cache_version(cached) - except Exception as ex: - _log_cache_fallback('get_app_settings_cache_version', ex, log_event_func=log_event) - return _get_app_settings_cache_version_fallback(log_event_func=log_event) - - def bump_app_settings_cache_version_redis(): - try: - return _normalize_cache_version(redis_client.incr(APP_SETTINGS_CACHE_VERSION_KEY)) - except Exception as ex: - _log_cache_fallback('bump_app_settings_cache_version', ex, log_event_func=log_event) - return _bump_app_settings_cache_version_fallback(log_event_func=log_event) - - def get_ttl_cached_app_settings_version_redis(): - now = time.time() - with _app_cache_lock: - if APP_SETTINGS_SHARED_VERSION_CACHE.get('expires_at', 0) > now: - return _normalize_cache_version(APP_SETTINGS_SHARED_VERSION_CACHE.get('value')) - - shared_version = get_app_settings_cache_version_redis() - _set_ttl_cached_version(APP_SETTINGS_SHARED_VERSION_CACHE, shared_version) - return shared_version - - def update_settings_cache_redis(new_settings): - global APP_SETTINGS_CACHE, APP_SETTINGS_CACHE_VERSION - try: - redis_client.set(APP_SETTINGS_CACHE_KEY, json.dumps(new_settings)) - shared_version = get_app_settings_cache_version_redis() - with _app_cache_lock: - APP_SETTINGS_CACHE = copy.deepcopy(new_settings or {}) - APP_SETTINGS_CACHE_VERSION = shared_version - _set_ttl_cached_version(APP_SETTINGS_SHARED_VERSION_CACHE, shared_version) - except Exception as ex: - _log_cache_fallback('update_settings_cache', ex, log_event_func=log_event) - _update_settings_cache_fallback(new_settings, log_event_func=log_event) - - def get_settings_cache_redis(): - global APP_SETTINGS_CACHE, APP_SETTINGS_CACHE_VERSION - try: - shared_version = get_ttl_cached_app_settings_version_redis() - with _app_cache_lock: - if APP_SETTINGS_CACHE and APP_SETTINGS_CACHE_VERSION == shared_version: - return copy.deepcopy(APP_SETTINGS_CACHE) - - cached = redis_client.get(APP_SETTINGS_CACHE_KEY) - if cached is None: - return _get_settings_cache_fallback(log_event_func=log_event) - loaded_settings = json.loads(cached) - with _app_cache_lock: - APP_SETTINGS_CACHE = copy.deepcopy(loaded_settings or {}) - APP_SETTINGS_CACHE_VERSION = shared_version - return copy.deepcopy(loaded_settings or {}) - except Exception as ex: - _log_cache_fallback('get_settings_cache', ex, log_event_func=log_event) - return _get_settings_cache_fallback(log_event_func=log_event) - def get_stream_session_metadata_key(cache_key): return f'STREAM_SESSION_META:{cache_key}' @@ -900,10 +806,6 @@ def bump_governance_cache_version_redis(): _log_cache_fallback('bump_governance_cache_version', ex, log_event_func=log_event) return _bump_governance_cache_version_fallback(log_event_func=log_event) - update_settings_cache = update_settings_cache_redis - get_settings_cache = get_settings_cache_redis - get_app_settings_cache_version = get_app_settings_cache_version_redis - bump_app_settings_cache_version = bump_app_settings_cache_version_redis initialize_stream_session_cache = initialize_stream_session_cache_redis set_stream_session_meta = set_stream_session_meta_redis get_stream_session_meta = get_stream_session_meta_redis @@ -917,35 +819,6 @@ def bump_governance_cache_version_redis(): bump_governance_cache_version = bump_governance_cache_version_redis else: - def update_settings_cache_mem(new_settings): - global APP_SETTINGS_CACHE, APP_SETTINGS_CACHE_VERSION - shared_version = get_app_settings_cache_version_mem() - with _app_cache_lock: - APP_SETTINGS_CACHE = new_settings - APP_SETTINGS_CACHE_VERSION = shared_version - - def get_settings_cache_mem(): - global APP_SETTINGS_CACHE, APP_SETTINGS_CACHE_VERSION - shared_version = get_app_settings_cache_version_mem() - with _app_cache_lock: - if APP_SETTINGS_CACHE and APP_SETTINGS_CACHE_VERSION == shared_version: - return APP_SETTINGS_CACHE - - try: - from config import cosmos_settings_container - loaded_settings = cosmos_settings_container.read_item( - item='app_settings', - partition_key='app_settings', - ) - with _app_cache_lock: - APP_SETTINGS_CACHE = loaded_settings - APP_SETTINGS_CACHE_VERSION = shared_version - return loaded_settings - except Exception as ex: - _logger.warning("[ASC] Failed to refresh app settings cache from Cosmos; using local cache fallback: %s", ex) - with _app_cache_lock: - return APP_SETTINGS_CACHE - def initialize_stream_session_cache_mem(cache_key, metadata, ttl_seconds=None): expiration_timestamp = _get_expiration_timestamp(ttl_seconds) with _app_cache_lock: @@ -1035,55 +908,6 @@ def delete_user_ui_settings_cache_mem(user_id): with _app_cache_lock: APP_USER_UI_SETTINGS_CACHE.pop(user_id, None) - def get_app_settings_cache_version_mem(): - global APP_SETTINGS_CACHE_VERSION - try: - from config import cosmos_settings_container - return _get_ttl_cached_cosmos_version( - APP_SETTINGS_SHARED_VERSION_CACHE, - cosmos_settings_container, - APP_SETTINGS_CACHE_VERSION_DOC_ID, - APP_SETTINGS_CACHE_VERSION, - log_event_func=log_event, - ) - except Exception as ex: - _logger.warning("[ASC] Shared cache version read failed; using local version fallback: %s", ex) - log_event( - "[ASC] Shared cache version read failed; using local version fallback.", - extra={'version_doc_id': APP_SETTINGS_CACHE_VERSION_DOC_ID, 'error': str(ex)}, - level=logging.WARNING, - ) - with _app_cache_lock: - return APP_SETTINGS_CACHE_VERSION - - def bump_app_settings_cache_version_mem(): - global APP_SETTINGS_CACHE_VERSION - try: - from config import cosmos_settings_container - bumped_version = _bump_cosmos_cache_version( - cosmos_settings_container, - APP_SETTINGS_CACHE_VERSION_DOC_ID, - log_event_func=log_event, - ) - if bumped_version is not None: - with _app_cache_lock: - APP_SETTINGS_CACHE_VERSION = bumped_version - _set_ttl_cached_version(APP_SETTINGS_SHARED_VERSION_CACHE, bumped_version) - return bumped_version - except Exception as ex: - _logger.warning("[ASC] Shared cache version bump failed; using local version fallback: %s", ex) - log_event( - "[ASC] Shared cache version bump failed; using local version fallback.", - extra={'version_doc_id': APP_SETTINGS_CACHE_VERSION_DOC_ID, 'error': str(ex)}, - level=logging.WARNING, - ) - - with _app_cache_lock: - APP_SETTINGS_CACHE_VERSION += 1 - fallback_version = APP_SETTINGS_CACHE_VERSION - _set_ttl_cached_version(APP_SETTINGS_SHARED_VERSION_CACHE, fallback_version) - return fallback_version - def get_governance_cache_version_mem(): global APP_GOVERNANCE_CACHE_VERSION try: @@ -1133,10 +957,6 @@ def bump_governance_cache_version_mem(): _set_ttl_cached_version(APP_GOVERNANCE_SHARED_VERSION_CACHE, fallback_version) return fallback_version - update_settings_cache = update_settings_cache_mem - get_settings_cache = get_settings_cache_mem - get_app_settings_cache_version = get_app_settings_cache_version_mem - bump_app_settings_cache_version = bump_app_settings_cache_version_mem initialize_stream_session_cache = initialize_stream_session_cache_mem set_stream_session_meta = set_stream_session_meta_mem get_stream_session_meta = get_stream_session_meta_mem diff --git a/application/single_app/app_settings_store.py b/application/single_app/app_settings_store.py new file mode 100644 index 000000000..7927eb92c --- /dev/null +++ b/application/single_app/app_settings_store.py @@ -0,0 +1,207 @@ +# app_settings_store.py +"""Shared settings reads and fenced, optimistic writes; never cache settings in a worker.""" + +import copy +import json +import time +import uuid + +from azure.core import MatchConditions +from azure.cosmos.exceptions import ( + CosmosAccessConditionFailedError, + CosmosResourceExistsError, + CosmosResourceNotFoundError, +) +from redis.exceptions import RedisError + + +SETTINGS_ID = "app_settings" +SETTINGS_STATE_KEY = "APP_SETTINGS_STATE_V2" +SETTINGS_REVISION_FIELD = "_settings_revision" +WRITE_LEASE_SECONDS = 30 +MAX_WRITE_ATTEMPTS = 5 +COSMOS_METADATA_FIELDS = {"_etag", "_rid", "_self", "_attachments", "_ts"} + +# A single key keeps publication atomic on both clustered and non-clustered Redis. +# Pending records deliberately have no TTL: a crashed writer must not expose old data. +COMPARE_AND_SET = """ +local current = redis.call('GET', KEYS[1]) +if (current or '') ~= ARGV[1] then return 0 end +redis.call('SET', KEYS[1], ARGV[2]) +return 1 +""" + + +class SettingsConflictError(RuntimeError): + """The settings changed after the caller's read.""" + + +class SettingsUnavailableError(RuntimeError): + """A settings write cannot safely start or finish.""" + + +class AppSettingsStore: + def __init__(self, container, redis_client=None, *, redis_required=False, on_fallback=None): + self.container = container + self.redis = redis_client + self.redis_required = redis_required + self.on_fallback = on_fallback + + def _fallback(self, error): + if self.on_fallback is not None: + self.on_fallback(error) + + def _read_cosmos(self, session_token=None): + headers = {} + + def capture_headers(response_headers, _body): + headers.update(response_headers) + + document = self.container.read_item( + item=SETTINGS_ID, + partition_key=SETTINGS_ID, + session_token=session_token, + response_hook=capture_headers, + ) + return copy.deepcopy(document), headers.get("x-ms-session-token", session_token) + + @staticmethod + def _decode(raw): + if raw is None: + return None + state = json.loads(raw) + if not isinstance(state, dict) or state.get("state") not in {"ready", "pending"}: + raise SettingsUnavailableError("Invalid shared settings state.") + if state["state"] == "ready": + document = state.get("document") + if not isinstance(document, dict) or not document.get("_etag"): + raise SettingsUnavailableError("Invalid shared settings document.") + elif not isinstance(state.get("deadline"), (int, float)): + raise SettingsUnavailableError("Invalid shared settings write marker.") + return state + + def _raw_state(self): + if self.redis is None: + raise SettingsUnavailableError("Configured Redis is unavailable; settings were not saved.") + return self.redis.get(SETTINGS_STATE_KEY) + + def _compare_and_set(self, previous, replacement): + return bool(self.redis.eval(COMPARE_AND_SET, 1, SETTINGS_STATE_KEY, previous or "", replacement)) + + def read(self, *, use_cosmos=False): + if not self.redis_required: + return self._read_cosmos()[0] + try: + raw = self._raw_state() + state = self._decode(raw) + if state and state["state"] == "ready" and not use_cosmos: + return copy.deepcopy(state["document"]) + token = state.get("session_token") if state else None + if use_cosmos or (state and state["deadline"] > time.time()): + return self._read_cosmos(token)[0] + self._read_cosmos(token) + # Cache misses and abandoned writes are repaired with an ETag-checked + # write. A plain GET/SET could publish an older session snapshot. + return self._write(lambda document: document, observed_raw=raw) + except (RedisError, SettingsUnavailableError, SettingsConflictError, ValueError) as error: + self._fallback(error) + return self._read_cosmos()[0] + + def write(self, transform, *, expected_etag=None, defaults=None): + """Apply a change to authoritative settings, conditional on the read ETag.""" + try: + return self._write(transform, expected_etag=expected_etag, defaults=defaults) + except RedisError as error: + raise SettingsUnavailableError( + "Unable to confirm the settings save. Reload and verify before retrying." + ) from error + + def _write(self, transform, *, expected_etag=None, defaults=None, observed_raw=None): + marker = None + session_token = None + if self.redis_required: + raw = self._raw_state() + if observed_raw is not None and raw != observed_raw: + raise SettingsConflictError("Shared settings changed; retry the read.") + state = self._decode(raw) + if state: + session_token = state.get("session_token") + if state["state"] == "pending" and state["deadline"] > time.time(): + raise SettingsUnavailableError("Another settings save is in progress. Please retry.") + if ( + state["state"] == "ready" + and expected_etag is not None + and state["document"]["_etag"] != expected_etag + ): + raise SettingsConflictError("Settings changed. Reload before saving again.") + marker = json.dumps({ + "state": "pending", + "owner": uuid.uuid4().hex, + "deadline": time.time() + WRITE_LEASE_SECONDS, + "session_token": session_token, + }) + if not self._compare_and_set(raw, marker): + raise SettingsConflictError("Another worker started a settings save.") + + for _ in range(MAX_WRITE_ATTEMPTS): + try: + current, session_token = self._read_cosmos(session_token) + except CosmosResourceNotFoundError: + if defaults is None: + raise + current = copy.deepcopy(defaults) + + if expected_etag is not None and current.get("_etag") != expected_etag: + # Leave the marker pending. Readers use Cosmos until safe repair; + # never publish a snapshot that has not passed an ETag check. + raise SettingsConflictError("Settings changed. Reload before saving again.") + candidate = transform(copy.deepcopy(current)) + candidate = { + key: copy.deepcopy(value) + for key, value in candidate.items() + if key not in COSMOS_METADATA_FIELDS + } + candidate["id"] = SETTINGS_ID + candidate[SETTINGS_REVISION_FIELD] = int(current.get(SETTINGS_REVISION_FIELD, 0)) + 1 + + if marker is not None: + # Fencing plus Cosmos OCC prevents an expired writer committing + # over the replacement writer, even if it resumes much later. + raw = self._raw_state() + if raw not in (marker, marker.encode("utf-8")): + raise SettingsConflictError("Settings write ownership expired.") + + headers = {} + + def capture_headers(response_headers, _body): + headers.update(response_headers) + + try: + if current.get("_etag"): + stored = self.container.replace_item( + item=SETTINGS_ID, + body=candidate, + etag=current["_etag"], + match_condition=MatchConditions.IfNotModified, + session_token=session_token, + response_hook=capture_headers, + ) + else: + stored = self.container.create_item(body=candidate, response_hook=capture_headers) + except (CosmosAccessConditionFailedError, CosmosResourceExistsError): + if expected_etag is not None: + raise SettingsConflictError("Settings changed during the save.") + continue + + if marker is not None: + ready = json.dumps({ + "state": "ready", + "document": dict(stored), + "session_token": headers.get("x-ms-session-token", session_token), + }) + if not self._compare_and_set(marker, ready): + raise SettingsUnavailableError( + "The database save completed but shared publication was superseded. Reload to verify." + ) + return copy.deepcopy(stored) + raise SettingsConflictError("Settings kept changing; reload and retry.") diff --git a/application/single_app/background_tasks.py b/application/single_app/background_tasks.py index b6cc370b1..55123ebe5 100644 --- a/application/single_app/background_tasks.py +++ b/application/single_app/background_tasks.py @@ -197,7 +197,7 @@ def check_logging_timers_once(): """Disable temporary logging settings after their timer expires.""" settings = get_settings() current_time = datetime.now() - settings_changed = False + settings_updates = {} if ( settings.get('enable_debug_logging', False) @@ -213,10 +213,9 @@ def check_logging_timers_once(): if turnoff_time and current_time >= turnoff_time: debug_print(f"logging timer expired at {turnoff_time}. Disabling debug logging.") - settings['enable_debug_logging'] = False - settings['debug_logging_timer_enabled'] = False - settings['debug_logging_turnoff_time'] = None - settings_changed = True + settings_updates['enable_debug_logging'] = False + settings_updates['debug_logging_timer_enabled'] = False + settings_updates['debug_logging_turnoff_time'] = None if ( settings.get('enable_file_processing_logs', False) @@ -232,14 +231,12 @@ def check_logging_timers_once(): if turnoff_time and current_time >= turnoff_time: print(f"File processing logs timer expired at {turnoff_time}. Disabling file processing logs.") - settings['enable_file_processing_logs'] = False - settings['file_processing_logs_timer_enabled'] = False - settings['file_processing_logs_turnoff_time'] = None - settings_changed = True + settings_updates['enable_file_processing_logs'] = False + settings_updates['file_processing_logs_timer_enabled'] = False + settings_updates['file_processing_logs_turnoff_time'] = None - if settings_changed: - update_settings(settings) - print("Logging settings updated due to timer expiration.") + if settings_updates: + return update_settings(settings_updates, expected_etag=settings.get('_etag')) def check_expired_approvals_once(): @@ -321,14 +318,15 @@ def _seed_control_center_auto_refresh_next_run(settings, current_time): """Persist the next Control Center auto-refresh run when schedule fields are missing.""" schedule = get_control_center_auto_refresh_schedule(settings) next_run = calculate_next_control_center_auto_refresh_run(settings, current_time=current_time) - update_settings({ + if not update_settings({ 'control_center_auto_refresh_enabled': settings.get('control_center_auto_refresh_enabled', True), 'control_center_auto_refresh_time': schedule['time'], 'control_center_auto_refresh_hour': schedule['hour'], 'control_center_auto_refresh_minute': schedule['minute'], 'control_center_auto_refresh_timezone': schedule['timezone'], 'control_center_auto_refresh_next_run': next_run.isoformat(), - }) + }, expected_etag=settings.get('_etag')): + raise RuntimeError('Unable to save the next Control Center refresh time.') return next_run @@ -471,7 +469,11 @@ def check_cosmos_throughput_autoscale_once(): result = evaluate_and_apply_cosmos_throughput_scaling(settings, refresh_id=refresh_id) settings_update = result.get('settings_update') or {} if settings_update: - update_settings(settings_update) + expected_etag = settings.get('_etag') if 'cosmos_throughput_container_policies' in settings_update else None + if not update_settings(settings_update, expected_etag=expected_etag): + result['success'] = False + result['error'] = 'Unable to save Cosmos throughput runtime settings.' + return result decision = result.get('decision') or {} scale_result = result.get('scale_result') or {} log_event( diff --git a/application/single_app/config.py b/application/single_app/config.py index e78cf6160..fc1da1a85 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -98,7 +98,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.024" +VERSION = "0.261.026" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_appinsights.py b/application/single_app/functions_appinsights.py index 05101ee29..26277b326 100644 --- a/application/single_app/functions_appinsights.py +++ b/application/single_app/functions_appinsights.py @@ -312,16 +312,19 @@ def _build_external_event_extra( def _load_logging_settings() -> Dict[str, Any]: - """Read cached settings first and fall back to live settings when needed.""" + """Read shared settings without recursively logging a cache failure.""" if getattr(_logging_settings_load_state, 'active', False): return {} + _logging_settings_load_state.active = True try: cache = app_settings_cache.get_settings_cache() if isinstance(cache, dict): return cache except Exception: pass + finally: + _logging_settings_load_state.active = False return {} diff --git a/application/single_app/functions_control_center.py b/application/single_app/functions_control_center.py index d50ea0859..eb434f926 100644 --- a/application/single_app/functions_control_center.py +++ b/application/single_app/functions_control_center.py @@ -233,29 +233,32 @@ def execute_control_center_refresh(manual_execution=False): settings = get_settings() if settings: current_time = datetime.now(timezone.utc) - settings['control_center_last_refresh'] = current_time.isoformat() - - schedule = get_control_center_auto_refresh_schedule(settings) - settings['control_center_auto_refresh_time'] = schedule['time'] - settings['control_center_auto_refresh_hour'] = schedule['hour'] - settings['control_center_auto_refresh_minute'] = schedule['minute'] - settings['control_center_auto_refresh_timezone'] = schedule['timezone'] + settings_updates = { + 'control_center_last_refresh': current_time.isoformat(), + } # Calculate next scheduled auto-refresh time if enabled if settings.get('control_center_auto_refresh_enabled', True): next_run = calculate_next_control_center_auto_refresh_run(settings, current_time=current_time) - settings['control_center_auto_refresh_next_run'] = next_run.isoformat() + settings_updates['control_center_auto_refresh_next_run'] = next_run.isoformat() else: - settings['control_center_auto_refresh_next_run'] = None + settings_updates['control_center_auto_refresh_next_run'] = None - update_success = update_settings(settings) + update_success = update_settings(settings_updates) if update_success: debug_print("โœ… [AUTO-REFRESH] Admin settings updated with refresh timestamp") else: + results['success'] = False + results['error'] = 'Unable to save Control Center refresh settings.' debug_print("โš ๏ธ [AUTO-REFRESH] Failed to update admin settings") - + else: + results['success'] = False + results['error'] = 'Unable to load Control Center refresh settings.' + except Exception as settings_error: + results['success'] = False + results['error'] = 'Unable to save Control Center refresh settings.' debug_print(f"โŒ [AUTO-REFRESH] Admin settings update failed: {settings_error}") # Log the activity diff --git a/application/single_app/functions_redis_monitoring.py b/application/single_app/functions_redis_monitoring.py index 79bd2bfd6..12443418c 100644 --- a/application/single_app/functions_redis_monitoring.py +++ b/application/single_app/functions_redis_monitoring.py @@ -7,6 +7,7 @@ import app_settings_cache import functions_redis_client +from app_settings_store import SETTINGS_STATE_KEY REDIS_MONITORING_STATUS_DISABLED = "disabled" @@ -38,6 +39,10 @@ "key", ) REDIS_EXPLORER_REDACTED_VALUE = "[REDACTED]" +# Old deployments can leave these keys behind; recognize them without depending +# on the removed worker-cache implementation or treating them as current state. +REDIS_LEGACY_SETTINGS_PAYLOAD_KEY = "APP_SETTINGS_CACHE" +REDIS_LEGACY_SETTINGS_VERSION_KEY = "APP_SETTINGS_CACHE_VERSION" REDIS_EXPLORER_RESTRICTED_PREVIEW = ( "Preview restricted because the Redis key name indicates session, token, cookie, or credential data." ) @@ -402,19 +407,26 @@ def _resolve_redis_keys(keys, dai_hash_resolver=None): ) continue - if normalized_key == app_settings_cache.APP_SETTINGS_CACHE_KEY: + if normalized_key == SETTINGS_STATE_KEY: + resolutions[normalized_key] = _build_resolution_payload( + "app_settings_state", + "Shared app settings state", + resolved=True, + note="Current settings publication record: ready document/revision or pending write marker. Sensitive preview fields are redacted.", + ) + elif normalized_key == REDIS_LEGACY_SETTINGS_PAYLOAD_KEY: resolutions[normalized_key] = _build_resolution_payload( "app_settings_cache", - "App settings cache payload", + "Legacy app settings cache payload", resolved=True, - note="Global app settings cache payload.", + note="Legacy settings payload; not used by the current shared settings store.", ) - elif normalized_key == app_settings_cache.APP_SETTINGS_CACHE_VERSION_KEY: + elif normalized_key == REDIS_LEGACY_SETTINGS_VERSION_KEY: resolutions[normalized_key] = _build_resolution_payload( "app_settings_cache_version", - "App settings cache version", + "Legacy app settings cache version", resolved=True, - note="Global app settings cache invalidation version.", + note="Legacy invalidation counter; current settings carry their revision in the shared state record.", ) if dai_version_hashes: diff --git a/application/single_app/functions_retention_policy.py b/application/single_app/functions_retention_policy.py index 4ae7c016e..dc6ec2629 100644 --- a/application/single_app/functions_retention_policy.py +++ b/application/single_app/functions_retention_policy.py @@ -544,16 +544,21 @@ def execute_retention_policy(workspace_scopes=None, manual_execution=False): results['public'] = public_results # Update last run time in settings - settings['retention_policy_last_run'] = datetime.now(timezone.utc).isoformat() + settings_updates = { + 'retention_policy_last_run': datetime.now(timezone.utc).isoformat(), + } # Calculate next run time (scheduled for configured hour next day) execution_hour = settings.get('retention_policy_execution_hour', 2) next_run = datetime.now(timezone.utc).replace(hour=execution_hour, minute=0, second=0, microsecond=0) if next_run <= datetime.now(timezone.utc): next_run += timedelta(days=1) - settings['retention_policy_next_run'] = next_run.isoformat() + settings_updates['retention_policy_next_run'] = next_run.isoformat() - update_settings(settings) + if not update_settings(settings_updates): + results['success'] = False + results['errors'].append('Unable to save retention policy execution settings.') + return results debug_print(f"Retention policy execution completed: {results}") return results diff --git a/application/single_app/functions_service_health.py b/application/single_app/functions_service_health.py index b936e369e..4dcba402b 100644 --- a/application/single_app/functions_service_health.py +++ b/application/single_app/functions_service_health.py @@ -107,7 +107,10 @@ def record_semantic_search_quota_exceeded(error=None, source="hybrid_search"): "source": source, "occurrence_count": occurrence_count, } - if not update_settings({"service_health": service_health}): + if not update_settings( + {"service_health": service_health}, + expected_etag=settings.get("_etag"), + ): raise RuntimeError("update_settings returned False while recording semantic quota warning.") log_event( "[SERVICE_HEALTH] Azure AI Search semantic quota exceeded.", @@ -144,7 +147,10 @@ def clear_semantic_search_quota_warning(source="hybrid_search"): cleared_health["last_cleared_at"] = _utc_now_iso() cleared_health["source"] = source service_health[SEMANTIC_SEARCH_HEALTH_KEY] = cleared_health - if not update_settings({"service_health": service_health}): + if not update_settings( + {"service_health": service_health}, + expected_etag=settings.get("_etag"), + ): raise RuntimeError("update_settings returned False while clearing semantic quota warning.") log_event( "[SERVICE_HEALTH] Azure AI Search semantic quota warning cleared after successful search.", diff --git a/application/single_app/functions_settings.py b/application/single_app/functions_settings.py index daf2356f1..6aa403d73 100644 --- a/application/single_app/functions_settings.py +++ b/application/single_app/functions_settings.py @@ -5,6 +5,12 @@ from flask import g, has_request_context, jsonify, request, session +from app_settings_store import ( + COSMOS_METADATA_FIELDS, + SETTINGS_REVISION_FIELD, + SettingsConflictError, + SettingsUnavailableError, +) from config import * from functions_appinsights import log_event from functions_content_safety import ( @@ -37,7 +43,6 @@ ) from functions_service_health import get_default_service_health import app_settings_cache -import inspect import copy import os import json @@ -1204,45 +1209,6 @@ def _should_sync_session_profile(target_user_id, actor_user_id, allow_cross_user return bool(normalized_target_user_id and normalized_actor_user_id and normalized_target_user_id == normalized_actor_user_id) -def _refresh_app_settings_cache_after_write(settings_payload, context="app_settings_write"): - """Update shared/local settings cache around a version bump.""" - cache_updater = getattr(app_settings_cache, "update_settings_cache", None) - version_bumper = getattr(app_settings_cache, "bump_app_settings_cache_version", None) - - def _update_cache(stage): - if not callable(cache_updater): - return - try: - cache_updater(copy.deepcopy(settings_payload)) - except Exception as cache_error: - log_event( - "App settings cache update failed after settings write.", - extra={ - "context": context, - "stage": stage, - "error": str(cache_error) - }, - level=logging.WARNING - ) - - _update_cache("before_version_bump") - - if callable(version_bumper): - try: - version_bumper() - except Exception as version_error: - log_event( - "App settings cache version bump failed after settings write.", - extra={ - "context": context, - "error": str(version_error) - }, - level=logging.WARNING - ) - - _update_cache("after_version_bump") - - def _env_flag_enabled(name): return str(os.environ.get(name, '')).strip().lower() in {'1', 'true', 'yes', 'on'} @@ -1946,76 +1912,7 @@ def _format_result(settings_payload, source): return settings_payload, source return settings_payload - try: - # Attempt to read the existing doc - if use_cosmos: - settings_item = cosmos_settings_container.read_item( - item="app_settings", - partition_key="app_settings" - ) - settings_source = "cosmos_forced" - log_event( - "App settings loaded from Cosmos DB (forced).", - extra={ - "settings_source": settings_source, - "use_cosmos": True - }, - level=logging.INFO - ) - else: - settings_item = None - settings_source = "cache" - - cache_accessor = getattr(app_settings_cache, "get_settings_cache", None) - if callable(cache_accessor): - try: - settings_item = cache_accessor() - except Exception as cache_error: - settings_item = None - log_event( - "Error reading app settings from cache accessor.", - extra={ - "error": str(cache_error) - }, - level=logging.WARNING - ) - - if not settings_item: - settings_source = "cosmos_fallback" - settings_item = cosmos_settings_container.read_item( - item="app_settings", - partition_key="app_settings" - ) - - frame = inspect.currentframe() - caller = frame.f_back # the function that called *this* code - - if caller is not None: - code = caller.f_code - caller_file = code.co_filename - caller_line = caller.f_lineno - caller_func = code.co_name - - log_event( - "App settings cache miss. Falling back to Cosmos DB.", - extra={ - "settings_source": settings_source, - "caller_file": caller_file, - "caller_line": caller_line, - "caller_func": caller_func - }, - level=logging.WARNING - ) - else: - - log_event( - "App settings cache miss. Falling back to Cosmos DB (no caller frame).", - extra={ - "settings_source": settings_source - }, - level=logging.WARNING - ) - + def normalize_loaded_settings(settings_item): legacy_control_center_schedule = ( 'control_center_auto_refresh_timezone' not in settings_item ) @@ -2037,9 +1934,8 @@ def _format_result(settings_payload, source): legacy_control_center_time = f"{legacy_hour:02d}:{legacy_minute:02d}" # Merge default_settings in, to fill in any missing or nested keys - merge_changed = deep_merge_dicts(default_settings, settings_item) + deep_merge_dicts(default_settings, settings_item) merged = settings_item - control_center_schedule_migration_updated = False if legacy_control_center_schedule: if legacy_control_center_time == '06:00': merged['control_center_auto_refresh_time'] = '02:00' @@ -2049,65 +1945,52 @@ def _format_result(settings_payload, source): else: merged['control_center_auto_refresh_timezone'] = 'UTC' merged['control_center_auto_refresh_next_run'] = None - control_center_schedule_migration_updated = True - enhanced_extraction_migration_updated = False if legacy_enhanced_extraction and legacy_enhanced_extraction_mode in ('layout', 'auto'): merged['enable_enhanced_extraction'] = True - enhanced_extraction_migration_updated = True - migration_updated = apply_custom_endpoint_setting_migration(merged) - assignment_settings_updated = normalize_group_workflow_assignment_settings(merged) - promoted_popular_settings_updated = normalize_agents_page_promoted_popular_settings(merged) - document_access_index_settings_updated = normalize_document_access_index_required_settings(merged) - inbound_mcp_settings_updated = normalize_inbound_mcp_settings(merged) - public_workspace_display_settings_updated = normalize_public_workspace_display_settings(merged) - key_vault_reminder_settings_updated = normalize_key_vault_reminder_settings(merged) - model_endpoint_identity_header_settings_updated = normalize_model_endpoint_identity_header_settings(merged) - tabular_parity_durable_preflight_settings_updated = normalize_tabular_parity_durable_preflight_defaults(merged) + apply_custom_endpoint_setting_migration(merged) + normalize_group_workflow_assignment_settings(merged) + normalize_agents_page_promoted_popular_settings(merged) + normalize_document_access_index_required_settings(merged) + normalize_inbound_mcp_settings(merged) + normalize_public_workspace_display_settings(merged) + normalize_key_vault_reminder_settings(merged) + normalize_model_endpoint_identity_header_settings(merged) + normalize_tabular_parity_durable_preflight_defaults(merged) merged['enable_tabular_processing_plugin'] = is_tabular_processing_enabled(merged) - # If merging added anything new, upsert back to Cosmos so future reads remain up to date - if ( - merge_changed - or control_center_schedule_migration_updated - or enhanced_extraction_migration_updated - or migration_updated - or assignment_settings_updated - or promoted_popular_settings_updated - or document_access_index_settings_updated - or inbound_mcp_settings_updated - or public_workspace_display_settings_updated - or key_vault_reminder_settings_updated - or model_endpoint_identity_header_settings_updated - or tabular_parity_durable_preflight_settings_updated - ): - cosmos_settings_container.upsert_item(merged) - _refresh_app_settings_cache_after_write(merged, context="merge_upsert") - - log_event( - "App settings defaults or migrations were persisted to Cosmos DB.", - extra={ - "settings_source": settings_source - }, - level=logging.INFO - ) - return _format_result(attach_public_workspace_label_context(merged), settings_source) - else: - # If merged is unchanged, no new keys needed - return _format_result(attach_public_workspace_label_context(merged), settings_source) - - except CosmosResourceNotFoundError: - cosmos_settings_container.create_item(body=default_settings) - _refresh_app_settings_cache_after_write(default_settings, context="default_create") + return merged - log_event( - "App settings document not found. Default settings created in Cosmos DB.", - extra={ - "settings_source": "cosmos_default_created" - }, - level=logging.WARNING - ) - return _format_result(attach_public_workspace_label_context(default_settings), "cosmos_default_created") + try: + store = app_settings_cache.get_settings_store() + settings_source = "cosmos_forced" if use_cosmos else "shared" + try: + settings_item = store.read(use_cosmos=use_cosmos) + except CosmosResourceNotFoundError: + settings_item = store.write(normalize_loaded_settings, defaults=default_settings) + settings_source = "cosmos_default_created" + merged = normalize_loaded_settings(copy.deepcopy(settings_item)) + if merged != settings_item: + try: + # Re-run migrations against the authoritative document, not the + # read snapshot. OCC retries preserve concurrent admin changes. + merged = store.write(normalize_loaded_settings) + except (SettingsUnavailableError, SettingsConflictError) as error: + # Reads remain available during an outage; migrations are deferred, + # not reported as persisted or written via a second version source. + merged = normalize_loaded_settings(store.read(use_cosmos=True)) + log_event( + "[ASC] Settings migration deferred; shared writes are unavailable.", + extra={"error_type": type(error).__name__}, + level=logging.WARNING, + ) + else: + log_event( + "[ASC] App settings defaults or migrations were persisted.", + extra={"settings_source": settings_source}, + level=logging.INFO, + ) + return _format_result(attach_public_workspace_label_context(merged), settings_source) except Exception as e: log_event( @@ -2132,12 +2015,18 @@ def get_rate_limit_message(settings=None): return build_rate_limit_message(resolved_settings) -def update_settings(new_settings): - try: - # always fetch the latest settings doc, which includes your merges - settings_item = get_settings() +def update_settings(new_settings, *, expected_etag=None): + """Merge intended changes into Cosmos with OCC and shared-cache publication.""" + expected_etag = expected_etag or new_settings.get("_etag") + updates = { + key: copy.deepcopy(value) + for key, value in new_settings.items() + if key not in COSMOS_METADATA_FIELDS | {SETTINGS_REVISION_FIELD, "id"} + } + + def apply_updates(settings_item): existing_multi_endpoint_enabled = settings_item.get('enable_multi_model_endpoints', False) - settings_item.update(new_settings) + settings_item.update(updates) normalize_group_workflow_assignment_settings(settings_item) normalize_agents_page_promoted_popular_settings(settings_item) normalize_document_access_index_required_settings(settings_item) @@ -2150,18 +2039,20 @@ def update_settings(new_settings): settings_item.get('enable_multi_model_endpoints', False), ) settings_item['enable_tabular_processing_plugin'] = is_tabular_processing_enabled(settings_item) - cosmos_settings_container.upsert_item(settings_item) - _refresh_app_settings_cache_after_write(settings_item, context="update_settings") + return settings_item + + try: + app_settings_cache.get_settings_store().write(apply_updates, expected_etag=expected_etag) log_event( - "App settings updated successfully.", + "[ASC] App settings updated and published successfully.", level=logging.INFO ) return True except Exception as e: log_event( - "Error updating app settings.", + "[ASC] Unable to confirm settings save; reload and verify before retrying.", extra={ - "error": str(e) + "error_type": type(e).__name__ }, level=logging.ERROR, exceptionTraceback=True diff --git a/application/single_app/plugin_validation_endpoint.py b/application/single_app/plugin_validation_endpoint.py index 0803ecb9d..6cd868d60 100644 --- a/application/single_app/plugin_validation_endpoint.py +++ b/application/single_app/plugin_validation_endpoint.py @@ -4,11 +4,14 @@ """ import logging +from copy import deepcopy from flask import Blueprint, current_app, jsonify, request from functions_appinsights import log_event from functions_authentication import admin_required, admin_required_blueprint, login_required, user_required, user_required_blueprint +from functions_global_actions import save_global_action +from functions_settings import get_settings, update_settings from json_schema_validation import apply_plugin_validation_defaults from semantic_kernel_plugins.plugin_health_checker import PluginErrorRecovery, PluginHealthChecker from semantic_kernel_plugins.plugin_loader import discover_plugins @@ -252,10 +255,8 @@ def repair_plugin(plugin_name): Attempt to repair a plugin that has issues. """ try: - from functions_settings import get_settings, update_settings - settings = get_settings() - plugins = settings.get('semantic_kernel_plugins', []) + plugins = deepcopy(settings.get('semantic_kernel_plugins', [])) # Find the plugin plugin_index = None @@ -312,20 +313,18 @@ def normalize(s): plugin_manifest['metadata']['original_errors'] = instantiation_errors plugins[plugin_index] = plugin_manifest - # NOTE: Update container-based storage instead of legacy settings - from functions_global_actions import save_global_action try: - # Save to container instead of settings - save_global_action(plugin_manifest) - # Remove from legacy settings if present - if 'semantic_kernel_plugins' in settings: - del settings['semantic_kernel_plugins'] - update_settings(settings) + saved_to_container = bool(save_global_action(plugin_manifest)) except Exception as e: - print(f"Error updating plugin in container storage: {e}") - # Fallback to settings update if container fails - settings['semantic_kernel_plugins'] = plugins - update_settings(settings) + log_event("[PLUGIN_REPAIR] Container save failed; using legacy settings.", + extra={'error_type': type(e).__name__}, level=logging.WARNING) + saved_to_container = False + remaining_plugins = plugins[:plugin_index] + plugins[plugin_index + 1:] if saved_to_container else plugins + if not update_settings( + {'semantic_kernel_plugins': remaining_plugins}, + expected_etag=settings.get('_etag'), + ): + return jsonify({'success': False, 'error': 'Unable to save the plugin repair.'}), 500 return jsonify({ 'success': True, @@ -349,20 +348,18 @@ def normalize(s): plugin_manifest['metadata']['repair_timestamp'] = health_report.get('timestamp') plugins[plugin_index] = plugin_manifest - # NOTE: Update container-based storage instead of legacy settings - from functions_global_actions import save_global_action try: - # Save to container instead of settings - save_global_action(plugin_manifest) - # Remove from legacy settings if present - if 'semantic_kernel_plugins' in settings: - del settings['semantic_kernel_plugins'] - update_settings(settings) + saved_to_container = bool(save_global_action(plugin_manifest)) except Exception as e: - print(f"Error updating plugin in container storage: {e}") - # Fallback to settings update if container fails - settings['semantic_kernel_plugins'] = plugins - update_settings(settings) + log_event("[PLUGIN_REPAIR] Container save failed; using legacy settings.", + extra={'error_type': type(e).__name__}, level=logging.WARNING) + saved_to_container = False + remaining_plugins = plugins[:plugin_index] + plugins[plugin_index + 1:] if saved_to_container else plugins + if not update_settings( + {'semantic_kernel_plugins': remaining_plugins}, + expected_etag=settings.get('_etag'), + ): + return jsonify({'success': False, 'error': 'Unable to save the plugin repair.'}), 500 return jsonify({ 'success': True, @@ -380,5 +377,5 @@ def normalize(s): log_event(f"[PLUGIN_REPAIR] Error repairing {plugin_name}: {str(e)}", level=logging.ERROR) return jsonify({ 'success': False, - 'error': f'Repair failed: {str(e)}' + 'error': 'Unable to repair the plugin.' }), 500 diff --git a/application/single_app/route_backend_agents.py b/application/single_app/route_backend_agents.py index 9b03a9ff2..5cfe219fc 100644 --- a/application/single_app/route_backend_agents.py +++ b/application/single_app/route_backend_agents.py @@ -4,6 +4,7 @@ import uuid import logging import builtins +from copy import deepcopy from azure.identity import DefaultAzureCredential, get_bearer_token_provider from flask import Blueprint, jsonify, request, current_app, session from config import ( @@ -854,13 +855,15 @@ def _maybe_disable_multi_endpoint_migration_notice(settings, preview): if preview['summary']['ready_to_migrate'] or preview['summary']['needs_default_model']: return False - notice = settings.get('multi_endpoint_migration_notice', {}) or {} + notice = dict(settings.get('multi_endpoint_migration_notice', {}) or {}) if not notice.get('enabled', False): return False notice['enabled'] = False - update_settings({'multi_endpoint_migration_notice': notice}) - return True + return update_settings( + {'multi_endpoint_migration_notice': notice}, + expected_etag=settings.get('_etag'), + ) # === AGENT GUID GENERATION ENDPOINT === @bpa.route('/api/agents/generate_id', methods=['GET']) @@ -1516,9 +1519,10 @@ def set_selected_agent(): return jsonify({'error': 'Agent not found.'}), 404 # Set global_selected_agent field only - settings = get_settings() - settings['global_selected_agent'] = { 'name': agent_name, 'is_global': True, 'is_group': False } - update_settings(settings) + if not update_settings({ + 'global_selected_agent': {'name': agent_name, 'is_global': True, 'is_group': False}, + }): + return jsonify({'error': 'Failed to set default agent.'}), 500 log_event("Global selected agent set", extra={"action": "set-global-selected", "agent_name": agent_name, "user": str(get_current_user_id())}) # --- HOT RELOAD TRIGGER --- setattr(builtins, "kernel_reload_needed", True) @@ -1643,14 +1647,21 @@ def set_agent_enabled(agent_name): enabled_agents = get_global_agents() if enabled_agents: fallback_agent_name = enabled_agents[0].get('name') - settings['global_selected_agent'] = { + selected_agent_update = { 'name': fallback_agent_name, 'is_global': True, 'is_group': False, } else: - settings['global_selected_agent'] = {} - update_settings(settings) + selected_agent_update = {} + if not update_settings( + {'global_selected_agent': selected_agent_update}, + expected_etag=settings.get('_etag'), + ): + setattr(builtins, "kernel_reload_needed", True) + return jsonify({ + 'error': 'Agent state changed, but the default selection could not be saved. Reload and retry.' + }), 500 log_agent_update( user_id=str(get_current_user_id()), @@ -1929,7 +1940,7 @@ def update_agent_setting(setting_name): if 'value' not in data: return jsonify({'error': 'Missing value in request.'}), 400 value = data['value'] - settings = get_settings() + settings = deepcopy(get_settings()) keys = setting_name.split('.') target = settings for k in keys[:-1]: @@ -1942,7 +1953,11 @@ def update_agent_setting(setting_name): target[key] = value else: return jsonify({'error': 'Only simple values (str, int, float, bool, None) are allowed.'}), 400 - update_settings(settings) + if not update_settings( + {keys[0]: settings[keys[0]]}, + expected_etag=settings.get('_etag') if len(keys) > 1 else None, + ): + return jsonify({'error': 'Failed to update agent setting.'}), 500 log_event("Agent setting updated", extra={ "setting": setting_name, @@ -2117,14 +2132,16 @@ def orchestration_settings(): return jsonify({"error": "max_rounds_per_agent must be an integer > 0 for group_chat."}), 400 # Save settings - settings = get_settings() - settings["orchestration_type"] = orchestration_type - settings["enable_multi_agent_orchestration"] = enable_multi + settings_updates = { + "orchestration_type": orchestration_type, + "enable_multi_agent_orchestration": enable_multi, + } if orchestration_type == "group_chat": - settings["max_rounds_per_agent"] = max_rounds + settings_updates["max_rounds_per_agent"] = max_rounds else: - settings["max_rounds_per_agent"] = 1 - update_settings(settings) + settings_updates["max_rounds_per_agent"] = 1 + if not update_settings(settings_updates): + return jsonify({'error': 'Failed to update orchestration settings.'}), 500 # --- HOT RELOAD TRIGGER --- setattr(builtins, "kernel_reload_needed", True) return jsonify({'success': True}) diff --git a/application/single_app/route_backend_control_center.py b/application/single_app/route_backend_control_center.py index b28aa659f..5a34d077f 100644 --- a/application/single_app/route_backend_control_center.py +++ b/application/single_app/route_backend_control_center.py @@ -5827,25 +5827,24 @@ def api_refresh_control_center_data(): # Update admin settings with refresh timestamp debug_print("๐Ÿ”„ [REFRESH DEBUG] Updating admin settings...") try: - from functions_settings import get_settings, update_settings - - settings = get_settings() - if settings: - settings['control_center_last_refresh'] = datetime.now(timezone.utc).isoformat() - update_success = update_settings(settings) - - if not update_success: - debug_print("โš ๏ธ [REFRESH DEBUG] Failed to update admin settings") - debug_print("Failed to update admin settings with refresh timestamp") - else: - debug_print("โœ… [REFRESH DEBUG] Admin settings updated successfully") - debug_print("Updated admin settings with refresh timestamp") - else: - debug_print("โš ๏ธ [REFRESH DEBUG] Could not get admin settings") + update_success = update_settings({ + 'control_center_last_refresh': datetime.now(timezone.utc).isoformat(), + }) + if not update_success: + return jsonify({ + 'success': False, + 'error': 'Data refreshed, but the refresh timestamp could not be saved.' + }), 500 + debug_print("โœ… [REFRESH DEBUG] Admin settings updated successfully") + debug_print("Updated admin settings with refresh timestamp") except Exception as admin_error: debug_print(f"โŒ [REFRESH DEBUG] Admin settings update failed: {admin_error}") debug_print(f"Error updating admin settings: {admin_error}") + return jsonify({ + 'success': False, + 'error': 'Data refreshed, but the refresh timestamp could not be saved.' + }), 500 debug_print(f"๐ŸŽ‰ [REFRESH DEBUG] Refresh completed! Users - Refreshed: {refreshed_count}, Failed: {failed_count}. Groups - Refreshed: {groups_refreshed_count}, Failed: {groups_failed_count}") debug_print(f"Control Center data refresh completed. Users: {refreshed_count} refreshed, {failed_count} failed. Groups: {groups_refreshed_count} refreshed, {groups_failed_count} failed") diff --git a/application/single_app/route_backend_retention_policy.py b/application/single_app/route_backend_retention_policy.py index ecc43d72d..6384760bb 100644 --- a/application/single_app/route_backend_retention_policy.py +++ b/application/single_app/route_backend_retention_policy.py @@ -63,35 +63,39 @@ def update_retention_policy_settings(): """ try: data = request.get_json() - settings = get_settings() + settings_updates = {} # Update settings if provided if 'enable_retention_policy_personal' in data: - settings['enable_retention_policy_personal'] = bool(data['enable_retention_policy_personal']) + settings_updates['enable_retention_policy_personal'] = bool(data['enable_retention_policy_personal']) if 'enable_retention_policy_group' in data: - settings['enable_retention_policy_group'] = bool(data['enable_retention_policy_group']) + settings_updates['enable_retention_policy_group'] = bool(data['enable_retention_policy_group']) if 'enable_retention_policy_public' in data: - settings['enable_retention_policy_public'] = bool(data['enable_retention_policy_public']) + settings_updates['enable_retention_policy_public'] = bool(data['enable_retention_policy_public']) if 'retention_policy_execution_hour' in data: hour = int(data['retention_policy_execution_hour']) if 0 <= hour <= 23: - settings['retention_policy_execution_hour'] = hour + settings_updates['retention_policy_execution_hour'] = hour # Recalculate next run time next_run = datetime.now(timezone.utc).replace(hour=hour, minute=0, second=0, microsecond=0) if next_run <= datetime.now(timezone.utc): next_run += timedelta(days=1) - settings['retention_policy_next_run'] = next_run.isoformat() + settings_updates['retention_policy_next_run'] = next_run.isoformat() else: return jsonify({ 'success': False, 'error': 'Execution hour must be between 0 and 23' }), 400 - update_settings(settings) + if not update_settings(settings_updates): + return jsonify({ + 'success': False, + 'error': 'Failed to update retention policy settings' + }), 500 return jsonify({ 'success': True, diff --git a/application/single_app/route_backend_settings.py b/application/single_app/route_backend_settings.py index ebf577c63..49c54719d 100644 --- a/application/single_app/route_backend_settings.py +++ b/application/single_app/route_backend_settings.py @@ -1052,12 +1052,17 @@ def scale_cosmos_throughput_admin(): scale_result['direction'] = direction scale_result['reason'] = f'manual_{direction}' - update_settings(build_runtime_update( + settings_updates = build_runtime_update( status=status, decision={'direction': direction, 'reason': f'manual_{direction}'}, scale_result=scale_result, settings=settings, - )) + ) + expected_etag = settings.get('_etag') if 'cosmos_throughput_container_policies' in settings_updates else None + if not update_settings(settings_updates, expected_etag=expected_etag): + return jsonify({ + 'error': 'Throughput changed, but its runtime settings could not be saved. Reload and verify before retrying.' + }), 500 log_general_admin_action( admin_user_id=admin_user_id, admin_email=admin_email, @@ -1131,12 +1136,17 @@ def convert_cosmos_throughput_to_autoscale_admin(): scale_result['direction'] = 'convert_to_autoscale' scale_result['reason'] = 'manual_to_autoscale_conversion' - update_settings(build_runtime_update( + settings_updates = build_runtime_update( status=status, decision=decision, scale_result=scale_result, settings=settings, - )) + ) + expected_etag = settings.get('_etag') if 'cosmos_throughput_container_policies' in settings_updates else None + if not update_settings(settings_updates, expected_etag=expected_etag): + return jsonify({ + 'error': 'Throughput mode changed, but its runtime settings could not be saved. Reload and verify before retrying.' + }), 500 log_general_admin_action( admin_user_id=admin_user_id, admin_email=admin_email, diff --git a/application/single_app/route_custom_pages.py b/application/single_app/route_custom_pages.py index 9a327dbc7..41fcba41c 100644 --- a/application/single_app/route_custom_pages.py +++ b/application/single_app/route_custom_pages.py @@ -238,11 +238,14 @@ def admin_create_request_access_custom_page(): return jsonify({"error": "; ".join(errors)}), 400 saved = save_custom_page(request_access_page, user_id=_current_admin_user_id()) - update_settings({ + if not update_settings({ "access_request_button_enabled": True, "access_request_button_text": "Request Access", "access_request_page_url": "/custom/request-access", - }) + }): + return jsonify({ + "error": "Page saved, but the access request settings could not be saved. Reload and verify before retrying." + }), 500 return jsonify({"page": saved, "access_request_button_enabled": True}), 201 @bp.route("/api/admin/custom-pages/", methods=["PUT"]) diff --git a/application/single_app/route_frontend_admin_settings.py b/application/single_app/route_frontend_admin_settings.py index f7e137abe..558e1a4ad 100644 --- a/application/single_app/route_frontend_admin_settings.py +++ b/application/single_app/route_frontend_admin_settings.py @@ -576,7 +576,8 @@ def admin_settings(): normalized_endpoints, endpoints_changed = normalize_model_endpoints(settings.get('model_endpoints', [])) if endpoints_changed: - update_settings({'model_endpoints': normalized_endpoints}) + if update_settings({'model_endpoints': normalized_endpoints}, expected_etag=settings.get('_etag')): + settings = get_settings() settings['model_endpoints'] = normalized_endpoints frontend_model_endpoints = sanitize_model_endpoints_for_frontend(normalized_endpoints) @@ -973,8 +974,8 @@ def admin_settings(): new_settings['update_available'] = False # Update settings to persist these values - update_settings(new_settings) - settings.update(new_settings) + if update_settings(new_settings): + settings = get_settings() except Exception as e: print(f"Error checking for updates: {e}") log_event(f"Error checking for updates: {e}", level=logging.ERROR) @@ -984,8 +985,8 @@ def admin_settings(): update_available = _is_update_version_newer(latest_version, current_version) if settings.get('update_available') != update_available: try: - update_settings({'update_available': update_available}) - settings['update_available'] = update_available + if update_settings({'update_available': update_available}): + settings = get_settings() except Exception as e: log_event(f"Error normalizing cached update availability: {e}", level=logging.WARNING) @@ -1049,6 +1050,10 @@ def admin_settings(): if request.method == 'POST': form_data = request.form # Use a variable for easier access user_id = get_current_user_id() + settings_etag = form_data.get('admin_settings_etag', '') + if not settings_etag or settings_etag != settings.get('_etag'): + flash("Settings changed since this page was loaded. Review the latest settings and try again.", "warning") + return redirect(url_for('frontend_admin_settings.admin_settings')) def admin_secret(field_name, form_field_name=None): submitted_value = form_data.get(form_field_name or field_name, '').strip() @@ -3215,7 +3220,7 @@ def is_valid_url(url): # --- Update settings in DB --- # new_settings now contains either the new logo/favicon base64 or the original ones - if update_settings(new_settings): + if update_settings(new_settings, expected_etag=settings_etag): flash("Admin settings updated successfully.", "success") if enable_custom_pages and not custom_pages_was_enabled and custom_pages_restart_acknowledged: log_general_admin_action( @@ -3300,7 +3305,11 @@ def is_valid_url(url): print(f"Warning sending chunk size notification: {e}") else: - flash("Failed to update admin settings.", "danger") + flash( + "Unable to confirm the settings save. Reload and verify the values before retrying. " + "Another save may be in progress, or Redis may be unavailable.", + "danger", + ) # Redirect back to settings page diff --git a/application/single_app/simplechat_scheduler.py b/application/single_app/simplechat_scheduler.py index 2435227f7..584590014 100644 --- a/application/single_app/simplechat_scheduler.py +++ b/application/single_app/simplechat_scheduler.py @@ -22,7 +22,6 @@ def initialize_scheduler_runtime(): settings, get_redis_cache_infrastructure_endpoint(redis_hostname) ) - app_settings_cache.update_settings_cache(settings) initialize_clients(settings) setup_appinsights_logging(settings) logging.basicConfig(level=logging.DEBUG) diff --git a/application/single_app/templates/admin_settings.html b/application/single_app/templates/admin_settings.html index b4ddd18a5..9e408f544 100644 --- a/application/single_app/templates/admin_settings.html +++ b/application/single_app/templates/admin_settings.html @@ -559,6 +559,7 @@

Admin Settings

{% include "_semantic_search_health_warning.html" %}
+ diff --git a/docs/admin/scale.md b/docs/admin/scale.md index baf702617..bc173b0a1 100644 --- a/docs/admin/scale.md +++ b/docs/admin/scale.md @@ -44,6 +44,46 @@ Azure Cache for Redis Basic, Standard, and Premium retire on September 30, 2028. deployments provision Azure Managed Redis; an existing Azure Cache for Redis instance keeps working, and moving to Azure Managed Redis is a host name change rather than a code change. +### Admin settings consistency + +Implemented in **0.261.025**. App settings no longer have a worker-local snapshot or +a 15-second version-check delay. With Redis enabled, workers read the shared settings +document on every lookup; without Redis, they read Cosmos directly. Other caches, +including conversation, user UI, and governance caches, retain their own policies. + +Settings changes use Cosmos ETag checks and a shared Redis write marker. An older +writer or starting worker cannot replace the shared settings with its earlier +snapshot. The admin form also carries the revision it displayed: if another save +changed that revision, reload and review the new values before saving again. + +When configured Redis is unavailable, **settings saves are rejected** rather than +silently writing only to Cosmos. Reads can fall back to Cosmos; if both services +are unavailable, the app does not serve an old worker snapshot. Cosmos fallback +reads still use the account/client's Session consistency, so this is not a promise +of global strong consistency during an outage. + +A failure after the database write can leave the outcome unconfirmed. The UI asks +you to reload and verify instead of reporting success or promising a rollback. +The shared pending marker prevents readers from using the previous Redis value. +After its 30-second write lease expires, a read can repair publication using a +conditional Cosmos write. No fixed redirect delay is needed. + +Deploy this change to all web workers and the scheduler together. Older versions +do not participate in the new publication protocol. Redis connection or enablement +changes require a coordinated restart of all workers; do not leave workers using +different cache backends. Do not share a Redis database between independent +SimpleChat deployments: the cache keys are application-wide. + +Validation: `functional_tests/test_app_settings_store_consistency.py` covers +multi-worker reads, conditional writes, failure recovery, and expired writers. +`ui_tests/test_admin_settings_save_consistency.py` covers form revisions and includes +an optional authenticated stale-form check. + +As of **0.261.026**, Redis Explorer identifies `APP_SETTINGS_STATE_V2` as the current +shared settings record. Old `APP_SETTINGS_CACHE` and `APP_SETTINGS_CACHE_VERSION` +keys are labeled legacy; their presence does not mean workers still read them. +Previews redact credentials and the Cosmos session token in ready or pending records. + ### Redis Metrics {#redis-monitoring-section} The Redis Metrics section reports the service and port SimpleChat resolved, along with live diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index ee28c350f..f467aa442 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,28 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.261.026)** + +#### Bug Fixes + +* **Redis Explorer Shared Settings Compatibility** + * Fixed key browsing and previews failing after removal of the worker-local settings cache. + * Explorer now recognizes the current shared settings record and labels leftover settings payload/version keys as legacy, without restoring worker caching. + * Preserves credential and Cosmos session-token redaction in previews. + * Added offline coverage for Azure Managed Redis and Azure Cache for Redis, including service-specific ports, key/managed-identity authentication, and app-cache/session clients. + * (Ref: [#1477](https://github.com/microsoft/simplechat/issues/1477), `functions_redis_monitoring.py`, `test_cosmos_wave5a3_redis_monitoring.py`) + +### **(v0.261.025)** + +#### Bug Fixes + +* **Admin Settings Consistency Across Workers** + * Removed worker-local admin settings snapshots so reloads read shared Redis settings, or Cosmos directly when Redis is disabled. + * Added conflict-checked writes and coordinated cache publication to prevent stale metadata updates, worker startup, and interrupted saves from restoring older settings. + * Stale admin forms now require a reload. Saves are rejected when configured Redis is unavailable; unconfirmed saves prompt verification rather than reporting success. + * Reads retain Cosmos fallback without serving an old worker snapshot. Deploy all web workers and the scheduler together; Cosmos fallback remains subject to Session consistency. + * (Ref: [#1477](https://github.com/microsoft/simplechat/issues/1477), `app_settings_store.py`, `app_settings_cache.py`, `functions_settings.py`, admin settings form, auxiliary settings writers, `docs/admin/scale.md`) + ### **(v0.261.023)** #### New Features diff --git a/functional_tests/test_app_settings_auxiliary_writers.py b/functional_tests/test_app_settings_auxiliary_writers.py new file mode 100644 index 000000000..97ef258a9 --- /dev/null +++ b/functional_tests/test_app_settings_auxiliary_writers.py @@ -0,0 +1,625 @@ +# test_app_settings_auxiliary_writers.py +""" +Functional tests for auxiliary app-settings writers. +Version: 0.261.025 +Implemented in: 0.261.025 + +Execute isolated production functions through AST extraction, without importing +application configuration or contacting Redis, Cosmos DB, or other cloud services. +Verify field-only updates, optimistic concurrency, and rejected-write responses. +""" + +import ast +from copy import deepcopy +from datetime import datetime, timedelta, timezone +import logging +from pathlib import Path +import sys +from types import SimpleNamespace +import uuid + +import pytest + + +APP_DIR = Path(__file__).resolve().parents[1] / "application" / "single_app" +WRITER_FILES = ( + "background_tasks.py", + "functions_control_center.py", + "functions_retention_policy.py", + "functions_service_health.py", + "plugin_validation_endpoint.py", + "route_backend_agents.py", + "route_backend_control_center.py", + "route_backend_retention_policy.py", + "route_backend_settings.py", + "route_custom_pages.py", +) + + +class SettingsStore: + """Keep a stale reader snapshot separate from the latest persisted document.""" + + def __init__(self, settings=None, *, available=True, conflict=False): + self.snapshot = { + "_etag": '"original"', + "app_title": "stale title", + "unrelated_secret": "must not be written", + **(settings or {}), + } + self.live = deepcopy(self.snapshot) + self.live["app_title"] = "latest title" + if conflict: + self.live["_etag"] = '"concurrent-write"' + self.available = available + self.calls = [] + + def read(self): + return self.snapshot + + def update(self, updates, *, expected_etag=None): + self.calls.append((deepcopy(updates), expected_etag)) + if not self.available: + return False + if expected_etag is not None and expected_etag != self.live["_etag"]: + return False + self.live.update(deepcopy(updates)) + return True + + +class IsolateFunction(ast.NodeTransformer): + """Remove route decorators and replace inline imports with injected doubles.""" + + def visit_FunctionDef(self, node): + node.decorator_list = [] + return self.generic_visit(node) + + def visit_Import(self, node): + return None + + def visit_ImportFrom(self, node): + return None + + +def load_function(filename, function_name, store, **overrides): + tree = ast.parse((APP_DIR / filename).read_text(encoding="utf-8")) + functions = [ + node for node in ast.walk(tree) + if isinstance(node, ast.FunctionDef) and node.name == function_name + ] + assert len(functions) == 1, function_name + function = IsolateFunction().visit(functions[0]) + module = ast.fix_missing_locations(ast.Module(body=[function], type_ignores=[])) + empty_container = SimpleNamespace(query_items=lambda **kwargs: []) + namespace = { + "datetime": datetime, + "timedelta": timedelta, + "timezone": timezone, + "logging": logging, + "uuid": uuid, + "deepcopy": deepcopy, + "get_settings": store.read, + "update_settings": store.update, + "jsonify": lambda payload: payload, + "debug_print": lambda *args, **kwargs: None, + "log_event": lambda *args, **kwargs: None, + "get_current_user_id": lambda: "admin", + "builtins": SimpleNamespace(kernel_reload_needed=False), + "request": SimpleNamespace( + json={}, method="POST", get_json=lambda **kwargs: {}, + ), + "cosmos_user_settings_container": empty_container, + "cosmos_groups_container": empty_container, + **overrides, + } + exec(compile(module, str(APP_DIR / filename), "exec"), namespace) + return namespace[function_name] + + +def response_parts(response): + return response if isinstance(response, tuple) else (response, 200) + + +def assert_delta(store, expected_keys, *, etag=None): + assert len(store.calls) == 1 + updates, expected_etag = store.calls[0] + assert set(updates) == set(expected_keys) + assert expected_etag == etag + assert store.live["app_title"] == "latest title" + assert "unrelated_secret" not in updates + + +@pytest.mark.parametrize("filename", WRITER_FILES) +def test_no_full_settings_snapshot_is_written(filename): + tree = ast.parse((APP_DIR / filename).read_text(encoding="utf-8")) + calls = [ + node for node in ast.walk(tree) + if isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == "update_settings" + ] + assert calls + for call in calls: + assert not (isinstance(call.args[0], ast.Name) and call.args[0].id == "settings") + + +@pytest.mark.parametrize("logging_type", ["debug", "file_processing", "both"]) +@pytest.mark.parametrize("available,conflict", [(True, False), (False, False), (True, True)]) +def test_logging_expiration_uses_guarded_deltas(logging_type, available, conflict): + expiry = (datetime.now() - timedelta(days=1)).isoformat() + fields = { + "debug": ("enable_debug_logging", "debug_logging_timer_enabled", "debug_logging_turnoff_time"), + "file_processing": ( + "enable_file_processing_logs", "file_processing_logs_timer_enabled", + "file_processing_logs_turnoff_time", + ), + } + selected = fields.values() if logging_type == "both" else [fields[logging_type]] + settings = {} + expected_keys = [] + for enabled_key, timer_key, expiry_key in selected: + settings.update({enabled_key: True, timer_key: True, expiry_key: expiry}) + expected_keys.extend([enabled_key, timer_key, expiry_key]) + store = SettingsStore(settings, available=available, conflict=conflict) + original = deepcopy(store.snapshot) + check = load_function("background_tasks.py", "check_logging_timers_once", store) + + assert check() is (available and not conflict) + assert_delta(store, expected_keys, etag='"original"') + assert store.snapshot == original + if not available or conflict: + assert all(store.live[key] == value for key, value in settings.items()) + + +def test_logging_expiration_does_not_write_before_expiry(): + store = SettingsStore({ + "enable_debug_logging": True, + "debug_logging_timer_enabled": True, + "debug_logging_turnoff_time": (datetime.now() + timedelta(days=1)).isoformat(), + }) + check = load_function("background_tasks.py", "check_logging_timers_once", store) + check() + assert store.calls == [] + + +@pytest.mark.parametrize("available,conflict", [(True, False), (False, False), (True, True)]) +def test_refresh_schedule_seed_rejects_failed_or_stale_writes(available, conflict): + store = SettingsStore(available=available, conflict=conflict) + next_run = datetime(2026, 9, 10, 6, tzinfo=timezone.utc) + seed = load_function( + "background_tasks.py", "_seed_control_center_auto_refresh_next_run", store, + get_control_center_auto_refresh_schedule=lambda settings: { + "time": "02:00", "hour": 2, "minute": 0, "timezone": "America/New_York", + }, + calculate_next_control_center_auto_refresh_run=lambda *args, **kwargs: next_run, + ) + if available and not conflict: + assert seed(store.snapshot, next_run) == next_run + else: + with pytest.raises(RuntimeError, match="Unable to save"): + seed(store.snapshot, next_run) + assert_delta(store, { + "control_center_auto_refresh_enabled", "control_center_auto_refresh_time", + "control_center_auto_refresh_hour", "control_center_auto_refresh_minute", + "control_center_auto_refresh_timezone", "control_center_auto_refresh_next_run", + }, etag='"original"') + + +@pytest.mark.parametrize("available", [True, False]) +@pytest.mark.parametrize("nested", [True, False]) +def test_autoscale_runtime_writes_report_failure_and_guard_policies(available, nested): + store = SettingsStore({"cosmos_throughput_autoscale_enabled": True}, available=available) + delta = {"cosmos_throughput_last_checked_at": "now"} + if nested: + delta["cosmos_throughput_container_policies"] = {"messages": {"last_scale_up_at": "now"}} + released = [] + lock = object() + check = load_function( + "background_tasks.py", "check_cosmos_throughput_autoscale_once", store, + acquire_distributed_task_lock=lambda *args, **kwargs: lock, + release_distributed_task_lock=released.append, + evaluate_and_apply_cosmos_throughput_scaling=lambda *args, **kwargs: { + "settings_update": delta, + }, + ) + result = check() + assert_delta(store, delta, etag='"original"' if nested else None) + assert released == [lock] + if not available: + assert result["success"] is False + assert "Unable to save" in result["error"] + + +@pytest.mark.parametrize("available", [True, False]) +@pytest.mark.parametrize("enabled", [True, False]) +def test_scheduled_control_center_refresh_writes_only_runtime_fields(available, enabled): + store = SettingsStore({ + "control_center_auto_refresh_enabled": enabled, + "control_center_auto_refresh_timezone": "UTC", + }, available=available) + run = load_function( + "functions_control_center.py", "execute_control_center_refresh", store, + calculate_next_control_center_auto_refresh_run=lambda *args, **kwargs: datetime.now(timezone.utc), + ) + result = run() + assert result["success"] is available + assert_delta(store, {"control_center_last_refresh", "control_center_auto_refresh_next_run"}) + assert (store.calls[0][0]["control_center_auto_refresh_next_run"] is not None) is enabled + if not available: + assert result["error"] == "Unable to save Control Center refresh settings." + + +@pytest.mark.parametrize("available", [True, False]) +def test_retention_execution_writes_only_runtime_fields(available): + store = SettingsStore({"enable_retention_policy_personal": True}, available=available) + run = load_function( + "functions_retention_policy.py", "execute_retention_policy", store, + process_personal_retention=lambda: {"conversations": 0, "documents": 0, "users_affected": 0}, + ) + result = run() + assert result["success"] is available + assert_delta(store, {"retention_policy_last_run", "retention_policy_next_run"}) + if not available: + assert result["errors"] == ["Unable to save retention policy execution settings."] + + +@pytest.mark.parametrize("available", [True, False]) +@pytest.mark.parametrize("payload", [ + {"enable_retention_policy_personal": True}, + {"enable_retention_policy_group": False, "enable_retention_policy_public": True}, + {"retention_policy_execution_hour": 8}, +]) +def test_retention_admin_updates_only_submitted_fields(available, payload): + store = SettingsStore(available=available) + route = load_function( + "route_backend_retention_policy.py", "update_retention_policy_settings", store, + request=SimpleNamespace(get_json=lambda: payload), + ) + result, status = response_parts(route()) + expected = set(payload) + if "retention_policy_execution_hour" in payload: + expected.add("retention_policy_next_run") + assert_delta(store, expected) + assert result["success"] is available + assert status == (200 if available else 500) + + +def test_invalid_retention_hour_does_not_write_settings(): + store = SettingsStore() + route = load_function( + "route_backend_retention_policy.py", "update_retention_policy_settings", store, + request=SimpleNamespace(get_json=lambda: {"retention_policy_execution_hour": 24}), + ) + assert response_parts(route())[1] == 400 + assert store.calls == [] + + +@pytest.mark.parametrize("available", [True, False]) +def test_manual_control_center_refresh_reports_timestamp_save_failure(available): + store = SettingsStore(available=available) + route = load_function("route_backend_control_center.py", "api_refresh_control_center_data", store) + result, status = response_parts(route()) + assert_delta(store, {"control_center_last_refresh"}) + assert result["success"] is available + assert status == (200 if available else 500) + + +@pytest.mark.parametrize("available,conflict", [(True, False), (False, False), (True, True)]) +def test_migration_notice_uses_original_etag_and_reports_write_result(available, conflict): + notice = {"enabled": True, "other_field": "preserve"} + store = SettingsStore({"multi_endpoint_migration_notice": notice}, available=available, conflict=conflict) + disable = load_function("route_backend_agents.py", "_maybe_disable_multi_endpoint_migration_notice", store) + preview = {"summary": {"ready_to_migrate": 0, "needs_default_model": 0}} + + assert disable(store.snapshot, preview) is (available and not conflict) + assert_delta(store, {"multi_endpoint_migration_notice"}, etag='"original"') + assert store.calls[0][0]["multi_endpoint_migration_notice"] == { + "enabled": False, "other_field": "preserve", + } + assert store.snapshot["multi_endpoint_migration_notice"]["enabled"] is True + if not available or conflict: + assert store.live["multi_endpoint_migration_notice"]["enabled"] is True + + +@pytest.mark.parametrize("available", [True, False]) +def test_global_agent_selection_is_an_explicit_field_update(available): + store = SettingsStore(available=available) + route = load_function( + "route_backend_agents.py", "set_selected_agent", store, + request=SimpleNamespace(json={"name": "chosen"}), + get_global_agents=lambda: [{"name": "chosen"}], + ) + result, status = response_parts(route()) + assert_delta(store, {"global_selected_agent"}) + assert status == (200 if available else 500) + assert result.get("success", False) is available + assert route.__globals__["builtins"].kernel_reload_needed is available + + +@pytest.mark.parametrize("available,conflict", [(True, False), (False, False), (True, True)]) +@pytest.mark.parametrize("has_fallback", [True, False]) +def test_disabling_selected_agent_checks_etag_before_replacing_selection(available, conflict, has_fallback): + store = SettingsStore({ + "global_selected_agent": {"name": "disabled"}, + }, available=available, conflict=conflict) + route = load_function( + "route_backend_agents.py", "set_agent_enabled", store, + request=SimpleNamespace(get_json=lambda **kwargs: {"is_enabled": False}), + get_global_agents=lambda **kwargs: ( + [{"name": "disabled", "id": "agent-id"}] if kwargs.get("include_disabled") + else ([{"name": "fallback"}] if has_fallback else []) + ), + update_global_agent_enabled=lambda *args, **kwargs: True, + log_agent_update=lambda **kwargs: None, + ) + result, status = response_parts(route("disabled")) + assert_delta(store, {"global_selected_agent"}, etag='"original"') + assert status == (200 if available and not conflict else 500) + assert result.get("success", False) is (available and not conflict) + assert route.__globals__["builtins"].kernel_reload_needed is True + if not available or conflict: + assert store.live["global_selected_agent"] == {"name": "disabled"} + + +@pytest.mark.parametrize("setting_name", ["simple_value", "nested.branch.value"]) +@pytest.mark.parametrize("available,conflict", [(True, False), (False, False), (True, True)]) +def test_agent_setting_updates_keep_unrelated_fields_and_guard_nested_edits(setting_name, available, conflict): + store = SettingsStore({ + "nested": {"branch": {"value": "old", "sibling": "keep"}, "other": "keep"}, + }, available=available, conflict=conflict) + original = deepcopy(store.snapshot) + route = load_function( + "route_backend_agents.py", "update_agent_setting", store, + request=SimpleNamespace(json={"value": "new"}), + ) + result, status = response_parts(route(setting_name)) + nested = "." in setting_name + expected_success = available and not (conflict and nested) + assert_delta(store, {setting_name.split(".")[0]}, etag='"original"' if nested else None) + assert status == (200 if expected_success else 500) + assert result.get("success", False) is expected_success + assert store.snapshot == original + if nested: + assert store.calls[0][0]["nested"] == { + "branch": {"value": "new", "sibling": "keep"}, "other": "keep", + } + + +@pytest.mark.parametrize("available", [True, False]) +@pytest.mark.parametrize("orchestration_type", ["group_chat", "single"]) +def test_orchestration_writes_only_owned_fields(available, orchestration_type): + store = SettingsStore(available=available) + route = load_function( + "route_backend_agents.py", "orchestration_settings", store, + request=SimpleNamespace(method="POST", json={ + "orchestration_type": orchestration_type, "max_rounds_per_agent": 3, + }), + get_agent_orchestration_types=lambda: [ + {"value": "group_chat", "agent_mode": "multi"}, + {"value": "single", "agent_mode": "single"}, + ], + ) + result, status = response_parts(route()) + assert_delta(store, {"orchestration_type", "enable_multi_agent_orchestration", "max_rounds_per_agent"}) + assert status == (200 if available else 500) + assert result.get("success", False) is available + + +@pytest.mark.parametrize("fallback", [True, False]) +@pytest.mark.parametrize("container_result", ["saved", "failed", "exception"]) +@pytest.mark.parametrize("available,conflict", [(True, False), (False, False), (True, True)]) +def test_plugin_repairs_preserve_other_plugins_and_reject_failed_writes( + fallback, container_result, available, conflict, +): + store = SettingsStore({ + "semantic_kernel_plugins": [ + {"name": "repair-me", "type": "Example"}, + {"name": "keep-me", "type": "Other"}, + ], + }, available=available, conflict=conflict) + original = deepcopy(store.snapshot) + plugin = object() + health_checker = SimpleNamespace( + create_plugin_safely=lambda *args: (None, ["error"]) if fallback else (plugin, []), + check_plugin_health=lambda *args: {"is_healthy": False, "errors": ["error"], "timestamp": "now"}, + ) + recovery = SimpleNamespace( + create_fallback_plugin=lambda *args: plugin, + attempt_plugin_repair=lambda *args: (plugin, True), + ) + + def save_global_action(manifest): + if container_result == "exception": + raise RuntimeError("container unavailable") + return manifest if container_result == "saved" else None + + route = load_function( + "plugin_validation_endpoint.py", "repair_plugin", store, + discover_plugins=lambda: {"Example": object}, + PluginHealthChecker=health_checker, + PluginErrorRecovery=recovery, + save_global_action=save_global_action, + ) + result, status = response_parts(route("repair-me")) + assert_delta(store, {"semantic_kernel_plugins"}, etag='"original"') + assert status == (200 if available and not conflict else 500) + assert result["success"] is (available and not conflict) + assert store.snapshot == original + written_plugins = store.calls[0][0]["semantic_kernel_plugins"] + assert written_plugins[-1] == original["semantic_kernel_plugins"][-1] + if container_result == "saved": + assert len(written_plugins) == 1 + else: + assert len(written_plugins) == 2 + assert written_plugins[0]["metadata"]["status"] == ("fallback" if fallback else "repaired") + if not available or conflict: + assert store.live["semantic_kernel_plugins"] == original["semantic_kernel_plugins"] + + +def load_service_health_function(function_name, store): + filename = "functions_service_health.py" + tree = ast.parse((APP_DIR / filename).read_text(encoding="utf-8")) + namespace = { + node.targets[0].id: ast.literal_eval(node.value) + for node in tree.body + if isinstance(node, ast.Assign) + and isinstance(node.targets[0], ast.Name) + and node.targets[0].id.startswith("SEMANTIC_SEARCH_") + } + for helper_name in ("get_default_service_health", "_utc_now_iso", "_sanitize_error_summary"): + namespace[helper_name] = load_function(filename, helper_name, store, **namespace) + return load_function(filename, function_name, store, **namespace) + + +@pytest.mark.parametrize("function_name", [ + "record_semantic_search_quota_exceeded", + "clear_semantic_search_quota_warning", +]) +@pytest.mark.parametrize("available,conflict", [(True, False), (False, False), (True, True)]) +def test_service_health_updates_guard_nested_state(function_name, available, conflict): + store = SettingsStore({ + "service_health": { + "semantic_search": { + "status": "quota_exceeded", "occurrence_count": 5, + "first_seen_at": "first occurrence", + }, + "other_service": {"status": "keep"}, + }, + }, available=available, conflict=conflict) + if conflict: + store.live["service_health"]["other_service"]["status"] = "newer status" + original = deepcopy(store.snapshot) + before_write = deepcopy(store.live) + update = load_service_health_function(function_name, store) + result = update(source="test") + + assert_delta(store, {"service_health"}, etag='"original"') + assert store.snapshot == original + written_health = store.calls[0][0]["service_health"] + assert written_health["other_service"] == {"status": "keep"} + recording = function_name == "record_semantic_search_quota_exceeded" + if recording: + assert written_health["semantic_search"]["occurrence_count"] == 6 + assert written_health["semantic_search"]["first_seen_at"] == "first occurrence" + else: + assert written_health["semantic_search"]["status"] == "ok" + if available and not conflict: + assert result == (written_health["semantic_search"] if recording else True) + else: + assert result is (None if recording else False) + assert store.live == before_write + + +def test_service_health_clear_without_warning_does_not_write(): + store = SettingsStore({ + "service_health": {"semantic_search": {"status": "ok"}}, + }, available=False) + clear = load_service_health_function("clear_semantic_search_quota_warning", store) + assert clear() is False + assert store.calls == [] + + +@pytest.mark.parametrize("function_name", [ + "scale_cosmos_throughput_admin", + "convert_cosmos_throughput_to_autoscale_admin", +]) +@pytest.mark.parametrize("nested", [True, False]) +@pytest.mark.parametrize("available,conflict", [(True, False), (False, False), (True, True)]) +def test_throughput_routes_guard_policies_and_report_failed_runtime_saves( + function_name, nested, available, conflict, +): + policies = {"messages": {"last_scale_up_at": "before", "enabled": True}} + store = SettingsStore({ + "cosmos_throughput_container_policies": policies, + }, available=available, conflict=conflict) + delta = {"cosmos_throughput_last_checked_at": "now"} + if nested: + delta["cosmos_throughput_container_policies"] = { + "messages": {"last_scale_up_at": "now", "enabled": True}, + } + scaling_calls = [] + audit_calls = [] + scope = "container" if nested else "database" + container_name = "messages" if nested else "" + scale_result = { + "scope": scope, "container_name": container_name, + "from_ru": 400, "to_ru": 800, "mode": "manual", + "from_mode": "manual", "to_mode": "autoscale", + } + + def set_database_throughput(settings, target_ru, **kwargs): + scaling_calls.append((target_ru, kwargs)) + return dict(scale_result) + + def build_runtime_update(**kwargs): + assert kwargs["settings"] is store.snapshot + return deepcopy(delta) + + route = load_function( + "route_backend_settings.py", function_name, store, + session={"user": {"email": "admin@example.test"}}, + request=SimpleNamespace(get_json=lambda **kwargs: { + "direction": "up", "container_name": container_name, + }), + get_cosmos_throughput_status=lambda *args, **kwargs: {"throughput": {"current_ru": 400}}, + calculate_manual_scale_target=lambda *args, **kwargs: 800, + calculate_manual_to_autoscale_target=lambda *args, **kwargs: 800, + set_database_throughput=set_database_throughput, + build_runtime_update=build_runtime_update, + log_general_admin_action=lambda **kwargs: audit_calls.append(kwargs), + CosmosThroughputError=ValueError, + ) + result, status = response_parts(route()) + expected_success = available and not (conflict and nested) + assert_delta(store, delta, etag='"original"' if nested else None) + assert len(scaling_calls) == 1 + assert bool(audit_calls) is expected_success + assert status == (200 if expected_success else 500) + assert result.get("success", False) is expected_success + if expected_success: + expected = { + "success": True, "scope": scope, "container_name": container_name, + "from_ru": 400, "to_ru": 800, + } + if function_name == "scale_cosmos_throughput_admin": + expected.update({"direction": "up", "mode": "manual"}) + else: + expected.update({ + "from_mode": "manual", "to_mode": "autoscale", + "reason": "manual_to_autoscale_conversion", + }) + assert result == expected + else: + assert "runtime settings could not be saved" in result["error"] + assert store.live["cosmos_throughput_container_policies"] == policies + + +@pytest.mark.parametrize("available,conflict", [(True, False), (False, False), (True, True)]) +def test_custom_request_access_page_reports_failed_setting_save(available, conflict): + store = SettingsStore(available=available, conflict=conflict) + saved_pages = [] + + def save_custom_page(payload, **kwargs): + saved_pages.append(deepcopy(payload)) + return payload + + route = load_function( + "route_custom_pages.py", "admin_create_request_access_custom_page", store, + validate_custom_page_metadata=lambda *args, **kwargs: [], + save_custom_page=save_custom_page, + _current_admin_user_id=lambda: "admin", + ) + result, status = response_parts(route()) + assert_delta(store, { + "access_request_button_enabled", "access_request_button_text", "access_request_page_url", + }) + assert len(saved_pages) == 1 + assert status == (201 if available else 500) + if available: + assert result == {"page": saved_pages[0], "access_request_button_enabled": True} + else: + assert "access_request_button_enabled" not in result + assert "Page saved" in result["error"] + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-q"])) diff --git a/functional_tests/test_app_settings_cache_versioning.py b/functional_tests/test_app_settings_cache_versioning.py index 302b7ec62..461881319 100644 --- a/functional_tests/test_app_settings_cache_versioning.py +++ b/functional_tests/test_app_settings_cache_versioning.py @@ -2,12 +2,11 @@ #!/usr/bin/env python3 """ Functional test for shared app settings and governance cache versioning. -Version: 0.242.020 +Version: 0.261.025 Implemented in: 0.242.020 -This test ensures Redis deployments keep shared version keys and non-Redis -multi-worker deployments use Cosmos-backed version documents with bounded local -version-read TTLs for app settings and governance policy caches. +Settings versions are now carried with the shared document. Governance caches +retain their separate version documents and bounded local version-read TTLs. """ import os @@ -31,33 +30,21 @@ def test_app_settings_cache_shared_version_contract(): cache_content = _read("application", "single_app", "app_settings_cache.py") settings_content = _read("application", "single_app", "functions_settings.py") + store_content = _read("application", "single_app", "app_settings_store.py") for marker in [ - "APP_SETTINGS_CACHE_VERSION_KEY", - "APP_SETTINGS_CACHE_VERSION_DOC_ID", - "CACHE_VERSION_READ_TTL_SECONDS = 15", - "get_app_settings_cache_version_redis", - "bump_app_settings_cache_version_redis", - "get_app_settings_cache_version_mem", - "bump_app_settings_cache_version_mem", + "get_settings_store", + "get_app_settings_cache_version = _get_settings_revision", "cosmos_settings_container", - "_get_ttl_cached_cosmos_version(", - "APP_SETTINGS_SHARED_VERSION_CACHE", ]: assert marker in cache_content, f"Missing app settings cache version marker: {marker}" - assert "bump_app_settings_cache_version" in settings_content, ( - "Expected app settings writes to bump shared app settings cache version" - ) - assert "_refresh_app_settings_cache_after_write(merged, context=\"merge_upsert\")" in settings_content, ( - "Expected merge upsert path to refresh and version app settings cache" - ) - assert "_refresh_app_settings_cache_after_write(settings_item, context=\"update_settings\")" in settings_content, ( - "Expected update_settings path to refresh and version app settings cache" - ) - assert "before_version_bump" in settings_content and "after_version_bump" in settings_content, ( - "Expected cache refresh helper to write payload before and after version bump" - ) + assert "APP_SETTINGS_SHARED_VERSION_CACHE" not in cache_content + assert "APP_SETTINGS_CACHE = " not in cache_content + assert "store.write(normalize_loaded_settings)" in settings_content + assert "write(apply_updates, expected_etag=expected_etag)" in settings_content + assert 'candidate[SETTINGS_REVISION_FIELD]' in store_content + assert '"document": dict(stored)' in store_content print("PASS: app settings shared cache version contract verified") diff --git a/functional_tests/test_app_settings_store_consistency.py b/functional_tests/test_app_settings_store_consistency.py new file mode 100644 index 000000000..cd40e6eaa --- /dev/null +++ b/functional_tests/test_app_settings_store_consistency.py @@ -0,0 +1,470 @@ +# test_app_settings_store_consistency.py +""" +Regression tests for shared settings and conditional writes. +Version: 0.261.025 +Implemented in: 0.261.025 + +Independent store objects represent workers. Fake services exercise ETag conflicts, +interrupted publication and lease expiry without network access or wall-clock sleeps. +""" + +import ast +import copy +import importlib.util +import json +import logging +from pathlib import Path +import socket +import secrets +import sys +from types import SimpleNamespace +import types + +import pytest +from azure.core import MatchConditions +from azure.cosmos.exceptions import ( + CosmosAccessConditionFailedError, + CosmosResourceExistsError, + CosmosResourceNotFoundError, +) +from redis.exceptions import ConnectionError as RedisConnectionError + + +ROOT = Path(__file__).resolve().parents[1] +APP = ROOT / "application" / "single_app" +SPEC = importlib.util.spec_from_file_location("settings_store_under_test", APP / "app_settings_store.py") +store_module = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(store_module) +AppSettingsStore = store_module.AppSettingsStore +SettingsConflictError = store_module.SettingsConflictError +SettingsUnavailableError = store_module.SettingsUnavailableError + + +class FakeRedis: + def __init__(self): + self.raw = None + self.failed = False + self.fail_publication = False + self.reads = 0 + + def get(self, key): + assert key == store_module.SETTINGS_STATE_KEY + self.reads += 1 + if self.failed: + raise RedisConnectionError("offline") + return self.raw + + def eval(self, script, key_count, key, previous, replacement): + assert script == store_module.COMPARE_AND_SET + assert key_count == 1 and key == store_module.SETTINGS_STATE_KEY + if self.failed or (self.fail_publication and json.loads(replacement)["state"] == "ready"): + raise RedisConnectionError("offline") + previous = previous.decode() if isinstance(previous, bytes) else previous + if (self.raw.decode() if self.raw else "") != previous: + return 0 + self.raw = replacement.encode() + return 1 + + +class FakeCosmos: + def __init__(self): + self.etag = 1 + self.document = {"id": "app_settings", "_etag": "1", "enabled": False} + self.before_replace = None + self.after_replace = None + self.before_create = None + self.failed = False + self.writes = 0 + self.tokens = [] + + def _response(self, response_hook=None): + if response_hook: + response_hook({"x-ms-session-token": f"token-{self.etag}"}, self.document) + return copy.deepcopy(self.document) + + def read_item(self, item, partition_key, *, response_hook=None, session_token=None): + assert item == partition_key == "app_settings" + self.tokens.append(session_token) + if self.failed: + raise RuntimeError("Cosmos offline") + if self.document is None: + raise CosmosResourceNotFoundError(status_code=404, message="Missing") + return self._response(response_hook) + + def replace_item(self, item, body, *, etag, match_condition, response_hook=None, session_token=None): + assert match_condition == MatchConditions.IfNotModified + callback, self.before_replace = self.before_replace, None + if callback: + callback() + if self.document["_etag"] != etag: + raise CosmosAccessConditionFailedError(status_code=412, message="Conflict") + self.etag += 1 + self.writes += 1 + self.document = {**copy.deepcopy(body), "_etag": str(self.etag)} + result = self._response(response_hook) + callback, self.after_replace = self.after_replace, None + if callback: + callback() + return result + + def create_item(self, body, *, response_hook=None): + callback, self.before_create = self.before_create, None + if callback: + callback() + if self.document is not None: + raise CosmosResourceExistsError(status_code=409, message="Exists") + self.document = {**copy.deepcopy(body), "_etag": str(self.etag)} + self.writes += 1 + return self._response(response_hook) + + +@pytest.fixture +def world(monkeypatch): + def deny_network(*_args, **_kwargs): + raise AssertionError("Tests must not contact external services") + + monkeypatch.setattr(socket.socket, "connect", deny_network) + now = [1000.0] + monkeypatch.setattr(store_module.time, "time", lambda: now[0]) + cosmos, redis = FakeCosmos(), FakeRedis() + fallback = [] + workers = [ + AppSettingsStore(cosmos, redis, redis_required=True, on_fallback=fallback.append) + for _ in range(3) + ] + return SimpleNamespace(cosmos=cosmos, redis=redis, a=workers[0], b=workers[1], + c=workers[2], now=now, fallback=fallback) + + +def change(**updates): + return lambda document: {**document, **updates} + + +def test_every_worker_reads_shared_state_after_completed_save(world): + assert not world.a.read()["enabled"] + assert not world.b.read()["enabled"] + stored = world.a.write(change(enabled=True)) + for worker in (world.b, world.a, world.c, world.b): + assert worker.read()["enabled"] + assert worker.read()["_etag"] == stored["_etag"] + + +def test_no_redis_mode_has_no_worker_snapshot(world): + a, b = AppSettingsStore(world.cosmos), AppSettingsStore(world.cosmos) + assert not b.read()["enabled"] + a.write(change(enabled=True)) + assert b.read()["enabled"] + + +def test_returned_settings_do_not_mutate_shared_document(world): + settings = world.a.read() + settings["enabled"] = "unsaved" + assert world.b.read()["enabled"] is False + + +def test_partial_writes_merge_authoritative_document_on_conflict(world): + store = AppSettingsStore(world.cosmos) + world.cosmos.before_replace = lambda: store.write(change(enabled=True)) + stored = store.write(change(last_update_check_time="new")) + assert stored["enabled"] + assert stored["last_update_check_time"] == "new" + assert world.cosmos.writes == 2 + + +def test_stale_form_is_rejected_without_poisoning_cache(world): + stale = world.a.read() + world.b.write(change(enabled=True)) + before = world.redis.raw + with pytest.raises(SettingsConflictError): + world.a.write(change(enabled=False), expected_etag=stale["_etag"]) + assert world.redis.raw == before + assert world.b.read()["enabled"] + + +def test_unavailable_redis_rejects_save_before_cosmos_write(world): + world.a.read() + before = copy.deepcopy(world.cosmos.document) + world.redis.failed = True + with pytest.raises(SettingsUnavailableError): + world.a.write(change(enabled=True)) + assert world.cosmos.document == before + assert world.b.read() == before + assert world.fallback + + +def test_client_construction_failure_does_not_enable_cosmos_only_writes(world): + store = AppSettingsStore(world.cosmos, redis_required=True) + with pytest.raises(SettingsUnavailableError): + store.write(change(enabled=True)) + assert store.read()["enabled"] is False + assert world.cosmos.writes == 0 + + +def test_interrupted_publication_never_restores_previous_payload(world): + world.a.read() + world.redis.fail_publication = True + with pytest.raises(SettingsUnavailableError): + world.a.write(change(enabled=True)) + assert json.loads(world.redis.raw)["state"] == "pending" + assert world.b.read()["enabled"] + world.redis.fail_publication = False + world.now[0] += store_module.WRITE_LEASE_SECONDS + 1 + assert world.c.read()["enabled"] + assert json.loads(world.redis.raw)["state"] == "ready" + + +def test_expired_writer_cannot_commit_over_recovery_and_new_save(world): + world.a.read() + + def take_over(): + world.now[0] += store_module.WRITE_LEASE_SECONDS + 1 + world.b.read() + world.b.write(change(enabled=True)) + + world.cosmos.before_replace = take_over + with pytest.raises(SettingsConflictError): + world.a.write(change(enabled=False)) + assert world.c.read()["enabled"] + + +def test_delayed_publication_cannot_replace_newer_ready_state(world): + world.a.read() + + def newer_save(): + world.now[0] += store_module.WRITE_LEASE_SECONDS + 1 + world.b.read() + world.b.write(change(enabled=True)) + + world.cosmos.after_replace = newer_save + with pytest.raises(SettingsUnavailableError): + world.a.write(change(enabled=False)) + assert world.c.read()["enabled"] + + +def test_migration_retries_against_newer_document(world): + store = AppSettingsStore(world.cosmos) + world.cosmos.before_replace = lambda: store.write(change(enabled=True)) + + def migrate(current): + current.setdefault("new_default", "default") + return current + + assert store.write(migrate)["enabled"] + assert world.cosmos.document["new_default"] == "default" + + +def test_session_token_travels_with_shared_payload(world): + world.a.read() + state = json.loads(world.redis.raw) + world.b.read(use_cosmos=True) + assert world.cosmos.tokens[-1] == state["session_token"] + world.b.write(change(enabled=True)) + assert state["session_token"] in world.cosmos.tokens + + +def test_cosmos_failure_cannot_return_old_worker_data(world): + world.a.read() + world.redis.failed = True + world.cosmos.failed = True + with pytest.raises(RuntimeError, match="Cosmos offline"): + world.b.read() + + +def test_creation_and_creation_race_preserve_winner(world): + world.cosmos.document = None + store = AppSettingsStore(world.cosmos) + world.cosmos.before_create = lambda: AppSettingsStore(world.cosmos).write( + lambda current: current, + defaults={"id": "app_settings", "enabled": True}, + ) + result = store.write(lambda current: current, defaults={"id": "app_settings", "enabled": False}) + assert result["enabled"] + + +def load_update_settings(store): + tree = ast.parse((APP / "functions_settings.py").read_text(encoding="utf-8-sig")) + names = {"update_settings", "coerce_multi_model_endpoint_enablement"} + namespace = { + "copy": copy, "logging": logging, + "COSMOS_METADATA_FIELDS": store_module.COSMOS_METADATA_FIELDS, + "SETTINGS_REVISION_FIELD": store_module.SETTINGS_REVISION_FIELD, + "app_settings_cache": SimpleNamespace(get_settings_store=lambda: store), + "log_event": lambda *_args, **_kwargs: None, + "is_tabular_processing_enabled": lambda _settings: False, + } + for name in ( + "normalize_group_workflow_assignment_settings", "normalize_agents_page_promoted_popular_settings", + "normalize_document_access_index_required_settings", "normalize_inbound_mcp_settings", + "normalize_public_workspace_display_settings", "normalize_key_vault_reminder_settings", + "normalize_model_endpoint_identity_header_settings", + ): + namespace[name] = lambda settings: None + nodes = [node for node in tree.body if isinstance(node, ast.FunctionDef) and node.name in names] + exec(compile(ast.Module(body=nodes, type_ignores=[]), "functions_settings.py", "exec"), namespace) + return namespace["update_settings"] + + +def test_real_update_settings_rejects_old_full_snapshot_and_merges_deltas(world): + old = world.a.read() + update_a, update_b = load_update_settings(world.a), load_update_settings(world.b) + assert update_a({"enabled": True}) + assert update_b({"last_update_check_time": "new"}) + assert world.c.read()["enabled"] + old["last_update_check_time"] = "stale" + assert update_b(old) is False + assert world.c.read()["enabled"] + + +def test_startup_does_not_publish_bootstrap_snapshot(): + for filename in ("app.py", "simplechat_scheduler.py"): + source = (APP / filename).read_text(encoding="utf-8-sig") + assert "app_settings_cache.update_settings_cache(settings)" not in source + + +def load_get_settings(store): + """Use the real getter/default-merge flow, isolating unrelated feature normalizers.""" + tree = ast.parse((APP / "functions_settings.py").read_text(encoding="utf-8-sig")) + definitions = {node.name: node for node in tree.body if isinstance(node, ast.FunctionDef)} + getter = definitions["get_settings"] + namespace = { + "copy": copy, "logging": logging, "secrets": secrets, + "app_settings_cache": SimpleNamespace(get_settings_store=lambda: store), + "CosmosResourceNotFoundError": CosmosResourceNotFoundError, + "SettingsConflictError": SettingsConflictError, + "SettingsUnavailableError": SettingsUnavailableError, + "log_event": lambda *_args, **_kwargs: None, + "_apply_tabular_parity_env_kill_switch": lambda settings: settings, + "attach_public_workspace_label_context": lambda settings: settings, + "normalize_document_intelligence_pdf_image_extraction_mode": lambda mode: mode, + "is_tabular_processing_enabled": lambda settings: False, + } + for node in ast.walk(getter.body[0]): + if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load) and node.id not in namespace: + if node.id.startswith("get_default_"): + namespace[node.id] = lambda: {} + elif node.id == "INBOUND_MCP_SETTINGS_DEFAULTS": + namespace[node.id] = {} + else: + namespace[node.id] = 1 if "MAX" in node.id or "MIN" in node.id or "SIZE" in node.id or "PAGES" in node.id else "" + normalizer = next(node for node in getter.body if isinstance(node, ast.FunctionDef) and node.name == "normalize_loaded_settings") + for node in ast.walk(normalizer): + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name): + name = node.func.id + if name.startswith(("normalize_", "apply_custom_")) and name not in namespace: + namespace[name] = lambda settings: False + nodes = [getter, definitions["deep_merge_dicts"]] + exec(compile(ast.Module(body=nodes, type_ignores=[]), "functions_settings.py", "exec"), namespace) + return namespace["get_settings"] + + +def test_real_get_settings_migrations_are_stable_after_publication(world): + getter = load_get_settings(world.a) + first = getter() + assert first is not None + writes = world.cosmos.writes + second = getter() + assert second == first + assert world.cosmos.writes == writes + + +def test_real_get_settings_migration_does_not_revert_an_admin_write(world): + world.a.read() + getter = load_get_settings(world.a) + + def concurrent_writer(): + AppSettingsStore(world.cosmos).write(change(enabled=True)) + + world.cosmos.before_replace = concurrent_writer + result = getter() + assert result is not None and result["enabled"] + assert world.cosmos.document["enabled"] + + +def test_real_get_settings_defers_migration_during_redis_outage(world): + world.redis.failed = True + before = copy.deepcopy(world.cosmos.document) + result = load_get_settings(world.a)() + assert result is not None + assert world.cosmos.document == before + + +def test_real_get_settings_creates_defaults_without_overwriting_winner(world): + world.cosmos.document = None + getter = load_get_settings(AppSettingsStore(world.cosmos)) + assert getter() is not None + assert world.cosmos.document["id"] == "app_settings" + + +def test_missing_shared_document_can_be_initialized_without_an_abandoned_marker(world): + world.cosmos.document = None + assert load_get_settings(world.a)() is not None + assert json.loads(world.redis.raw)["state"] == "ready" + + +def test_logging_guard_handles_recursive_cache_failure(): + tree = ast.parse((APP / "functions_appinsights.py").read_text(encoding="utf-8-sig")) + definition = next(node for node in tree.body if isinstance(node, ast.FunctionDef) and node.name == "_load_logging_settings") + namespace = { + "_logging_settings_load_state": SimpleNamespace(), + "Dict": dict, + "Any": object, + } + calls = [] + + def recursive_cache_read(): + calls.append(1) + assert namespace["_load_logging_settings"]() == {} + raise RedisConnectionError("offline") + + namespace["app_settings_cache"] = SimpleNamespace(get_settings_cache=recursive_cache_read) + exec(compile(ast.Module(body=[definition], type_ignores=[]), "functions_appinsights.py", "exec"), namespace) + assert namespace["_load_logging_settings"]() == {} + assert calls == [1] + assert namespace["_logging_settings_load_state"].active is False + + +def load_cache_module(world, monkeypatch): + monkeypatch.syspath_prepend(str(APP)) + config = types.ModuleType("config") + config.cosmos_settings_container = world.cosmos + insights = types.ModuleType("functions_appinsights") + insights.log_event = lambda *_args, **_kwargs: None + client_module = types.ModuleType("functions_redis_client") + client_module.AUTH_TYPE_MANAGED_IDENTITY = "managed_identity" + client_module.CREDENTIAL_PURPOSE_APP_CACHE = "app_cache" + client_module.create_redis_client = lambda **_kwargs: world.redis + monkeypatch.setitem(sys.modules, "config", config) + monkeypatch.setitem(sys.modules, "functions_appinsights", insights) + monkeypatch.setitem(sys.modules, "functions_redis_client", client_module) + spec = importlib.util.spec_from_file_location("cache_wiring_under_test", APP / "app_settings_cache.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def test_actual_worker_configuration_does_not_publish_startup_snapshot(world, monkeypatch): + world.cosmos.document.update(enable_redis_cache=True, redis_url="unused.invalid") + cache = load_cache_module(world, monkeypatch) + snapshot = cache.get_settings_store().read() + world.b.write(change(enabled=True)) + cache.configure_app_cache(snapshot) + assert cache.get_settings_cache()["enabled"] + cache.update_settings_cache(snapshot) + assert world.c.read()["enabled"] + assert not hasattr(cache, "APP_SETTINGS_CACHE") + + +def test_actual_cache_initialization_failure_keeps_write_requirement(world, monkeypatch): + world.cosmos.document.update(enable_redis_cache=True, redis_url="unused.invalid") + cache = load_cache_module(world, monkeypatch) + + def unavailable_client(**_kwargs): + raise ValueError("Client cannot be created") + + cache.create_redis_client = unavailable_client + cache.configure_app_cache(world.cosmos.document) + assert cache.APP_SETTINGS_STORE.redis_required is True + assert cache.get_settings_cache()["enabled"] is False + with pytest.raises(RuntimeError, match="Configured Redis is unavailable"): + cache.APP_SETTINGS_STORE.write(change(enabled=True)) + assert world.cosmos.writes == 0 diff --git a/functional_tests/test_content_understanding_extraction_engine.py b/functional_tests/test_content_understanding_extraction_engine.py index 3b6b5c706..d83a70e48 100644 --- a/functional_tests/test_content_understanding_extraction_engine.py +++ b/functional_tests/test_content_understanding_extraction_engine.py @@ -2,7 +2,7 @@ # test_content_understanding_extraction_engine.py """ Functional test for Enhanced extraction backed by Azure AI Content Understanding. -Version: 0.250.224 +Version: 0.261.025 Implemented in: 0.250.221 This test ensures that the Content Understanding client parses analyzer results into the same @@ -631,11 +631,11 @@ def test_enhanced_extraction_upgrade_migration_contract(): assert_contains(settings, "legacy_enhanced_extraction = 'enable_enhanced_extraction' not in settings_item", "legacy toggle detection before merge") assert_contains(settings, "if legacy_enhanced_extraction and legacy_enhanced_extraction_mode in ('layout', 'auto'):", "migration condition") assert_contains(settings, "merged['enable_enhanced_extraction'] = True", "migration backfill") - assert_contains(settings, "or enhanced_extraction_migration_updated", "migration persisted to Cosmos") + assert_contains(settings, "merged = store.write(normalize_loaded_settings)", "migration conditionally persisted to Cosmos") # The migration must run before the merge fills the key in with its False default. detection_index = settings.index("legacy_enhanced_extraction = 'enable_enhanced_extraction' not in settings_item") - merge_index = settings.index("merge_changed = deep_merge_dicts(default_settings, settings_item)") + merge_index = settings.index("deep_merge_dicts(default_settings, settings_item)") if detection_index > merge_index: raise AssertionError("Legacy toggle detection must happen before deep_merge_dicts fills the default.") diff --git a/functional_tests/test_cosmos_wave1_cache_fallback.py b/functional_tests/test_cosmos_wave1_cache_fallback.py index 7a0150f39..c61eff2c9 100644 --- a/functional_tests/test_cosmos_wave1_cache_fallback.py +++ b/functional_tests/test_cosmos_wave1_cache_fallback.py @@ -2,7 +2,7 @@ #!/usr/bin/env python3 """ Functional test for Cosmos Wave 1 cache fallback behavior. -Version: 0.250.005 +Version: 0.261.025 Implemented in: 0.250.005 This test ensures Redis failures in the app cache layer fall back to @@ -14,6 +14,7 @@ import os import sys import types +from redis.exceptions import ConnectionError as RedisConnectionError ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -39,7 +40,7 @@ def _copy_with_new_etag(self, body): item["_etag"] = f"etag-{self._etag_counter}" return item - def read_item(self, item, partition_key): + def read_item(self, item, partition_key, **kwargs): if item not in self.items: raise FakeCosmosError(404, f"Missing item {item}") return copy.deepcopy(self.items[item]) @@ -74,10 +75,10 @@ def __init__(self, *args, **kwargs): pass def get(self, *args, **kwargs): - raise RuntimeError("redis unavailable") + raise RedisConnectionError("redis unavailable") def set(self, *args, **kwargs): - raise RuntimeError("redis unavailable") + raise RedisConnectionError("redis unavailable") def setex(self, *args, **kwargs): raise RuntimeError("redis unavailable") @@ -131,6 +132,7 @@ def test_redis_runtime_failure_falls_back_to_cosmos_settings(): container.items["app_settings"] = { "id": "app_settings", "feature_flag": "from-cosmos", + "_settings_revision": 7, } container.items["app_settings_cache_version"] = { "id": "app_settings_cache_version", diff --git a/functional_tests/test_cosmos_wave5a3_redis_monitoring.py b/functional_tests/test_cosmos_wave5a3_redis_monitoring.py index e4c3866e3..68b4eaf03 100644 --- a/functional_tests/test_cosmos_wave5a3_redis_monitoring.py +++ b/functional_tests/test_cosmos_wave5a3_redis_monitoring.py @@ -1,11 +1,12 @@ -#!/usr/bin/env python3 # test_cosmos_wave5a3_redis_monitoring.py +#!/usr/bin/env python3 """ Functional test for Wave 5A3 Redis monitoring. -Version: 0.250.043 +Version: 0.261.026 Implemented in: 0.250.026 Redis Explorer implemented in: 0.250.040 Redis Explorer DAI resolution implemented in: 0.250.043 +Shared settings state compatibility fixed in: 0.261.026 This test ensures Redis monitoring reports sanitized health, memory, stats, keyspace, DAI cache hygiene, runtime signals, and read-only Redis Explorer @@ -16,12 +17,15 @@ import json import os import sys +from unittest.mock import patch REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) APP_DIR = os.path.join(REPO_ROOT, "application", "single_app") sys.path.insert(0, APP_DIR) import app_settings_cache +import functions_redis_client +from app_settings_store import SETTINGS_STATE_KEY from functions_redis_monitoring import ( get_redis_explorer_keys, get_redis_explorer_value, @@ -385,6 +389,169 @@ def test_redis_explorer_restricts_session_key_preview(): assert "session-cookie-secret" not in serialized_preview +def test_redis_explorer_resolves_shared_and_legacy_settings_keys(): + """Use the real cache module, which no longer exports legacy settings constants.""" + client = FakeRedisClient() + settings = {"enable_redis_cache": True, "redis_url": "example.redis.cache.windows.net"} + expected_kinds = { + SETTINGS_STATE_KEY: "app_settings_state", + "APP_SETTINGS_CACHE": "app_settings_cache", + "APP_SETTINGS_CACHE_VERSION": "app_settings_cache_version", + } + for key in expected_kinds: + client.items[key] = {"type": "string", "value": "{}", "ttl": -1} + + page = get_redis_explorer_keys( + settings, + app_cache_client=client, + key_filter="APP_SETTINGS", + dai_hash_resolver=_fake_dai_hash_resolver, + ) + assert page["success"] is True + assert {item["key"]: item["resolution"]["kind"] for item in page["keys"]} == expected_kinds + for key, kind in expected_kinds.items(): + preview = get_redis_explorer_value(settings, key=key, app_cache_client=client) + assert preview["success"] is True + assert preview["resolution"]["kind"] == kind + if key != SETTINGS_STATE_KEY: + assert "Legacy" in preview["resolution"]["label"] + + +def test_redis_explorer_shared_state_previews_redact_credentials(): + """Both ready and pending records must hide the Cosmos session token.""" + client = FakeRedisClient() + settings = {"enable_redis_cache": True, "redis_url": "example.redis.cache.windows.net"} + for state in ("ready", "pending"): + payload = { + "state": state, + "session_token": "private-cosmos-session-value", + } + if state == "ready": + payload["document"] = { + "app_title": "Visible application title", + "_settings_revision": 12, + "redis_key": "private-redis-key", + "azure_openai_gpt_key": "private-model-key", + "nested": {"client_secret": "private-client-secret"}, + } + else: + payload["owner"] = "write-owner" + payload["deadline"] = 1000 + client.items[SETTINGS_STATE_KEY] = { + "type": "string", + "value": json.dumps(payload), + "ttl": -1, + } + preview = get_redis_explorer_value(settings, key=SETTINGS_STATE_KEY, app_cache_client=client) + assert preview["success"] is True + assert preview["redacted"] is True + decoded = json.loads(preview["preview"]) + assert decoded["state"] == state + assert decoded["session_token"] == "[REDACTED]" + if state == "ready": + assert decoded["document"]["app_title"] == "Visible application title" + for secret in ( + "private-cosmos-session-value", + "private-redis-key", + "private-model-key", + "private-client-secret", + ): + assert secret not in json.dumps(preview) + + +def test_redis_explorer_client_factory_works_for_both_azure_services(): + """Exercise real service/port routing and Explorer with fake Redis I/O only.""" + services = ( + ("example.redis.cache.windows.net", "azure_cache_for_redis", 6380), + ("example.eastus.redis.azure.net", "azure_managed_redis", 10000), + ) + credential_provider = object() + + class ExplorerRedisClient(FakeRedisClient): + def __init__(self, **kwargs): + super().__init__() + self.connection_options = kwargs + self.items[SETTINGS_STATE_KEY] = { + "type": "string", + "value": json.dumps({ + "state": "ready", + "document": {"app_title": "Shared settings", "redis_key": "private-settings-key"}, + "session_token": "private-cosmos-token", + }), + "ttl": -1, + } + + def info(self): + info = super().info() + if self.connection_options["port"] == 10000: + # Enterprise INFO can omit counters; Explorer must not depend on them. + for key in ("maxclients", "tracking_clients", "total_error_replies", "db0"): + info.pop(key, None) + return info + + for hostname, service_type, port in services: + for auth_type in ("key", "managed_identity"): + settings = { + "enable_redis_cache": True, + "redis_url": hostname, + "redis_auth_type": auth_type, + "redis_key": "private-connection-key", + } + with ( + patch.object(functions_redis_client, "Redis", ExplorerRedisClient), + patch.object(functions_redis_client, "get_redis_credential_provider", return_value=credential_provider), + ): + client = functions_redis_client.create_redis_client(settings=settings) + assert client.connection_options["host"] == hostname + assert client.connection_options["port"] == port + assert client.connection_options["ssl"] is True + assert client.connection_options["db"] == 0 + if auth_type == "managed_identity": + assert client.connection_options["credential_provider"] is credential_provider + assert "password" not in client.connection_options + else: + assert client.connection_options["password"] == "private-connection-key" + + for source in ("app_cache", "session"): + client_args = ( + {"app_cache_client": client} + if source == "app_cache" + else {"session_redis_client": client, "session_type": "redis"} + ) + with patch.object(app_settings_cache, "get_app_cache_redis_client", return_value=None): + status = get_redis_monitoring_status(settings, **client_args) + assert status["health"]["status"] == "healthy" + assert status["configuration"]["service_type"] == service_type + assert status["configuration"]["port"] == port + assert status["runtime"]["monitoring_source"] == source + seen_keys = [] + cursor = 0 + while True: + page = get_redis_explorer_keys( + settings, + cursor=cursor, + page_size=2, + dai_hash_resolver=_fake_dai_hash_resolver, + **client_args, + ) + assert page["success"] is True + seen_keys.extend(item["key"] for item in page["keys"]) + if not page["has_more"]: + break + cursor = page["next_cursor"] + assert len(seen_keys) <= len(client.items) + assert set(seen_keys) == set(client.items) + preview = get_redis_explorer_value(settings, key=SETTINGS_STATE_KEY, **client_args) + assert preview["success"] is True + assert preview["resolution"]["kind"] == "app_settings_state" + serialized = json.dumps(preview) + assert "Shared settings" in serialized + assert "private-settings-key" not in serialized + assert "private-cosmos-token" not in serialized + assert "private-connection-key" not in serialized + print(f"PASS: {service_type}, TLS {port}, {auth_type}, {source}") + + if __name__ == "__main__": tests = [ test_redis_monitoring_healthy_metrics, @@ -394,6 +561,9 @@ def test_redis_explorer_restricts_session_key_preview(): test_redis_explorer_resolves_dai_version_marker_metadata, test_redis_explorer_value_sanitizes_json_preview, test_redis_explorer_restricts_session_key_preview, + test_redis_explorer_resolves_shared_and_legacy_settings_keys, + test_redis_explorer_shared_state_previews_redact_credentials, + test_redis_explorer_client_factory_works_for_both_azure_services, ] results = [] for test in tests: diff --git a/functional_tests/test_get_settings_merge_bool_regression.py b/functional_tests/test_get_settings_merge_bool_regression.py index 3beae5232..44bd294b1 100644 --- a/functional_tests/test_get_settings_merge_bool_regression.py +++ b/functional_tests/test_get_settings_merge_bool_regression.py @@ -1,7 +1,8 @@ +# test_get_settings_merge_bool_regression.py #!/usr/bin/env python3 """ Functional test for get_settings deep-merge bool regression. -Version: 0.240.006 +Version: 0.261.025 Implemented in: 0.240.006 This test ensures get_settings treats deep_merge_dicts() return as a change flag @@ -24,14 +25,14 @@ def test_get_settings_uses_merge_changed_flag(): with open(target_path, 'r', encoding='utf-8') as file_handle: content = file_handle.read() - assert "merge_changed = deep_merge_dicts(default_settings, settings_item)" in content, ( - "Expected merge_changed assignment not found" + assert "deep_merge_dicts(default_settings, settings_item)" in content, ( + "Expected in-place default merge not found" ) assert "merged = settings_item" in content, ( "Expected merged dict assignment not found" ) - assert "if merge_changed or migration_updated:" in content, ( - "Expected merge change flag check not found" + assert "if merged != settings_item:" in content, ( + "Expected change detection against the unmodified snapshot" ) old_pattern = re.compile(r"merged\s*=\s*deep_merge_dicts\(default_settings,\s*settings_item\)") diff --git a/functional_tests/test_settings_deep_merge_persistence_fix.py b/functional_tests/test_settings_deep_merge_persistence_fix.py index d4f45f0cc..440d34d8e 100644 --- a/functional_tests/test_settings_deep_merge_persistence_fix.py +++ b/functional_tests/test_settings_deep_merge_persistence_fix.py @@ -1,260 +1,77 @@ -#!/usr/bin/env python3 # test_settings_deep_merge_persistence_fix.py """ -Functional test for settings deep-merge persistence fix. -Version: v0.240.002 -Implemented in: v0.240.002 - -This test ensures merge persistence logic is validated using AST structure checks -and controlled runtime behavior validation for deep_merge_dicts. +Settings default merging and safe persistence regression checks. +Version: 0.261.025 +Implemented in: 0.240.002 +Updated for conditional shared-settings writes in: 0.261.025 """ import ast -import os +import copy +from pathlib import Path import sys -import traceback -sys.path.append(os.path.dirname(os.path.abspath(__file__))) +from test_support.versioning import assert_app_version_at_least -def _read_file(*path_parts): - file_path = os.path.join( - os.path.dirname(os.path.abspath(__file__)), - "..", - *path_parts - ) - with open(file_path, "r", encoding="utf-8") as file_handle: - return file_handle.read() +SOURCE_PATH = Path(__file__).resolve().parents[1] / "application" / "single_app" / "functions_settings.py" def _load_functions_settings_ast(): - """Load and parse functions_settings.py into an AST tree.""" - source = _read_file("application", "single_app", "functions_settings.py") + source = SOURCE_PATH.read_text(encoding="utf-8-sig") return source, ast.parse(source) -def _find_top_level_function(module_tree, function_name): - """Find a top-level function definition by name.""" - for node in module_tree.body: - if isinstance(node, ast.FunctionDef) and node.name == function_name: - return node - return None +def _find_top_level_function(tree, name): + return next(node for node in tree.body if isinstance(node, ast.FunctionDef) and node.name == name) -def test_get_settings_merge_detection_ast_wiring(): - """Validate get_settings merge persistence logic through AST structure checks.""" - print("๐Ÿ” Testing get_settings merge persistence AST wiring...") - _, module_tree = _load_functions_settings_ast() - - has_copy_import = any( - isinstance(node, ast.Import) and any(alias.name == "copy" for alias in node.names) - for node in module_tree.body +def test_get_settings_merge_detection_ast_wiring(): + """Read normalization must not blindly upsert its earlier snapshot.""" + _, tree = _load_functions_settings_ast() + getter = _find_top_level_function(tree, "get_settings") + calls = [node for node in ast.walk(getter) if isinstance(node, ast.Call)] + assert any( + isinstance(call.func, ast.Attribute) + and call.func.attr == "write" + and call.args + and isinstance(call.args[0], ast.Name) + and call.args[0].id == "normalize_loaded_settings" + for call in calls + ) + assert not any( + isinstance(call.func, ast.Attribute) and call.func.attr == "upsert_item" + for call in calls + ) + assert any( + isinstance(node, ast.Compare) + and isinstance(node.left, ast.Name) + and node.left.id == "merged" + and isinstance(node.ops[0], ast.NotEq) + for node in ast.walk(getter) ) - assert has_copy_import, "Missing 'import copy' in functions_settings.py" - - get_settings_def = _find_top_level_function(module_tree, "get_settings") - assert get_settings_def is not None, "Missing get_settings function in functions_settings.py" - - has_merged_assignment = False - has_settings_changed_assignment = False - merge_comparison_if = None - - for node in ast.walk(get_settings_def): - if isinstance(node, ast.Assign): - for target in node.targets: - if isinstance(target, ast.Name) and target.id == "merged": - if ( - isinstance(node.value, ast.Name) - and node.value.id == "settings_item" - ): - has_merged_assignment = True - - if isinstance(target, ast.Name) and target.id == "settings_changed": - if ( - isinstance(node.value, ast.Call) - and isinstance(node.value.func, ast.Name) - and node.value.func.id == "deep_merge_dicts" - and len(node.value.args) == 2 - and isinstance(node.value.args[0], ast.Name) - and node.value.args[0].id == "default_settings" - and isinstance(node.value.args[1], ast.Name) - and node.value.args[1].id == "merged" - ): - has_settings_changed_assignment = True - - if isinstance(node, ast.If) and isinstance(node.test, ast.Name): - if node.test.id == "settings_changed": - merge_comparison_if = node - - assert has_merged_assignment, "Missing merged = settings_item assignment" - assert has_settings_changed_assignment, "Missing settings_changed = deep_merge_dicts(default_settings, merged) assignment" - assert merge_comparison_if is not None, "Missing if settings_changed branch" - - has_upsert_in_branch = False - has_log_event_in_branch = False - - for node in ast.walk(merge_comparison_if): - if ( - isinstance(node, ast.Call) - and isinstance(node.func, ast.Attribute) - and isinstance(node.func.value, ast.Name) - and node.func.value.id == "cosmos_settings_container" - and node.func.attr == "upsert_item" - and len(node.args) == 1 - and isinstance(node.args[0], ast.Name) - and node.args[0].id == "merged" - ): - has_upsert_in_branch = True - - if ( - isinstance(node, ast.Call) - and isinstance(node.func, ast.Name) - and node.func.id == "log_event" - and node.args - and isinstance(node.args[0], ast.Constant) - and isinstance(node.args[0].value, str) - and "missing keys were merged and persisted to Cosmos DB" in node.args[0].value - ): - has_log_event_in_branch = True - - assert has_upsert_in_branch, "Missing cosmos_settings_container.upsert_item(merged) in merge-detected branch" - assert has_log_event_in_branch, "Missing merge persistence log_event message in merge-detected branch" - - print("โœ… Get_settings merge persistence AST wiring is present") def test_deep_merge_dicts_ast_behavior_wiring(): - """Validate deep_merge_dicts merge behavior through AST structure checks.""" - print("๐Ÿ” Testing deep_merge_dicts AST behavior wiring...") - - _, module_tree = _load_functions_settings_ast() - deep_merge_def = _find_top_level_function(module_tree, "deep_merge_dicts") - assert deep_merge_def is not None, "Missing deep_merge_dicts function in functions_settings.py" - - has_not_in_guard = False - has_missing_key_assignment = False - has_recursive_merge_call = False - has_changed_init = False - has_changed_true_assignment = False - returns_changed = False + """Defaults are added recursively without overwriting explicit saved values.""" + _, tree = _load_functions_settings_ast() + definition = _find_top_level_function(tree, "deep_merge_dicts") + namespace = {"copy": copy} + exec(compile(ast.Module(body=[definition], type_ignores=[]), str(SOURCE_PATH), "exec"), namespace) + merge = namespace["deep_merge_dicts"] + settings = {"enabled": False, "nested": {"user_choice": "saved"}} + defaults = {"enabled": True, "nested": {"user_choice": "default", "new_key": 1}} + assert merge(defaults, settings) is True + assert settings == {"enabled": False, "nested": {"user_choice": "saved", "new_key": 1}} + assert merge(defaults, settings) is False - for node in ast.walk(deep_merge_def): - if isinstance(node, ast.Compare): - if ( - isinstance(node.left, ast.Name) - and node.left.id == "k" - and len(node.ops) == 1 - and isinstance(node.ops[0], ast.NotIn) - and len(node.comparators) == 1 - and isinstance(node.comparators[0], ast.Name) - and node.comparators[0].id == "existing_dict" - ): - has_not_in_guard = True - - if isinstance(node, ast.Assign) and len(node.targets) == 1: - target = node.targets[0] - - if ( - isinstance(target, ast.Name) - and target.id == "changed" - and isinstance(node.value, ast.Constant) - and node.value.value is False - ): - has_changed_init = True - - if ( - isinstance(target, ast.Name) - and target.id == "changed" - and isinstance(node.value, ast.Constant) - and node.value.value is True - ): - has_changed_true_assignment = True - - if ( - isinstance(target, ast.Subscript) - and isinstance(target.value, ast.Name) - and target.value.id == "existing_dict" - and isinstance(node.value, ast.Name) - and node.value.id == "default_val" - ): - has_missing_key_assignment = True - - if ( - isinstance(node, ast.Call) - and isinstance(node.func, ast.Name) - and node.func.id == "deep_merge_dicts" - and len(node.args) == 2 - and isinstance(node.args[0], ast.Name) - and node.args[0].id == "default_val" - and isinstance(node.args[1], ast.Name) - and node.args[1].id == "existing_val" - ): - has_recursive_merge_call = True - - if isinstance(node, ast.Return): - if isinstance(node.value, ast.Name) and node.value.id == "changed": - returns_changed = True - - assert has_not_in_guard, "Missing 'if k not in existing_dict' guard in deep_merge_dicts" - assert has_missing_key_assignment, "Missing existing_dict[k] = default_val assignment in deep_merge_dicts" - assert has_recursive_merge_call, "Missing recursive deep_merge_dicts(default_val, existing_val) call" - assert has_changed_init, "Missing changed = False initialization in deep_merge_dicts" - assert has_changed_true_assignment, "Missing changed = True assignment in deep_merge_dicts" - assert returns_changed, "Missing return changed in deep_merge_dicts" - - print("โœ… deep_merge_dicts AST behavior wiring is present") def test_version_alignment_for_fix_release(): - """Validate config version reflects this fix release.""" - print("๐Ÿ” Testing fix release version alignment...") - - config_content = _read_file("application", "single_app", "config.py") - - required_markers = [ - "VERSION = \"0.240.002\"" - ] - - missing_markers = [marker for marker in required_markers if marker not in config_content] - assert not missing_markers, f"Missing config markers: {missing_markers}" - - print("โœ… Fix release version markers are aligned") - - -def main(): - """Run all functional checks for deep-merge persistence fix.""" - print("๐Ÿงช Running Settings Deep Merge Persistence Functional Tests...\n") - - tests = [ - test_get_settings_merge_detection_ast_wiring, - test_deep_merge_dicts_ast_behavior_wiring, - test_version_alignment_for_fix_release - ] - - results = [] - for test in tests: - print(f"\n๐Ÿงช Running {test.__name__}...") - try: - test() - results.append(True) - except AssertionError as error: - print(f"โŒ {test.__name__} failed: {error}") - results.append(False) - except Exception as error: - print(f"โŒ {test.__name__} error: {error}") - traceback.print_exc() - results.append(False) - - success = all(results) - print(f"\n๐Ÿ“Š Results: {sum(results)}/{len(results)} tests passed") - - if success: - print("โœ… All deep-merge persistence functional tests passed!") - else: - print("โŒ Some deep-merge persistence functional tests failed.") - - return success + assert_app_version_at_least("0.261.025") if __name__ == "__main__": - test_success = main() - sys.exit(0 if test_success else 1) + test_get_settings_merge_detection_ast_wiring() + test_deep_merge_dicts_ast_behavior_wiring() + test_version_alignment_for_fix_release() + sys.exit(0) diff --git a/functional_tests/test_tabular_parity_stale_settings_migration.py b/functional_tests/test_tabular_parity_stale_settings_migration.py index 43718c940..07a95afc0 100644 --- a/functional_tests/test_tabular_parity_stale_settings_migration.py +++ b/functional_tests/test_tabular_parity_stale_settings_migration.py @@ -2,7 +2,7 @@ #!/usr/bin/env python3 """ Functional test for the tabular durable-preflight parity stale-settings migration. -Version: 0.250.198 +Version: 0.261.025 Implemented in: 0.250.198 deep_merge_dicts() (used by get_settings() to merge code-level defaults into a @@ -144,18 +144,17 @@ def test_migration_is_wired_into_get_settings_merge_flow(): assert_app_version_at_least(IMPLEMENTED_VERSION) source = SETTINGS_FILE.read_text(encoding="utf-8") - assert "tabular_parity_durable_preflight_settings_updated = normalize_tabular_parity_durable_preflight_defaults(merged)" in source, ( + assert "normalize_tabular_parity_durable_preflight_defaults(merged)" in source, ( "get_settings() must call normalize_tabular_parity_durable_preflight_defaults(merged) " "during its merge/migration step" ) get_settings_start = source.index("def get_settings(") - upsert_condition_start = source.index("cosmos_settings_container.upsert_item(merged)", get_settings_start) - condition_block = source[get_settings_start:upsert_condition_start] + write_start = source.index("merged = store.write(normalize_loaded_settings)", get_settings_start) + condition_block = source[get_settings_start:write_start] - assert "or tabular_parity_durable_preflight_settings_updated" in condition_block, ( - "tabular_parity_durable_preflight_settings_updated must be included in the " - "upsert-trigger condition so corrected values are persisted back to Cosmos DB" + assert "if merged != settings_item:" in condition_block, ( + "Corrected values must trigger conflict-safe persistence." ) diff --git a/ui_tests/test_admin_settings_save_consistency.py b/ui_tests/test_admin_settings_save_consistency.py new file mode 100644 index 000000000..c131725bd --- /dev/null +++ b/ui_tests/test_admin_settings_save_consistency.py @@ -0,0 +1,61 @@ +# test_admin_settings_save_consistency.py +""" +Admin form concurrency contract and optional authenticated browser regression. +Version: 0.261.025 +Implemented in: 0.261.025 + +The live test submits only an invalid revision, so it cannot change any settings. +""" + +import os +from pathlib import Path +import re + +from jinja2 import Environment +import pytest +from playwright.sync_api import expect + + +ROOT = Path(__file__).resolve().parents[1] + + +def test_admin_form_carries_escaped_revision(): + source = (ROOT / "application" / "single_app" / "templates" / "admin_settings.html").read_text(encoding="utf-8") + field = re.search(r']+name="admin_settings_etag"[^>]+>', source) + assert field is not None + rendered = Environment(autoescape=True).from_string(field.group()).render(settings={"_etag": '"revision"'}) + assert ""revision"" in rendered + + +@pytest.mark.ui +def test_stale_admin_form_is_rejected_before_settings_are_parsed(playwright): + base_url = os.getenv("SIMPLECHAT_UI_BASE_URL", "").rstrip("/") + storage_state = os.getenv("SIMPLECHAT_UI_ADMIN_STORAGE_STATE") or os.getenv("SIMPLECHAT_UI_STORAGE_STATE") + if not base_url or not storage_state or not Path(storage_state).is_file(): + pytest.skip("An authenticated admin UI test environment is required.") + browser = playwright.chromium.launch() + context = browser.new_context(storage_state=storage_state) + page = context.new_page() + try: + response = page.goto(f"{base_url}/admin/settings", wait_until="domcontentloaded") + assert response and response.ok + field = page.locator('input[name="admin_settings_etag"]') + expect(field).to_have_count(1) + assert field.input_value() + page.evaluate("""() => { + const form = document.getElementById('admin-settings-form'); + for (const element of form.elements) element.disabled = true; + const revision = document.createElement('input'); + revision.type = 'hidden'; + revision.name = 'admin_settings_etag'; + revision.value = 'deliberately-stale-test-revision'; + form.appendChild(revision); + form.submit(); + }""") + expect(page.get_by_text( + "Settings changed since this page was loaded. Review the latest settings and try again.", + exact=True, + )).to_be_visible() + finally: + context.close() + browser.close() From 3faf39645bf560582b4f28a1d07f3d3bbe1bb200 Mon Sep 17 00:00:00 2001 From: Bionic711 Date: Wed, 9 Sep 2026 14:56:36 -0500 Subject: [PATCH 2/3] Remove cache bootstrap import cycles and strengthen regression checks Address CodeQL py/cyclic-import and py/side-effect-in-assert in PR #1478. Refs #1477. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...cation_of_functional_tests.instructions.md | 9 + .../instructions/python-lang.instructions.md | 13 +- .../prepare-for-pull-request.prompt.md | 8 + application/single_app/app.py | 5 +- application/single_app/app_settings_cache.py | 140 +++++----- application/single_app/config.py | 2 +- application/single_app/functions_settings.py | 46 +++- .../single_app/simplechat_scheduler.py | 9 +- docs/admin/scale.md | 10 + docs/explanation/release_notes.md | 11 + .../test_app_settings_cache_versioning.py | 6 +- .../test_app_settings_import_boundaries.py | 243 ++++++++++++++++++ .../test_app_settings_store_consistency.py | 120 +++++---- .../test_cosmos_wave1_cache_fallback.py | 56 ++-- .../test_redis_entra_token_auth.py | 19 +- 15 files changed, 525 insertions(+), 172 deletions(-) create mode 100644 functional_tests/test_app_settings_import_boundaries.py diff --git a/.github/instructions/location_of_functional_tests.instructions.md b/.github/instructions/location_of_functional_tests.instructions.md index 77b88727b..9e33efad2 100644 --- a/.github/instructions/location_of_functional_tests.instructions.md +++ b/.github/instructions/location_of_functional_tests.instructions.md @@ -121,6 +121,15 @@ if __name__ == "__main__": ## ๐Ÿ” **Test Discovery & Reuse** +### Import Lifecycle and Assertion Safety + +- Execute setup, mutations, database/cache operations, callbacks, and any getter that can initialize or refresh state **before** an assertion. Assert on the captured result. Python removes `assert` expressions under `-O`; pytest rewriting does not justify side effects in them. +- For example, use `saved = update_settings(changes)` followed by `assert saved`, not `assert update_settings(changes)`. +- Import-cycle regressions need fresh-process tests of real modules, with network calls blocked. Include both import orders, early bootstrap, web/scheduler wiring, and failure paths; a fake `config` module or an AST-only function test can conceal the exact cycle being tested. +- Test normal and optimized Python when verifying that required test operations cannot disappear. An optimized run is not proof of assertion coverage; use explicit checks in its subprocess probe. +- Restore every injected module, callback, environment variable, and monkeypatch after a test. Prefer scoped fixtures/context managers over persistent `sys.modules` replacements. +- Read each CodeQL alert's exact rule and path. Cover the full affected pattern, not just the one flagged line, and rerun the relevant integration tests. + ### **Before Creating New Tests:** 1. **Search existing tests**: `grep -r "test_.*{feature}" functional_tests/` 2. **Check for similar patterns**: Look for tests in the same feature area diff --git a/.github/instructions/python-lang.instructions.md b/.github/instructions/python-lang.instructions.md index 7f5f350c1..66a783f37 100644 --- a/.github/instructions/python-lang.instructions.md +++ b/.github/instructions/python-lang.instructions.md @@ -12,7 +12,18 @@ applyTo: '**/*.py' ## Rule: Imports Must Be Organized and at the Top of the File !IMPORTANT -- IMPORTANT: `from` and `import` statements MUST be grouped at the top of the document after the module docstring, unless otherwise indicated by the code writer or for performance reasons in which case the import should be as close as possible to the usage with a comment explaining why the import is not at the top of the file. CodeQL hammers us on this in the findings. If you find imports that are not at the top of the file, move them to the top and add a comment if there is a reason they cannot be moved. This also helps prevent multuple imports of the same module in different places which can lead to confusion and maintenance issues. +- Group imports after the module docstring by default. Before moving or adding any import, trace the dependency chain and initialization timing. Do not mechanically hoist a local import: that can turn a deferred dependency into a startup failure. Local imports require a concrete lifecycle or performance justification. + +## Rule: Preserve Settings and Bootstrap Dependency Boundaries + +- A local import delays execution; it does **not** remove a cycle in the dependency graph. Never claim a cycle is fixed merely because the import moved inside a function, or hide it with `try/except ImportError`, `getattr`, or a success-shaped fallback. +- `config.py` constructs Azure clients and imports logging. Treat `config`, `functions_settings`, logging, cache modules, and Redis/Key Vault helpers as a startup dependency chain, not interchangeable utility modules. +- Configure cache/client behavior from the **settings object already supplied by the caller**. Do not import `config`, `cosmos_settings_container`, or another settings owner back into a lower-level cache helper to rediscover that configuration. +- Pass storage handles, factories, and logging callbacks explicitly from the owning settings/bootstrap layer. Keep those runtime objects separate from the settings dictionary: never persist them, copy them into Redis settings payloads, or pass them to the browser. +- Keep `app_settings_cache.py` and `app_settings_store.py` below their owners in the dependency graph. Neither may directly or transitively import `config`, `functions_settings`, `functions_appinsights`, or the configuration-dependent Redis factory. The web app and scheduler supply the factory; the settings owner supplies initialized storage dependencies. +- Use `import app_settings_cache` and module-qualified access for dynamically configured accessors. Importing an accessor by value can retain the pre-initialization `None` or an obsolete implementation. +- On bootstrap changes, inspect both normal web startup and the scheduler, Redis-enabled/disabled/error paths, and calls that occur before initialization. An uninitialized accessor must not silently import its owner or initialize cloud resources. +- Validate with real-module cold imports in fresh processes and blocked network access, plus static dependency checks that include function-local imports. Stub external I/O, not the module boundary under test. Compilation and AST-extracted function tests alone do not prove import safety. ## Rule: Indentation, Logging, and Decorators - Use 4 spaces per indentation level. No tabs. diff --git a/.github/prompts/prepare-for-pull-request.prompt.md b/.github/prompts/prepare-for-pull-request.prompt.md index 305b0b2e5..796750887 100644 --- a/.github/prompts/prepare-for-pull-request.prompt.md +++ b/.github/prompts/prepare-for-pull-request.prompt.md @@ -83,6 +83,14 @@ Always run: - A Python syntax compile check for changed Python files, and at minimum the Python files under `application/single_app` that GitHub compiles. - Any new or changed test files directly. +When imports, settings/cache initialization, or logging bootstrap changed: + +- Trace the complete dependency chain, including function-local imports and both web and scheduler startup. A local import is not proof that a cycle was removed. +- Verify lower-level cache helpers use the caller's settings object and explicitly supplied runtime dependencies; do not let them import `config` or the settings owner back into the cache. +- Run `functional_tests/test_app_settings_import_boundaries.py` and the relevant real-module bootstrap tests with network access blocked. Do not rely only on syntax compilation, AST-extracted functions, or stubs for modules at the boundary under test. +- Inspect test assertions for side effects, including getters that populate caches. Execute those operations before assertions and assert only on their results. +- Review CodeQL alert annotations and review threads, not only the workflow job conclusion. A successful analysis job can still publish blocking findings. Do not mark those findings resolved based on compilation alone. + When Python route files changed: - Run `python scripts/check_swagger_routes.py `. diff --git a/application/single_app/app.py b/application/single_app/app.py index 45af06ef4..5c77d1b17 100644 --- a/application/single_app/app.py +++ b/application/single_app/app.py @@ -285,9 +285,10 @@ def initialize_application(force=False): print("Initializing application...") settings = get_settings(use_cosmos=True) redis_hostname = settings.get('redis_url', '').strip().split('.')[0] - app_settings_cache.configure_app_cache( + configure_application_cache( settings, - get_redis_cache_infrastructure_endpoint(redis_hostname) + get_redis_cache_infrastructure_endpoint(redis_hostname), + redis_client_factory=functions_redis_client.create_redis_client, ) sanitized_settings = sanitize_settings_for_logging(settings) debug_print(f"DEBUG:Application settings: {sanitized_settings}") diff --git a/application/single_app/app_settings_cache.py b/application/single_app/app_settings_cache.py index 6c3ad0716..864161d4d 100644 --- a/application/single_app/app_settings_cache.py +++ b/application/single_app/app_settings_cache.py @@ -10,25 +10,31 @@ import copy import threading import time +from dataclasses import dataclass from datetime import datetime, timedelta +from typing import Callable + from azure.core.exceptions import AzureError -from azure.cosmos.exceptions import CosmosResourceNotFoundError +from azure.cosmos import ContainerProxy +from redis import Redis from redis.exceptions import RedisError from app_settings_store import AppSettingsStore, SETTINGS_REVISION_FIELD -# Redis client construction lives in functions_redis_client so session, cache, and admin -# diagnostics code paths share one place that resolves service type, port, and credentials. -from functions_redis_client import ( - AUTH_TYPE_MANAGED_IDENTITY, - CREDENTIAL_PURPOSE_APP_CACHE, - create_redis_client, -) -# Logging/configuration imports are deferred to avoid startup dependency cycles. +@dataclass(frozen=True) +class AppCacheDependencies: + """Runtime dependencies supplied by the settings owner, never stored in settings.""" + + settings_container: ContainerProxy + governance_container: ContainerProxy + create_redis_client: Callable[..., Redis] + log_event: Callable[..., None] + _logger = logging.getLogger(__name__) APP_SETTINGS_STORE = None +APP_CACHE_DEPENDENCIES = None APP_USER_UI_SETTINGS_CACHE = {} APP_STREAM_SESSION_METADATA = {} APP_STREAM_SESSION_EVENTS = {} @@ -67,10 +73,10 @@ def create_redis_managed_identity_client(redis_url, settings=None, **redis_kwarg Retained as a thin wrapper so existing callers keep working; the port, TLS, and credential provider are resolved by functions_redis_client. """ - return create_redis_client( + return _get_cache_dependencies().create_redis_client( settings=settings, redis_url=redis_url, - auth_type=AUTH_TYPE_MANAGED_IDENTITY, + auth_type='managed_identity', **redis_kwargs ) @@ -146,50 +152,53 @@ def _set_ttl_cached_version(version_cache, version): def _log_settings_fallback(error): - # Logging reads settings itself; the logging entrypoint guards re-entrancy. - from functions_appinsights import log_event - - log_event( + _get_cache_dependencies().log_event( "[ASC] Shared settings unavailable; reading Cosmos without a worker snapshot.", extra={'error_type': type(error).__name__}, level=logging.WARNING, ) -def get_settings_store(): - """Initialize connections lazily, without retaining any settings payload.""" - global APP_SETTINGS_STORE, get_settings_cache, update_settings_cache - global get_app_settings_cache_version +def _get_cache_dependencies(): + if APP_CACHE_DEPENDENCIES is None: + raise RuntimeError("App cache dependencies must be supplied by the settings owner before use.") + return APP_CACHE_DEPENDENCIES - if APP_SETTINGS_STORE is None: - # config imports logging/cache during startup; defer until it is initialized. - from config import cosmos_settings_container - try: - initial = cosmos_settings_container.read_item(item='app_settings', partition_key='app_settings') - except CosmosResourceNotFoundError: - initial = {} - required = bool(initial.get('enable_redis_cache', False)) - redis_client = None - if required: - try: - redis_client = create_redis_client( - settings=initial, - credential_purpose=CREDENTIAL_PURPOSE_APP_CACHE, - socket_connect_timeout=5, - socket_timeout=5, - ) - except (RedisError, AzureError, ValueError) as error: - _log_settings_fallback(error) - APP_SETTINGS_STORE = AppSettingsStore( - cosmos_settings_container, - redis_client, - redis_required=required, - on_fallback=_log_settings_fallback, - ) +def configure_settings_store(settings, *, dependencies): + """Build connections from supplied settings and separately injected runtime dependencies.""" + global APP_SETTINGS_STORE, APP_CACHE_DEPENDENCIES, get_settings_cache, update_settings_cache + global get_app_settings_cache_version + + if not isinstance(settings, dict): + raise TypeError("App cache configuration requires a settings object.") + APP_CACHE_DEPENDENCIES = dependencies + required = bool(settings.get('enable_redis_cache', False)) + APP_SETTINGS_STORE = AppSettingsStore( + dependencies.settings_container, + redis_required=required, + on_fallback=_log_settings_fallback, + ) get_settings_cache = APP_SETTINGS_STORE.read update_settings_cache = _refresh_authoritative_settings get_app_settings_cache_version = _get_settings_revision + if required: + try: + APP_SETTINGS_STORE.redis = dependencies.create_redis_client( + settings=settings, + credential_purpose='app_cache', + socket_connect_timeout=5, + socket_timeout=5, + ) + except (RedisError, AzureError, ValueError) as error: + _log_settings_fallback(error) + return APP_SETTINGS_STORE + + +def get_settings_store(): + """Return initialized shared storage without importing or loading configuration.""" + if APP_SETTINGS_STORE is None: + raise RuntimeError("App settings store must be configured by the settings owner before use.") return APP_SETTINGS_STORE @@ -221,8 +230,7 @@ def _build_cosmos_cache_doc_id(cache_key): def _get_cosmos_cache_container(): - from config import cosmos_settings_container - return cosmos_settings_container + return _get_cache_dependencies().settings_container def _serialize_datetime(value): @@ -309,10 +317,9 @@ def _get_settings_cache_fallback(log_event_func=None): def _get_governance_cache_version_fallback(log_event_func=None): global APP_GOVERNANCE_CACHE_VERSION try: - from config import cosmos_governance_policies_container return _get_ttl_cached_cosmos_version( APP_GOVERNANCE_SHARED_VERSION_CACHE, - cosmos_governance_policies_container, + _get_cache_dependencies().governance_container, GOVERNANCE_CACHE_VERSION_DOC_ID, APP_GOVERNANCE_CACHE_VERSION, log_event_func=log_event_func, @@ -332,9 +339,8 @@ def _get_governance_cache_version_fallback(log_event_func=None): def _bump_governance_cache_version_fallback(log_event_func=None): global APP_GOVERNANCE_CACHE_VERSION try: - from config import cosmos_governance_policies_container bumped_version = _bump_cosmos_cache_version( - cosmos_governance_policies_container, + _get_cache_dependencies().governance_container, GOVERNANCE_CACHE_VERSION_DOC_ID, log_event_func=log_event_func, ) @@ -594,8 +600,8 @@ def get_app_cache_redis_client(): return APP_REDIS_CLIENT if app_cache_is_using_redis else None -def configure_app_cache(settings, redis_cache_endpoint=None): - global update_settings_cache, get_settings_cache, APP_SETTINGS_STORE +def configure_app_cache(settings, redis_cache_endpoint=None, *, dependencies): + global update_settings_cache, get_settings_cache global APP_USER_UI_SETTINGS_CACHE, APP_STREAM_SESSION_METADATA, APP_STREAM_SESSION_EVENTS global APP_GOVERNANCE_CACHE_VERSION, APP_GOVERNANCE_SHARED_VERSION_CACHE global initialize_stream_session_cache, set_stream_session_meta, get_stream_session_meta @@ -605,17 +611,10 @@ def configure_app_cache(settings, redis_cache_endpoint=None): global get_governance_cache_version, bump_governance_cache_version global app_cache_is_using_redis global APP_REDIS_CLIENT - # Local import to avoid circular dependency: functions_keyvault imports app_settings_cache. - from functions_appinsights import log_event - from config import cosmos_settings_container + log_event = dependencies.log_event use_redis = settings.get('enable_redis_cache', False) - APP_SETTINGS_STORE = AppSettingsStore( - cosmos_settings_container, - redis_required=use_redis, - on_fallback=_log_settings_fallback, - ) - get_settings_store() + store = configure_settings_store(settings, dependencies=dependencies) app_cache_is_using_redis = False APP_REDIS_CLIENT = None @@ -632,17 +631,12 @@ def configure_app_cache(settings, redis_cache_endpoint=None): else: log_event("[ASC] Redis enabled using Access Key", level=logging.INFO) - # Pass settings directly: get_settings_cache() is still None at this point - # because configure_app_cache has not finished initialising the cache yet. - redis_client = create_redis_client( - settings=settings, - credential_purpose=CREDENTIAL_PURPOSE_APP_CACHE, - socket_connect_timeout=5, - socket_timeout=5, - ) + redis_client = store.redis + if redis_client is None: + _assign_fallback_cache_functions(log_event_func=log_event) + return app_cache_is_using_redis = True APP_REDIS_CLIENT = redis_client - APP_SETTINGS_STORE.redis = redis_client except Exception as redis_init_error: _log_cache_fallback('redis_initialization', redis_init_error, log_event_func=log_event) _assign_fallback_cache_functions(log_event_func=log_event) @@ -911,10 +905,9 @@ def delete_user_ui_settings_cache_mem(user_id): def get_governance_cache_version_mem(): global APP_GOVERNANCE_CACHE_VERSION try: - from config import cosmos_governance_policies_container return _get_ttl_cached_cosmos_version( APP_GOVERNANCE_SHARED_VERSION_CACHE, - cosmos_governance_policies_container, + dependencies.governance_container, GOVERNANCE_CACHE_VERSION_DOC_ID, APP_GOVERNANCE_CACHE_VERSION, log_event_func=log_event, @@ -932,9 +925,8 @@ def get_governance_cache_version_mem(): def bump_governance_cache_version_mem(): global APP_GOVERNANCE_CACHE_VERSION try: - from config import cosmos_governance_policies_container bumped_version = _bump_cosmos_cache_version( - cosmos_governance_policies_container, + dependencies.governance_container, GOVERNANCE_CACHE_VERSION_DOC_ID, log_event_func=log_event, ) diff --git a/application/single_app/config.py b/application/single_app/config.py index fc1da1a85..517716bc7 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -98,7 +98,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.026" +VERSION = "0.261.027" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_settings.py b/application/single_app/functions_settings.py index 6aa403d73..26e420e58 100644 --- a/application/single_app/functions_settings.py +++ b/application/single_app/functions_settings.py @@ -2,10 +2,12 @@ from functools import wraps import logging +import threading from flask import g, has_request_context, jsonify, request, session from app_settings_store import ( + AppSettingsStore, COSMOS_METADATA_FIELDS, SETTINGS_REVISION_FIELD, SettingsConflictError, @@ -56,6 +58,7 @@ USER_SETTINGS_REQUEST_CACHE_ATTR = "simplechat_user_settings_request_cache" +_settings_store_init_lock = threading.Lock() FONT_SIZE_PREFERENCES = ("xs", "s", "m", "l", "xl") DEFAULT_FONT_SIZE_PREFERENCE = "m" CHAT_COMPLETION_AUDIO_SOUND_IDS = ( @@ -1209,6 +1212,45 @@ def _should_sync_session_profile(target_user_id, actor_user_id, allow_cross_user return bool(normalized_target_user_id and normalized_actor_user_id and normalized_target_user_id == normalized_actor_user_id) +def _get_app_cache_dependencies(redis_client_factory): + return app_settings_cache.AppCacheDependencies( + settings_container=cosmos_settings_container, + governance_container=cosmos_governance_policies_container, + create_redis_client=redis_client_factory, + log_event=log_event, + ) + + +def _get_app_settings_store(): + """Read bootstrap settings here; the cache must never import its owner or config.""" + with _settings_store_init_lock: + if app_settings_cache.APP_SETTINGS_STORE is None: + try: + settings = cosmos_settings_container.read_item( + item="app_settings", + partition_key="app_settings", + ) + except CosmosResourceNotFoundError: + settings = {} + # Before web/scheduler configuration, Redis-required writes must fail + # closed. This temporary reader is not installed as a worker cache. + return AppSettingsStore( + cosmos_settings_container, + redis_required=bool(settings.get('enable_redis_cache', False)), + ) + return app_settings_cache.get_settings_store() + + +def configure_application_cache(settings, redis_cache_endpoint=None, *, redis_client_factory): + """Supply runtime dependencies separately from the persisted settings object.""" + with _settings_store_init_lock: + app_settings_cache.configure_app_cache( + settings, + redis_cache_endpoint, + dependencies=_get_app_cache_dependencies(redis_client_factory), + ) + + def _env_flag_enabled(name): return str(os.environ.get(name, '')).strip().lower() in {'1', 'true', 'yes', 'on'} @@ -1962,7 +2004,7 @@ def normalize_loaded_settings(settings_item): return merged try: - store = app_settings_cache.get_settings_store() + store = _get_app_settings_store() settings_source = "cosmos_forced" if use_cosmos else "shared" try: settings_item = store.read(use_cosmos=use_cosmos) @@ -2042,7 +2084,7 @@ def apply_updates(settings_item): return settings_item try: - app_settings_cache.get_settings_store().write(apply_updates, expected_etag=expected_etag) + _get_app_settings_store().write(apply_updates, expected_etag=expected_etag) log_event( "[ASC] App settings updated and published successfully.", level=logging.INFO diff --git a/application/single_app/simplechat_scheduler.py b/application/single_app/simplechat_scheduler.py index 584590014..13ec5d06b 100644 --- a/application/single_app/simplechat_scheduler.py +++ b/application/single_app/simplechat_scheduler.py @@ -6,11 +6,11 @@ import os import sys -import app_settings_cache from background_tasks import run_scheduler_forever +import functions_redis_client from config import get_redis_cache_infrastructure_endpoint, initialize_clients from functions_appinsights import setup_appinsights_logging -from functions_settings import get_settings +from functions_settings import configure_application_cache, get_settings def initialize_scheduler_runtime(): @@ -18,9 +18,10 @@ def initialize_scheduler_runtime(): print('Initializing SimpleChat scheduler runtime...') settings = get_settings(use_cosmos=True) redis_hostname = settings.get('redis_url', '').strip().split('.')[0] - app_settings_cache.configure_app_cache( + configure_application_cache( settings, - get_redis_cache_infrastructure_endpoint(redis_hostname) + get_redis_cache_infrastructure_endpoint(redis_hostname), + redis_client_factory=functions_redis_client.create_redis_client, ) initialize_clients(settings) setup_appinsights_logging(settings) diff --git a/docs/admin/scale.md b/docs/admin/scale.md index bc173b0a1..d82847a67 100644 --- a/docs/admin/scale.md +++ b/docs/admin/scale.md @@ -84,6 +84,16 @@ shared settings record. Old `APP_SETTINGS_CACHE` and `APP_SETTINGS_CACHE_VERSION keys are labeled legacy; their presence does not mean workers still read them. Previews redact credentials and the Cosmos session token in ready or pending records. +In **0.261.027**, cache initialization uses the settings object supplied by the web +or scheduler startup path. The settings owner supplies database handles and logging +callbacks separately; the startup path supplies the Redis client factory. Cache +helpers no longer import `config` or rediscover configuration while initializing. +These runtime dependencies are never added to the stored settings document. +Before cache initialization, settings reads remain available through the owning +settings layer, but Redis-required writes remain blocked until its client is configured. +`functional_tests/test_app_settings_import_boundaries.py` checks cold imports, +dependency direction, and normal/optimized Python startup probes without network access. + ### Redis Metrics {#redis-monitoring-section} The Redis Metrics section reports the service and port SimpleChat resolved, along with live diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index f467aa442..7444ffb52 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,17 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.261.027)** + +#### Bug Fixes + +* **Settings Cache Bootstrap Import Boundaries** + * Removed reverse imports from cache helpers into application configuration, preventing the import cycle identified during review. + * Web and scheduler startup now configure the cache from the supplied settings object, with storage handles, the Redis factory, and logging callbacks passed separately. + * Preserves shared settings reads and fail-closed Redis writes without restoring worker-local settings snapshots. + * Added real-module cold-start probes for normal and optimized Python, moved state-changing test operations outside assertions, and strengthened repository instructions for dependency and startup validation. + * (Ref: [#1477](https://github.com/microsoft/simplechat/issues/1477), [PR #1478](https://github.com/microsoft/simplechat/pull/1478), `app_settings_cache.py`, `functions_settings.py`, `test_app_settings_import_boundaries.py`) + ### **(v0.261.026)** #### Bug Fixes diff --git a/functional_tests/test_app_settings_cache_versioning.py b/functional_tests/test_app_settings_cache_versioning.py index 461881319..90a7014d9 100644 --- a/functional_tests/test_app_settings_cache_versioning.py +++ b/functional_tests/test_app_settings_cache_versioning.py @@ -2,7 +2,7 @@ #!/usr/bin/env python3 """ Functional test for shared app settings and governance cache versioning. -Version: 0.261.025 +Version: 0.261.027 Implemented in: 0.242.020 Settings versions are now carried with the shared document. Governance caches @@ -35,7 +35,7 @@ def test_app_settings_cache_shared_version_contract(): for marker in [ "get_settings_store", "get_app_settings_cache_version = _get_settings_revision", - "cosmos_settings_container", + "dependencies.settings_container", ]: assert marker in cache_content, f"Missing app settings cache version marker: {marker}" @@ -63,7 +63,7 @@ def test_governance_cache_cosmos_fallback_contract(): "bump_governance_cache_version_redis", "get_governance_cache_version_mem", "bump_governance_cache_version_mem", - "cosmos_governance_policies_container", + "dependencies.governance_container", ]: assert marker in cache_content, f"Missing governance cache version marker: {marker}" diff --git a/functional_tests/test_app_settings_import_boundaries.py b/functional_tests/test_app_settings_import_boundaries.py new file mode 100644 index 000000000..003e9e8ea --- /dev/null +++ b/functional_tests/test_app_settings_import_boundaries.py @@ -0,0 +1,243 @@ +# test_app_settings_import_boundaries.py +""" +Regression coverage for CodeQL py/cyclic-import and py/side-effect-in-assert. +Version: 0.261.027 +Implemented in: 0.261.027 + +Import real cache/logging modules in fresh interpreters with configuration imports +and network access blocked. Exercise initialization from supplied settings in both +normal and optimized Python; module stubs cannot mask the dependency boundary. +""" + +import ast +from pathlib import Path +import subprocess +import sys + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +APP = ROOT / "application" / "single_app" +FORBIDDEN_MODULES = {"config", "functions_settings", "functions_appinsights", "functions_redis_client"} +BOOTSTRAP_PROBE = r''' +import ast +import builtins +import copy +import importlib +import json +import logging +from pathlib import Path +import socket +import sys +import threading +from unittest.mock import patch + +sys.path.insert(0, sys.argv[1]) +forbidden = { + "config", "single_app.config", "application.single_app.config", + "functions_settings", "functions_redis_client", +} +real_import = builtins.__import__ + +def guarded_import(name, *args, **kwargs): + if name in forbidden: + raise AssertionError("Reverse bootstrap import: " + name) + return real_import(name, *args, **kwargs) + +def no_network(*args, **kwargs): + raise AssertionError("Network access during import/bootstrap") + +def check(condition, message): + if not condition: + raise AssertionError(message) + +class Container: + def __init__(self): + self.reads = 0 + self.version = 0 + self.redis_enabled = False + + def read_item(self, item, partition_key, **kwargs): + self.reads += 1 + if item == "app_settings": + return { + "id": item, "_etag": "test-etag", "from_cosmos": True, + "enable_redis_cache": self.redis_enabled, + } + return {"id": item, "version": self.version} + + def upsert_item(self, body): + self.version = body["version"] + +class Redis: + def get(self, key): + return json.dumps({"state": "ready", "document": { + "id": "app_settings", "_etag": "test-etag", "from_redis": True, + }}) + +with patch.object(builtins, "__import__", guarded_import), patch.object(socket.socket, "connect", no_network): + importlib.import_module(sys.argv[2]) + cache = importlib.import_module("app_settings_cache") + insights = importlib.import_module("functions_appinsights") + logger = logging.getLogger("import_boundary_probe") + logger.handlers = [logging.NullHandler()] + logger.propagate = False + insights._appinsights_logger = logger + settings_container, governance_container = Container(), Container() + factory_calls = [] + factory_failure = [False] + + def factory(**kwargs): + factory_calls.append(kwargs) + if factory_failure[0]: + raise ValueError("synthetic Redis construction failure") + return Redis() + + dependencies = cache.AppCacheDependencies( + settings_container=settings_container, + governance_container=governance_container, + create_redis_client=factory, + log_event=insights.log_event, + ) + try: + cache.get_settings_store() + except RuntimeError: + pass + else: + raise AssertionError("Unconfigured accessor silently bootstrapped") + + from azure.cosmos.exceptions import CosmosResourceNotFoundError + from app_settings_store import AppSettingsStore + owner_source = (Path(sys.argv[1]) / "functions_settings.py").read_text(encoding="utf-8-sig") + owner_tree = ast.parse(owner_source) + owner_nodes = [ + node for node in owner_tree.body + if isinstance(node, ast.FunctionDef) + and node.name in { + "_get_app_cache_dependencies", "_get_app_settings_store", + "configure_application_cache", + } + ] + owner = { + "app_settings_cache": cache, + "_settings_store_init_lock": threading.Lock(), + "AppSettingsStore": AppSettingsStore, + "CosmosResourceNotFoundError": CosmosResourceNotFoundError, + "cosmos_settings_container": settings_container, + "cosmos_governance_policies_container": governance_container, + "log_event": insights.log_event, + } + exec(compile(ast.Module(body=owner_nodes, type_ignores=[]), "settings_owner_bootstrap", "exec"), owner) + + for enabled in (False, True): + cache.APP_SETTINGS_STORE = None + cache.get_settings_cache = None + settings_container.redis_enabled = enabled + bootstrap = owner["_get_app_settings_store"]() + check(bootstrap.redis_required == enabled, "Bootstrap ignored persisted Redis enablement") + check(cache.APP_SETTINGS_STORE is None, "Temporary bootstrap reader was installed as a cache") + if enabled: + try: + bootstrap.write(lambda document: document) + except RuntimeError: + pass + else: + raise AssertionError("Bootstrap allowed Redis-required writes without a Redis client") + settings = {"enable_redis_cache": enabled, "redis_url": "unused.invalid"} + original = copy.deepcopy(settings) + reads_before = settings_container.reads + owner["configure_application_cache"](settings, redis_client_factory=factory) + check(settings == original, "Configuration mutated the settings payload") + check(settings_container.reads == reads_before, "Configuration reloaded settings") + configured = owner["_get_app_settings_store"]() + check(configured is cache.APP_SETTINGS_STORE, "Owner did not return the configured shared store") + check(settings_container.reads == reads_before, "Initialized owner reloaded bootstrap settings") + loaded = cache.get_settings_cache() + check(loaded.get("from_redis" if enabled else "from_cosmos"), "Wrong settings backend") + if enabled: + check(factory_calls[-1]["settings"] is settings, "Factory ignored the supplied settings") + else: + version = cache.get_governance_cache_version() + bumped = cache.bump_governance_cache_version() + check(bumped == version + 1, "Injected governance container was not used") + + factory_failure[0] = True + cache.configure_app_cache( + {"enable_redis_cache": True, "redis_url": "unused.invalid"}, + dependencies=dependencies, + ) + check(cache.get_settings_store().redis_required, "Redis write requirement was silently disabled") + loaded = cache.get_settings_cache() + check(loaded.get("from_cosmos"), "Failed Redis did not use the injected Cosmos reader") + check(not any(name in sys.modules for name in forbidden), "Bootstrap loaded a forbidden module") +print("PASS: cold imports and settings-driven configuration; no reverse imports or network") +''' + + +@pytest.mark.parametrize("first_module", ["app_settings_cache", "functions_appinsights"]) +@pytest.mark.parametrize("optimized", [False, True]) +def test_real_imports_and_bootstrap_do_not_reenter_config(first_module, optimized): + command = [sys.executable, "-B"] + if optimized: + command.append("-O") + result = subprocess.run( + command + ["-c", BOOTSTRAP_PROBE, str(APP), first_module], + capture_output=True, + text=True, + timeout=60, + check=False, + ) + assert result.returncode == 0, result.stdout + result.stderr + assert "PASS: cold imports" in result.stdout + + +def test_cache_dependency_graph_has_no_reverse_imports(): + """Inspect function-local imports too: deferring an edge does not remove it.""" + pending = ["app_settings_cache"] + inspected = set() + while pending: + name = pending.pop() + if name in inspected: + continue + inspected.add(name) + tree = ast.parse((APP / f"{name}.py").read_text(encoding="utf-8-sig")) + for node in ast.walk(tree): + imports = [] + if isinstance(node, ast.ImportFrom): + imports = [node.module or ""] + elif isinstance(node, ast.Import): + imports = [alias.name for alias in node.names] + for imported in imports: + normalized = imported.removeprefix("application.").removeprefix("single_app.").split(".")[0] + assert normalized not in FORBIDDEN_MODULES, f"{name}:{node.lineno} imports {imported}" + if (APP / f"{normalized}.py").is_file(): + pending.append(normalized) + assert "app_settings_store" in inspected + + +def test_consistency_assertions_only_inspect_results(): + """No settings operations, getter bootstrap, or database writes inside asserts.""" + tree = ast.parse((ROOT / "functional_tests" / "test_app_settings_store_consistency.py").read_text(encoding="utf-8")) + pure_calls = {"len", "hasattr", "isinstance", "json.loads"} + for node in ast.walk(tree): + if isinstance(node, ast.Assert): + for descendant in ast.walk(node.test): + if isinstance(descendant, ast.Call): + function = ast.unparse(descendant.func) + assert function in pure_calls, f"Line {node.lineno}: execute {function} before asserting its result" + + +def test_web_and_scheduler_pass_the_real_factory_through_the_settings_owner(): + for filename in ("app.py", "simplechat_scheduler.py"): + tree = ast.parse((APP / filename).read_text(encoding="utf-8-sig")) + calls = [ + node for node in ast.walk(tree) + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) + and node.func.id == "configure_application_cache" + ] + assert len(calls) == 1 + call = calls[0] + assert isinstance(call.args[0], ast.Name) and call.args[0].id == "settings" + keywords = {kw.arg: ast.unparse(kw.value) for kw in call.keywords} + assert keywords["redis_client_factory"] == "functions_redis_client.create_redis_client" diff --git a/functional_tests/test_app_settings_store_consistency.py b/functional_tests/test_app_settings_store_consistency.py index cd40e6eaa..d652bb881 100644 --- a/functional_tests/test_app_settings_store_consistency.py +++ b/functional_tests/test_app_settings_store_consistency.py @@ -1,7 +1,7 @@ # test_app_settings_store_consistency.py """ Regression tests for shared settings and conditional writes. -Version: 0.261.025 +Version: 0.261.027 Implemented in: 0.261.025 Independent store objects represent workers. Fake services exercise ETag conflicts, @@ -18,7 +18,6 @@ import secrets import sys from types import SimpleNamespace -import types import pytest from azure.core import MatchConditions @@ -141,25 +140,31 @@ def change(**updates): def test_every_worker_reads_shared_state_after_completed_save(world): - assert not world.a.read()["enabled"] - assert not world.b.read()["enabled"] + initial_a = world.a.read() + initial_b = world.b.read() + assert not initial_a["enabled"] + assert not initial_b["enabled"] stored = world.a.write(change(enabled=True)) for worker in (world.b, world.a, world.c, world.b): - assert worker.read()["enabled"] - assert worker.read()["_etag"] == stored["_etag"] + observed = worker.read() + assert observed["enabled"] + assert observed["_etag"] == stored["_etag"] def test_no_redis_mode_has_no_worker_snapshot(world): a, b = AppSettingsStore(world.cosmos), AppSettingsStore(world.cosmos) - assert not b.read()["enabled"] + before = b.read() + assert not before["enabled"] a.write(change(enabled=True)) - assert b.read()["enabled"] + after = b.read() + assert after["enabled"] def test_returned_settings_do_not_mutate_shared_document(world): settings = world.a.read() settings["enabled"] = "unsaved" - assert world.b.read()["enabled"] is False + observed = world.b.read() + assert observed["enabled"] is False def test_partial_writes_merge_authoritative_document_on_conflict(world): @@ -178,7 +183,8 @@ def test_stale_form_is_rejected_without_poisoning_cache(world): with pytest.raises(SettingsConflictError): world.a.write(change(enabled=False), expected_etag=stale["_etag"]) assert world.redis.raw == before - assert world.b.read()["enabled"] + observed = world.b.read() + assert observed["enabled"] def test_unavailable_redis_rejects_save_before_cosmos_write(world): @@ -188,7 +194,8 @@ def test_unavailable_redis_rejects_save_before_cosmos_write(world): with pytest.raises(SettingsUnavailableError): world.a.write(change(enabled=True)) assert world.cosmos.document == before - assert world.b.read() == before + observed = world.b.read() + assert observed == before assert world.fallback @@ -196,7 +203,8 @@ def test_client_construction_failure_does_not_enable_cosmos_only_writes(world): store = AppSettingsStore(world.cosmos, redis_required=True) with pytest.raises(SettingsUnavailableError): store.write(change(enabled=True)) - assert store.read()["enabled"] is False + observed = store.read() + assert observed["enabled"] is False assert world.cosmos.writes == 0 @@ -206,10 +214,12 @@ def test_interrupted_publication_never_restores_previous_payload(world): with pytest.raises(SettingsUnavailableError): world.a.write(change(enabled=True)) assert json.loads(world.redis.raw)["state"] == "pending" - assert world.b.read()["enabled"] + observed = world.b.read() + assert observed["enabled"] world.redis.fail_publication = False world.now[0] += store_module.WRITE_LEASE_SECONDS + 1 - assert world.c.read()["enabled"] + recovered = world.c.read() + assert recovered["enabled"] assert json.loads(world.redis.raw)["state"] == "ready" @@ -224,7 +234,8 @@ def take_over(): world.cosmos.before_replace = take_over with pytest.raises(SettingsConflictError): world.a.write(change(enabled=False)) - assert world.c.read()["enabled"] + observed = world.c.read() + assert observed["enabled"] def test_delayed_publication_cannot_replace_newer_ready_state(world): @@ -238,7 +249,8 @@ def newer_save(): world.cosmos.after_replace = newer_save with pytest.raises(SettingsUnavailableError): world.a.write(change(enabled=False)) - assert world.c.read()["enabled"] + observed = world.c.read() + assert observed["enabled"] def test_migration_retries_against_newer_document(world): @@ -249,7 +261,8 @@ def migrate(current): current.setdefault("new_default", "default") return current - assert store.write(migrate)["enabled"] + migrated = store.write(migrate) + assert migrated["enabled"] assert world.cosmos.document["new_default"] == "default" @@ -288,7 +301,7 @@ def load_update_settings(store): "copy": copy, "logging": logging, "COSMOS_METADATA_FIELDS": store_module.COSMOS_METADATA_FIELDS, "SETTINGS_REVISION_FIELD": store_module.SETTINGS_REVISION_FIELD, - "app_settings_cache": SimpleNamespace(get_settings_store=lambda: store), + "_get_app_settings_store": lambda: store, "log_event": lambda *_args, **_kwargs: None, "is_tabular_processing_enabled": lambda _settings: False, } @@ -307,12 +320,17 @@ def load_update_settings(store): def test_real_update_settings_rejects_old_full_snapshot_and_merges_deltas(world): old = world.a.read() update_a, update_b = load_update_settings(world.a), load_update_settings(world.b) - assert update_a({"enabled": True}) - assert update_b({"last_update_check_time": "new"}) - assert world.c.read()["enabled"] + saved_a = update_a({"enabled": True}) + saved_b = update_b({"last_update_check_time": "new"}) + observed = world.c.read() + assert saved_a + assert saved_b + assert observed["enabled"] old["last_update_check_time"] = "stale" - assert update_b(old) is False - assert world.c.read()["enabled"] + stale_saved = update_b(old) + observed = world.c.read() + assert stale_saved is False + assert observed["enabled"] def test_startup_does_not_publish_bootstrap_snapshot(): @@ -328,7 +346,7 @@ def load_get_settings(store): getter = definitions["get_settings"] namespace = { "copy": copy, "logging": logging, "secrets": secrets, - "app_settings_cache": SimpleNamespace(get_settings_store=lambda: store), + "_get_app_settings_store": lambda: store, "CosmosResourceNotFoundError": CosmosResourceNotFoundError, "SettingsConflictError": SettingsConflictError, "SettingsUnavailableError": SettingsUnavailableError, @@ -391,13 +409,16 @@ def test_real_get_settings_defers_migration_during_redis_outage(world): def test_real_get_settings_creates_defaults_without_overwriting_winner(world): world.cosmos.document = None getter = load_get_settings(AppSettingsStore(world.cosmos)) - assert getter() is not None + created = getter() + assert created is not None assert world.cosmos.document["id"] == "app_settings" def test_missing_shared_document_can_be_initialized_without_an_abandoned_marker(world): world.cosmos.document = None - assert load_get_settings(world.a)() is not None + getter = load_get_settings(world.a) + created = getter() + assert created is not None assert json.loads(world.redis.raw)["state"] == "ready" @@ -413,44 +434,48 @@ def test_logging_guard_handles_recursive_cache_failure(): def recursive_cache_read(): calls.append(1) - assert namespace["_load_logging_settings"]() == {} + recursive_result = namespace["_load_logging_settings"]() + assert recursive_result == {} raise RedisConnectionError("offline") namespace["app_settings_cache"] = SimpleNamespace(get_settings_cache=recursive_cache_read) exec(compile(ast.Module(body=[definition], type_ignores=[]), "functions_appinsights.py", "exec"), namespace) - assert namespace["_load_logging_settings"]() == {} + result = namespace["_load_logging_settings"]() + assert result == {} assert calls == [1] assert namespace["_logging_settings_load_state"].active is False def load_cache_module(world, monkeypatch): monkeypatch.syspath_prepend(str(APP)) - config = types.ModuleType("config") - config.cosmos_settings_container = world.cosmos - insights = types.ModuleType("functions_appinsights") - insights.log_event = lambda *_args, **_kwargs: None - client_module = types.ModuleType("functions_redis_client") - client_module.AUTH_TYPE_MANAGED_IDENTITY = "managed_identity" - client_module.CREDENTIAL_PURPOSE_APP_CACHE = "app_cache" - client_module.create_redis_client = lambda **_kwargs: world.redis - monkeypatch.setitem(sys.modules, "config", config) - monkeypatch.setitem(sys.modules, "functions_appinsights", insights) - monkeypatch.setitem(sys.modules, "functions_redis_client", client_module) spec = importlib.util.spec_from_file_location("cache_wiring_under_test", APP / "app_settings_cache.py") module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) return module +def cache_dependencies(cache, world, factory=None): + return cache.AppCacheDependencies( + settings_container=world.cosmos, + governance_container=world.cosmos, + create_redis_client=factory or (lambda **_kwargs: world.redis), + log_event=lambda *_args, **_kwargs: None, + ) + + def test_actual_worker_configuration_does_not_publish_startup_snapshot(world, monkeypatch): world.cosmos.document.update(enable_redis_cache=True, redis_url="unused.invalid") cache = load_cache_module(world, monkeypatch) - snapshot = cache.get_settings_store().read() + snapshot = copy.deepcopy(world.cosmos.document) + dependencies = cache_dependencies(cache, world) + cache.configure_settings_store(snapshot, dependencies=dependencies) world.b.write(change(enabled=True)) - cache.configure_app_cache(snapshot) - assert cache.get_settings_cache()["enabled"] + cache.configure_app_cache(snapshot, dependencies=dependencies) + observed = cache.get_settings_cache() + assert observed["enabled"] cache.update_settings_cache(snapshot) - assert world.c.read()["enabled"] + observed = world.c.read() + assert observed["enabled"] assert not hasattr(cache, "APP_SETTINGS_CACHE") @@ -461,10 +486,13 @@ def test_actual_cache_initialization_failure_keeps_write_requirement(world, monk def unavailable_client(**_kwargs): raise ValueError("Client cannot be created") - cache.create_redis_client = unavailable_client - cache.configure_app_cache(world.cosmos.document) + cache.configure_app_cache( + world.cosmos.document, + dependencies=cache_dependencies(cache, world, unavailable_client), + ) assert cache.APP_SETTINGS_STORE.redis_required is True - assert cache.get_settings_cache()["enabled"] is False + observed = cache.get_settings_cache() + assert observed["enabled"] is False with pytest.raises(RuntimeError, match="Configured Redis is unavailable"): cache.APP_SETTINGS_STORE.write(change(enabled=True)) assert world.cosmos.writes == 0 diff --git a/functional_tests/test_cosmos_wave1_cache_fallback.py b/functional_tests/test_cosmos_wave1_cache_fallback.py index c61eff2c9..608b5a97c 100644 --- a/functional_tests/test_cosmos_wave1_cache_fallback.py +++ b/functional_tests/test_cosmos_wave1_cache_fallback.py @@ -2,7 +2,7 @@ #!/usr/bin/env python3 """ Functional test for Cosmos Wave 1 cache fallback behavior. -Version: 0.261.025 +Version: 0.261.027 Implemented in: 0.250.005 This test ensures Redis failures in the app cache layer fall back to @@ -13,7 +13,6 @@ import importlib import os import sys -import types from redis.exceptions import ConnectionError as RedisConnectionError @@ -98,32 +97,26 @@ def pipeline(self): class RaisingRedis(FailingRedis): def __init__(self, *args, **kwargs): - raise RuntimeError("redis initialization failed") + raise RedisConnectionError("redis initialization failed") -def _install_fake_modules(container): - fake_config = types.ModuleType("config") - fake_config.cosmos_settings_container = container - fake_config.exceptions = types.SimpleNamespace() - sys.modules["config"] = fake_config +def _load_cache_module(): + spec = importlib.util.spec_from_file_location( + "cache_fallback_under_test", + os.path.join(SINGLE_APP_DIR, "app_settings_cache.py"), + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module - fake_appinsights = types.ModuleType("functions_appinsights") - fake_appinsights.log_event = lambda *args, **kwargs: None - sys.modules["functions_appinsights"] = fake_appinsights - -def _load_cache_module(container): - _install_fake_modules(container) - sys.modules.pop("app_settings_cache", None) - sys.modules.pop("functions_redis_client", None) - return importlib.import_module("app_settings_cache") - - -def _set_redis_client_class(redis_class): - """Redis clients are built in functions_redis_client, so patch it there.""" - redis_client_module = importlib.import_module("functions_redis_client") - redis_client_module.Redis = redis_class - return redis_client_module +def _dependencies(cache, container, redis_class): + return cache.AppCacheDependencies( + settings_container=container, + governance_container=container, + create_redis_client=redis_class, + log_event=lambda *args, **kwargs: None, + ) def test_redis_runtime_failure_falls_back_to_cosmos_settings(): @@ -139,15 +132,14 @@ def test_redis_runtime_failure_falls_back_to_cosmos_settings(): "type": "cache_version", "version": 7, } - cache_module = _load_cache_module(container) - _set_redis_client_class(FailingRedis) + cache_module = _load_cache_module() cache_module.configure_app_cache({ "enable_redis_cache": True, "redis_url": "simplechat.redis.cache.windows.net", "redis_key": "test-key", "redis_auth_type": "key", - }) + }, dependencies=_dependencies(cache_module, container, FailingRedis)) cached_settings = cache_module.get_settings_cache() @@ -158,15 +150,14 @@ def test_redis_runtime_failure_falls_back_to_cosmos_settings(): def test_redis_write_failure_persists_user_ui_cache_to_cosmos(): """A Redis write failure should persist lightweight UI cache data in Cosmos.""" container = FakeCosmosContainer() - cache_module = _load_cache_module(container) - _set_redis_client_class(FailingRedis) + cache_module = _load_cache_module() cache_module.configure_app_cache({ "enable_redis_cache": True, "redis_url": "simplechat.redis.cache.windows.net", "redis_key": "test-key", "redis_auth_type": "key", - }) + }, dependencies=_dependencies(cache_module, container, FailingRedis)) cache_module.set_user_ui_settings_cache("user-1", {"theme": "dark"}, ttl_seconds=60) cached_settings = cache_module.get_user_ui_settings_cache("user-1") @@ -182,15 +173,14 @@ def test_redis_initialization_failure_assigns_fallback_functions(): "id": "app_settings", "feature_flag": "fallback-configured", } - cache_module = _load_cache_module(container) - _set_redis_client_class(RaisingRedis) + cache_module = _load_cache_module() cache_module.configure_app_cache({ "enable_redis_cache": True, "redis_url": "simplechat.redis.cache.windows.net", "redis_key": "test-key", "redis_auth_type": "key", - }) + }, dependencies=_dependencies(cache_module, container, RaisingRedis)) assert cache_module.app_cache_is_using_redis is False assert cache_module.get_settings_cache()["feature_flag"] == "fallback-configured" diff --git a/functional_tests/test_redis_entra_token_auth.py b/functional_tests/test_redis_entra_token_auth.py index e5894ed9b..42bf42249 100644 --- a/functional_tests/test_redis_entra_token_auth.py +++ b/functional_tests/test_redis_entra_token_auth.py @@ -1,7 +1,8 @@ +# test_redis_entra_token_auth.py #!/usr/bin/env python3 """ Functional test for Redis Microsoft Entra token authentication wiring. -Version: 0.261.010 +Version: 0.261.027 Implemented in: 0.242.070 Updated in: 0.261.010 for Azure Managed Redis support. @@ -18,6 +19,7 @@ import sys import time from types import SimpleNamespace +from unittest.mock import patch ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) APP_DIR = os.path.join(ROOT_DIR, "application", "single_app") @@ -83,9 +85,16 @@ def test_create_redis_managed_identity_client_uses_credential_provider(): import app_settings_cache import functions_redis_client as redis_client - original_redis = redis_client.Redis - try: - redis_client.Redis = _CapturingRedis + dependencies = app_settings_cache.AppCacheDependencies( + settings_container=None, + governance_container=None, + create_redis_client=redis_client.create_redis_client, + log_event=lambda *args, **kwargs: None, + ) + with ( + patch.object(redis_client, "Redis", _CapturingRedis), + patch.object(app_settings_cache, "APP_CACHE_DEPENDENCIES", dependencies), + ): app_settings_cache.create_redis_managed_identity_client( "example.redis.cache.usgovcloudapi.net", @@ -100,8 +109,6 @@ def test_create_redis_managed_identity_client_uses_credential_provider(): socket_timeout=5, ) managed = dict(_CapturingRedis.captured_kwargs) - finally: - redis_client.Redis = original_redis assert classic["host"] == "example.redis.cache.usgovcloudapi.net" assert classic["port"] == 6380 From 4b0c836856ff37018a202a017d789f904f975b33 Mon Sep 17 00:00:00 2001 From: Bionic711 Date: Wed, 9 Sep 2026 15:03:23 -0500 Subject: [PATCH 3/3] Remove unused import from settings regression tests Address CodeQL py/unused-import follow-up in PR #1478. Refs #1477. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- functional_tests/test_app_settings_store_consistency.py | 1 - 1 file changed, 1 deletion(-) diff --git a/functional_tests/test_app_settings_store_consistency.py b/functional_tests/test_app_settings_store_consistency.py index d652bb881..5c93bd8e0 100644 --- a/functional_tests/test_app_settings_store_consistency.py +++ b/functional_tests/test_app_settings_store_consistency.py @@ -16,7 +16,6 @@ from pathlib import Path import socket import secrets -import sys from types import SimpleNamespace import pytest