anthropics / anthropics/anthropic-sdk-python

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

未关闭
#1,749 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
Python
星标
3.9k
派生
853
平均合并
1 天 18 小时
30 天内合并 PR
11

描述

### 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.

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。