fix(desktop): stop model-switch markers duplicating user messages - #68665
Closed
Dolverin wants to merge 1 commit into
Closed
fix(desktop): stop model-switch markers duplicating user messages#68665Dolverin wants to merge 1 commit into
Dolverin wants to merge 1 commit into
Conversation
preserveLocalPendingTurnMessages pairs local optimistic rows with the authoritative transcript by role ordinal. The gateway persists model-switch notices as role=user (tui_gateway/server.py), so a stored [System:] marker between two real user turns shifts every later user ordinal: the optimistic row misses its committed copy and is appended again at the end of the transcript — the duplicate user bubble stacked at the bottom of the chat. Skip gateway system markers on both sides of the ordinal pairing. Fixes NousResearch#67603
Collaborator
|
Superseded by #69861, which carries this marker-exclusion fix (credited to @Dolverin via Heads-up on why it needed reshaping: since this PR was opened, the compression fix #69682 added a |
OutThisLife
added a commit
that referenced
this pull request
Jul 23, 2026
…bleed, and [System:] bubble (#69861) * fix(desktop): stop model-switch dup + route recovery resumes to the owning profile Fixes two Desktop session-reconciliation symptoms from #67603. Symptom 1 — duplicated user bubble after a model switch. The gateway persists model-switch / personality notices as role=user `[System: …]` rows (tui_gateway/server.py) so strict OpenAI-compatible providers don't reject a non-leading system message (#48338). `preserveLocalPendingTurnMessages` paired local optimistic rows with the stored transcript by user-role ordinal, so a marker between two real user turns shifted every later ordinal and the optimistic row was re-appended at the bottom. The single trailing-marker case is already covered by the compression-era `latestAuthoritativeUser` guard, but two switches around one turn (marker before AND after the committed prompt) still duplicated it. Exclude `[System:` bookkeeping markers from ordinal pairing on both sides. Symptom 2 — a session appearing under two profiles. The main resume path already resolves a session's owning profile via `resolveStoredSession` (cache → active backend → cross-profile probe), but the recovery `session.resume` calls (stale runtime id, session-not-found, wedged loop, redirect) omitted `profile`, so the gateway fell back to the launch-profile DB and forked the conversation into the wrong profile. Route every recovery resume — and an uncached right-click branch — through the same resolver so the profile is carried even for sessions outside the paginated sidebar window (the cache-miss gap). Tests: discriminating two-switch marker test (fails before, passes after); cache-hit + cross-profile cache-miss coverage for the recovery resume and for branching an uncached session. Supersedes #68665 and #63590. Closes #67603. Co-authored-by: Dolverin <5910064+Dolverin@users.noreply.github.com> Co-authored-by: oliviaaaa7788 <274182427+oliviaaaa7788@users.noreply.github.com> * fix(desktop): scope the remembered session id per profile A single global `hermes.desktop.lastSessionId` key remembered ONE session across every profile, so a relaunch or cold start under profile B would try to restore a session owned by profile A — reinforcing the impression that a conversation had bled between profiles (#67603, second symptom). Key the remembered id by the session's owning profile (resolved from the session row's `profile`, falling back to the active gateway profile), read it back for the active profile on restore, and clear an exhausted session under its owner. The default profile keeps the original unsuffixed key so existing installs' remembered session survives the upgrade. Co-authored-by: oliviaaaa7788 <oliviaaaa7788@users.noreply.github.com> * fix(gateway): hide [System:] bookkeeping markers from every transcript Model-switch and personality notices are persisted as role=user `[System: …]` rows so strict providers accept them mid-history, but they are model-facing runtime metadata, not user turns. `_history_to_messages` — the single display projection every client reads — passed them straight through, so on resume or reload they rendered as a fake user bubble in the desktop, TUI, CLI, and web transcripts. Drop them in that projection. The raw marker stays in `session["history"]` for the model, so nothing changes for inference; only the display loses a row that never belonged to the user. This also removes the stored marker from the payload the desktop reconciles against, killing the ordinal shift that duplicated the optimistic prompt (#67603) at its source — the desktop-side marker exclusion remains as a fallback for older backends. Co-authored-by: Dolverin <Dolverin@users.noreply.github.com> --------- Co-authored-by: Dolverin <5910064+Dolverin@users.noreply.github.com> Co-authored-by: oliviaaaa7788 <274182427+oliviaaaa7788@users.noreply.github.com> Co-authored-by: oliviaaaa7788 <oliviaaaa7788@users.noreply.github.com> Co-authored-by: Dolverin <Dolverin@users.noreply.github.com>
pull Bot
pushed a commit
to licoba/hermes-agent
that referenced
this pull request
Jul 23, 2026
…wning profile Fixes two Desktop session-reconciliation symptoms from NousResearch#67603. Symptom 1 — duplicated user bubble after a model switch. The gateway persists model-switch / personality notices as role=user `[System: …]` rows (tui_gateway/server.py) so strict OpenAI-compatible providers don't reject a non-leading system message (NousResearch#48338). `preserveLocalPendingTurnMessages` paired local optimistic rows with the stored transcript by user-role ordinal, so a marker between two real user turns shifted every later ordinal and the optimistic row was re-appended at the bottom. The single trailing-marker case is already covered by the compression-era `latestAuthoritativeUser` guard, but two switches around one turn (marker before AND after the committed prompt) still duplicated it. Exclude `[System:` bookkeeping markers from ordinal pairing on both sides. Symptom 2 — a session appearing under two profiles. The main resume path already resolves a session's owning profile via `resolveStoredSession` (cache → active backend → cross-profile probe), but the recovery `session.resume` calls (stale runtime id, session-not-found, wedged loop, redirect) omitted `profile`, so the gateway fell back to the launch-profile DB and forked the conversation into the wrong profile. Route every recovery resume — and an uncached right-click branch — through the same resolver so the profile is carried even for sessions outside the paginated sidebar window (the cache-miss gap). Tests: discriminating two-switch marker test (fails before, passes after); cache-hit + cross-profile cache-miss coverage for the recovery resume and for branching an uncached session. Supersedes NousResearch#68665 and NousResearch#63590. Closes NousResearch#67603. Co-authored-by: Dolverin <5910064+Dolverin@users.noreply.github.com> Co-authored-by: oliviaaaa7788 <274182427+oliviaaaa7788@users.noreply.github.com>
pierrenode
added a commit
to pierrenode/hermes-agent
that referenced
this pull request
Jul 28, 2026
…ume preview _display_resumed_history() (the CLI's "Previous Conversation" recap shown on /resume and hermes --resume) is a separate display projection from tui_gateway/server.py::_history_to_messages (the one desktop/TUI/web read). It special-cases the display_kind values it happens to know about, but two classes of bookkeeping/scaffolding rows fell through to the plain role=="user" branch and rendered as fake You: bubbles: 1. Personality-change/clear marker — tui_gateway.server's _set_personality() writes a plain role=user "[System: ...]" row with no display_kind. _history_to_messages already hides it via _is_display_hidden_marker() (a role + "[System:" prefix check, independent of display_kind); the CLI's recap never picked up the same check. 2. /skill invocations — persisted expanded (activation note plus the entire skill body). _history_to_messages already collapses this onto the invocation the user typed via describe_skill_invocation(); the CLI's recap showed up to 300 raw characters of the expanded body as if the user had written it. This is the third time this general bug class has been closed without covering the CLI path — NousResearch#68665 / NousResearch#69861 fixed the duplicate-bubble symptom on desktop, and a same-day sibling commit widened _history_to_messages for "desktop, TUI, CLI, and web transcripts" even though that function is never on the CLI's `hermes --resume` code path. Fix: - Personality marker: same role == "user" + "[System:" prefix check as _is_display_hidden_marker(), added to _display_resumed_history()'s history loop. Positioned after the display_kind special cases (so model_switch's "◈ model changed" event line is unaffected) and before the plain role == "user" handling. - Skill invocations: describe_skill_invocation() (from agent.skill_commands, the same helper tui_gateway/server.py uses) is called on role=="user" content; a match renders as "◈ skill invoked: /work — fix the leak" instead of the expanded body, consistent with the existing model-switch / async-delegation event lines. Deliberately not importing tui_gateway.server to reuse _is_display_hidden_marker directly — that module is not otherwise on the CLI's import graph and is a large gateway-oriented module. The duplicated prefix check is commented to point back at _is_display_hidden_marker so the two stay in sync if the marker wording/role ever changes. Tests: 4 regression tests (personality-change marker, personality-cleared marker, skill invocation with instruction, bare skill invocation) in tests/cli/test_resume_display.py — all assert the raw scaffolding never appears and the collapsed/hidden form renders correctly, while real conversation turns around them still render. Mutation-verified: each fix reverted independently, confirmed both fail with the exact bug's failure mode.
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…wning profile Fixes two Desktop session-reconciliation symptoms from NousResearch#67603. Symptom 1 — duplicated user bubble after a model switch. The gateway persists model-switch / personality notices as role=user `[System: …]` rows (tui_gateway/server.py) so strict OpenAI-compatible providers don't reject a non-leading system message (NousResearch#48338). `preserveLocalPendingTurnMessages` paired local optimistic rows with the stored transcript by user-role ordinal, so a marker between two real user turns shifted every later ordinal and the optimistic row was re-appended at the bottom. The single trailing-marker case is already covered by the compression-era `latestAuthoritativeUser` guard, but two switches around one turn (marker before AND after the committed prompt) still duplicated it. Exclude `[System:` bookkeeping markers from ordinal pairing on both sides. Symptom 2 — a session appearing under two profiles. The main resume path already resolves a session's owning profile via `resolveStoredSession` (cache → active backend → cross-profile probe), but the recovery `session.resume` calls (stale runtime id, session-not-found, wedged loop, redirect) omitted `profile`, so the gateway fell back to the launch-profile DB and forked the conversation into the wrong profile. Route every recovery resume — and an uncached right-click branch — through the same resolver so the profile is carried even for sessions outside the paginated sidebar window (the cache-miss gap). Tests: discriminating two-switch marker test (fails before, passes after); cache-hit + cross-profile cache-miss coverage for the recovery resume and for branching an uncached session. Supersedes NousResearch#68665 and NousResearch#63590. Closes NousResearch#67603. Co-authored-by: Dolverin <5910064+Dolverin@users.noreply.github.com> Co-authored-by: oliviaaaa7788 <274182427+oliviaaaa7788@users.noreply.github.com>
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…ile session bleed, and [System:] bubble (NousResearch#69861) * fix(desktop): stop model-switch dup + route recovery resumes to the owning profile Fixes two Desktop session-reconciliation symptoms from NousResearch#67603. Symptom 1 — duplicated user bubble after a model switch. The gateway persists model-switch / personality notices as role=user `[System: …]` rows (tui_gateway/server.py) so strict OpenAI-compatible providers don't reject a non-leading system message (NousResearch#48338). `preserveLocalPendingTurnMessages` paired local optimistic rows with the stored transcript by user-role ordinal, so a marker between two real user turns shifted every later ordinal and the optimistic row was re-appended at the bottom. The single trailing-marker case is already covered by the compression-era `latestAuthoritativeUser` guard, but two switches around one turn (marker before AND after the committed prompt) still duplicated it. Exclude `[System:` bookkeeping markers from ordinal pairing on both sides. Symptom 2 — a session appearing under two profiles. The main resume path already resolves a session's owning profile via `resolveStoredSession` (cache → active backend → cross-profile probe), but the recovery `session.resume` calls (stale runtime id, session-not-found, wedged loop, redirect) omitted `profile`, so the gateway fell back to the launch-profile DB and forked the conversation into the wrong profile. Route every recovery resume — and an uncached right-click branch — through the same resolver so the profile is carried even for sessions outside the paginated sidebar window (the cache-miss gap). Tests: discriminating two-switch marker test (fails before, passes after); cache-hit + cross-profile cache-miss coverage for the recovery resume and for branching an uncached session. Supersedes NousResearch#68665 and NousResearch#63590. Closes NousResearch#67603. Co-authored-by: Dolverin <5910064+Dolverin@users.noreply.github.com> Co-authored-by: oliviaaaa7788 <274182427+oliviaaaa7788@users.noreply.github.com> * fix(desktop): scope the remembered session id per profile A single global `hermes.desktop.lastSessionId` key remembered ONE session across every profile, so a relaunch or cold start under profile B would try to restore a session owned by profile A — reinforcing the impression that a conversation had bled between profiles (NousResearch#67603, second symptom). Key the remembered id by the session's owning profile (resolved from the session row's `profile`, falling back to the active gateway profile), read it back for the active profile on restore, and clear an exhausted session under its owner. The default profile keeps the original unsuffixed key so existing installs' remembered session survives the upgrade. Co-authored-by: oliviaaaa7788 <oliviaaaa7788@users.noreply.github.com> * fix(gateway): hide [System:] bookkeeping markers from every transcript Model-switch and personality notices are persisted as role=user `[System: …]` rows so strict providers accept them mid-history, but they are model-facing runtime metadata, not user turns. `_history_to_messages` — the single display projection every client reads — passed them straight through, so on resume or reload they rendered as a fake user bubble in the desktop, TUI, CLI, and web transcripts. Drop them in that projection. The raw marker stays in `session["history"]` for the model, so nothing changes for inference; only the display loses a row that never belonged to the user. This also removes the stored marker from the payload the desktop reconciles against, killing the ordinal shift that duplicated the optimistic prompt (NousResearch#67603) at its source — the desktop-side marker exclusion remains as a fallback for older backends. Co-authored-by: Dolverin <Dolverin@users.noreply.github.com> --------- Co-authored-by: Dolverin <5910064+Dolverin@users.noreply.github.com> Co-authored-by: oliviaaaa7788 <274182427+oliviaaaa7788@users.noreply.github.com> Co-authored-by: oliviaaaa7788 <oliviaaaa7788@users.noreply.github.com> Co-authored-by: Dolverin <Dolverin@users.noreply.github.com>
pierrenode
added a commit
to pierrenode/hermes-agent
that referenced
this pull request
Aug 11, 2026
…ume preview _display_resumed_history() (the CLI's "Previous Conversation" recap shown on /resume and hermes --resume) is a separate display projection from tui_gateway/server.py::_history_to_messages (the one desktop/TUI/web read). It special-cases the display_kind values it happens to know about, but two classes of bookkeeping/scaffolding rows fell through to the plain role=="user" branch and rendered as fake You: bubbles: 1. Personality-change/clear marker — tui_gateway.server's _set_personality() writes a plain role=user "[System: ...]" row with no display_kind. _history_to_messages already hides it via _is_display_hidden_marker() (a role + "[System:" prefix check, independent of display_kind); the CLI's recap never picked up the same check. 2. /skill invocations — persisted expanded (activation note plus the entire skill body). _history_to_messages already collapses this onto the invocation the user typed via describe_skill_invocation(); the CLI's recap showed up to 300 raw characters of the expanded body as if the user had written it. This is the third time this general bug class has been closed without covering the CLI path — NousResearch#68665 / NousResearch#69861 fixed the duplicate-bubble symptom on desktop, and a same-day sibling commit widened _history_to_messages for "desktop, TUI, CLI, and web transcripts" even though that function is never on the CLI's `hermes --resume` code path. Fix: - Personality marker: same role == "user" + "[System:" prefix check as _is_display_hidden_marker(), added to _display_resumed_history()'s history loop. Positioned after the display_kind special cases (so model_switch's "◈ model changed" event line is unaffected) and before the plain role == "user" handling. - Skill invocations: describe_skill_invocation() (from agent.skill_commands, the same helper tui_gateway/server.py uses) is called on role=="user" content; a match renders as "◈ skill invoked: /work — fix the leak" instead of the expanded body, consistent with the existing model-switch / async-delegation event lines. Deliberately not importing tui_gateway.server to reuse _is_display_hidden_marker directly — that module is not otherwise on the CLI's import graph and is a large gateway-oriented module. The duplicated prefix check is commented to point back at _is_display_hidden_marker so the two stay in sync if the marker wording/role ever changes. Tests: 4 regression tests (personality-change marker, personality-cleared marker, skill invocation with instruction, bare skill invocation) in tests/cli/test_resume_display.py — all assert the raw scaffolding never appears and the collapsed/hidden form renders correctly, while real conversation turns around them still render. Mutation-verified: each fix reverted independently, confirmed both fail with the exact bug's failure mode.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #67603 — Desktop duplicates a user message and stacks the copy at the bottom of the chat after a mid-session model switch.
Root cause
The gateway persists model-switch notices as
role=user(tui_gateway/server.py,_append_model_switch_marker) — deliberately, because strict OpenAI-compatible providers reject non-leading system messages (#48338). The authoritative transcript therefore contains one more user row than the live renderer state.preserveLocalPendingTurnMessages(apps/desktop/src/app/session/hooks/use-session-actions/utils.ts) pairs local optimistic rows with the authoritative transcript by role ordinal (user:N). A stored[System:]marker between two real user turns shifts every later user ordinal: the optimistic row (user-<ts>) no longer matches its committed copy (nextByRoleOrdinalreturns the marker, text mismatch), so it is treated as uncommitted and appended a second time at the end of the transcript — exactly the reported "same user message rendered twice, stacked at the bottom". DB inspection confirms the transcript itself holds only one copy (reported in #67603 and reproduced independently on Windows, v0.19.0: 1 DB row + 1 stored[System:]marker in the affected session).Fix
Skip gateway bookkeeping markers (
role=userrows whose text starts with[System:) when building the role-ordinal maps on both sides of the pairing, so ordinals count real user turns only. Sibling paths assessed:reconcileResumeMessagesuses the same ordinal scheme but its grafts are guarded by exact text equality, so shifted user ordinals are benign there (user rows carry no reasoning parts);preserveLocalAssistantErrorsmatches by id/text, not ordinals.Test plan
[System:]marker + live optimistic turn → fails before the fix (optimistic user row appended as duplicate), passes after (toBe(next), nothing appended)vitest run src/app/session/hooks/use-session-actions/utils.test.ts src/lib/chat-messages.test.ts→ 61/61 passtsc -p . --noEmitcleanmainwithout this change