fix(agents/graphs/auto_debug): return update dicts from node functions instead of mutating state in-place

ISSUES CLOSED: #10494
#10496

CHANGELOG.md

@@ -5,6 +5,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+- **Fixed auto_debug node LangGraph contract violations** (#10494, #10496): Fixed four auto_debug node functions violating LangGraph's node contract — changed return type from full state object to partial update dicts, and replaced in-place mutation with immutable copy patterns.
+
 - **`task-implementor` posts work-started notification comments** (#11031): Both
   the `issue_impl` and `pr_fix` procedures now post an informational "work
   started" comment to the Forgejo issue/PR before beginning implementation.

CONTRIBUTORS.md

@@ -19,7 +19,7 @@ Below are some of the specific details of various contributions.
 * HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool.
 * HAL 9000 has contributed concurrency safety improvements, including thread-safe context tier management (issue #7547) for parallel plan execution.
 * HAL 9000 has contributed the plan concurrency race-condition fix (#7989): wired `LockService` into the plan lifecycle, guarding `execute_plan()` and `apply_plan()` with plan-level advisory locks and unique per-invocation owner identities to prevent silent concurrent state corruption.
-<<<<<<< HEAD
+
 * HAL 9000 has contributed the bug-hunt-pool-supervisor non-blocking tracking fix (#7875 / PR #7957): updated step 5 to be best-effort and added rule 9 to prevent the automation-tracking-manager call from blocking the main supervisor loop.
 * Jeffrey Phillips Freeman has contributed the complete AUTO-BUG-POOL to AUTO-BUG-SUP tracking prefix fix across agent-system-specification.md, automation-tracking.md documentation and agent-system-specification.md spec document, replaced with correct `AUTO-BUG-SUP` prefix used by the bug-hunt-pool-supervisor agent (#7875).
 * HAL 9000 has contributed the plugin entry point security hardening fix (#7476): enforced entry point allowlist validation before importing plugin modules to prevent malicious plugin loading.

features/steps/tdd_auto_debug_state_mutation_steps.py

@@ -0,0 +1,249 @@
+"""Step definitions for tdd_auto_debug_state_mutation.feature.
+
+These steps verify that AutoDebug node functions return new state dicts
+instead of mutating the input state in-place, respecting the LangGraph
+node contract.
+"""
+
+from __future__ import annotations
+
+import json
+from copy import deepcopy
+from typing import Any
+
+from behave import given, then, when
+
+from cleveragents.agents.graphs.auto_debug import AutoDebugAgent, AutoDebugState
+
+
+class _MockResponse:
+    """Minimal response object with a content attribute."""
+
+    def __init__(self, content: str) -> None:
+        self.content = content
+
+
+class _MockLLM:
+    """Stub LLM that returns a fixed content string."""
+
+    def __init__(self, content: str = "Mock LLM response") -> None:
+        self._content = content
+
+    def invoke(self, messages: Any) -> _MockResponse:
+        return _MockResponse(self._content)
+
+
+class _InvalidValidationLLM:
+    """Stub LLM that returns an invalid validation response."""
+
+    def invoke(self, messages: Any) -> _MockResponse:
+        return _MockResponse(
+            json.dumps(
+                {
+                    "is_valid": False,
+                    "reasoning": "Fix failed",
+                    "issues": ["error persists"],
+                }
+            )
+        )
+
+
+def _base_state(**overrides: Any) -> AutoDebugState:
+    """Create a minimal valid AutoDebugState with optional overrides."""
+    state: AutoDebugState = {
+        "messages": [],
+        "context": {},
+        "result": None,
+        "error": None,
+        "metadata": {},
+        "error_message": "NameError: name 'x' is not defined",
+        "code_context": "print(x)",
+        "attempted_fixes": [],
+        "current_fix": {},
+        "fix_validated": False,
+    }
+    state.update(overrides)  # type: ignore[typeddict-item]
+    return state
+
+
+@given("the auto debug state mutation module is imported")
+def step_module_imported(context: Any) -> None:
+    """Verify the auto_debug module is importable."""
+    assert AutoDebugAgent is not None
+    assert AutoDebugState is not None
+
+
+@given("an auto debug agent for mutation testing")
+def step_create_agent(context: Any) -> None:
+    """Create an AutoDebugAgent with a mock LLM."""
+    context.mutation_agent = AutoDebugAgent(llm=_MockLLM(), max_fix_attempts=3)
+
+
+@given("an initial auto debug state for mutation testing")
+def step_initial_state(context: Any) -> None:
+    """Create an initial state with empty messages list."""
+    context.mutation_input_state = _base_state(messages=[])
+    context.mutation_original_messages_len = len(
+        context.mutation_input_state["messages"]
+    )
+
+
+@given("a state with an error analysis for mutation testing")
+def step_state_with_analysis(context: Any) -> None:
+    """Create a state with an error analysis message."""
+    context.mutation_input_state = _base_state(
+        messages=[
+            {
+                "role": "assistant",
+                "content": "Detected a NameError",
+                "type": "error_analysis",
+            }
+        ],
+        current_fix={},
+    )
+    context.mutation_original_current_fix = deepcopy(
+        context.mutation_input_state["current_fix"]
+    )
+
+
+@given("a state with a current fix for mutation testing")
+def step_state_with_current_fix(context: Any) -> None:
+    """Create a state with a current fix."""
+    context.mutation_agent = AutoDebugAgent(llm=_MockLLM(), max_fix_attempts=3)
+    context.mutation_input_state = _base_state(
+        current_fix={
+            "description": "Proposed fix",
+            "code": "x = 42\nprint(x)",
+            "files_to_modify": ["main.py"],
+        },
+        fix_validated=False,
+    )
+    context.mutation_original_fix_validated = context.mutation_input_state[
+        "fix_validated"
+    ]
+
+
+@given("a state with a current fix and invalid validation for mutation testing")
+def step_state_with_invalid_validation(context: Any) -> None:
+    """Create a state with a current fix that will fail validation."""
+    context.mutation_agent = AutoDebugAgent(
+        llm=_InvalidValidationLLM(), max_fix_attempts=3
+    )
+    context.mutation_input_state = _base_state(
+        current_fix={
+            "description": "Proposed fix",
+            "code": "x = 42\nprint(x)",
+            "files_to_modify": ["main.py"],
+        },
+        attempted_fixes=[],
+        fix_validated=False,
+    )
+    context.mutation_original_attempted_fixes_len = len(
+        context.mutation_input_state["attempted_fixes"]
+    )
+
+
+@given("a state ready for finalization for mutation testing")
+def step_state_for_finalization(context: Any) -> None:
+    """Create a state ready for finalization."""
+    context.mutation_agent = AutoDebugAgent(llm=_MockLLM(), max_fix_attempts=3)
+    context.mutation_input_state = _base_state(
+        fix_validated=True,
+        current_fix={"description": "Final fix", "code": "# fixed"},
+        result=None,
+    )
+    context.mutation_original_result = context.mutation_input_state["result"]
+
+
+@when("I call _analyze_error and capture the result")
+def step_call_analyze_error(context: Any) -> None:
+    """Call _analyze_error and capture both input and output."""
+    context.mutation_result_state = context.mutation_agent._analyze_error(
+        context.mutation_input_state
+    )
+
+
+@when("I call _generate_fix and capture the result")
+def step_call_generate_fix(context: Any) -> None:
+    """Call _generate_fix and capture both input and output."""
+    context.mutation_result_state = context.mutation_agent._generate_fix(
+        context.mutation_input_state
+    )
+
+
+@when("I call _validate_fix and capture the result")
+def step_call_validate_fix(context: Any) -> None:
+    """Call _validate_fix and capture both input and output."""
+    context.mutation_result_state = context.mutation_agent._validate_fix(
+        context.mutation_input_state
+    )
+
+
+@when("I call _finalize and capture the result")
+def step_call_finalize(context: Any) -> None:
+    """Call _finalize and capture both input and output."""
+    context.mutation_result_state = context.mutation_agent._finalize(
+        context.mutation_input_state
+    )
+
+
+@then("the returned state should be a different object from the input state")
+def step_different_object(context: Any) -> None:
+    """Verify the returned state is a new dict, not the same object."""
+    assert context.mutation_result_state is not context.mutation_input_state, (
+        "Node function returned the same state object (mutated in-place). "
+        "LangGraph node functions must return a new dict."
+    )
+
+
+@then("the original state messages list should be unchanged")
+def step_messages_unchanged(context: Any) -> None:
+    """Verify the original state's messages list was not mutated."""
+    original_len = context.mutation_original_messages_len
+    actual_len = len(context.mutation_input_state["messages"])
+    assert actual_len == original_len, (
+        f"Original state messages list was mutated: "
+        f"expected {original_len} messages, got {actual_len}"
+    )
+
+
+@then("the original state current_fix should be unchanged")
+def step_current_fix_unchanged(context: Any) -> None:
+    """Verify the original state's current_fix was not mutated."""
+    original = context.mutation_original_current_fix
+    actual = context.mutation_input_state["current_fix"]
+    assert actual == original, (
+        f"Original state current_fix was mutated: expected {original!r}, got {actual!r}"
+    )
+
+
+@then("the original state fix_validated should be unchanged")
+def step_fix_validated_unchanged(context: Any) -> None:
+    """Verify the original state's fix_validated was not mutated."""
+    original = context.mutation_original_fix_validated
+    actual = context.mutation_input_state["fix_validated"]
+    assert actual == original, (
+        f"Original state fix_validated was mutated: "
+        f"expected {original!r}, got {actual!r}"
+    )
+
+
+@then("the original state attempted_fixes list should be unchanged")
+def step_attempted_fixes_unchanged(context: Any) -> None:
+    """Verify the original state's attempted_fixes list was not mutated."""
+    original_len = context.mutation_original_attempted_fixes_len
+    actual_len = len(context.mutation_input_state["attempted_fixes"])
+    assert actual_len == original_len, (
+        f"Original state attempted_fixes list was mutated: "
+        f"expected {original_len} items, got {actual_len}"
+    )
+
+
+@then("the original state result should be unchanged")
+def step_result_unchanged(context: Any) -> None:
+    """Verify the original state's result was not mutated."""
+    original = context.mutation_original_result
+    actual = context.mutation_input_state["result"]
+    assert actual == original, (
+        f"Original state result was mutated: expected {original!r}, got {actual!r}"
+    )

features/tdd_auto_debug_state_mutation.feature

@@ -0,0 +1,42 @@
+Feature: AutoDebug node functions must not mutate state in-place
+  As a LangGraph developer
+  I want AutoDebug node functions to return new state dicts
+  So that the LangGraph node contract is respected and state isolation is maintained
+
+  Background:
+    Given the auto debug state mutation module is imported
+
+  Scenario: _analyze_error does not mutate the original state object
+    Given an auto debug agent for mutation testing
+    And an initial auto debug state for mutation testing
+    When I call _analyze_error and capture the result
+    Then the returned state should be a different object from the input state
+    And the original state messages list should be unchanged
+
+  Scenario: _generate_fix does not mutate the original state object
+    Given an auto debug agent for mutation testing
+    And a state with an error analysis for mutation testing
+    When I call _generate_fix and capture the result
+    Then the returned state should be a different object from the input state
+    And the original state current_fix should be unchanged
+
+  Scenario: _validate_fix does not mutate the original state object
+    Given an auto debug agent for mutation testing
+    And a state with a current fix for mutation testing
+    When I call _validate_fix and capture the result
+    Then the returned state should be a different object from the input state
+    And the original state fix_validated should be unchanged
+
+  Scenario: _validate_fix with invalid fix does not mutate attempted_fixes in-place
+    Given an auto debug agent for mutation testing
+    And a state with a current fix and invalid validation for mutation testing
+    When I call _validate_fix and capture the result
+    Then the returned state should be a different object from the input state
+    And the original state attempted_fixes list should be unchanged
+
+  Scenario: _finalize does not mutate the original state object
+    Given an auto debug agent for mutation testing
+    And a state ready for finalization for mutation testing
+    When I call _finalize and capture the result
+    Then the returned state should be a different object from the input state
+    And the original state result should be unchanged

src/cleveragents/agents/graphs/auto_debug.py

@@ -89,7 +89,12 @@ class AutoDebugAgent:
 
         return workflow
 
-    def _analyze_error(self, state: AutoDebugState) -> AutoDebugState:
+    def _analyze_error(self, state: AutoDebugState) -> dict[str, Any]:
+        """Analyze the error message and return updated messages.
+
+        Returns a new partial state dict with the updated messages list,
+        without mutating the input state.
+        """
         logger.info("Analyzing error message")
 
         error_msg = state.get("error_message", "")
@@ -129,17 +134,24 @@ Analyze this error and provide insights."""
             logger.warning("LLM analysis failed, using fallback: %s", exc)
             analysis = "Error analysis completed"
 
-        state.setdefault("messages", []).append(
-            {
-                "role": "assistant",
-                "content": analysis,
-                "type": "error_analysis",
-            }
-        )
+        new_message: dict[str, Any] = {
+            "role": "assistant",
+            "content": analysis,
+            "type": "error_analysis",
+        }
+        updated_messages: list[dict[str, Any]] = [
+            *state.get("messages", []),
+            new_message,
+        ]
 
-        return state
+        return {"messages": updated_messages}
 
-    def _generate_fix(self, state: AutoDebugState) -> AutoDebugState:
+    def _generate_fix(self, state: AutoDebugState) -> dict[str, Any]:
+        """Generate a fix suggestion and return updated current_fix.
+
+        Returns a new partial state dict with the updated current_fix,
+        without mutating the input state.
+        """
         logger.info("Generating fix suggestion")
 
         error_analysis = next(
@@ -220,10 +232,14 @@ Generate fix attempt #{attempt_num}."""
                 "files_to_modify": [],
             }
 
-        state["current_fix"] = fix_data
-        return state
+        return {"current_fix": fix_data}
 
-    def _validate_fix(self, state: AutoDebugState) -> AutoDebugState:
+    def _validate_fix(self, state: AutoDebugState) -> dict[str, Any]:
+        """Validate the current fix and return updated validation state.
+
+        Returns a new partial state dict with the updated fix_validated and
+        attempted_fixes fields, without mutating the input state.
+        """
         logger.info("Validating fix")
 
         current_fix = state.get("current_fix", {})
@@ -278,14 +294,17 @@ Validate this fix."""
             logger.warning("LLM validation failed, using fallback: %s", exc)
             is_valid = True
 
-        state["fix_validated"] = is_valid
-
         if not is_valid:
-            attempted_fixes = state.get("attempted_fixes", [])
-            attempted_fixes.append(current_fix)
-            state["attempted_fixes"] = attempted_fixes
+            updated_attempted_fixes: list[dict[str, Any]] = [
+                *state.get("attempted_fixes", []),
+                current_fix,
+            ]
+            return {
+                "fix_validated": is_valid,
+                "attempted_fixes": updated_attempted_fixes,
+            }
 
-        return state
+        return {"fix_validated": is_valid}
 
     def _should_retry_fix(self, state: AutoDebugState) -> str:
         if not state.get("fix_validated", False):
@@ -294,15 +313,21 @@ Validate this fix."""
                 return "retry"
         return "done"
 
-    def _finalize(self, state: AutoDebugState) -> AutoDebugState:
+    def _finalize(self, state: AutoDebugState) -> dict[str, Any]:
+        """Finalize the auto-debug results and return updated result.
+
+        Returns a new partial state dict with the result field populated,
+        without mutating the input state.
+        """
         logger.info("Finalizing auto-debug results")
 
-        state["result"] = {
-            "success": state.get("fix_validated", False),
-            "fix": state.get("current_fix"),
-            "attempts": len(state.get("attempted_fixes", [])),
+        return {
+            "result": {
+                "success": state.get("fix_validated", False),
+                "fix": state.get("current_fix"),
+                "attempts": len(state.get("attempted_fixes", [])),
+            }
         }
-        return state
 
     def invoke(
         self, input_state: AutoDebugState, config: dict[str, Any] | None = None