fix: defensive copy in get_reviewer_agent and get_chat_agent [closes #1584] (#1742)

* fix: defensive copy in get_reviewer_agent and get_chat_agent [closes #1584]

Factory functions were mutating the caller's RunnableConfig in-place via
config['recursion_limit'] = DEFAULT_RECURSION_LIMIT. Add copy.deepcopy(config)
at the top of each factory and switch the recursion_limit write to setdefault
so a caller-supplied ceiling is respected.

* fix: preserve runtime config object identities

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>

---------

Co-authored-by: Fleet Agent <fleet-agent@langchain.dev>
Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com>
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
(cherry picked from commit c34e04f44da7ec7638cdafc71e0bb40e77070efd)

Co-authored-by: Jacob Albert <122248719+jacobalbert3@users.noreply.github.com>

agent/chat.py

@@ -115,8 +115,10 @@ async def _resolve_chat_model(configurable: dict) -> tuple[str, str]:
 
 async def get_chat_agent(config: RunnableConfig) -> Pregel:
     """Get a read-only PR chat agent. No sandbox; PR context comes via config."""
+    config = config.copy()
+    config["configurable"] = config["configurable"].copy()
+    config.setdefault("recursion_limit", DEFAULT_RECURSION_LIMIT)
     thread_id = config["configurable"].get("thread_id")
-    config["recursion_limit"] = DEFAULT_RECURSION_LIMIT
 
     if thread_id is None or not graph_loaded_for_execution(config):
         return create_deep_agent(system_prompt="", tools=[]).with_config(config)

agent/reviewer.py

@@ -854,10 +854,11 @@ async def _resolve_grouping_model(
 
 async def get_reviewer_agent(config: RunnableConfig) -> Pregel:
     """Get or create a reviewer agent with a sandbox + prepped repo."""
+    config = config.copy()
+    config["configurable"] = config["configurable"].copy()
+    config.setdefault("recursion_limit", DEFAULT_RECURSION_LIMIT)
     thread_id = config["configurable"].get("thread_id", None)
 
-    config["recursion_limit"] = DEFAULT_RECURSION_LIMIT
-
     if thread_id is None or not graph_loaded_for_execution(config):
         logger.info("No thread_id or not for execution, returning reviewer agent without sandbox")
         return create_deep_agent(system_prompt="", tools=[]).with_config(config)

tests/reviewer/test_factory_config_isolation.py

@@ -0,0 +1,122 @@
+"""Tests that get_reviewer_agent and get_chat_agent do not mutate the caller's config."""
+
+from __future__ import annotations
+
+from unittest.mock import MagicMock, patch
+
+import pytest
+from langgraph.graph.state import RunnableConfig
+
+
+def _make_config(recursion_limit: int = 25) -> RunnableConfig:
+    return {
+        "configurable": {"thread_id": None},
+        "recursion_limit": recursion_limit,
+    }
+
+
+@pytest.mark.asyncio
+async def test_get_reviewer_agent_does_not_mutate_caller_config() -> None:
+    """get_reviewer_agent must not overwrite the caller's recursion_limit."""
+    from agent import reviewer
+
+    config = _make_config(recursion_limit=25)
+    original_limit = config["recursion_limit"]
+
+    fake_pregel = MagicMock()
+    fake_pregel.with_config = MagicMock(return_value=fake_pregel)
+
+    with patch("agent.reviewer.create_deep_agent", return_value=fake_pregel):
+        await reviewer.get_reviewer_agent(config)
+
+    assert config["recursion_limit"] == original_limit, (
+        f"get_reviewer_agent mutated caller's recursion_limit: "
+        f"expected {original_limit}, got {config['recursion_limit']}"
+    )
+
+
+@pytest.mark.asyncio
+async def test_get_reviewer_agent_applies_default_when_limit_unset() -> None:
+    """get_reviewer_agent should apply DEFAULT_RECURSION_LIMIT when the caller didn't set one."""
+    from agent import reviewer
+
+    config: RunnableConfig = {"configurable": {"thread_id": None}}
+
+    fake_pregel = MagicMock()
+    fake_pregel.with_config = MagicMock(return_value=fake_pregel)
+
+    with patch("agent.reviewer.create_deep_agent", return_value=fake_pregel):
+        await reviewer.get_reviewer_agent(config)
+
+    assert "recursion_limit" not in config
+
+
+@pytest.mark.asyncio
+async def test_get_chat_agent_does_not_mutate_caller_config() -> None:
+    """get_chat_agent must not overwrite the caller's recursion_limit."""
+    from agent import chat
+
+    config = _make_config(recursion_limit=50)
+    original_limit = config["recursion_limit"]
+
+    fake_pregel = MagicMock()
+    fake_pregel.with_config = MagicMock(return_value=fake_pregel)
+
+    with patch("agent.chat.create_deep_agent", return_value=fake_pregel):
+        await chat.get_chat_agent(config)
+
+    assert config["recursion_limit"] == original_limit, (
+        f"get_chat_agent mutated caller's recursion_limit: "
+        f"expected {original_limit}, got {config['recursion_limit']}"
+    )
+
+
+@pytest.mark.asyncio
+async def test_get_chat_agent_applies_default_when_limit_unset() -> None:
+    """get_chat_agent should apply DEFAULT_RECURSION_LIMIT when the caller didn't set one."""
+    from agent import chat
+
+    config: RunnableConfig = {"configurable": {"thread_id": None}}
+
+    fake_pregel = MagicMock()
+    fake_pregel.with_config = MagicMock(return_value=fake_pregel)
+
+    with patch("agent.chat.create_deep_agent", return_value=fake_pregel):
+        await chat.get_chat_agent(config)
+
+    assert "recursion_limit" not in config
+
+
+@pytest.mark.parametrize(
+    ("module_name", "factory_name"),
+    [("agent.reviewer", "get_reviewer_agent"), ("agent.chat", "get_chat_agent")],
+)
+@pytest.mark.asyncio
+async def test_factory_copies_config_dicts_but_preserves_runtime_objects(
+    module_name: str, factory_name: str
+) -> None:
+    """Factory config isolation must preserve callback and configurable value identities."""
+    module = __import__(module_name, fromlist=[factory_name])
+    factory = getattr(module, factory_name)
+    callback = object()
+    configurable_value = object()
+    callbacks = [callback]
+    config: RunnableConfig = {
+        "configurable": {"thread_id": None, "custom_key": configurable_value},
+        "callbacks": callbacks,
+    }
+
+    fake_pregel = MagicMock()
+    fake_pregel.with_config = MagicMock(return_value=fake_pregel)
+
+    with patch(f"{module_name}.create_deep_agent", return_value=fake_pregel):
+        await factory(config)
+
+    bound_config = fake_pregel.with_config.call_args.args[0]
+    assert bound_config is not config
+    assert bound_config["configurable"] is not config["configurable"]
+    assert bound_config["configurable"]["custom_key"] is configurable_value
+    assert bound_config["callbacks"] is callbacks
+    assert bound_config["callbacks"][0] is callback
+    assert "recursion_limit" not in config
+    assert config["configurable"] == {"thread_id": None, "custom_key": configurable_value}

tests/reviewer/test_reviewer.py

@@ -1288,13 +1288,17 @@ async def test_reviewer_populates_diff_line_set_from_github_api() -> None:
         patch("agent.utils.deferred_model.make_model", return_value=MagicMock()),
         patch("agent.reviewer.create_deep_agent", side_effect=fake_create_deep_agent),
     ):
-        await reviewer.get_reviewer_agent(config)
+        agent = await reviewer.get_reviewer_agent(config)
 
     mock_fetch_diff.assert_awaited_once_with(
         owner="acme", repo="repo", pr_number=42, token="gh-token"
     )
-    assert config["configurable"]["diff_text"] == pr_diff
-    assert config["configurable"]["diff_line_set"] == {"in_diff.py": {"RIGHT": {10}, "LEFT": {1}}}
+    # get_reviewer_agent copies the caller's config before mutating it (it must not
+    # mutate the caller's dict in place), so assert against the config actually
+    # bound to the returned agent rather than the original input dict.
+    bound_configurable = agent.config["configurable"]
+    assert bound_configurable["diff_text"] == pr_diff
+    assert bound_configurable["diff_line_set"] == {"in_diff.py": {"RIGHT": {10}, "LEFT": {1}}}
 
 
 @pytest.mark.asyncio
@@ -1353,10 +1357,14 @@ async def test_reviewer_leaves_validation_disabled_when_diff_fetch_fails() -> No
         patch("agent.utils.deferred_model.make_model", return_value=MagicMock()),
         patch("agent.reviewer.create_deep_agent", side_effect=fake_create_deep_agent),
     ):
-        await reviewer.get_reviewer_agent(config)
+        agent = await reviewer.get_reviewer_agent(config)
 
-    assert config["configurable"]["diff_text"] == ""
-    assert config["configurable"]["diff_line_set"] is None
+    # get_reviewer_agent copies the caller's config before mutating it (it must not
+    # mutate the caller's dict in place), so assert against the config actually
+    # bound to the returned agent rather than the original input dict.
+    bound_configurable = agent.config["configurable"]
+    assert bound_configurable["diff_text"] == ""
+    assert bound_configurable["diff_line_set"] is None
 
 
 @pytest.mark.asyncio