fix(session): end the turn loop by reply parent, not message ID order - #38387
fix(session): end the turn loop by reply parent, not message ID order#38387Itsnotaka wants to merge 4 commits into
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: I found a potentially related PR: PR #35872: fix: prevent agent loop self-reply caused by non-monotonic message IDs This PR appears to address a very similar issue — preventing an agent loop caused by non-monotonic message IDs, which directly relates to PR #38387's core problem where client-supplied UUIDs break message ID ordering and cause an infinite turn loop. Both PRs are tackling the session loop exit condition issue stemming from message ID ordering problems. |
The loop exited only when `lastUser.id < lastAssistant.id`, and
`MessageV2.latest` picks the newest user/assistant by lexicographic ID.
Message IDs from `Identifier.ascending` sort by creation time, so that
holds for IDs this loop mints.
`session.promptAsync` lets a client supply the user message ID, and it is
validated only by `Schema.isStartsWith("msg")`. A client ID that does not
sort below the IDs minted afterwards is therefore permanently the maximum:
it is always `lastUser`, the exit test is never true, and the loop keeps
re-prompting the model with a finished conversation until the process is
restarted.
Compare `lastAssistant.parentID === lastUser.id` instead. Every assistant
message in this file is created with `parentID` set to the user message it
answers, so the check reads the same intent from data the loop owns and is
unaffected by the format of a client-supplied ID.
`session.promptAsync` accepts a caller-supplied user message ID and stores it as-is; `MessageID` validates only the `msg` prefix. Ordering-sensitive code such as `MessageV2.latest` then compares it against IDs minted by `Identifier.ascending`, so an ID in any other format sorts arbitrarily against the rest of the session. Add `Identifier.isAscending` and check a supplied ID at both prompt entry points, so a client that mints its own IDs gets a clear error at the call that introduces the problem instead of a session that misbehaves later.
9854b66 to
be070f6
Compare
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Issue for this PR
Closes #35741
Type of change
What does this PR do?
Fixes a session turn loop caused by treating message IDs as timestamps.
Clients may supply
messageID, and the public schema only requires themsgprefix. If that ID sorts after IDs created by the server,MessageV2.latestcan keep selecting an older user message. A completed assistant reply then fails the loop's ID-order exit check, so the model is prompted again.This change:
time.created, with ID as the same-millisecond tie-breakerparentIDmatches the latest user messageHow did you verify your code works?
bun test test/session --timeout 30000 --only-failures: 370 passed, 7 skipped, 1 todobun test test/session/message-v2.test.ts test/session/prompt.test.ts --timeout 30000 --only-failures: 94 passed, 1 skippedbun typecheckfrompackages/opencodebun turbo typecheck: 30 successful targetsgit diff --checkScreenshots / recordings
Not applicable. This changes session ordering and loop behavior without changing UI.
Checklist