From fd5a9aeae46c627764912b826af283f15dfbc255 Mon Sep 17 00:00:00 2001
From: Willem Jiang <willem.jiang@gmail.com>
Date: Sat, 25 Oct 2025 22:06:19 +0800
Subject: [PATCH] fix: handle [ACCEPTED] feedback gracefully without TypeError
 in plan review  (#657)

* fix: handle [ACCEPTED] feedback gracefully without TypeError in plan review (#607)

- Add explicit None/empty feedback check to prevent processing None values
- Normalize feedback string once using strip().upper() instead of repeated calls
- Replace TypeError exception with graceful fallback to planner node
- Handle invalid feedback formats by logging warning and returning to planner
- Maintain backward compatibility for '[ACCEPTED]' and '[EDIT_PLAN]' formats
- Add test cases for None feedback, empty string feedback, and invalid formats
- Update existing test to verify graceful handling instead of exception raising

* Update src/graph/nodes.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
---
 src/graph/nodes.py              | 16 +++++++++++++---
 tests/integration/test_nodes.py | 31 ++++++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/src/graph/nodes.py b/src/graph/nodes.py
index e97fd1e..aa417ed 100644
--- a/src/graph/nodes.py
+++ b/src/graph/nodes.py
@@ -320,8 +320,17 @@ def human_feedback_node(
     if not auto_accepted_plan:
         feedback = interrupt("Please Review the Plan.")
 
+        # Handle None or empty feedback
+        if not feedback:
+            logger.warning(f"Received empty or None feedback: {feedback}. Returning to planner for new plan.")
+            return Command(goto="planner")
+
+        # Normalize feedback string
+        feedback_normalized = str(feedback).strip().upper()
+
         # if the feedback is not accepted, return the planner node
-        if feedback and str(feedback).upper().startswith("[EDIT_PLAN]"):
+        if feedback_normalized.startswith("[EDIT_PLAN]"):
+            logger.info(f"Plan edit requested by user: {feedback}")
             return Command(
                 update={
                     "messages": [
@@ -330,10 +339,11 @@ def human_feedback_node(
                 },
                 goto="planner",
             )
-        elif feedback and str(feedback).upper().startswith("[ACCEPTED]"):
+        elif feedback_normalized.startswith("[ACCEPTED]"):
             logger.info("Plan is accepted by user.")
         else:
-            raise TypeError(f"Interrupt value of {feedback} is not supported.")
+            logger.warning(f"Unsupported feedback format: {feedback}. Please use '[ACCEPTED]' to accept or '[EDIT_PLAN]' to edit.")
+            return Command(goto="planner")
 
     # if the plan is accepted, run the following node
     plan_iterations = state["plan_iterations"] if state.get("plan_iterations", 0) else 0
diff --git a/tests/integration/test_nodes.py b/tests/integration/test_nodes.py
index 074862e..f4a8de8 100644
--- a/tests/integration/test_nodes.py
+++ b/tests/integration/test_nodes.py
@@ -454,12 +454,37 @@ def test_human_feedback_node_accepted(monkeypatch, mock_state_base, mock_config)
 def test_human_feedback_node_invalid_interrupt(
     monkeypatch, mock_state_base, mock_config
 ):
-    # interrupt returns something else, should raise TypeError
+    # interrupt returns something else, should gracefully return to planner (not raise TypeError)
     state = dict(mock_state_base)
     state["auto_accepted_plan"] = False
     with patch("src.graph.nodes.interrupt", return_value="RANDOM_FEEDBACK"):
-        with pytest.raises(TypeError):
-            human_feedback_node(state, mock_config)
+        result = human_feedback_node(state, mock_config)
+        assert isinstance(result, Command)
+        assert result.goto == "planner"
+
+
+def test_human_feedback_node_none_feedback(
+    monkeypatch, mock_state_base, mock_config
+):
+    # interrupt returns None, should gracefully return to planner
+    state = dict(mock_state_base)
+    state["auto_accepted_plan"] = False
+    with patch("src.graph.nodes.interrupt", return_value=None):
+        result = human_feedback_node(state, mock_config)
+        assert isinstance(result, Command)
+        assert result.goto == "planner"
+
+
+def test_human_feedback_node_empty_feedback(
+    monkeypatch, mock_state_base, mock_config
+):
+    # interrupt returns empty string, should gracefully return to planner
+    state = dict(mock_state_base)
+    state["auto_accepted_plan"] = False
+    with patch("src.graph.nodes.interrupt", return_value=""):
+        result = human_feedback_node(state, mock_config)
+        assert isinstance(result, Command)
+        assert result.goto == "planner"
 
 
 def test_human_feedback_node_json_decode_error_first_iteration(