Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions client/src/components/Agents/MarketplaceContext.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'react';
import { ChatContext } from '~/Providers';
import { useChatHelpers } from '~/hooks';
import { ChatProvider } from '~/hooks/Chat/provider';

/**
* Minimal marketplace provider that provides only what SidePanel actually needs
Expand Down Expand Up @@ -38,12 +37,8 @@ export function useMarketplaceHost(): MarketplaceHost {
return host;
}

export const MarketplaceProvider: React.FC<MarketplaceProviderProps> = ({ children, host }) => {
const chatHelpers = useChatHelpers(0, 'new');

return (
<ChatContext.Provider value={chatHelpers}>
<MarketplaceHostContext.Provider value={host}>{children}</MarketplaceHostContext.Provider>
</ChatContext.Provider>
);
};
export const MarketplaceProvider: React.FC<MarketplaceProviderProps> = ({ children, host }) => (
<ChatProvider index={0} conversationId="new">
<MarketplaceHostContext.Provider value={host}>{children}</MarketplaceHostContext.Provider>
</ChatProvider>
);
12 changes: 7 additions & 5 deletions client/src/components/Agents/tests/MarketplaceContext.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ import { useChatContext } from '~/Providers';

const mockResetNewConversation = jest.fn();

jest.mock('~/hooks', () => ({
useChatHelpers: jest.fn(),
const mockUseChatHelpers = jest.fn();

jest.mock('~/hooks/Chat/useChatHelpers', () => ({
__esModule: true,
default: (index?: number, paramId?: string) => mockUseChatHelpers(index, paramId),
}));

const chatHelpers = {
Expand Down Expand Up @@ -51,15 +54,14 @@ const renderProvider = (children: React.ReactNode = <Consumer />) => {
describe('MarketplaceProvider', () => {
beforeEach(() => {
jest.clearAllMocks();
// eslint-disable-next-line @typescript-eslint/no-require-imports
const { useChatHelpers } = require('~/hooks');
(useChatHelpers as jest.Mock).mockReturnValue(chatHelpers);
mockUseChatHelpers.mockReturnValue(chatHelpers);
});

it('hands the marketplace the chat context its panels read', () => {
renderProvider();

expect(screen.getByTestId('conversation-id')).toHaveTextContent('marketplace');
expect(mockUseChatHelpers).toHaveBeenCalledWith(0, 'new');
});

it('passes the host reset straight through to the marketplace that asks for it', async () => {
Expand Down
84 changes: 84 additions & 0 deletions client/src/hooks/Chat/__tests__/provider.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import React from 'react';
import { renderHook } from '@testing-library/react';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import type { TConversation } from 'librechat-data-provider';
import type { ChatContract } from '../contract';
import { useChatActions } from '../facade';
import { ChatProvider } from '../provider';

const mockUseChatHelpers = jest.fn();

jest.mock('../useChatHelpers', () => ({
__esModule: true,
default: (index?: number, paramId?: string) => mockUseChatHelpers(index, paramId),
}));

const noop = () => undefined;

const contract: ChatContract = {
index: 0,
conversation: { conversationId: 'convo-1' } as TConversation,
setConversation: noop,
newConversation: noop,
preset: null,
setPreset: noop,
optionSettings: {},
setOptionSettings: noop,
getMessages: () => [],
messagesKey: 'convo-1',
setMessages: noop,
setSiblingIdx: noop,
latestMessageId: undefined,
latestMessageDepth: undefined,
ask: jest.fn(),
regenerate: jest.fn(),
isSubmitting: false,
setIsSubmitting: noop,
handleRegenerate: noop,
handleContinue: noop,
stopGenerating: jest.fn(() => Promise.resolve()),
handleStopGenerating: noop,
abortScroll: false,
setAbortScroll: noop,
files: new Map(),
setFiles: noop,
filesLoading: false,
setFilesLoading: noop,
showPopover: false,
setShowPopover: noop,
feedbackEnabled: false,
};

const renderUnder = (props: { index?: number; conversationId?: string }) => {
const queryClient = new QueryClient();
return renderHook(() => useChatActions(), {
wrapper: ({ children }) => (
<QueryClientProvider client={queryClient}>
<ChatProvider {...props}>{children}</ChatProvider>
</QueryClientProvider>
),
});
};

describe('ChatProvider', () => {
beforeEach(() => {
mockUseChatHelpers.mockReset();
mockUseChatHelpers.mockReturnValue(contract);
});

it('serves the pane contract it builds to the facade below it', async () => {
const { result } = renderUnder({ index: 1, conversationId: 'convo-1' });

expect(mockUseChatHelpers).toHaveBeenCalledWith(1, 'convo-1');
expect(result.current.id).toBe('convo-1');
expect(result.current.status).toBe('ready');
await result.current.stop();
expect(contract.stopGenerating).toHaveBeenCalledTimes(1);
});

it('builds the root pane by default', () => {
renderUnder({});

expect(mockUseChatHelpers).toHaveBeenCalledWith(0, undefined);
});
});
23 changes: 23 additions & 0 deletions client/src/hooks/Chat/provider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import type { ReactNode } from 'react';
import { ChatContext } from '~/Providers/ChatContext';
import useChatHelpers from './useChatHelpers';

/**
* Builds a pane's chat contract and serves it to `useChat`, `useChatActions` and
* `useChatContext` below. A host renders this instead of calling `useChatHelpers` itself, so
* components reach the chat only through the facade and its context.
*/
export function ChatProvider({
index = 0,
conversationId,
children,
}: {
/** The pane: `0` is the root pane, `1` the added (multi-convo) pane. */
index?: number;
/** The route's conversation id, which can run ahead of the pane's conversation. */
conversationId?: string;
children: ReactNode;
}) {
const chat = useChatHelpers(index, conversationId);
return <ChatContext.Provider value={chat}>{children}</ChatContext.Provider>;
}
5 changes: 3 additions & 2 deletions client/src/routes/__tests__/Marketplace.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import MarketplaceRoute from '../Marketplace';
const mockClearAllConversations = jest.fn();
const mockClearMessagesCache = jest.fn();

jest.mock('~/hooks', () => ({
useChatHelpers: jest.fn(() => ({})),
jest.mock('~/hooks/Chat/useChatHelpers', () => ({
__esModule: true,
default: jest.fn(() => ({})),
}));

jest.mock('~/utils/messages', () => ({
Expand Down
88 changes: 88 additions & 0 deletions e2e/specs/mock/scenarios/marketplace-start-chat.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { expect, test } from '@playwright/test';
import type { Page } from '@playwright/test';
import { withMongo } from '../db';
import { uniqueAgentName } from '../agents.helpers';
import {
getAccessToken,
messagesView,
replyPrompt,
replyText,
requestJson,
sendMessage,
} from '../helpers';

type AgentResponse = { id: string };

async function createAgent(page: Page, name: string): Promise<AgentResponse> {
const token = await getAccessToken(page);
return requestJson<AgentResponse>(page, {
path: '/api/agents',
token,
method: 'POST',
body: {
name,
description: 'Agent used by the marketplace start-chat scenario.',
instructions: 'Respond deterministically for the marketplace start-chat scenario.',
provider: 'Mock Provider A',
model: 'mock-model-a',
model_parameters: {},
},
});
}

async function cleanupAgent(agentId: string): Promise<void> {
await withMongo(async (db) => {
const agent = await db
.collection('agents')
.findOne({ id: agentId }, { projection: { _id: 1 } });
if (agent) {
await db.collection('aclentries').deleteMany({ resourceId: agent._id });
}
await db.collection('agents').deleteMany({ id: agentId });
});
}

test.describe('marketplace start chat', () => {
/* `getAccessToken` refreshes through a relative URL from the page, so the tab has to be
on the app before the test asks for a token. */
test.beforeEach(async ({ page }) => {
await page.goto('/agents/all', { timeout: 30_000 });
});

test('@scenario:a-chat-started-from-the-marketplace-answers-with-that-agent starting a chat from an agent card opens a new chat that the agent answers', async ({
page,
}) => {
test.setTimeout(120_000);
const name = uniqueAgentName('E2E Marketplace Start');
const agent = await createAgent(page, name);

try {
await page.goto(`/agents/all?q=${encodeURIComponent(name)}`, { timeout: 10_000 });
await expect(page.getByRole('heading', { name, exact: true })).toBeVisible({
timeout: 30_000,
});
/** The card's heading sits under its click layer; the button that opens the dialog
* takes its accessible name from that heading. */
await page.getByRole('button', { name, exact: true }).click();

await page.getByRole('button', { name: 'Start Chat' }).click();
/** The chat route applies `agent_id` on its query-param poll and then drops it from the
* URL, so waiting for it to go is waiting for the agent to be selected. The turn's request
* body is what proves which agent the chat started with. */
await expect(page).toHaveURL(/\/c\/new/, { timeout: 15_000 });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wait for the agent query settings before sending

This URL assertion succeeds immediately after navigation even while ?endpoint=agents&agent_id=... is still present, but useQueryParams does not apply those settings until its 100 ms polling callback runs (client/src/hooks/Input/useQueryParams.ts:263-352). On a fast browser run, sendMessage can therefore submit using the previous/default conversation, making the new scenario intermittently fail its agent_id assertion. Wait until the query parameters have been removed, or until the selected agent is otherwise observable, before sending.

Useful? React with 馃憤聽/ 馃憥.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4015926: the scenario waits for agent_id to leave the URL, which useQueryParams does once it has applied the agent, before sending. reviewctl verify passed on that head (desktop light, desktop dark, mobile).

await expect(page).not.toHaveURL(/agent_id=/, { timeout: 15_000 });

const label = `marketplace-start-${Date.now()}`;
const response = await sendMessage(page, replyPrompt(label));
expect(response.ok()).toBeTruthy();
await expect(messagesView(page).getByText(replyText(label))).toBeVisible({
timeout: 30_000,
});
expect(response.request().postDataJSON()).toEqual(
expect.objectContaining({ agent_id: agent.id }),
);
} finally {
await cleanupAgent(agent.id);
}
});
});
Loading