diff --git a/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts index dd1fe42633df..3c3bbbddd7fa 100644 --- a/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts +++ b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts @@ -3,7 +3,15 @@ import { useEffect, useRef } from 'react' import { closeActiveTab } from '@/app/chat/close-tab' import { storedSessionIdForNotification } from '@/lib/session-ids' import { respondToApprovalAction } from '@/store/native-notifications' -import { getRememberedRoute, getRememberedSessionId, setRememberedRoute, setRememberedSessionId } from '@/store/session' +import { $activeGatewayProfile } from '@/store/profile' +import { + $sessions, + getRememberedRoute, + getRememberedSessionId, + rememberedSessionProfile, + setRememberedRoute, + setRememberedSessionId +} from '@/store/session' import { onSessionsChanged } from '@/store/session-sync' import { openUpdatesWindow, startUpdatePoller, stopUpdatePoller } from '@/store/updates' import { isSecondaryWindow } from '@/store/windows' @@ -63,7 +71,10 @@ export function useDesktopIntegrations({ // you don't want to boot into a modal. useEffect(() => { if (routedSessionId) { - setRememberedSessionId(routedSessionId) + setRememberedSessionId( + routedSessionId, + rememberedSessionProfile($sessions.get(), routedSessionId, $activeGatewayProfile.get()) + ) } if (!isOverlayView(appViewForPath(locationPathname))) { @@ -92,7 +103,7 @@ export function useDesktopIntegrations({ return } - const last = getRememberedSessionId() + const last = getRememberedSessionId($activeGatewayProfile.get()) if (last) { navigate(sessionRoute(last), { replace: true }) @@ -100,8 +111,14 @@ export function useDesktopIntegrations({ }, [locationPathname, navigate]) useEffect(() => { - if (resumeExhaustedSessionId && getRememberedSessionId() === resumeExhaustedSessionId) { - setRememberedSessionId(null) + if (!resumeExhaustedSessionId) { + return + } + + const owner = rememberedSessionProfile($sessions.get(), resumeExhaustedSessionId, $activeGatewayProfile.get()) + + if (getRememberedSessionId(owner) === resumeExhaustedSessionId) { + setRememberedSessionId(null, owner) } }, [resumeExhaustedSessionId]) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx index f27fc5b750a5..a7de88bf4de3 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx @@ -3,6 +3,7 @@ import type { MutableRefObject } from 'react' import { useEffect, useRef } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { getSession } from '@/hermes' import { textPart } from '@/lib/chat-messages' import { $composerAttachments, $composerDraft, type ComposerAttachment, setComposerDraft } from '@/store/composer' import { $notifications, clearNotifications } from '@/store/notifications' @@ -25,6 +26,7 @@ import { uploadComposerAttachment, usePromptActions } from '.' vi.mock('@/hermes', () => ({ getProfiles: vi.fn(async () => ({ profiles: [] })), + getSession: vi.fn(), PROMPT_SUBMIT_REQUEST_TIMEOUT_MS: 1_800_000, setApiRequestProfile: vi.fn(), transcribeAudio: vi.fn() @@ -1834,6 +1836,99 @@ describe('usePromptActions sleep/wake session recovery', () => { expect(calls[2]?.params).toEqual({ session_id: RECOVERED_SESSION_ID, text: 'message after wake' }) }) + // #67603 (second symptom): a recovery resume must re-register on the session's + // OWNING profile. Resuming on whichever profile is live forks the conversation + // into the wrong profile's DB — the session then appears under both profiles. + it('carries the owning profile from the cache into the recovery resume', async () => { + setSessions(() => [sessionInfo({ id: STORED_SESSION_ID, profile: 'work' })]) + + const calls: { method: string; params?: Record }[] = [] + let submitAttempts = 0 + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + calls.push({ method, params }) + + if (method === 'prompt.submit') { + submitAttempts += 1 + + if (submitAttempts === 1) { + throw new Error('session not found') + } + + return {} as never + } + + if (method === 'session.resume') { + return { session_id: RECOVERED_SESSION_ID } as never + } + + return {} as never + }) + + let handle: HarnessHandle | null = null + await actRender( + (handle = h)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + storedSessionId={STORED_SESSION_ID} + /> + ) + + expect(await handle!.submitText('message after wake')).toBe(true) + expect(calls[1]?.params).toEqual({ session_id: STORED_SESSION_ID, source: 'desktop', profile: 'work' }) + + setSessions(() => []) + }) + + // The session lives on another profile and is outside the paginated sidebar + // cache: resolve it by id across profiles rather than resuming profile-blind. + it('resolves the owning profile across profiles when the session is not cached', async () => { + // module-factory vi.fn is not reset by restoreAllMocks — reset explicitly in + // the finally below so this resolved value never leaks into sibling tests. + setSessions(() => []) + vi.mocked(getSession).mockResolvedValue(sessionInfo({ id: STORED_SESSION_ID, profile: 'work' })) + + const calls: { method: string; params?: Record }[] = [] + let submitAttempts = 0 + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + calls.push({ method, params }) + + if (method === 'prompt.submit') { + submitAttempts += 1 + + if (submitAttempts === 1) { + throw new Error('session not found') + } + + return {} as never + } + + if (method === 'session.resume') { + return { session_id: RECOVERED_SESSION_ID } as never + } + + return {} as never + }) + + let handle: HarnessHandle | null = null + await actRender( + (handle = h)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + storedSessionId={STORED_SESSION_ID} + /> + ) + + expect(await handle!.submitText('message after wake')).toBe(true) + expect(calls[1]?.params).toEqual({ session_id: STORED_SESSION_ID, source: 'desktop', profile: 'work' }) + + vi.mocked(getSession).mockReset() + setSessions(() => []) + }) + it('background queue resume uses the queued stored id and leaves foreground runtime selected', async () => { const calls: { method: string; params?: Record }[] = [] let submitAttempts = 0 diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index f7edf24f14c1..1e314fe714a8 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -43,6 +43,7 @@ import type { ImageAttachResponse, SessionRedirectResponse } from '../../../types' +import { resolveSessionProfile } from '../use-session-actions/utils' import { applyBranchVisibility, @@ -601,9 +602,12 @@ export function usePromptActions({ if (isSessionNotFoundError(err) && selectedStoredSessionIdRef.current) { try { + const resumeProfile = await resolveSessionProfile(selectedStoredSessionIdRef.current) + const resumed = await requestGateway<{ session_id: string }>('session.resume', { session_id: selectedStoredSessionIdRef.current, - source: 'desktop' + source: 'desktop', + ...(resumeProfile ? { profile: resumeProfile } : {}) }) const recoveredId = resumed?.session_id @@ -700,9 +704,12 @@ export function usePromptActions({ // correction right after a reconnect isn't lost to the race. if (isSessionNotFoundError(err) && selectedStoredSessionIdRef.current) { try { + const resumeProfile = await resolveSessionProfile(selectedStoredSessionIdRef.current) + const resumed = await requestGateway<{ session_id: string }>('session.resume', { session_id: selectedStoredSessionIdRef.current, - source: 'desktop' + source: 'desktop', + ...(resumeProfile ? { profile: resumeProfile } : {}) }) const recoveredId = resumed?.session_id diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index deee5b0683ba..398a72f3be4a 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -24,6 +24,7 @@ import { setAwaitingResponse, setBusy, setMessages } from '@/store/session' import type { ClientSessionState } from '../../../types' import { sessionContextDrift } from '../session-context-drift' +import { resolveSessionProfile } from '../use-session-actions/utils' import { _submitInFlight, @@ -384,9 +385,14 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // background queue drain only has the durable id). Continue that target // conversation; only a genuine new-chat draft may create a new session. try { + // Re-register on the session's OWNING profile — resuming on whichever + // profile is live would fork the conversation into the wrong DB (#67603). + const resumeProfile = await resolveSessionProfile(targetStoredSessionId) + const resumed = await requestGateway<{ session_id: string }>('session.resume', { session_id: targetStoredSessionId, - source: 'desktop' + source: 'desktop', + ...(resumeProfile ? { profile: resumeProfile } : {}) }) const resumeDrift = sessionDriftReason() @@ -528,9 +534,12 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // backend loop (#55578 symptom d) rejects the submit even though // the stored session is fine — resume + retry instead of erroring // out and losing the session binding. + const resumeProfile = await resolveSessionProfile(recoverStoredSessionId) + const resumed = await requestGateway<{ session_id: string }>('session.resume', { session_id: recoverStoredSessionId, - source: 'desktop' + source: 'desktop', + ...(resumeProfile ? { profile: resumeProfile } : {}) }) const resumeRetryDrift = sessionDriftReason() diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx index 5188a11f8d26..f0a4139f7990 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx @@ -3,7 +3,7 @@ import type { MutableRefObject } from 'react' import { useEffect } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' -import { getSessionMessages, type SessionInfo } from '@/hermes' +import { getSession, getSessionMessages, type SessionInfo } from '@/hermes' import { createClientSessionState } from '@/lib/chat-runtime' import { clearSessionDraft, stashSessionDraft, takeSessionDraft } from '@/store/composer' import { $activeGatewayProfile, $newChatProfile, ensureGatewayProfile } from '@/store/profile' @@ -43,6 +43,7 @@ import { useSessionActions } from './use-session-actions' vi.mock('@/hermes', async importOriginal => ({ ...(await importOriginal>()), deleteSession: vi.fn(), + getSession: vi.fn(), getSessionMessages: vi.fn(), listAllProfileSessions: vi.fn(), setApiRequestProfile: vi.fn(), @@ -1088,6 +1089,39 @@ describe('branchStoredSession desktop source tagging', () => { source: 'desktop' }) }) + + // #67603: right-clicking a session outside the paginated sidebar window is a + // cache miss. Resolve its owning profile (cache → active → cross-profile) and + // swap to it before reading the transcript / creating the branch, so the fork + // is not created on whichever profile happens to be live. + it('resolves and swaps to the parent profile when the branched session is not cached', async () => { + setSessions([]) + vi.mocked(getSession).mockResolvedValue(storedSession({ id: 'stored-parent', message_count: 1, profile: 'work' })) + vi.mocked(getSessionMessages).mockResolvedValue({ + messages: [{ content: 'branch me', role: 'user', timestamp: 1 }], + session_id: 'stored-parent' + } as never) + + const requestGateway = vi.fn(async (method: string) => { + if (method === 'session.create') { + return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never + } + + return {} as never + }) + + let branchStoredSession: ((storedSessionId: string, sessionProfile?: string | null) => Promise) | null = + null + render( (branchStoredSession = branch)} requestGateway={requestGateway} />) + await waitFor(() => expect(branchStoredSession).not.toBeNull()) + + await expect(branchStoredSession!('stored-parent')).resolves.toBe(true) + + expect(ensureGatewayProfile).toHaveBeenCalledWith('work') + expect(getSessionMessages).toHaveBeenCalledWith('stored-parent', 'work') + + vi.mocked(getSession).mockReset() + }) }) // ── Warm-cache mapping integrity (the "open chat A, chat B loads" bug) ───────── diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 75aaf8cd6cab..a4ac8ec4ad72 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -1177,7 +1177,13 @@ export function useSessionActions({ async (storedSessionId: string, sessionProfile?: string | null): Promise => { clearNotifications() - const stored = $sessions.get().find(session => sessionMatchesStoredId(session, storedSessionId)) + // Right-clicking a session outside the paginated sidebar window is a cache + // miss: resolve it (cache → active backend → cross-profile) so the branch + // is created on the parent's OWNING profile, not whichever is live (#67603). + const stored = + $sessions.get().find(session => sessionMatchesStoredId(session, storedSessionId)) ?? + (sessionProfile ? undefined : await resolveStoredSession(storedSessionId)) + const profile = sessionProfile ?? stored?.profile try { diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts index d2fab21de6ca..2f0d7c05f5f9 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts @@ -361,6 +361,58 @@ describe('preserveLocalPendingTurnMessages', () => { expect(preserveLocalPendingTurnMessages(compressedAuthority, pollutedWarmCache)).toBe(compressedAuthority) }) + + // #67603: the gateway persists model-switch / personality notices as role=user + // ([System: …], tui_gateway/server.py). A single trailing marker is already + // handled by the latestAuthoritativeUser guard above, but TWO switches around + // one turn put a marker BEFORE the committed prompt (shifting its ordinal) and + // another AFTER it (so the prompt is no longer the last user row, so the text + // guard can't rescue it). Naive ordinal pairing then pairs the optimistic row + // against a marker, treats it as uncommitted, and re-appends it — the + // duplicated user bubble stacked at the bottom of the chat. + it('does not duplicate the optimistic prompt when markers bracket it (two model switches)', () => { + const marker = (name: string) => `[System: The active model for this chat has changed to ${name}.]` + + const previous = [ + msg('1-user', 'user', 'first'), + msg('2-assistant', 'assistant', 'first answer'), + msg('user-optimistic', 'user', 'second question') + ] + + const next = [ + msg('s1-user', 'user', 'first'), + msg('s2-assistant', 'assistant', 'first answer'), + msg('s3-marker', 'user', marker('k2')), + msg('s4-user', 'user', 'second question'), + msg('s5-assistant', 'assistant', 'second answer'), + msg('s6-marker', 'user', marker('k3')) + ] + + expect(preserveLocalPendingTurnMessages(next, previous)).toBe(next) + }) + + it('still keeps a genuinely uncommitted optimistic turn when a marker is present', () => { + const previous = [ + msg('1-user', 'user', 'first'), + msg('2-assistant', 'assistant', 'first answer'), + msg('user-optimistic', 'user', 'new question') + ] + + // The marker is persisted but the new prompt has not committed yet — the + // optimistic row must survive (marker exclusion must not over-correct). + const next = [ + msg('1-user-stored', 'user', 'first'), + msg('2-assistant-stored', 'assistant', 'first answer'), + msg('3-marker-stored', 'user', '[System: The active model for this chat has changed to k3.]') + ] + + expect(preserveLocalPendingTurnMessages(next, previous).map(message => message.id)).toEqual([ + '1-user-stored', + '2-assistant-stored', + '3-marker-stored', + 'user-optimistic' + ]) + }) }) describe('appendLiveSessionProjection', () => { diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts index f724a8f3e2cf..55781c3bcb7a 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts @@ -240,7 +240,17 @@ export function reconcileResumeMessages(nextMessages: ChatMessage[], previousMes * history, not in-flight work. The latest authoritative user confirms whether * that tail has persisted; any authoritative assistant at the same ordinal * supersedes the local stream. + * + * Gateway bookkeeping markers (the model-switch / personality notices written + * by tui_gateway/server.py) are persisted as role=user but are not user turns. + * They must not take part in ordinal pairing on either side: a stored marker + * between two real user turns shifts every later user ordinal, so the optimistic + * row misses its committed copy and is appended a second time at the end of the + * transcript — the duplicated user bubble of #67603. */ +const isGatewaySystemMarker = (message: ChatMessage): boolean => + message.role === 'user' && chatMessageText(message).trimStart().startsWith('[System:') + export function preserveLocalPendingTurnMessages( nextMessages: ChatMessage[], previousMessages: ChatMessage[] @@ -253,6 +263,10 @@ export function preserveLocalPendingTurnMessages( const nextRoleCounts = new Map() for (const message of nextMessages) { + if (isGatewaySystemMarker(message)) { + continue + } + const ordinal = nextRoleCounts.get(message.role) ?? 0 nextRoleCounts.set(message.role, ordinal + 1) nextByRoleOrdinal.set(`${message.role}:${ordinal}`, message) @@ -269,6 +283,10 @@ export function preserveLocalPendingTurnMessages( const preserved: ChatMessage[] = [] for (const message of previousMessages) { + if (isGatewaySystemMarker(message)) { + continue + } + const ordinal = previousRoleCounts.get(message.role) ?? 0 previousRoleCounts.set(message.role, ordinal + 1) @@ -497,6 +515,28 @@ export async function resolveStoredSession(storedSessionId: string): Promise { + if (!storedSessionId) { + return undefined + } + + const profile = (await resolveStoredSession(storedSessionId))?.profile?.trim() + + return profile || undefined +} + type SessionRuntimeStatePatch = Partial< Pick< ClientSessionState, diff --git a/apps/desktop/src/store/session.test.ts b/apps/desktop/src/store/session.test.ts index 6de3e67ed092..1bb0d304300f 100644 --- a/apps/desktop/src/store/session.test.ts +++ b/apps/desktop/src/store/session.test.ts @@ -11,10 +11,13 @@ import { $selectedStoredSessionId, $unreadFinishedSessionIds, applyConfiguredDefaultProjectDir, + getRememberedSessionId, mergeSessionPage, + rememberedSessionProfile, resolveComposerSessionKey, sessionPinId, setCurrentCwd, + setRememberedSessionId, setSelectedStoredSessionId, workspaceCwdForNewSession } from './session' @@ -399,3 +402,67 @@ describe('unread finished sessions', () => { expect($unreadFinishedSessionIds.get()).toEqual([]) }) }) + +describe('remembered session id (per profile)', () => { + beforeEach(() => { + localStorage.clear() + }) + + afterEach(() => { + localStorage.clear() + }) + + it('scopes the remembered session by profile so one profile cannot read another', () => { + setRememberedSessionId('work-session', 'ai-engineer') + setRememberedSessionId('personal-session', 'default') + + expect(getRememberedSessionId('ai-engineer')).toBe('work-session') + expect(getRememberedSessionId('default')).toBe('personal-session') + // A profile with nothing remembered does not inherit another's session. + expect(getRememberedSessionId('research')).toBeNull() + }) + + it('keeps the default profile on the legacy unsuffixed key for back-compat', () => { + // An existing install remembered its session under the pre-per-profile key. + localStorage.setItem('hermes.desktop.lastSessionId', 'legacy-session') + + expect(getRememberedSessionId('default')).toBe('legacy-session') + // Absent/blank profile normalizes to the default key too. + expect(getRememberedSessionId(undefined)).toBe('legacy-session') + expect(getRememberedSessionId('')).toBe('legacy-session') + expect(getRememberedSessionId(null)).toBe('legacy-session') + }) + + it('clearing one profile leaves the others intact', () => { + setRememberedSessionId('work-session', 'ai-engineer') + setRememberedSessionId('personal-session', 'default') + + setRememberedSessionId(null, 'ai-engineer') + + expect(getRememberedSessionId('ai-engineer')).toBeNull() + expect(getRememberedSessionId('default')).toBe('personal-session') + }) +}) + +describe('rememberedSessionProfile', () => { + it('keys by the session row owning profile, not the active one', () => { + const sessions = [session({ id: 'stored-1', profile: 'ai-engineer' })] + + expect(rememberedSessionProfile(sessions, 'stored-1', 'default')).toBe('ai-engineer') + }) + + it('matches on the lineage root so a compressed tip resolves its owner', () => { + const sessions = [session({ _lineage_root_id: 'root-1', id: 'tip-2', profile: 'work' })] + + expect(rememberedSessionProfile(sessions, 'root-1', 'default')).toBe('work') + }) + + it('falls back to the active profile for a session not yet in the list', () => { + expect(rememberedSessionProfile([], 'uncached', 'research')).toBe('research') + }) + + it('normalizes a blank active profile to default', () => { + expect(rememberedSessionProfile([], null, '')).toBe('default') + expect(rememberedSessionProfile([], null, null)).toBe('default') + }) +}) diff --git a/apps/desktop/src/store/session.ts b/apps/desktop/src/store/session.ts index 0f0a64ae6b47..0044a7b34a6f 100644 --- a/apps/desktop/src/store/session.ts +++ b/apps/desktop/src/store/session.ts @@ -26,10 +26,49 @@ const COMPOSER_FAST_KEY = 'hermes.desktop.composer.fast' // The last chat the user had open, so a relaunch lands back on it instead of an // empty new-chat. Stored (not runtime) id — the route is keyed by stored id. +// +// Scoped per profile: a single global key remembered ONE session across every +// profile, so relaunching (or a cold start) under profile B would try to +// restore a session that belongs to profile A — one of the ways a conversation +// appears to bleed between profiles (#63590). Each profile now remembers its +// own last session. The default profile keeps the original unsuffixed key so +// existing installs' remembered session survives the upgrade. const LAST_SESSION_KEY = 'hermes.desktop.lastSessionId' -export const getRememberedSessionId = (): null | string => storedString(LAST_SESSION_KEY) -export const setRememberedSessionId = (id: null | string) => persistString(LAST_SESSION_KEY, id) +function rememberedSessionKey(profile?: null | string): string { + const key = (profile ?? '').trim() + + return !key || key === 'default' ? LAST_SESSION_KEY : `${LAST_SESSION_KEY}.${key}` +} + +export const getRememberedSessionId = (profile?: null | string): null | string => + storedString(rememberedSessionKey(profile)) +export const setRememberedSessionId = (id: null | string, profile?: null | string) => + persistString(rememberedSessionKey(profile), id) + +/** + * The profile a routed session belongs to, for keying the remembered id. + * + * Prefer the owning profile recorded on the session row (the cross-profile + * aggregator tags each row), so the session is remembered under ITS profile + * even while a different one is live. Falls back to the active gateway profile + * for a session not yet in the in-memory list. + */ +export function rememberedSessionProfile( + sessions: readonly SessionInfo[], + sessionId: null | string, + activeProfile: null | string +): string { + if (sessionId) { + const owner = sessions.find(session => sessionMatchesStoredId(session, sessionId))?.profile?.trim() + + if (owner) { + return owner + } + } + + return (activeProfile ?? '').trim() || 'default' +} // The last non-overlay route (a page like /skills, or a session route), so a // relaunch lands back where you were instead of a bare new-chat. diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 161bdd671652..4c7b94dda2ac 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -1517,6 +1517,54 @@ def test_history_to_messages_renders_multimodal_content(): ] +def test_history_to_messages_hides_gateway_system_markers(): + # Model-switch / personality notices are persisted as role=user [System: …] + # rows so strict providers accept them mid-history, but they are model-facing + # metadata -- never a user turn. They must not render as a user bubble on any + # surface, and dropping them from the display projection also stops the + # stored marker from shifting the desktop's user-message ordinals and + # duplicating the optimistic prompt (#67603). + history = [ + {"role": "user", "content": "first question"}, + {"role": "assistant", "content": "first answer"}, + { + "role": "user", + "content": "[System: The active model for this chat has changed to k3.]", + }, + {"role": "user", "content": "second question"}, + {"role": "assistant", "content": "second answer"}, + { + "role": "user", + "content": ( + "[System: The user has changed the assistant's personality. " + "Adopt the new persona going forward.]" + ), + }, + ] + + assert server._history_to_messages(history) == [ + {"role": "user", "text": "first question"}, + {"role": "assistant", "text": "first answer"}, + {"role": "user", "text": "second question"}, + {"role": "assistant", "text": "second answer"}, + ] + + +def test_history_to_messages_keeps_real_user_bracket_text(): + # Only role=user rows whose text OPENS with the [System: marker sentinel are + # bookkeeping notices. A genuine user turn that merely mentions the token is + # a real message and stays visible. + history = [ + {"role": "user", "content": "why does [System: ...] show up in my chat?"}, + {"role": "assistant", "content": "it should not"}, + ] + + assert server._history_to_messages(history) == [ + {"role": "user", "text": "why does [System: ...] show up in my chat?"}, + {"role": "assistant", "text": "it should not"}, + ] + + def test_session_resume_uses_parent_lineage_for_display(monkeypatch): captured = {} diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 0488a5753b2d..096fb8b546ed 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -5670,6 +5670,20 @@ def _is_text_only_busy_payload(content: Any) -> bool: return False +def _is_display_hidden_marker(role: str | None, text: str) -> bool: + """Gateway bookkeeping notices (model-switch, personality) are persisted as + role=user ``[System: …]`` rows so strict providers accept them mid-history. + They are model-facing runtime metadata, not user turns, and must never + render as a user bubble in ANY client transcript (desktop, TUI, CLI, web). + + Filtering here — the single display projection every surface reads — hides + them everywhere while the raw marker stays in ``session["history"]`` for the + model. It also removes the stored marker from the payload the desktop + reconciles against, so it can no longer shift user-message ordinals and + duplicate the optimistic prompt (#67603).""" + return role == "user" and text.lstrip().startswith("[System:") + + def _history_to_messages(history: list[dict]) -> list[dict]: messages = [] tool_call_args = {} @@ -5681,6 +5695,8 @@ def _history_to_messages(history: list[dict]) -> list[dict]: if role not in {"user", "assistant", "tool", "system"}: continue content_text = _coerce_message_text(m.get("content")) + if _is_display_hidden_marker(role, content_text): + continue if role == "assistant" and m.get("tool_calls"): for tc in m["tool_calls"]: fn = tc.get("function", {})