anthropics / anthropics/anthropic-sdk-python

SessionToolRunner can re-execute a tool call after a stream reconnect if its result post failed

Đang mở
#1,749 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Python
Star
3.9k
Fork
853
Merge trung bình
1 ngày 18 giờ
Pull request đã merge (30 ngày)
11

Mô tả

### Summary

`SessionToolRunner` (the implementation behind `client.beta.sessions.events.tool_runner()`) can execute a tool call **twice** for the same `tool_use_id` when a stream reconnect happens after a result-post attempt has failed permanently (or exhausted its retries). For a side-effecting tool — `bash`, a file write, anything in `agent_toolset` — this is an at-least-once instead of at-most-once execution hazard: the same shell command can run a second time.

### Root cause

`_reconcile()` (called on every `_stream_loop` reconnect, including after any transient disconnect) re-lists the session's history and re-enqueues every `agent.tool_use` / `agent.custom_tool_use` event whose id is not yet in `self._answered`:

```python
unanswered = [ev for ev in pending if ev.id not in self._answered]
...
for ev in unanswered:
await self._send_work.send(ev)
```

`self._answered` is only populated when `_send_result` successfully posts the result. A call whose post permanently failed (e.g. a 400) or exhausted `SEND_RETRIES` is therefore indistinguishable, to `_reconcile`, from a call that was never dispatched at all — it gets sent back through `_send_work`, and `_dispatch_loop` hands it to `_execute`, which unconditionally re-runs the tool:

```python
async def _execute(self, ev: DispatchedToolUseEvent) -> None:
...
else:
content = await run_runnable_tool(tool, input_) # <-- runs again
...
```

Any stream disconnect (network blip, load balancer timeout, a redeploy) that happens to land after a tool ran but before its result was successfully posted will trigger this on the next `_reconcile()` pass.

### Reproduction

Deterministic, no live API needed — using the existing `FakeAsyncEvents` test harness in `tests/lib/tools/test_session_runner.py`:

```python
@pytest.mark.asyncio()
async def test_reconnect_does_not_re_execute_tool_after_failed_send(monkeypatch):
monkeypatch.setattr(session_runner_mod, "STREAM_BACKOFF_START", 0.001)
counter = {"calls": 0}

async def increment(_input):
counter["calls"] += 1
return "done"

tool = _FakeTool("inc", increment)
events = FakeAsyncEvents(
list_events=[_tool_use("tu_1", "inc", {})],
streams=[
_FakeStream([_StubEvent("noop")], raise_after=1, raise_with=_api_status_error(500)),
_FakeStream([_terminated()]),
],
send_failures=[_api_status_error(400)],
)

[_ async for _ in _run_with_fakes(events=events, tools=[tool])]

assert counter["calls"] == 1 # currently fails: counter["calls"] == 2
```

On current `main` (`d2f6543`) this fails with `counter["calls"] == 2` — the `inc` tool runs twice for `tu_1`.

### Expected behavior

Once a tool has run for a given `tool_use_id`, the runner should never run it again — a reconcile-triggered re-dispatch of an id whose tool already executed should only retry *posting* the already-computed result, not recompute it.

### Impact

Any self-hosted `EnvironmentWorker` / `SessionToolRunner` deployment using tools with side effects (the bundled `agent_toolset` bash/file tools, or a custom tool) is exposed to this on ordinary network instability, not just an unusual edge case — no multiagent/subagent setup or MCP involvement required to trigger it.

I have a fix + regression test ready and will open a PR shortly.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.