From 2d1676f655d37efee05b015fe6e021b68cabb5ee Mon Sep 17 00:00:00 2001 From: CleverThis Date: Sat, 16 May 2026 08:04:59 +0000 Subject: [PATCH] fix(agents/graphs/auto_debug): return partial state updates per LangGraph contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert all four node functions (_analyze_error, _generate_fix, _validate_fix, _finalize) from in-place state mutation to returning partial-state dicts as required by LangGraph's node contract. Also fix a fail-open security bug in _validate_fix where exception handlers defaulted is_valid=True → changed to is_valid=False so crashed LLM validators don't pass unvalidated fixes (closes #10496). --- src/cleveragents/agents/graphs/auto_debug.py | 115 +++++++++++++++---- 1 file changed, 90 insertions(+), 25 deletions(-) diff --git a/src/cleveragents/agents/graphs/auto_debug.py b/src/cleveragents/agents/graphs/auto_debug.py index 2bbf5b64b..0a5b9fb0d 100644 --- a/src/cleveragents/agents/graphs/auto_debug.py +++ b/src/cleveragents/agents/graphs/auto_debug.py @@ -89,7 +89,22 @@ 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 code context. + + Per LangGraph's node contract, this function returns a **partial-state** + dict (keys only for fields updated by this node); LangGraph merges + those keys into the existing workflow state. Mutating the input + ``state`` in-place is avoided — we build a new messages list via + unpacking so that concurrent graph runs and checkpoint writes stay + isolated, per HAL9000 observation #2 (LLM peer review). + + Args: + state: Current workflow state. + + Returns: + Partial-state dict with an appended ``messages`` entry. + """ logger.info("Analyzing error message") error_msg = state.get("error_message", "") @@ -129,17 +144,31 @@ 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", - } - ) + return { + "messages": [ + *state.get("messages", []), + { + "role": "assistant", + "content": analysis, + "type": "error_analysis", + }, + ], + } - return state + def _generate_fix(self, state: AutoDebugState) -> dict[str, Any]: + """Generate a fix suggestion. - def _generate_fix(self, state: AutoDebugState) -> AutoDebugState: + Per LangGraph's node contract, this function returns a **partial-state** + dict containing only the ``current_fix`` key it updates; omitted keys + (e.g., ``messages``, ``attempted_fixes``) are left unchanged in the + workflow state (LangGraph merges behavior). + + Args: + state: Current workflow state. + + Returns: + Partial-state dict with the new ``current_fix`` value. + """ logger.info("Generating fix suggestion") error_analysis = next( @@ -220,10 +249,28 @@ 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 proposed fix. + + Per LangGraph's node contract this function returns a **partial-state** + dict. When the fix is invalid we include both ``fix_validated`` and + ``attempted_fixes`` keys; when valid only ``fix_validated`` is returned + — LangGraph treats omitted keys as "leave unchanged". + + Security note: the exception handler defaults to **fail-close** + (``is_valid = False``) rather than the previous fail-open (``True``), + preventing a crashed LLM validator from accidentally marking an + unvalidated fix as acceptable. Closes #10496. + + Args: + state: Current workflow state. + + Returns: + Partial-state dict with validation result and, on failure, the + current fix appended to ``attempted_fixes``. + """ logger.info("Validating fix") current_fix = state.get("current_fix", {}) @@ -276,33 +323,51 @@ Validate this fix.""" ) except Exception as exc: # pragma: no cover - defensive logger.warning("LLM validation failed, using fallback: %s", exc) - is_valid = True - - state["fix_validated"] = is_valid + is_valid = False if not is_valid: attempted_fixes = state.get("attempted_fixes", []) - attempted_fixes.append(current_fix) - state["attempted_fixes"] = attempted_fixes + return { + "fix_validated": is_valid, + "attempted_fixes": [*attempted_fixes, current_fix], + } - return state + return {"fix_validated": is_valid} def _should_retry_fix(self, state: AutoDebugState) -> str: + """Decide whether to retry fix generation. + + This conditional-edge function is read-only from LangGraph's + perspective — any mutations it makes to ``state`` are silently + discarded. Only the keys that are returned by node functions are + merged into the workflow state. + """ if not state.get("fix_validated", False): attempts = len(state.get("attempted_fixes", [])) if attempts < self.max_fix_attempts: return "retry" return "done" - def _finalize(self, state: AutoDebugState) -> AutoDebugState: + def _finalize(self, state: AutoDebugState) -> dict[str, Any]: + """Finalize auto-debug results. + + Returns a partial-state dict containing only the ``result`` key. + + Args: + state: Current workflow state. + + Returns: + Partial-state dict with the final ``result`` summary. + """ 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