From e108955c20b2a4423ba9e5f974dfe6b45a83801f Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 11 Aug 2026 08:27:50 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B7=20ci:=20Enforce=20Durable=20Agent?= =?UTF-8?q?=20Finalization=20for=20E2E=20tests=20(#14740)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: enforce agent generation finalization * test(e2e): correlate canonical persisted turns --- e2e/specs/mock/agent-handoffs.spec.ts | 19 +-- e2e/specs/mock/agents.spec.ts | 4 +- e2e/specs/mock/chat.spec.ts | 16 +-- e2e/specs/mock/completion.spec.ts | 25 ++++ e2e/specs/mock/helpers.ts | 181 +++++++++++++++++++++++- e2e/specs/mock/mcp.spec.ts | 10 +- e2e/specs/mock/model-switching.spec.ts | 4 +- e2e/specs/mock/streaming.spec.ts | 4 +- e2e/specs/mock/subagent-results.spec.ts | 13 +- 9 files changed, 249 insertions(+), 27 deletions(-) create mode 100644 e2e/specs/mock/completion.spec.ts diff --git a/e2e/specs/mock/agent-handoffs.spec.ts b/e2e/specs/mock/agent-handoffs.spec.ts index f007319706..df3cbe3208 100644 --- a/e2e/specs/mock/agent-handoffs.spec.ts +++ b/e2e/specs/mock/agent-handoffs.spec.ts @@ -9,7 +9,7 @@ import { getAccessToken, messagesView, requestJson, - sendMessage, + sendMessageAndWaitForCompletion, } from './helpers'; const DESCRIPTION = 'Created by the mock end-to-end suite to verify agent handoffs.'; @@ -244,7 +244,7 @@ test.describe('agent handoffs', () => { await expect(page.getByRole('textbox', { name: 'Message input' })).toBeVisible(); const label = `scratch-bare-${Date.now()}`; - const response = await sendMessage( + const response = await sendMessageAndWaitForCompletion( page, handoffMarker(label, [ { @@ -714,7 +714,10 @@ test.describe('agent handoffs', () => { await selectAgentForChat(page, routerName); const noTransferLabel = `no-transfer-${Date.now()}`; - const noTransferResponse = await sendMessage(page, `E2E_REPLY:${noTransferLabel}`); + const noTransferResponse = await sendMessageAndWaitForCompletion( + page, + `E2E_REPLY:${noTransferLabel}`, + ); expect(noTransferResponse.ok()).toBeTruthy(); await expect( messagesView(page).getByText(`E2E reply ${noTransferLabel}`, { exact: true }), @@ -723,7 +726,7 @@ test.describe('agent handoffs', () => { messagesView(page).getByRole('button', { name: /^Transferred to / }), ).toHaveCount(0); - const response = await sendMessage( + const response = await sendMessageAndWaitForCompletion( page, handoffMarker(label, [ { @@ -774,7 +777,7 @@ test.describe('agent handoffs', () => { await expect(transferCard).toBeVisible(); const emptyLabel = `${label}-empty`; - const emptyResponse = await sendMessage( + const emptyResponse = await sendMessageAndWaitForCompletion( page, handoffMarker(emptyLabel, [ { @@ -839,7 +842,7 @@ test.describe('agent handoffs', () => { routerId = router.id; await selectAgentForChat(page, routerName); - const response = await sendMessage( + const response = await sendMessageAndWaitForCompletion( page, handoffMarker(label, [ { @@ -1000,7 +1003,7 @@ test.describe('agent handoffs', () => { routerId = router.id; await selectAgentForChat(page, routerName); - const response = await sendMessage( + const response = await sendMessageAndWaitForCompletion( page, handoffMarker(label, [ { @@ -1081,7 +1084,7 @@ test.describe('agent handoffs', () => { routerId = router.id; await selectAgentForChat(page, routerName); - const response = await sendMessage( + const response = await sendMessageAndWaitForCompletion( page, handoffMarker(label, [ { diff --git a/e2e/specs/mock/agents.spec.ts b/e2e/specs/mock/agents.spec.ts index 9a1fd8cfb0..e935b06dbb 100644 --- a/e2e/specs/mock/agents.spec.ts +++ b/e2e/specs/mock/agents.spec.ts @@ -8,7 +8,7 @@ import { uniqueAgentName, waitForPersistedAgent, } from './agents.helpers'; -import { MOCK_ENDPOINTS, mockReply, sendMessage } from './helpers'; +import { MOCK_ENDPOINTS, mockReply, sendMessageAndWaitForCompletion } from './helpers'; const DESCRIPTION = 'Use this agent to verify LibreChat agent creation in mock end-to-end tests.'; const INSTRUCTIONS = @@ -187,7 +187,7 @@ test.describe('agent builder', () => { await form.getByRole('button', { name: 'Select Agent' }).click(); - const response = await sendMessage(page, `hello from ${agentName}`); + const response = await sendMessageAndWaitForCompletion(page, `hello from ${agentName}`); expect(response.ok()).toBeTruthy(); await expect(mockReply(page)).toBeVisible({ timeout: 30000 }); } finally { diff --git a/e2e/specs/mock/chat.spec.ts b/e2e/specs/mock/chat.spec.ts index 4d05341b66..10ebb78754 100644 --- a/e2e/specs/mock/chat.spec.ts +++ b/e2e/specs/mock/chat.spec.ts @@ -9,7 +9,7 @@ import { replyText, replyPrompt, selectMockEndpoint, - sendMessage, + sendMessageAndWaitForCompletion, } from './helpers'; type UploadFixture = { @@ -94,7 +94,7 @@ test.describe('core chat loop', () => { await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); - const response = await sendMessage(page, userMessage); + const response = await sendMessageAndWaitForCompletion(page, userMessage); expect(response.ok()).toBeTruthy(); await expect(page.getByText(userMessage)).toBeVisible(); @@ -135,7 +135,7 @@ test.describe('core chat loop', () => { await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); - const response = await sendMessage(page, 'E2E_MARKDOWN_REPLY'); + const response = await sendMessageAndWaitForCompletion(page, 'E2E_MARKDOWN_REPLY'); expect(response.ok()).toBeTruthy(); const assistantMessage = messagesView(page) @@ -169,11 +169,11 @@ test.describe('core chat loop', () => { await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); - let response = await sendMessage(page, firstMessage); + let response = await sendMessageAndWaitForCompletion(page, firstMessage); expect(response.ok()).toBeTruthy(); await expect(mockReply(page)).toBeVisible(); - response = await sendMessage(page, followUpMessage); + response = await sendMessageAndWaitForCompletion(page, followUpMessage); expect(response.ok()).toBeTruthy(); await expect(page.getByText(followUpMessage)).toBeVisible(); @@ -206,10 +206,10 @@ test.describe('core chat loop', () => { await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); - let response = await sendMessage(page, firstMessage); + let response = await sendMessageAndWaitForCompletion(page, firstMessage); expect(response.ok()).toBeTruthy(); await expect(mockReply(page).first()).toBeVisible(); - response = await sendMessage(page, secondMessage); + response = await sendMessageAndWaitForCompletion(page, secondMessage); expect(response.ok()).toBeTruthy(); await expect(page.getByText(secondMessage)).toBeVisible(); @@ -273,7 +273,7 @@ test.describe('core chat loop', () => { // Build a three-turn thread (the "long running thread"), waiting for each // turn's unique reply to render before sending the next. for (const turn of turns) { - const response = await sendMessage(page, turn.prompt); + const response = await sendMessageAndWaitForCompletion(page, turn.prompt); expect(response.ok()).toBeTruthy(); await expect(messagesView(page).getByText(turn.reply)).toBeVisible({ timeout: 30000 }); } diff --git a/e2e/specs/mock/completion.spec.ts b/e2e/specs/mock/completion.spec.ts new file mode 100644 index 0000000000..bebc532a42 --- /dev/null +++ b/e2e/specs/mock/completion.spec.ts @@ -0,0 +1,25 @@ +import { expect, test } from '@playwright/test'; +import { + MOCK_ENDPOINTS, + NEW_CHAT_PATH, + messagesView, + selectMockEndpoint, + sendMessageAndWaitForCompletion, +} from './helpers'; + +test.describe('generation finalization invariant', () => { + test('rejects a persisted assistant error after successful generation admission', async ({ + page, + }) => { + test.setTimeout(90_000); + const label = `completion-error-${Date.now()}`; + + await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); + await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); + + await expect( + sendMessageAndWaitForCompletion(page, `E2E_FORCED_ERROR:${label}`), + ).rejects.toThrow('Persisted assistant response contains an unexpected error'); + await expect(messagesView(page).getByText(`E2E forced stream error ${label}`)).toBeVisible(); + }); +}); diff --git a/e2e/specs/mock/helpers.ts b/e2e/specs/mock/helpers.ts index 39bf0bf6f7..c4fba0bc2d 100644 --- a/e2e/specs/mock/helpers.ts +++ b/e2e/specs/mock/helpers.ts @@ -1,4 +1,6 @@ import { expect } from '@playwright/test'; +import { ContentTypes } from 'librechat-data-provider'; +import type { TMessage } from 'librechat-data-provider'; import type { Page, Response } from '@playwright/test'; /** Substring of the reply emitted by the mock LLM server. */ @@ -18,6 +20,16 @@ type RefreshTokenBody = { token?: string; }; +type AgentGenerationStart = { + conversationId?: string; +}; + +type CompletionOptions = { + timeout?: number; +}; + +const DEFAULT_COMPLETION_TIMEOUT = 20_000; + export function isAgentsStream(response: Response) { return isAgentGenerationStart(response); } @@ -90,7 +102,10 @@ export function mockReply(page: Page) { return messagesView(page).getByText(new RegExp(MOCK_REPLY_TEXT, 'i')); } -/** Type a message, send it, and wait for the streamed `/api/agents` response. */ +/** + * Type a message and wait only for generation admission. Use this lower-level + * helper when a test intentionally observes a live, paused, aborted, or failed run. + */ export async function sendMessage(page: Page, text: string): Promise { const input = page.getByRole('textbox', { name: 'Message input' }); await input.click(); @@ -102,6 +117,170 @@ export async function sendMessage(page: Page, text: string): Promise { return response; } +function formatPersistedMessages(messages: TMessage[]): string { + return JSON.stringify( + messages.map( + ({ content, error, isCreatedByUser, messageId, parentMessageId, text, unfinished }) => ({ + messageId, + parentMessageId, + isCreatedByUser, + unfinished, + error, + text: typeof text === 'string' ? text.slice(0, 200) : text, + content: content?.map((part) => ({ + type: part?.type, + ...(part?.type === ContentTypes.ERROR + ? { error: part[ContentTypes.ERROR], text: part.text } + : {}), + })), + }), + ), + null, + 2, + ); +} + +function conversationIdFromUrl(url: string): string | undefined { + const match = new URL(url).pathname.match(/^\/c\/([^/]+)\/?$/); + const conversationId = match?.[1]; + return conversationId && conversationId !== 'new' + ? decodeURIComponent(conversationId) + : undefined; +} + +/** + * Send a message and require the resulting assistant response to be durably finalized. + * A streamed answer is not success until its persisted message is terminal and error-free. + */ +export async function sendMessageAndWaitForCompletion( + page: Page, + text: string, + options: CompletionOptions = {}, +): Promise { + const token = await getAccessToken(page); + const existingConversationId = conversationIdFromUrl(page.url()); + /** The POST messageId is an optimistic UI placeholder; BaseClient persists a + * server-generated user ID. Snapshot history before admission so the new + * canonical user→assistant edge can be identified without matching prompt text. */ + const existingMessages = existingConversationId + ? await fetchJson( + page, + `/api/messages/${encodeURIComponent(existingConversationId)}`, + token, + ) + : []; + const existingMessageIds = new Set(existingMessages.map((message) => message.messageId)); + + const response = await sendMessage(page, text); + const start = (await response.json()) as AgentGenerationStart; + const conversationId = start.conversationId; + + if (!conversationId || conversationId === 'new') { + throw new Error( + `Generation admission did not identify a persisted turn: ${JSON.stringify({ + conversationId, + })}`, + ); + } + if (existingConversationId && existingConversationId !== conversationId) { + throw new Error( + `Generation admission changed conversations unexpectedly: ${JSON.stringify({ + existingConversationId, + conversationId, + })}`, + ); + } + + let assistantMessages: TMessage[] = []; + let newMessages: TMessage[] = []; + let latestMessages: TMessage[] = []; + let latestReadError: string | undefined; + + try { + await expect + .poll( + async () => { + try { + latestMessages = await fetchJson( + page, + `/api/messages/${encodeURIComponent(conversationId)}`, + token, + ); + latestReadError = undefined; + } catch (error) { + latestReadError = error instanceof Error ? error.message : String(error); + return false; + } + + newMessages = latestMessages.filter( + (message) => !existingMessageIds.has(message.messageId), + ); + const userMessageIds = new Set( + newMessages + .filter((message) => message.isCreatedByUser === true) + .map((message) => message.messageId), + ); + assistantMessages = newMessages.filter( + (message) => + message.isCreatedByUser === false && + message.parentMessageId != null && + userMessageIds.has(message.parentMessageId), + ); + return ( + userMessageIds.size > 0 && + assistantMessages.length > 0 && + assistantMessages.every((message) => message.unfinished === false) + ); + }, + { + timeout: options.timeout ?? DEFAULT_COMPLETION_TIMEOUT, + intervals: [250, 500, 1_000], + message: 'new assistant response should be durably finalized', + }, + ) + .toBe(true); + } catch (error) { + const pollError = error instanceof Error ? error.message : String(error); + throw new Error( + [ + 'Timed out waiting for the new assistant response to be durably finalized.', + latestReadError ? `Latest message read failed: ${latestReadError}` : undefined, + `Pre-existing message IDs: ${JSON.stringify([...existingMessageIds])}`, + `New persisted messages: ${formatPersistedMessages(newMessages)}`, + `Persisted messages: ${formatPersistedMessages(latestMessages)}`, + pollError, + ] + .filter(Boolean) + .join('\n'), + ); + } + + const failedMessage = assistantMessages.find( + (message) => + message.error === true || + message.content?.some((part) => part?.type === ContentTypes.ERROR) === true, + ); + if (failedMessage) { + throw new Error( + `Persisted assistant response contains an unexpected error: ${formatPersistedMessages([ + failedMessage, + ])}`, + ); + } + + if (!existingConversationId) { + await expect + .poll(() => conversationIdFromUrl(page.url()), { + timeout: 5_000, + intervals: [100, 250, 500], + message: 'new conversation route should use the admitted conversation ID', + }) + .toBe(conversationId); + } + + return response; +} + export async function getAccessToken(page: Page): Promise { const result = await page.evaluate(async () => { const response = await fetch('/api/auth/refresh', { diff --git a/e2e/specs/mock/mcp.spec.ts b/e2e/specs/mock/mcp.spec.ts index 632bf2c5c7..5aebaf14b8 100644 --- a/e2e/specs/mock/mcp.spec.ts +++ b/e2e/specs/mock/mcp.spec.ts @@ -8,7 +8,13 @@ import { uniqueAgentName, waitForPersistedAgent, } from './agents.helpers'; -import { MOCK_ENDPOINTS, fetchJson, getAccessToken, mockReply, sendMessage } from './helpers'; +import { + MOCK_ENDPOINTS, + fetchJson, + getAccessToken, + mockReply, + sendMessageAndWaitForCompletion, +} from './helpers'; const MCP_SERVER_NAME = 'e2e-memory'; const MCP_TOOL_NAME = 'remember_fact'; @@ -135,7 +141,7 @@ test.describe('agent builder MCP tools', () => { await reopenedForm.getByRole('button', { name: 'Select Agent' }).click(); - const response = await sendMessage(page, `hello from ${agentName}`); + const response = await sendMessageAndWaitForCompletion(page, `hello from ${agentName}`); expect(response.ok()).toBeTruthy(); await expect(mockReply(page)).toBeVisible({ timeout: 30000 }); } finally { diff --git a/e2e/specs/mock/model-switching.spec.ts b/e2e/specs/mock/model-switching.spec.ts index 7c9a62de90..2bbed04f11 100644 --- a/e2e/specs/mock/model-switching.spec.ts +++ b/e2e/specs/mock/model-switching.spec.ts @@ -4,7 +4,7 @@ import { NEW_CHAT_PATH, mockReply, selectMockEndpoint, - sendMessage, + sendMessageAndWaitForCompletion, } from './helpers'; test.describe('endpoint switching', () => { @@ -15,7 +15,7 @@ test.describe('endpoint switching', () => { await selectMockEndpoint(page, endpoint); - const response = await sendMessage(page, `hello ${endpoint.model}`); + const response = await sendMessageAndWaitForCompletion(page, `hello ${endpoint.model}`); expect(response.ok()).toBeTruthy(); await expect(mockReply(page)).toBeVisible(); }); diff --git a/e2e/specs/mock/streaming.spec.ts b/e2e/specs/mock/streaming.spec.ts index f8f6c7d90f..360b35e437 100644 --- a/e2e/specs/mock/streaming.spec.ts +++ b/e2e/specs/mock/streaming.spec.ts @@ -4,7 +4,7 @@ import { NEW_CHAT_PATH, messagesView, selectMockEndpoint, - sendMessage, + sendMessageAndWaitForCompletion, } from './helpers'; const ORDERED_PIECE_COUNT = 64; @@ -23,7 +23,7 @@ test.describe('stream transport fidelity', () => { await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); - const response = await sendMessage(page, `E2E_ORDERED_REPLY:${label}`); + const response = await sendMessageAndWaitForCompletion(page, `E2E_ORDERED_REPLY:${label}`); expect(response.ok()).toBeTruthy(); const assistantContent = messagesView(page) diff --git a/e2e/specs/mock/subagent-results.spec.ts b/e2e/specs/mock/subagent-results.spec.ts index 26d08b4380..382fc9bddf 100644 --- a/e2e/specs/mock/subagent-results.spec.ts +++ b/e2e/specs/mock/subagent-results.spec.ts @@ -2,7 +2,13 @@ import { expect, test } from '@playwright/test'; import type { Page } from '@playwright/test'; import type { AgentDetail } from './agents.helpers'; import { cleanupAgent, openAgentBuilder, uniqueAgentName } from './agents.helpers'; -import { MOCK_ENDPOINTS, getAccessToken, messagesView, requestJson, sendMessage } from './helpers'; +import { + MOCK_ENDPOINTS, + getAccessToken, + messagesView, + requestJson, + sendMessageAndWaitForCompletion, +} from './helpers'; async function createAgent( page: Page, @@ -57,7 +63,10 @@ test.describe('isolated subagent results', () => { parentId = parent.id; await selectAgent(page, parentName); - const response = await sendMessage(page, `E2E_SUBAGENT_RESULT:${child.id}:${label}`); + const response = await sendMessageAndWaitForCompletion( + page, + `E2E_SUBAGENT_RESULT:${child.id}:${label}`, + ); expect(response.ok()).toBeTruthy(); const expected = `E2E subagent streamed result ${label}`;