fix(hitl): answer invalid tool calls in server state on a parked close by Thanatos9404 · Pull Request #687 · EvoScientist/EvoScientist

Thanatos9404 · 2026-10-25T13:16:07Z

Description

Part of #509 (the first half).

Through the server gateway, thread state messages arrive as dicts. _unanswered_calls converts them with convert_to_messages, which keeps tool_calls but moves invalid_tool_calls into additional_kwargs. So when a parked HITL turn was closed, a malformed call on the last AI message got no result, and the close still checked out as clean. It now reads the invalid calls from the dict when the converted message has none. Message objects are handled as before.

Two new tests pass dict state through abandoned_tool_messages and close_parked_checkpoint. Both fail on main and pass here. The second half (cancelling the Rich CLI close) is left for a separate change.

Type of change

Checklist

I have read the Contributing Guidelines
This targets core functionality used by the majority of users (niche features belong in EvoSkills)
I have added/updated tests where applicable
uv run ruff check . passes
uv run pytest passes (on Windows, the symlink-privilege and git-config tests fail the same way on main)

Summary by CodeRabbit

Bug Fixes
- Improved recovery of paused workflows from checkpoints. Invalid tool calls stored in message metadata are now recognized when identifying unanswered calls.
- Checkpoint updates now return results for valid, ungated, and malformed tool calls. Invalid calls receive a result indicating that they were not executed, and completed calls are not incorrectly patched.

Through the server gateway, thread state messages arrive as dicts, and convert_to_messages moves their invalid_tool_calls into additional_kwargs. _unanswered_calls then missed a malformed call on the last AI message, so closing a parked HITL turn left it without a result while the verify step still passed. Read the invalid calls from the dict when the converted message has none. Part of EvoScientist#509.

coderabbitai · 2026-10-25T13:16:26Z

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 131a1049-7082-485c-bcf9-d763db297457

Commits

Reviewing files that changed from the base of the PR and between 8134b9a and 496f9f7.

Files selected for processing: 2

tests/test_hitl_loop.py
tests/test_parked_close.py

Files with no reviewable changes: 1

tests/test_hitl_loop.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

The unanswered-tool-call scan now reads invalid calls from AIMessage.additional_kwargs when the direct field is empty or absent. Tests cover abandoned calls and parked-state closure with valid and invalid calls.

Changes: Tool-call checkpoint state

Layer / File(s)	Summary
Find unanswered tool calls `EvoScientist/backends.py`, `tests/test_hitl_loop.py`, `tests/test_parked_close.py`	The scan reads invalid tool calls from `additional_kwargs` alongside valid calls. Tests cover abandoned invalid calls and parked-state closure with valid, ungated, and malformed calls.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: jfilipiuk, din0s

Merge Risk: Minimal · up to 496f9

No concrete merge-blocking risk remains for this change.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title accurately identifies the main change: handling invalid tool calls in server state when closing a parked HITL checkpoint.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

din0s

Hi @Thanatos9404,

Thank you for the PR. I just have one comment below:

convert_to_messages already keeps a dict's invalid_tool_calls in additional_kwargs, so read them from the converted message instead of zipping the raw messages back in.

din0s

cheers!

X-iZhang

Thanks for the fix, @Thanatos9404. One suggestion on the new close test, inline at tests/test_hitl_loop.py.

X-iZhang · 2026-10-26T14:54:32Z

+async def test_close_parked_checkpoint_answers_invalid_calls_in_dict_state():
+    """A server-backed close also answers the malformed call (#509)."""
+    from EvoScientist.backends import close_parked_checkpoint
+    from EvoScientist.gateway import GraphTarget
+
+    agent = FakeCheckpointAgent(
+        values={
+            "messages": [
+                {
+                    "type": "ai",
+                    "content": "",
+                    "tool_calls": [
+                        {
+                            "name": "execute",
+                            "args": {"command": "echo hi"},
+                            "id": "exec-1",
+                            "type": "tool_call",
+                        }
+                    ],
+                    "invalid_tool_calls": [
+                        {
+                            "name": "write_file",
+                            "args": "{not json",
+                            "id": "bad-1",
+                            "error": "parse",
+                            "type": "invalid_tool_call",
+                        }
+                    ],
+                }
+            ]
+        }
+    )
+
+    await close_parked_checkpoint(
+        FakeGraphGateway(checkpoint=agent),
+        GraphTarget(local_graph=None),
+        "t-dicts",
+        reason="round_limit",
+    )
+
+    patch = next(values for values, node in agent.updates if node == "tools")[
+        "messages"
+    ]
+    assert [message.tool_call_id for message in patch] == ["exec-1", "bad-1"]
+    assert "was not executed" in patch[1].content


This test runs on FakeGraphGateway, so it never goes through LangGraphServerGateway.get_state_snapshot / _thread_state_as_snapshot, the patch is written back as message objects rather than the dicts the server stores, and the thread is parked at tools rather than before tools as an approval close is. tests/test_parked_close.py already drives the close through the real LangGraphServerGateway on dict state, parked before tools (_ParkedServerThreads, _parked_server_state(), _server_gateway()), and holds the other invalid-call close tests.

Suggestion: drop this test and add the case there, next to test_the_close_through_the_server_gateway_classifies_like_the_local_one:

async def test_the_close_through_the_server_gateway_answers_an_invalid_call(): """A malformed call in the server's dict state is closed too (#509).""" state = _parked_server_state() state["values"]["messages"][-1]["invalid_tool_calls"] = [_invalid("bad-1")] threads = _ParkedServerThreads(threads=[], states={"t-srv": state}) await close_parked_checkpoint( _server_gateway(threads), GraphTarget(), "t-srv", reason="rejected" ) assert _results(_texts(threads.state_updates[1][1]["messages"])) == { "c1": ("asked", "rejected", True), "w1": ("not executed", "rejected", False), "bad-1": ("not executed", "rejected", False), } assert threads.states["t-srv"]["next"] == []

Makes sense, moved it in 496f9f7: the FakeGraphGateway test is gone and your test now sits in test_parked_close.py right after the classifies-like-the-local-one case. It fails with main's backends.py and passes on this branch.

Move the dict-state close test to test_parked_close.py so it runs through LangGraphServerGateway on the server's dict state, parked before tools.

X-iZhang requested review from X-iZhang and din0s October 26, 2026 10:20

din0s requested changes Oct 26, 2026

View reviewed changes

Comment thread EvoScientist/backends.py Resolved

din0s added bug Something isn't working needs changes Reviewers requested changes before merging external contribution Implementation from an external contributor labels Oct 26, 2026

refactor(hitl): read invalid tool calls from additional_kwargs

8134b9a

convert_to_messages already keeps a dict's invalid_tool_calls in additional_kwargs, so read them from the converted message instead of zipping the raw messages back in.

din0s approved these changes Oct 26, 2026

View reviewed changes

din0s removed the needs changes Reviewers requested changes before merging label Oct 26, 2026

X-iZhang reviewed Oct 26, 2026

View reviewed changes

test(hitl): close an invalid call through the server gateway

496f9f7

Move the dict-state close test to test_parked_close.py so it runs through LangGraphServerGateway on the server's dict state, parked before tools.

X-iZhang approved these changes Oct 26, 2026

View reviewed changes

X-iZhang merged commit cdfaa1d into EvoScientist:main Oct 26, 2026
10 checks passed

X-iZhang mentioned this pull request Oct 31, 2026

release: v0.4.0 #728

Merged

9 of 10 tasks

This was referenced Nov 11, 2026

Rich CLI and serve: closing a parked turn on the server backend cannot be interrupted #789

Open

Server-gateway HITL close drops invalid_tool_calls and cannot be cancelled #509

Closed

Navigation Menu

Repository navigation

fix(hitl): answer invalid tool calls in server state on a parked close - #687

fix(hitl): answer invalid tool calls in server state on a parked close#687
X-iZhang merged 3 commits into
EvoScientist:mainfrom
Thanatos9404:fix/hitl-close-dict-invalid-tool-calls

Thanatos9404 commented Oct 25, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 25, 2026 •

edited

Loading

Uh oh!

din0s left a comment

Uh oh!

Uh oh!

din0s left a comment

Uh oh!

X-iZhang left a comment

Uh oh!

X-iZhang Oct 26, 2026

Uh oh!

Thanatos9404 Oct 26, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Thanatos9404 commented Oct 25, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

din0s left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

din0s left a comment

Choose a reason for hiding this comment

Uh oh!

X-iZhang left a comment

Choose a reason for hiding this comment

Uh oh!

X-iZhang Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

Thanatos9404 Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Thanatos9404 commented Oct 25, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 25, 2026 •

edited

Loading