fix(session): use parentID instead of timestamp for loop exit condition#21294
fix(session): use parentID instead of timestamp for loop exit condition#21294YLRong wants to merge 1 commit intoanomalyco:devfrom
Conversation
Replace timestamp-based comparison with parentID check to prevent duplicate assistant responses caused by clock skew between client and server. When client and server have different system times, the ID-based comparison lastUser.id < lastAssistant.id fails, causing the prompt loop to continue and generate a second assistant response. This fix uses the explicit parentID relationship to determine if the assistant message is a response to the last user message, which is immune to clock skew issues. For backward compatibility, the timestamp comparison is kept as a fallback for messages that may not have parentID set. Fixes anomalyco#14935
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: I found some related PRs worth investigating: Potential Related PRs:
These PRs (especially #17010) may already be addressing the same clock skew issue with parentID checks. You should verify whether:
|
|
Thanks for the automated check! I've reviewed the related PRs mentioned (#17010 and #11443). Comparison with existing PRsPR #17010 by @ShenAC-SAC addresses the same core issue with an identical approach using Key differences in this PR (#21294):
If the maintainers prefer #17010's approach, I'm happy to close this in favor of that PR. Otherwise, the backward compatibility in this PR might be worth considering for safer rollout. cc: @ShenAC-SAC @khj809 |
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
Issue for this PR
Closes #14935
Type of change
What does this PR do?
Fixes duplicate assistant responses caused by clock skew between client and server systems.
Problem
When client and server have different system times (clock skew), the prompt loop's exit condition
lastUser.id < lastAssistant.idfails because message IDs are timestamp-based. This causes the loop to continue and generate a second assistant response for the same user message.The issue occurs when:
Solution
Replace timestamp-based comparison with explicit
parentIDrelationship check. The assistant message'sparentIDfield already stores the ID of the user message it responds to, making this a more reliable indicator than timestamp comparison.Changes:
packages/opencode/src/session/prompt.tsHow did you verify your code works?
I reproduced and verified the fix in my local environment:
The fix uses the existing
parentIDfield which is already populated correctly, so no database migration or data changes are needed.Checklist