developmentseed / developmentseed/mcp-toolsets-runtime

Two concurrent runs on one thread lose a turn, and both are told they succeeded

Open
#96 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Python
Stars
0
Forks
0
Avg merge
1d 18h
Merged PRs (30d)
39

Description

Two runs on one `thread_id` at the same time do not interleave. The one that finishes last wins, the other's turn is gone, and **both clients are told they succeeded** — including a `MESSAGES_SNAPSHOT` that claims the loser's turn is the thread.

### Reproduction

Two `POST /runs` against one `threadId`, launched together:

```
dA: RUN_FINISHED=True | its snapshot's user msgs: ['In one word: say APPLE']
dB: RUN_FINISHED=True | its snapshot's user msgs: ['In one word: say BANANA']

THREAD ACTUALLY HOLDS: ['In one word: say BANANA']
total messages: 2
```

APPLE's question and answer are not in the thread. Its client saw `RUN_FINISHED` and a snapshot asserting otherwise.

Against a real PostgreSQL checkpointer, on 0.7.0, with a real model.

### Why the snapshot makes it worse

`stream_turn` reads the thread with `aget_state` at the start and the graph writes it back at the end, so this is a lost update. That much is inherent to the current model.

What #86 changed is the reporting: the closing `MESSAGES_SNAPSHOT` exists to tell a client "this is the thread as the server holds it". For the losing run that statement is false at the moment it is sent, and it is the one event a client is told to trust over its own copy. Before #86 a client could not detect the loss either, but nothing had promised it anything.

### Not obviously the checkpointer's to fix

LangGraph's checkpointer does not lock a thread, and `AsyncPostgresSaver` writes per super-step rather than transactionally over a turn — so the two runs' writes interleave at the checkpoint level and the message reducer resolves to whichever wrote last.

Options, roughly in order of cost:

- **Document it.** A caller queues turns per thread. Cheapest, and honest — dss's `docs/API.md` now says so. Does nothing for a second browser tab.
- **Reject the overlap.** Track in-flight `thread_id`s in the process and answer the second run `409`. Correct for one replica, wrong the moment there are two.
- **Detect it.** Compare the checkpoint id seen at run start with the one at write time and fail the turn that lost. Needs a checkpointer that reports it, but it makes the failure visible rather than silent.
- **Serialise on the store.** An advisory lock per thread. Real fix, real cost, and only for Postgres.

Worth deciding what the contract *is* before choosing: "one run per thread at a time, enforced" or "concurrent runs, last writer wins, and we say so".

### Related

- The snapshot contract is #86.
- Found while widening dss onto 0.7.0 (ecmwf/dss-agentic-ai-services#130), which documents the constraint as a caveat for integrators.

Contributor guide

Open the contributing guide

Research direction

Start with stream_turn, the checkpointer calls described in the issue, and the snapshot contract in #86; compare that flow with the queueing caveat in dss's docs/API.md. First establish whether the intended contract is enforced single-run execution or documented last-writer-wins behavior. Done means the chosen contract is explicit and the reported success and snapshot behavior no longer contradict it.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.