fix: scope ticket cache per-PR and fix Dynaconf list merge leaks in tests (#2383)

pr_agent/mosaico/env_bridge.py

@@ -39,6 +39,12 @@ def apply_mosaico_env() -> None:
     if model_name:
         model = model_name if "/" in model_name else f"openai/{model_name}"
         settings.set("CONFIG.MODEL", model)
+        # Dynaconf merge_enabled=True appends list values instead of replacing;
+        # pop the leaf first so the subsequent .set() produces a fresh list.
+        # Use case-insensitive matching because Dynaconf can normalize key casing.
+        for _k in list(settings.CONFIG.keys()):
+            if _k.lower() == "fallback_models":
+                settings.CONFIG.pop(_k, None)
         settings.set("CONFIG.FALLBACK_MODELS", [])
         # MOSAICO models are not in pr-agent's built-in MAX_TOKENS table; declare a budget
         # so reviews don't fail with "not defined in MAX_TOKENS". Overridable via env.
@@ -68,6 +74,12 @@ def _register_langfuse_callback(settings) -> None:
         current = [c for c in (settings.get(key, []) or []) if c != LEGACY_LANGFUSE_CALLBACK]
         if LANGFUSE_CALLBACK_NAME not in current:
             current.append(LANGFUSE_CALLBACK_NAME)
+        # Dynaconf merge_enabled=True appends list values; pop the leaf first.
+        # Use case-insensitive matching because Dynaconf can normalize key casing.
+        leaf = key.split(".", 1)[-1]
+        for _k in list(settings.LITELLM.keys()):
+            if _k.lower() == leaf.lower():
+                settings.LITELLM.pop(_k, None)
         settings.set(key, current)
     settings.set("LITELLM.ENABLE_CALLBACKS", True)
     get_logger().info("MOSAICO: registered Langfuse litellm callback.")

pr_agent/settings/configuration.toml

@@ -84,6 +84,7 @@ require_security_review=true
 require_estimate_contribution_time_cost=false
 require_todo_scan=false
 require_ticket_analysis_review=true
+cache_tickets=true
 # general options
 publish_output_no_suggestions=true # Set to "false" if you only need the reviewer's remarks (not labels, not "security audit", etc.) and want to avoid noisy "No major issues detected" comments.
 persistent_comment=true

pr_agent/tools/ticket_pr_compliance_check.py

@@ -1,3 +1,4 @@
+import hashlib
 import re
 import traceback
 
@@ -244,7 +245,11 @@ async def extract_and_cache_pr_tickets(git_provider, vars):
     if not get_settings().get('pr_reviewer.require_ticket_analysis_review', False):
         return
 
-    related_tickets = get_settings().get('related_tickets', [])
+    use_cache = get_settings().get('pr_reviewer.cache_tickets', True)
+    pr_url = getattr(git_provider, 'pr_url', None)
+    cache_key = f'related_tickets_{hashlib.md5(pr_url.encode()).hexdigest()}' if pr_url else 'related_tickets'
+
+    related_tickets = get_settings().get(cache_key, []) if use_cache else []
 
     if not related_tickets:
         tickets_content = await extract_tickets(git_provider)
@@ -262,7 +267,8 @@ async def extract_and_cache_pr_tickets(git_provider, vars):
                               artifact={"tickets": related_tickets})
 
             vars['related_tickets'] = related_tickets
-            get_settings().set('related_tickets', related_tickets)
+            if use_cache:
+                get_settings().set(cache_key, related_tickets)
     else:
         get_logger().info("Using cached tickets", artifact={"tickets": related_tickets})
         vars['related_tickets'] = related_tickets

tests/unittest/_settings_helpers.py

@@ -53,10 +53,14 @@ def _remove_key(settings, key):
 
 
 def restore_settings(snapshot):
-    """Restore ``snapshot``; truly remove entries whose snapshot is SENTINEL."""
+    """Restore ``snapshot``; truly remove entries whose snapshot is SENTINEL.
+
+    Because ``merge_enabled=True`` causes ``settings.set()`` to *append* list
+    values rather than replace them, the key is always removed first via
+    ``_remove_key`` before setting the restored value.
+    """
     settings = get_settings()
     for key, value in snapshot.items():
-        if value is SENTINEL:
-            _remove_key(settings, key)
-        else:
+        _remove_key(settings, key)
+        if value is not SENTINEL:
             settings.set(key, value)

tests/unittest/test_ignore_repositories.py

@@ -4,6 +4,7 @@
 from pr_agent.servers.bitbucket_app import should_process_pr_logic as bitbucket_should_process_pr_logic
 from pr_agent.servers.github_app import should_process_pr_logic as github_should_process_pr_logic
 from pr_agent.servers.gitlab_webhook import should_process_pr_logic as gitlab_should_process_pr_logic
+from tests.unittest._settings_helpers import _remove_key
 
 
 def make_bitbucket_payload(full_name):
@@ -42,11 +43,15 @@ def make_gitlab_body(full_name):
 
 class TestIgnoreRepositories:
     def setup_method(self):
-        get_settings().set("CONFIG.IGNORE_REPOSITORIES", [])
+        settings = get_settings()
+        _remove_key(settings, "CONFIG.IGNORE_REPOSITORIES")
+        settings.set("CONFIG.IGNORE_REPOSITORIES", [])
 
     @pytest.mark.parametrize("provider_name, provider_func, body_func", PROVIDERS)
     def test_should_ignore_matching_repository(self, provider_name, provider_func, body_func):
-        get_settings().set("CONFIG.IGNORE_REPOSITORIES", ["org/repo-to-ignore"])
+        settings = get_settings()
+        _remove_key(settings, "CONFIG.IGNORE_REPOSITORIES")
+        settings.set("CONFIG.IGNORE_REPOSITORIES", ["org/repo-to-ignore"])
         body = {
             "pull_request": {},
             "repository": {"full_name": "org/repo-to-ignore"},
@@ -58,7 +63,9 @@ def test_should_ignore_matching_repository(self, provider_name, provider_func, b
 
     @pytest.mark.parametrize("provider_name, provider_func, body_func", PROVIDERS)
     def test_should_not_ignore_non_matching_repository(self, provider_name, provider_func, body_func):
-        get_settings().set("CONFIG.IGNORE_REPOSITORIES", ["org/repo-to-ignore"])
+        settings = get_settings()
+        _remove_key(settings, "CONFIG.IGNORE_REPOSITORIES")
+        settings.set("CONFIG.IGNORE_REPOSITORIES", ["org/repo-to-ignore"])
         body = {
             "pull_request": {},
             "repository": {"full_name": "org/other-repo"},

tests/unittest/test_mosaico_env_bridge.py

@@ -13,6 +13,7 @@
                                          langfuse_env_present)
 from pr_agent.mosaico.observability import (mosaico_log_context,
                                             parse_observability_metadata)
+from tests.unittest._settings_helpers import _remove_key
 
 _SNAPSHOT_KEYS = [
     "OPENAI.API_BASE",
@@ -41,10 +42,8 @@ def restore_settings():
     litellm_failure = litellm.failure_callback
     yield
     for k, v in snapshot.items():
-        if v is _SENTINEL:
-            # Key was absent before; best-effort reset to empty so the global is not polluted.
-            global_settings.set(k, [] if k.endswith("CALLBACK") or k == "CONFIG.FALLBACK_MODELS" else None)
-        else:
+        _remove_key(global_settings, k)
+        if v is not _SENTINEL:
             global_settings.set(k, v)
     litellm.success_callback = litellm_success
     litellm.failure_callback = litellm_failure

tests/unittest/test_retry_with_fallback_models.py

@@ -5,7 +5,7 @@
 from pr_agent.algo.pr_processing import retry_with_fallback_models
 from pr_agent.algo.utils import ModelType
 from pr_agent.config_loader import get_settings
-from tests.unittest._settings_helpers import SENTINEL, restore_settings, snapshot_settings
+from tests.unittest._settings_helpers import SENTINEL, restore_settings, snapshot_settings, _remove_key
 
 _TRACKED_KEYS = (
     "config.model",
@@ -28,10 +28,13 @@ def _restore_settings(snapshot):
 def test_primary_model_success_invoked_once_and_returns_value():
     snapshot = _snapshot_settings()
     try:
-        get_settings().set("config.model", "primary-model")
-        get_settings().set("config.fallback_models", ["fallback-1", "fallback-2"])
-        get_settings().set("openai.deployment_id", None)
-        get_settings().set("openai.fallback_deployments", [])
+        settings = get_settings()
+        _remove_key(settings, "config.fallback_models")
+        settings.set("config.model", "primary-model")
+        settings.set("config.fallback_models", ["fallback-1", "fallback-2"])
+        _remove_key(settings, "openai.fallback_deployments")
+        settings.set("openai.deployment_id", None)
+        settings.set("openai.fallback_deployments", [])
 
         calls = []
 
@@ -50,10 +53,13 @@ async def fake_f(model):
 def test_primary_fails_fallback_succeeds():
     snapshot = _snapshot_settings()
     try:
-        get_settings().set("config.model", "primary-model")
-        get_settings().set("config.fallback_models", ["fallback-1", "fallback-2"])
-        get_settings().set("openai.deployment_id", None)
-        get_settings().set("openai.fallback_deployments", [])
+        settings = get_settings()
+        _remove_key(settings, "config.fallback_models")
+        settings.set("config.model", "primary-model")
+        settings.set("config.fallback_models", ["fallback-1", "fallback-2"])
+        _remove_key(settings, "openai.fallback_deployments")
+        settings.set("openai.deployment_id", None)
+        settings.set("openai.fallback_deployments", [])
 
         calls = []
 
@@ -74,10 +80,13 @@ async def fake_f(model):
 def test_all_models_fail_raises_with_aggregate_message_and_cause():
     snapshot = _snapshot_settings()
     try:
-        get_settings().set("config.model", "primary-model")
-        get_settings().set("config.fallback_models", ["fallback-1"])
-        get_settings().set("openai.deployment_id", None)
-        get_settings().set("openai.fallback_deployments", [])
+        settings = get_settings()
+        _remove_key(settings, "config.fallback_models")
+        settings.set("config.model", "primary-model")
+        settings.set("config.fallback_models", ["fallback-1"])
+        _remove_key(settings, "openai.fallback_deployments")
+        settings.set("openai.deployment_id", None)
+        settings.set("openai.fallback_deployments", [])
 
         last_error = ValueError("last failure")
         attempted = []
@@ -102,10 +111,13 @@ async def fake_f(model):
 def test_deployment_id_updated_per_attempt():
     snapshot = _snapshot_settings()
     try:
-        get_settings().set("config.model", "primary-model")
-        get_settings().set("config.fallback_models", ["fallback-1", "fallback-2"])
-        get_settings().set("openai.deployment_id", "deployment-primary")
-        get_settings().set(
+        settings = get_settings()
+        _remove_key(settings, "config.fallback_models")
+        _remove_key(settings, "openai.fallback_deployments")
+        settings.set("config.model", "primary-model")
+        settings.set("config.fallback_models", ["fallback-1", "fallback-2"])
+        settings.set("openai.deployment_id", "deployment-primary")
+        settings.set(
             "openai.fallback_deployments",
             ["deployment-fb1", "deployment-fb2"],
         )
@@ -134,11 +146,14 @@ async def fake_f(model):
 def test_weak_model_type_uses_weak_setting_and_forwards_identifier():
     snapshot = _snapshot_settings()
     try:
-        get_settings().set("config.model", "regular-model")
-        get_settings().set("config.model_weak", "weak-model-id")
-        get_settings().set("config.fallback_models", [])
-        get_settings().set("openai.deployment_id", None)
-        get_settings().set("openai.fallback_deployments", [])
+        settings = get_settings()
+        _remove_key(settings, "config.fallback_models")
+        _remove_key(settings, "openai.fallback_deployments")
+        settings.set("config.model", "regular-model")
+        settings.set("config.model_weak", "weak-model-id")
+        settings.set("config.fallback_models", [])
+        settings.set("openai.deployment_id", None)
+        settings.set("openai.fallback_deployments", [])
 
         calls = []
 
@@ -159,11 +174,14 @@ async def fake_f(model):
 def test_reasoning_model_type_uses_reasoning_setting():
     snapshot = _snapshot_settings()
     try:
-        get_settings().set("config.model", "regular-model")
-        get_settings().set("config.model_reasoning", "reasoning-model-id")
-        get_settings().set("config.fallback_models", [])
-        get_settings().set("openai.deployment_id", None)
-        get_settings().set("openai.fallback_deployments", [])
+        settings = get_settings()
+        _remove_key(settings, "config.fallback_models")
+        _remove_key(settings, "openai.fallback_deployments")
+        settings.set("config.model", "regular-model")
+        settings.set("config.model_reasoning", "reasoning-model-id")
+        settings.set("config.fallback_models", [])
+        settings.set("openai.deployment_id", None)
+        settings.set("openai.fallback_deployments", [])
 
         calls = []
 

tests/unittest/test_ticket_extraction_async.py

@@ -100,11 +100,16 @@ def settings_snapshot():
     """
     s = get_settings()
     snapshot = snapshot_settings(
-        ["related_tickets", "pr_reviewer.require_ticket_analysis_review"]
+        [
+            "related_tickets",
+            "pr_reviewer.require_ticket_analysis_review",
+            "pr_reviewer.cache_tickets",
+        ]
     )
     # Reset to known defaults for each test
     s.set("related_tickets", [])
     s.set("pr_reviewer.require_ticket_analysis_review", False)
+    s.set("pr_reviewer.cache_tickets", True)
     try:
         yield s
     finally:
@@ -483,3 +488,74 @@ async def _empty(_):
         vars_ = {}
         asyncio.run(extract_and_cache_pr_tickets(object(), vars_))
         assert "related_tickets" not in vars_
+
+    def test_per_pr_cache_isolation(
+        self, settings_snapshot, monkeypatch
+    ):
+        settings_snapshot.set("pr_reviewer.require_ticket_analysis_review", True)
+
+        tickets_pr_a = [{"ticket_id": 11111, "title": "PR A ticket"}]
+        tickets_pr_b = [{"ticket_id": 22222, "title": "PR B ticket"}]
+
+        call_count = {"n": 0}
+
+        async def _side_effect(git_provider):
+            call_count["n"] += 1
+            if "11111" in (getattr(git_provider, "pr_url", "") or ""):
+                return tickets_pr_a
+            return tickets_pr_b
+
+        monkeypatch.setattr(tpc, "extract_tickets", _side_effect)
+
+        class _Provider:
+            pass
+
+        provider_a = _Provider()
+        provider_a.pr_url = "https://dev.azure.com/org/project/_git/repo/pullrequest/11111"
+        provider_b = _Provider()
+        provider_b.pr_url = "https://dev.azure.com/org/project/_git/repo/pullrequest/22222"
+
+        # First PR — should fetch
+        vars_a = {}
+        asyncio.run(extract_and_cache_pr_tickets(provider_a, vars_a))
+        assert vars_a["related_tickets"] == tickets_pr_a
+        assert call_count["n"] == 1
+
+        # Second PR — different URL, should fetch fresh
+        vars_b = {}
+        asyncio.run(extract_and_cache_pr_tickets(provider_b, vars_b))
+        assert vars_b["related_tickets"] == tickets_pr_b
+        assert call_count["n"] == 2
+
+        # Repeat first PR — should use cache, not fetch
+        vars_a2 = {}
+        asyncio.run(extract_and_cache_pr_tickets(provider_a, vars_a2))
+        assert vars_a2["related_tickets"] == tickets_pr_a
+        assert call_count["n"] == 2  # no additional fetch
+
+    def test_cache_tickets_disabled_always_fetches(
+        self, settings_snapshot, monkeypatch
+    ):
+        settings_snapshot.set("pr_reviewer.require_ticket_analysis_review", True)
+        settings_snapshot.set("pr_reviewer.cache_tickets", False)
+
+        call_count = {"n": 0}
+
+        async def _side_effect(_):
+            call_count["n"] += 1
+            return [{"ticket_id": call_count["n"], "title": f"fetch {call_count['n']}"}]
+
+        monkeypatch.setattr(tpc, "extract_tickets", _side_effect)
+
+        provider = object()
+
+        vars1 = {}
+        asyncio.run(extract_and_cache_pr_tickets(provider, vars1))
+        assert vars1["related_tickets"] == [{"ticket_id": 1, "title": "fetch 1"}]
+        assert call_count["n"] == 1
+
+        # Second call should also fetch (no cache)
+        vars2 = {}
+        asyncio.run(extract_and_cache_pr_tickets(provider, vars2))
+        assert vars2["related_tickets"] == [{"ticket_id": 2, "title": "fetch 2"}]
+        assert call_count["n"] == 2