Skip to content

Conversation

@khj809
Copy link

@khj809 khj809 commented Jan 31, 2026

What does this PR do?

  1. Fixed a bug where lastUser is incorrectly identified in multi-turn situations and exits the loop prematurely without generating a response.
    In the below:

    for (let i = msgs.length - 1; i >= 0; i--) {
    const msg = msgs[i]
    if (!lastUser && msg.info.role === "user") lastUser = msg.info as MessageV2.User
    msgs are traversed in reverse order, but msgs is not guaranteed to be chronologically ordered. The loop then identifies the wrong lastUser and the exit condition is incorrectly evaluated.
    Fix: Sort msgs by created timestamp to ensure chronological order

  2. Changed the loop exit condition from lexicographical ID comparison to a parent-child relationship check to prevent infinite loops.
    In the below:

    if (
    lastAssistant?.finish &&
    !["tool-calls", "unknown"].includes(lastAssistant.finish) &&
    lastUser.id < lastAssistant.id
    ) {
    even if lastUser and lastAssistant are correctly found, if lastUser.id is lexicographically higher than lastAssistant.id, it does not break the loop and continues the steps infinitely.
    Fix: Change exit condition from ID comparison to lastFinished.parentID === lastUser.id, which properly checks if the finished message is a response to the user message

How did you verify your code works?

It was hard to reproduce the issue, since sometimes the conditions are evaluated as expected by chance. I tested a multi-turn conversation scenario several times by logging msgs and the IDs of messages.

@github-actions
Copy link
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions
Copy link
Contributor

The following comment was made by an LLM, it may be inaccurate:

Based on my search results, I found one potentially related PR:

PR #7425: fix(session): sort messages by time.created instead of alphabetical order

This PR appears to be addressing a similar issue - sorting messages by creation timestamp instead of alphabetical order. This relates directly to the first fix in PR #11443 where messages need to be sorted chronologically by the created timestamp to ensure correct lastUser identification.

The other results (PRs #4710, #7756, #10123) are unrelated feature additions to session handling and don't appear to be duplicates of the specific bug fixes in the current PR.

@khj809 khj809 mentioned this pull request Jan 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant