langgenius / langgenius/dify

[Bug] Dify Agent can append non-terminal events after a terminal transition

Open
#40,765 0 comments 2 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
156k
Forks
24.6k
Avg merge
22h 9m
Merged PRs (30d)
610

Description

### Self Checks

- [x] I have read the [Contributing Guide](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md) and [Language Policy](https://github.com/langgenius/dify/issues/1542).
- [x] This is only for bug report, if you would like to ask a question, please head to [Discussions](https://github.com/langgenius/dify/discussions/categories/general).
- [x] I have searched for existing issues [search for existing issues](https://github.com/langgenius/dify/issues), including closed ones.
- [x] I confirm that I am using English to submit this report, otherwise it will be closed.
- [x] 【中文用户 & Non English User】请使用英语提交,否则会被关闭 :)
- [x] Please do not modify this template :) and fill in all the required fields.

### Dify version

`main` at `f8e05ebfc0789b2a83c25d610ebd1aca295931cb` (2026-08-14)

### Cloud or Self Hosted

Self Hosted (Source)

### Summary

The standalone Dify Agent runtime atomically commits the first terminal event and its matching run status, but non-terminal events are still appended through an unconditional Redis Stream `XADD`.

When a cancellation is accepted by one API process while the runner-owning process has an in-flight Pydantic AI event, the terminal event can be followed by a late non-terminal event:

```text
run_started -> run_cancelled -> pydantic_ai_event
```

This violates the event-stream contract that consumers can stop after the first terminal event.

### Steps to reproduce

1. Start a run through one `RedisRunStore` instance.
2. From a second Redis client, call `finalize_run()` with `RunCancelledEvent`.
3. From the original writer, call `append_event()` with `RunStartedEvent` or a Pydantic AI stream event.
4. Read the run stream with `get_events()`.

On current `main`, step 3 performs an unconditional `XADD`, so the returned sequence is:

```text
run_cancelled -> run_started
```

The same ordering can occur with a real in-flight model event during route-independent cancellation.

### ✔️ Expected Behavior

Once a terminal event wins, it seals the run stream. Later non-terminal writes should not persist, consume a cursor, or refresh retention.

### ❌ Actual Behavior

Late non-terminal writes remain accepted and can appear after the terminal cursor.

### Why this matters

- SSE consumers stop at the terminal event, while a later polling request can observe an additional event. The two observation paths can therefore disagree.
- A late event refreshes the stream and run-record TTL even though the run is already terminal.
- The cancellation observer reduces the race window, but it cannot remove the cross-process check-to-write race.

### Root cause

`RedisRunStore.finalize_run()` uses a Lua compare-and-set operation over the run record and event stream. In contrast, `RedisRunStore.append_event()` writes through a separate transaction without checking the durable run status.

The following interleaving is therefore possible:

1. Process A passes its local `is_cancelled()` check and prepares a stream event.
2. Process B atomically commits `run_cancelled` and changes the run record to `cancelled`.
3. Process A performs its unconditional `XADD` after the terminal cursor.

A status read in the runner would still be unsafe because cancellation can win between that read and `XADD`.

### Correctness invariant

For each run, non-terminal appends and terminal finalization should share a linearization point:

- If a non-terminal append wins first, it receives a lower cursor and the terminal event follows it.
- If terminal finalization wins first, the late non-terminal append is rejected without consuming a cursor or refreshing retention.
- The first terminal event remains both the only terminal event and the last retained event.

### Proposed implementation

- Deepen the existing `RunEventSink` contract: `append_event()` succeeds only while the durable run status is `running`.
- Replace the non-terminal Redis pipeline with a two-key Lua operation that performs `GET record -> status check -> XADD -> refresh both TTLs` atomically.
- Raise a runtime-owned coordination error carrying the winning terminal status when the stream is already sealed.
- Make `AgentRunRunner` treat that error as normal coordination and stop without attempting to append a second terminal event.
- Give the in-memory adapter the same observable behavior.

No Redis keys, event JSON, SSE cursors, HTTP DTOs, or protocol versions need to change.

### Verification plan

- In-memory tests for all three terminal statuses.
- Local Redis-adapter tests for accepted, sealed, and missing-record writes.
- Runner tests for a terminal winning before `run_started` and before an in-flight Pydantic AI event.
- Real-Redis tests with two clients for deterministic late writes and concurrent append/finalize races.
- Documentation for the terminal-is-last invariant and rolling-deployment limitation.

### Scope exclusions

This proposal does not add run recovery, worker handoff, tool-side-effect exactly-once semantics, a new Redis key, or a new event type. During a rolling deployment, the stronger invariant is available only after all event-writing processes have been upgraded.

I have a focused implementation and real-Redis race tests ready and would like to work on this if the direction is acceptable.

Contributor guide

Open the contributing guide

Research direction

Start with RedisRunStore.finalize_run() and append_event(), then trace the RunEventSink contract, AgentRunRunner, and the in-memory adapter. Run the proposed in-memory, Redis-adapter, runner, and real-Redis race tests. Done means a terminal event seals the stream, rejects later non-terminal writes without refreshing retention, and runner coordination handles that rejection normally.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, redis
Domain
backend, distributed-systems, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.