fix(session): use parentID instead of timestamp for loop exit condition#21365
fix(session): use parentID instead of timestamp for loop exit condition#21365okottorika wants to merge 2 commits intoanomalyco:devfrom
Conversation
The loop exit condition compared user.id < assistant.id which breaks when client/server clocks diverge. Web UI generates message IDs client-side while server generates assistant IDs, causing clock skew that makes user IDs sort after assistant IDs. This triggers an infinite loop generating duplicate assistant messages. Fix checks assistant.parentID === user.id instead, which correctly identifies whether the assistant message is responding to the current user message regardless of ID ordering. Fixes anomalyco#21335 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
github-actions
bot
added
the
needs:compliance
|
The following comment was made by an LLM, it may be inaccurate: Potential Duplicate PRs FoundPR #21334 - fix(session): use parentID instead of timestamp for loop exit condition
PR #21361 - fix: prevent web ui session loop by removing client-side message id g...
PR #14307 - fix: use parentID matching instead of ID ordering for prompt loop exit and message rendering
PR #11869 - fix: server-generated message IDs to prevent client/server clock skew
The most likely duplicate is PR #21334, which uses the exact same solution ( |
okottorika
changed the title
github-actions
bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
The test was creating different IDs for parentID and user message ID due to the global counter incrementing on each Identifier.create() call. Create the user ID first and reuse it for both the user message and the assistant's parentID. Also added the missing text part for the assistant message to match the seed helper. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Great job @okottorika! But it seems like many have already reported this bug and many opened PRs to fix it. I had a look at the contributing policy and don't really understand what was blocking on these previous fixes, apart maybe from the maintainers' lack of time to review? @thdxr, @adamdotdevin Is there anything else needed from us to get this merged? |
2 participants
Issue for this PR
Closes #21335
Type of change
What does this PR do?
The Web UI triggers duplicate assistant responses when sending a prompt. The processing loop fails to exit because its condition
lastUser.id < lastAssistant.idrelies on timestamp-based ID ordering. Since the Web UI generates user message IDs client-side and the server generates assistant IDs independently, clock skew between the two breaks the exit condition, causing the loop to continue generating responses.The fix replaces the ID ordering check with
lastAssistant.parentID === lastUser.id, which directly verifies the assistant message belongs to the current user prompt. This field is already populated and does not require any schema changes.The TUI is not affected because it uses the synchronous
session.promptendpoint rather thansession.promptAsync.How did you verify your code works?
Added a test case in
packages/opencode/test/session/prompt-effect.test.tsthat simulates clock skew by creating messages with timestamps that would sort incorrectly under the old logic. The test confirms the loop exits correctly using theparentIDcheck regardless of ID ordering.The existing test "loop exits immediately when last assistant has stop finish" continues to pass since the seeded assistant message already has
parentIDset to the user message ID.Screenshots / recordings
N/A - no UI changes
Checklist