Skip to content

fix(desktop): preserve redirect reply order - #81762

Closed
0809android wants to merge 1 commit into
NousResearch:mainfrom
0809android:agent/desktop-message-order
Closed

0809android wants to merge 1 commit into
NousResearch:mainfrom
0809android:agent/desktop-message-order

Conversation

@0809android

@0809android 0809android commented Aug 8, 2026

Copy link
Copy Markdown

Summary

Fix reply ordering when a prompt is redirected during an active response. The existing partial reply is sealed, the corrected user prompt stays after it, and the retried reply starts after the correction.

Validation

  • 119 targeted UI tests passed
  • Verified in Hermes Local Fix on macOS

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 8, 2026
@0809android
0809android marked this pull request as ready for review August 9, 2026 02:43
@spfcraze

spfcraze commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
The new index > latestUserIndex guard excludes the sealed partial from the completion fallback, so the aborted turn's late message.complete — the race the diff's own comment acknowledges — appends a duplicate assistant row after the correction instead of settling the partial in place.

Problems:

  • The redirect path seals the active stream (pending: false, interim: true), appends the correction at the tail, and sets streamId: null before awaiting the gateway. A message.complete for the aborted turn therefore reaches completeAssistantMessage — which reads const streamId = state.streamId — with the fallback path, not the streamId match.
  • The fallback now only matches assistant rows with index > latestUserIndex, and the sealed partial sits before the correction's user row, outside that window. The pre-existing interimBoundaryPending && finalContinuesInterim branch in apps/desktop/src/app/session/hooks/use-message-stream/index.ts — built to settle a sealed interim the completion continues — never sees the partial.
  • With no fallback match, a non-empty finalText appends newAssistantFromCompletion() after the correction: a second assistant row carrying the aborted turn's text.

Solution:
Keep the sealed partial eligible for the finalContinuesInterim settle when the redirect set interimBoundaryPending; the prefix match already keeps the retried turn's own completion from settling the partial, so the acknowledged late completion completes it in place instead of duplicating it.


Checked against 2b52de8 — the tip of agent/desktop-message-order when this was written — and 35cbad5, main at the same moment.

@0809android
0809android marked this pull request as draft August 9, 2026 12:44
@0809android

Copy link
Copy Markdown
Author

Closing after re-checking the Desktop history and current intent. #69739 deliberately established the correction-before-redirected-reply contract, and #73882 was closed for reversing that ordering. This PR changes the same contract without an issue or maintainer alignment, and the current patch also leaves a late-completion duplication race. I should not ask maintainers to review a transcript-contract redesign that was not agreed first.

@0809android 0809android closed this Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants