fix(session): use server timestamps instead of IDs in runLoop exit condition#28637
fix(session): use server timestamps instead of IDs in runLoop exit condition#28637kagura-agent wants to merge 1 commit into
Conversation
…ndition The runLoop exit condition compared lastUser.id < lastAssistant.id to determine message ordering. Since message IDs encode Date.now() in their first 12 hex chars, this breaks when the web client provides a client-generated messageID timestamped ahead of the server clock. Replace the ID comparison with time.created comparison. Both User and Assistant time.created are set server-side via Date.now(), making them immune to client clock skew. Closes anomalyco#28618
|
The following comment was made by an LLM, it may be inaccurate: I found some potentially related PRs that addressed similar issues with message ID comparisons in the runLoop: Related PRs:
These PRs all appear to be working on the same area of code (the session prompt loop and message comparison logic). While they may have used different solutions (parentID, transcript position, message positions vs. timestamps), they're related to the same problem domain of fixing the exit condition. You may want to check if these prior approaches were considered or if there's any historical context that explains why the current timestamp comparison approach is preferred. |
Issue for this PR
Closes #28618
Type of change
What does this PR do?
The runLoop exit condition at
prompt.ts:1272useslastUser.id < lastAssistant.idto check whether the user message predates the assistant response. Message IDs encodeDate.now()in their first 12 hex chars. When the web client sends a client-generatedmessageID(timestamped with the browser clock) and the browser is ahead of the server, the user ID sorts after the assistant ID — the exit check fails, the loop fires a second LLM call with a<system-reminder>wrap.The fix replaces the ID comparison with
lastUser.time.created <= lastAssistant.time.created. Both fields are set server-side viaDate.now()when the message is created, so they are immune to client clock skew.How did you verify your code works?
User.time.createdis set at line 722 (time: { created: Date.now() }), always server-side regardless of whetherinput.messageIDis client-providedAssistant.time.createdis also set server-sidebun test test/session/— 39 pass, 16 fail (all pre-existing upstream failures, identical without this change)bun test test/session/message-v2.test.ts— 36/36 passScreenshots / recordings
N/A — backend logic change, no UI impact.
Checklist