From ead210927dc9145150a5faaf36ece5ecaefa66c7 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 17 Jul 2026 16:22:38 -0400 Subject: [PATCH 1/7] 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] --------- Co-authored-by: Fleet Agent Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com> Co-authored-by: open-swe[bot] (cherry picked from commit c34e04f44da7ec7638cdafc71e0bb40e77070efd) Co-authored-by: Jacob Albert <122248719+jacobalbert3@users.noreply.github.com> --- agent/chat.py | 4 +- agent/reviewer.py | 5 +- .../reviewer/test_factory_config_isolation.py | 122 ++++++++++++++++++ tests/reviewer/test_reviewer.py | 20 ++- 4 files changed, 142 insertions(+), 9 deletions(-) create mode 100644 tests/reviewer/test_factory_config_isolation.py diff --git a/agent/chat.py b/agent/chat.py index 45dae8a4..4798407c 100644 --- a/agent/chat.py +++ b/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) diff --git a/agent/reviewer.py b/agent/reviewer.py index 6c2db45a..ac7285fa 100644 --- a/agent/reviewer.py +++ b/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) diff --git a/tests/reviewer/test_factory_config_isolation.py b/tests/reviewer/test_factory_config_isolation.py new file mode 100644 index 00000000..14fe78bf --- /dev/null +++ b/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} diff --git a/tests/reviewer/test_reviewer.py b/tests/reviewer/test_reviewer.py index 7a5fb3a1..1073c356 100644 --- a/tests/reviewer/test_reviewer.py +++ b/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 From 58941d1edd38e0f8f8cbf744f72332671da3bb88 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 17 Jul 2026 16:22:38 -0400 Subject: [PATCH 2/7] fix: normalize dashboard label rendering (#1732) * fix: normalize tool call rendering Co-authored-by: open-swe[bot] * fix: simplify tool call labels Co-authored-by: open-swe[bot] * fix: capitalize dashboard UI labels Co-authored-by: open-swe[bot] --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] (cherry picked from commit d714586ce6928e4848214a8fce78cc4597ff2684) Co-authored-by: Ramon Nogueira --- .../agents/components/AgentsThreadsPage.tsx | 8 +- .../components/WorkflowApprovalCard.tsx | 4 +- .../agents/components/chat/CloudPromptBar.tsx | 2 +- .../components/chat/ToolExecution.test.ts | 39 +++++++++ .../agents/components/chat/ToolExecution.tsx | 57 +------------ .../components/chat/toolExecutionDisplay.ts | 82 +++++++++++++++++++ .../components/messages/ThinkingSpinner.tsx | 22 ++--- .../features/agents/experiments/PromptBar.tsx | 2 +- .../features/agents/lib/streamMessagesToUi.ts | 8 +- 9 files changed, 149 insertions(+), 75 deletions(-) create mode 100644 ui/src/features/agents/components/chat/ToolExecution.test.ts create mode 100644 ui/src/features/agents/components/chat/toolExecutionDisplay.ts diff --git a/ui/src/features/agents/components/AgentsThreadsPage.tsx b/ui/src/features/agents/components/AgentsThreadsPage.tsx index 1d89dd01..04f96cc3 100644 --- a/ui/src/features/agents/components/AgentsThreadsPage.tsx +++ b/ui/src/features/agents/components/AgentsThreadsPage.tsx @@ -61,6 +61,10 @@ function triToBool(value: TriState): boolean | undefined { return undefined } +function displayStatus(status: AgentStatus): string { + return STATUS_OPTIONS.find((option) => option.value === status)?.label ?? status +} + export function AgentsThreadsPage({ filters, onFiltersChange, @@ -279,8 +283,8 @@ function ThreadListItem({ thread }: { thread: AgentThread }) {

{thread.title}

- {thread.repoFullName || "no repo"} · {thread.status} - {isResolved ? " · resolved" : ""} + {thread.repoFullName || "No repo"} · {displayStatus(thread.status)} + {isResolved ? " · Resolved" : ""}