From b500ccc3113b2405098e2cd16a161f8cae41ce5f Mon Sep 17 00:00:00 2001 From: jfilipiuk Date: Mon, 17 Aug 2026 09:10:37 +0200 Subject: [PATCH] fix: prevent session-emptying crash on resume commands (#428) * fix: prevent session-emptying crash on resume commands * docs: fix stale docstrings in goto=None crash tests and patch - Correct checkpoint corruption claim: the error state replaces the previous conversation state (messages: [], files: {}), it IS corrupted. - Replace WebUI-specific language with UI-agnostic wording. - Remove references to uncommitted local notes files. - Add upstream issue reference (langchain-ai/langgraph#5656). * fix: wrap _control_branch instead of reimplementing, use dataclasses.replace, add END routing and session preservation tests * fix: rebind map_cmd on already-loaded consumers, rewrite crash-path tests to exercise __start__ via checkpoint deletion --- EvoScientist/llm/patches.py | 78 ++++++++ tests/test_command_goto_none_crash.py | 251 ++++++++++++++++++++++++++ 2 files changed, 329 insertions(+) create mode 100644 tests/test_command_goto_none_crash.py diff --git a/EvoScientist/llm/patches.py b/EvoScientist/llm/patches.py index dcd30aa..c239517 100644 --- a/EvoScientist/llm/patches.py +++ b/EvoScientist/llm/patches.py @@ -14,6 +14,8 @@ Patches: - _patch_openrouter_strip_responses_reasoning: drop OpenAI-Responses encrypted reasoning items (rs_* id) from outgoing OpenRouter messages (store=false → "Item with id rs_... not found") + - _patch_langgraph_goto_none_crash: handle Command(goto=None) + defensively in langgraph and langgraph-api (langgraph 1.2.9 workaround) Utilities: - _is_ccproxy_codex: detect ccproxy Codex OAuth adapter @@ -22,8 +24,10 @@ Utilities: from __future__ import annotations +import dataclasses import functools import os +import sys from collections.abc import ( AsyncIterator, Awaitable, @@ -1235,3 +1239,77 @@ def _patch_deepagents_model_passthrough() -> None: ds_mod._build_start_tool = _patched_build_start ds_mod._build_update_tool = _patched_build_update _model_passthrough_patched = True + + +# --------------------------------------------------------------------------- +# Patch: LangGraph 1.2.9 _control_branch crashes with TypeError when +# Command.goto is None (e.g. from a resume command). We patch both the +# generator (map_cmd) and the consumer (_control_branch). +# --------------------------------------------------------------------------- +def _patch_langgraph_goto_none_crash() -> None: + """Handle Command(goto=None) defensively in langgraph and langgraph-api. + + Either patch alone prevents the production crash; both together cover + all code paths (a Command(goto=None) could in principle be created + by code other than map_cmd). + + Matches upstream issue langchain-ai/langgraph#5656. + """ + # ── 1. map_cmd — prevent goto=None from being created ──────────────── + try: + import langgraph_api.command as _cmd_mod + from langgraph.types import Command as _Command + + _orig_map_cmd = getattr(_cmd_mod, "map_cmd", None) + if _orig_map_cmd and not hasattr(_orig_map_cmd, "_evosci_goto_none_fix"): + + @functools.wraps(_orig_map_cmd) + def _patched_map_cmd(cmd: Any) -> _Command: + result = _orig_map_cmd(cmd) + if result.goto is None: + return dataclasses.replace(result, goto=[]) + return result + + _patched_map_cmd._evosci_goto_none_fix = True # type: ignore[attr-defined] + _cmd_mod.map_cmd = _patched_map_cmd + # Rebind on already-loaded consumers that grabbed map_cmd via + # ``from langgraph_api.command import map_cmd`` — those hold the + # original reference and wouldn't see the module-level rebind. + for _name in ("langgraph_api.stream", "langgraph_api.grpc.ops.threads"): + _mod = sys.modules.get(_name) + if _mod is not None and getattr(_mod, "map_cmd", None) is _orig_map_cmd: + _mod.map_cmd = _patched_map_cmd + except ImportError: + pass + + # ── 2. _control_branch — handle goto=None defensively ─────────────── + # Wrap (not reimplement) the upstream function so only the goto=None→[] + # normalization is ours; all other routing logic delegates to the + # original, avoiding drift as upstream evolves. + try: + import langgraph.graph.state as _state_mod + from langgraph.types import Command as _Command + + _orig_ctrl = getattr(_state_mod, "_control_branch", None) + if _orig_ctrl and not hasattr(_orig_ctrl, "_evosci_goto_none_fix"): + + def _fix_goto(cmd: Any) -> Any: + if isinstance(cmd, _Command) and cmd.goto is None: + return dataclasses.replace(cmd, goto=[]) + return cmd + + @functools.wraps(_orig_ctrl) + def _patched_ctrl(value: Any) -> Sequence[tuple[str, Any]]: + if isinstance(value, (list, tuple)): + value = [_fix_goto(v) for v in value] + else: + value = _fix_goto(value) + return _orig_ctrl(value) + + _patched_ctrl._evosci_goto_none_fix = True # type: ignore[attr-defined] + _state_mod._control_branch = _patched_ctrl + except (ImportError, AttributeError): + pass + + +_patch_langgraph_goto_none_crash() diff --git a/tests/test_command_goto_none_crash.py b/tests/test_command_goto_none_crash.py new file mode 100644 index 0000000..671b1f8 --- /dev/null +++ b/tests/test_command_goto_none_crash.py @@ -0,0 +1,251 @@ +"""Tests for the session-emptying ``Command(goto=None)`` crash. + +Root cause +---------- +When a client sends a ``command`` parameter (e.g. ``{"resume": {...}}``) in +the run request body **without** a ``goto`` field, ``langgraph_api.command.map_cmd`` +produces ``Command(goto=None, resume=...)``. LangGraph 1.2.9's +``_control_branch`` (``langgraph/graph/state.py``) then tries to iterate +over ``None``:: + + goto_targets = ( + [command.goto] if isinstance(command.goto, (Send, str)) else command.goto + ) + for go in goto_targets: # ← TypeError: 'NoneType' object is not iterable + +The crash happens at the ``__start__`` pseudo-node — before any model call, +before any middleware. The thread status is set to ``"error"`` and the +checkpoint ``values`` are reset to defaults (``messages: []``, ``files: {}``, +``async_tasks: {}``), corrupting the previous conversation state. Every UI +(WebUI, TUI, CLI) renders a blank session. + +These tests verify two properties: + +1. **No crash**: ``Command(goto=None)`` is handled gracefully (treated as + "no goto targets"), not as a ``TypeError``. +2. **Crash path coverage**: when a thread's checkpoint has been deleted + (mimicking a cancelled/rolled-back run), a subsequent ``goto=None`` + command reaches ``__start__`` and ``_control_branch`` — the exact crash + path. On ``main`` this crashes; with the patch it completes cleanly and + any update payload is applied. + +Matches upstream issue langchain-ai/langgraph#5656. +""" + +from __future__ import annotations + +from typing import Annotated + +import pytest +from langgraph.checkpoint.memory import MemorySaver +from langgraph.graph import END, START, StateGraph +from langgraph.graph.message import add_messages +from langgraph.types import Command +from typing_extensions import TypedDict + +# Apply the upstream-workaround patches (map_cmd + _control_branch) so the +# tests verify the post-fix behaviour. +from EvoScientist.llm import patches # noqa: F401 (side-effect) + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +class _State(TypedDict): + messages: Annotated[list, add_messages] + + +@pytest.fixture +def minimal_graph(): + """A minimal compiled StateGraph with one node and a MemorySaver checkpointer. + + The checkpointer is required for ``Command(resume=...)`` — without it + LangGraph raises ``RuntimeError: Cannot use Command(resume=...) without + checkpointer`` before reaching the crash site. + """ + builder = StateGraph(_State) + builder.add_node("model", lambda s: s) + builder.add_edge(START, "model") + builder.add_edge("model", END) + return builder.compile(checkpointer=MemorySaver()) + + +@pytest.fixture +def thread_config(): + return {"configurable": {"thread_id": "test-thread"}} + + +# --------------------------------------------------------------------------- +# 1. map_cmd should not produce goto=None +# --------------------------------------------------------------------------- + + +class TestMapCmdGotoNone: + """``map_cmd`` must not emit ``Command(goto=None)``. + + A ``Command`` with ``goto=None`` crashes ``_control_branch`` when used as + graph input. The fix is to emit ``goto=[]`` so that the "no goto targets" + path is taken cleanly. + """ + + def test_resume_only_command_goto_is_empty_list(self): + """A resume command without goto must produce ``goto=[]``, not ``None``.""" + from langgraph_api.command import map_cmd + + result = map_cmd({"resume": {"status": "answered", "answers": ["yes"]}}) + + assert result.goto == [], ( + "map_cmd produces Command(goto=None) for resume-only commands, " + "which crashes _control_branch when used as graph input" + ) + + def test_update_only_command_goto_is_empty_list(self): + """An update command without goto must produce ``goto=[]``, not ``None``.""" + from langgraph_api.command import map_cmd + + result = map_cmd({"update": {"messages": [{"role": "user", "content": "hi"}]}}) + + assert result.goto == [], ( + "map_cmd produces Command(goto=None) for update-only commands, " + "which crashes _control_branch when used as graph input" + ) + + +# --------------------------------------------------------------------------- +# 2. Command(goto=None) as graph input should not crash +# --------------------------------------------------------------------------- + + +class TestCommandGotoNoneAsInput: + """Feeding ``Command(goto=None)`` as graph input should be handled + gracefully, not crash with ``TypeError``. + """ + + async def test_resume_command_does_not_crash(self, minimal_graph, thread_config): + """``Command(goto=None, resume=...)`` — the exact shape ``map_cmd`` + produces for a resume request — should not raise.""" + cmd = Command(goto=None, resume={"answer": "yes"}) + # Should complete without TypeError + await minimal_graph.ainvoke(cmd, config=thread_config) + + async def test_update_command_does_not_crash(self, minimal_graph, thread_config): + """``Command(goto=None, update=...)`` should also not raise.""" + cmd = Command( + goto=None, + update={"messages": [{"role": "user", "content": "hi"}]}, + ) + await minimal_graph.ainvoke(cmd, config=thread_config) + + async def test_normal_dict_input_still_works(self, minimal_graph, thread_config): + """Regression guard: regular dict input is unaffected.""" + result = await minimal_graph.ainvoke( + {"messages": [{"role": "user", "content": "hello"}]}, + config=thread_config, + ) + assert len(result["messages"]) == 1 + + async def test_command_with_goto_still_works(self, minimal_graph, thread_config): + """Regression guard: a Command with a valid goto is unaffected.""" + cmd = Command(goto="model", update={}) + result = await minimal_graph.ainvoke(cmd, config=thread_config) + assert len(result["messages"]) == 0, ( + "Command with valid goto should complete and return state" + ) + + +# --------------------------------------------------------------------------- +# 3. Crash path via __start__: goto=None command after checkpoint deletion +# --------------------------------------------------------------------------- + + +class TestGotoNoneCrashPath: + """Tests that exercise the actual crash path: ``_control_branch`` via the + ``__start__`` pseudo-node. + + The scenario mimics the production trigger: a thread that previously had + messages has its checkpoint deleted (e.g. by a cancelled/rolled-back run), + then a resume or update command with ``goto=None`` fires against the same + thread. With no checkpoint, ``is_resuming`` is ``False``, so LangGraph + takes the ``map_input`` path, writes the ``Command`` to the ``START`` + channel, and ``__start__`` fires — calling ``_control_branch`` with + ``Command(goto=None)``. + + On ``main`` (unpatched), this crashes with ``TypeError: 'NoneType' object + is not iterable`` and the thread is left in an error state with default + values. With the patch, ``goto=None`` is normalized to ``[]`` and the + command completes cleanly. + """ + + async def test_resume_after_checkpoint_deletion_does_not_crash( + self, minimal_graph, thread_config + ): + """Seed messages, delete the checkpoint (mimicking rollback), then + resume — reaches ``__start__`` with ``goto=None``, crashes on main.""" + # Seed: create a checkpoint with one message + await minimal_graph.ainvoke( + {"messages": [{"role": "user", "content": "hello"}]}, + config=thread_config, + ) + # Delete the checkpoint — mimics a cancelled/rolled-back run + minimal_graph.checkpointer.delete_thread( + thread_config["configurable"]["thread_id"] + ) + # Act: resume command with goto=None — __start__ fires, _control_branch + # is called with Command(goto=None). Crashes on main, completes with patch. + cmd = Command(goto=None, resume={"answer": "yes"}) + await minimal_graph.ainvoke(cmd, config=thread_config) + + async def test_update_after_checkpoint_deletion_applies_update( + self, minimal_graph, thread_config + ): + """Seed messages, delete the checkpoint (mimicking rollback), then + send an update command — the update must be applied despite going + through ``__start__`` with ``goto=None``.""" + # Seed: create a checkpoint with one message + await minimal_graph.ainvoke( + {"messages": [{"role": "user", "content": "hello"}]}, + config=thread_config, + ) + # Delete the checkpoint — mimics a cancelled/rolled-back run + minimal_graph.checkpointer.delete_thread( + thread_config["configurable"]["thread_id"] + ) + # Act: update command with goto=None — __start__ fires, _control_branch + # is called. Crashes on main, applies the update with patch. + cmd = Command( + goto=None, + update={"messages": [{"role": "user", "content": "world"}]}, + ) + await minimal_graph.ainvoke(cmd, config=thread_config) + # Assert: the update was applied (not lost to a crash) + state = await minimal_graph.aget_state(thread_config) + assert len(state.values.get("messages", [])) >= 1, ( + "Update was lost: no messages after goto=None command via __start__" + ) + assert state.values["messages"][0].content == "world" + + +# --------------------------------------------------------------------------- +# 4. _control_branch directly: END routing must not produce a branch +# --------------------------------------------------------------------------- + + +class TestControlBranchEndRouting: + """Regression guard: ``Command(goto=END)`` must not produce a spurious + ``branch:to:__end__`` channel. The original ``_control_branch`` skips + branching for ``END``; the patched version must preserve that behavior. + """ + + def test_goto_end_is_not_a_branch_target(self): + """``_control_branch(Command(goto=END))`` must return ``[]`` — END + is a terminal sentinel, not a node to branch to.""" + from langgraph.graph.state import _control_branch + + assert _control_branch(Command(goto=END)) == [] + + def test_goto_end_in_list_is_not_a_branch_target(self): + """Same check with ``goto=[END]`` (list form).""" + from langgraph.graph.state import _control_branch + + assert _control_branch(Command(goto=[END])) == []