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 4413467c88..4e66433c1a 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 @@ -196,6 +196,10 @@ function preserveStructuralParts(message: ChatMessage, previous: ChatMessage): C // attachmentRefs — composer-side metadata; already reconciled in reconcileResumeMessages // serverRowSpan — backend rows the folded message covers; the older-page offset // accounting reads it, the transcript never paints it +// systemNotice — hydration's provenance flag for a backend-authored notice +// (a model switch, a process completion); the stale-transcript +// compare reads it, while the visible system row is painted +// from role + parts, and role is already COMPARED // // If your new field affects what the user sees in the transcript, add it to // COMPARED. If it's metadata that shouldn't trigger a re-render, add it to @@ -229,7 +233,7 @@ const COMPARED_FIELDS = [ 'durationS' ] as const -const IGNORED_FIELDS = ['attachmentRefs', 'parts', 'serverRowSpan'] as const +const IGNORED_FIELDS = ['attachmentRefs', 'parts', 'serverRowSpan', 'systemNotice'] as const // Compile-time check: every ChatMessagePart discriminant must be handled by // chatPartsEquivalent. If @assistant-ui adds a new part type, this fails tsc. diff --git a/apps/desktop/src/lib/chat-messages/hydration.ts b/apps/desktop/src/lib/chat-messages/hydration.ts index 31e886a2ca..68b0136ee8 100644 --- a/apps/desktop/src/lib/chat-messages/hydration.ts +++ b/apps/desktop/src/lib/chat-messages/hydration.ts @@ -140,6 +140,25 @@ function transcriptContent(displayKind: SessionMessage['display_kind'], content: return displayKind === 'hidden' ? null : content } +/** + * Backend-authored transcript notices. The gateway persists these itself and no + * view "sent" them, so they render as system rows but are not authored + * transcript content (see `ChatMessage.systemNotice`). + */ +const NOTICE_DISPLAY_KINDS = [ + 'model_switch', + 'async_delegation_complete', + 'process_complete', + 'auto_continue', + 'personality_switch', + // Hermes closing a failed turn, not the model speaking. + 'failed_turn' +] as const + +function isMachineNotice(displayKind: SessionMessage['display_kind']): boolean { + return displayKind !== undefined && (NOTICE_DISPLAY_KINDS as readonly string[]).includes(displayKind) +} + // A remote backend older than this app serves display_metadata as raw JSON text, // and `in` throws on a primitive — which used to fail the whole session resume. function parseDisplayMetadata(metadata: SessionMessage['display_metadata']): null | Record { @@ -348,16 +367,7 @@ export function toChatMessages(messages: SessionMessage[]): ChatMessage[] { timelineDisplayContent(message, displayContentForMessage(message.role, content)) ) - const displayRole = - message.display_kind === 'model_switch' || - message.display_kind === 'async_delegation_complete' || - message.display_kind === 'process_complete' || - message.display_kind === 'auto_continue' || - message.display_kind === 'personality_switch' || - // Hermes closing a failed turn, not the model speaking. - message.display_kind === 'failed_turn' - ? 'system' - : message.role + const displayRole = isMachineNotice(message.display_kind) ? 'system' : message.role // Persisted user turns carry `@image:` directive lines inline in // the text (see tui_gateway/server.py's persist-time rewrite). The @@ -496,6 +506,7 @@ export function toChatMessages(messages: SessionMessage[]): ChatMessage[] { ? { asyncResult: asyncResultBody(displayContentForMessage(message.role, message.content || content)) } : {}), ...(message.display_kind === 'process_complete' ? { asyncResultKind: 'process' as const } : {}), + ...(isMachineNotice(message.display_kind) ? { systemNotice: true } : {}), timestamp: earliestTimestamp(message.timestamp, ...parts.map(part => part.timestamp)), ...(rowId !== undefined ? { rowId } : {}), ...(pendingAbsorbedRows > 0 ? { serverRowSpan: pendingAbsorbedRows + 1 } : {}), diff --git a/apps/desktop/src/lib/chat-messages/types.ts b/apps/desktop/src/lib/chat-messages/types.ts index 082480ff3b..970bb04195 100644 --- a/apps/desktop/src/lib/chat-messages/types.ts +++ b/apps/desktop/src/lib/chat-messages/types.ts @@ -66,6 +66,13 @@ export type ChatMessage = { serverRowSpan?: number /** Emoji reactions on this message — one per author (see MessageReaction). */ reactions?: MessageReaction[] + /** Backend-authored transcript notice rather than a message any view sent: a + * model switch, an auto-continue, a background-process completion. It renders + * on the timeline like any other system row but belongs to no view, so the + * stale-transcript compare must not count it (see + * `messagesIfTranscriptBehind`) — counting it made one model switch report a + * second window ahead and refuse every send. */ + systemNotice?: boolean } export type GatewayEventPayload = { diff --git a/apps/desktop/src/lib/stale-transcript-guard.test.ts b/apps/desktop/src/lib/stale-transcript-guard.test.ts new file mode 100644 index 0000000000..89c6563ee6 --- /dev/null +++ b/apps/desktop/src/lib/stale-transcript-guard.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest' + +import type { SessionMessage } from '@/types/hermes' + +import { toChatMessages } from './chat-messages' +import { messagesIfTranscriptBehind } from './stale-transcript-guard' + +/** + * The guard refuses a send when the authoritative latest page holds MORE + * transcript than the window does, rather than forking the session (#65047). + * It measures that difference with `remoteChat.length > localMessages.length` + * after `toChatMessages`. + * + * A backend-authored NOTICE also arrives as a `ChatMessage` (`model changed`, + * `background agent work finished`): `tui_gateway/server.py` persists + * `display_kind=model_switch` with `role=user` on an in-place model switch, and + * hydration renders it as a system row. Nothing about it means another window + * exists — but it inflates the page by one message per event, so the guard + * reported "This window was behind another view of the same chat" to a user who + * had only switched models, refused the send, and said the same thing on every + * retry. Staleness must be measured in AUTHORED content, not array length. + */ + +const row = (over: Partial & Pick): SessionMessage => ({ + content: '', + timestamp: 1_700_000_000, + ...over +}) + +const userTurn = (id: number, text: string): SessionMessage => + row({ content: text, id, role: 'user', timestamp: 1_700_000_000 + id }) + +const assistantTurn = (id: number, text: string): SessionMessage => + row({ content: text, id, role: 'assistant', timestamp: 1_700_000_000 + id }) + +/** What an in-place model switch persists, exactly as the gateway writes it. */ +const modelSwitchNotice = (id: number): SessionMessage => + row({ content: 'switched to another model', display_kind: 'model_switch', id, role: 'user', timestamp: 1_700_000_000 + id }) + +describe('messagesIfTranscriptBehind', () => { + it('does not treat a backend-authored notice as another view being ahead', () => { + const windowMessages = toChatMessages([userTurn(1, 'ask'), assistantTurn(2, 'answer')]) + const page = toChatMessages([userTurn(1, 'ask'), assistantTurn(2, 'answer'), modelSwitchNotice(3)]) + + // The notice is a real message in the page: this is what skews a length compare. + expect(page).toHaveLength(3) + expect(page.map(message => message.role)).toEqual(['user', 'assistant', 'system']) + + expect(messagesIfTranscriptBehind(windowMessages, page)).toBeNull() + }) + + it('still refuses when another view advanced the chat with a real turn', () => { + const windowMessages = toChatMessages([userTurn(1, 'ask'), assistantTurn(2, 'answer')]) + + const page = toChatMessages([ + userTurn(1, 'ask'), + assistantTurn(2, 'answer'), + userTurn(4, 'sent from another window') + ]) + + expect(messagesIfTranscriptBehind(windowMessages, page)).not.toBeNull() + }) + + it('still refuses when a notice arrives alongside a reply this window never saw', () => { + const windowMessages = toChatMessages([userTurn(1, 'ask'), assistantTurn(2, 'answer')]) + + const page = toChatMessages([ + userTurn(1, 'ask'), + assistantTurn(2, 'answer'), + modelSwitchNotice(3), + assistantTurn(4, 'a reply this window never rendered') + ]) + + expect(messagesIfTranscriptBehind(windowMessages, page)).not.toBeNull() + }) + + it('is current when both sides hold the same notices and the same turns', () => { + const rows = [userTurn(1, 'ask'), assistantTurn(2, 'answer'), modelSwitchNotice(3)] + + expect(messagesIfTranscriptBehind(toChatMessages(rows), toChatMessages(rows))).toBeNull() + }) + + it('is current when the page is empty, and refreshes a window that holds nothing', () => { + const rows = [userTurn(1, 'ask'), assistantTurn(2, 'answer')] + + expect(messagesIfTranscriptBehind(toChatMessages(rows), [])).toBeNull() + expect(messagesIfTranscriptBehind([], toChatMessages(rows))).toEqual(toChatMessages(rows)) + }) +}) diff --git a/apps/desktop/src/lib/stale-transcript-guard.ts b/apps/desktop/src/lib/stale-transcript-guard.ts index b6ac5c9f7e..9c7faccbba 100644 --- a/apps/desktop/src/lib/stale-transcript-guard.ts +++ b/apps/desktop/src/lib/stale-transcript-guard.ts @@ -20,15 +20,26 @@ export function profileScopeForSessionOwner(owner: SessionOwnerScope): ProfileSc } } +/** + * Transcript content a view actually authored. Backend-written notices + * (`ChatMessage.systemNotice`) render on the timeline but belong to no view, so + * counting them reports a second window that does not exist: an in-place model + * switch alone refused every send with "this window was behind another view of + * the same chat". + */ +function authoredMessageCount(messages: ChatMessage[]): number { + return messages.reduce((count, message) => (message.systemNotice ? count : count + 1), 0) +} + /** * Chat messages to install when the authoritative latest page is ahead of the * local view. Null when the local view is current. * - * Length is compared after `toChatMessages`, so tool rows folded into an - * assistant bubble are not "ahead". A backfilled prefix is kept when the - * refreshed tail anchors inside it. Live stream ids that do not anchor still - * use length, so the window that just finished the turn is not blocked when - * the counts match. + * Authored content is compared after `toChatMessages`, so tool rows folded into + * an assistant bubble are not "ahead", and neither is a backend-authored + * notice. A backfilled prefix is kept when the refreshed tail anchors inside + * it. Live stream ids that do not anchor still use the count, so the window + * that just finished the turn is not blocked when the counts match. */ export function messagesIfTranscriptBehind( localMessages: ChatMessage[], @@ -43,12 +54,13 @@ export function messagesIfTranscriptBehind( } const grafted = graftRefreshedTailOntoBackfill(remoteChat, localMessages) + const localAuthored = authoredMessageCount(localMessages) if (grafted === remoteChat) { - return remoteChat.length > localMessages.length ? remoteChat : null + return authoredMessageCount(remoteChat) > localAuthored ? remoteChat : null } - return grafted.length > localMessages.length ? grafted : null + return authoredMessageCount(grafted) > localAuthored ? grafted : null } /**