Skip to content
Open
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
10 changes: 5 additions & 5 deletions src/components/configuration/FieldProfilePopover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import type * as t from '@/types';
import { ProfileValueModal, getDefaultValue } from './ProfileValueModal';
import { DeleteProfileValueModal } from './DeleteProfileValueModal';
import { EditButton, TrashButton } from '@/components/shared';
import { getControlType, serializeModalValue } from './utils';
import { useProfileMutations, useLocalize } from '@/hooks';
import { availableScopesOptions } from '@/server';
import { getScopeTypeConfig } from '@/constants';
import { serializeKVPairs, cn } from '@/utils';
import { getControlType } from './utils';
import { cn } from '@/utils';

export function FieldProfilePopover({
fieldPath,
Expand Down Expand Up @@ -71,7 +71,7 @@ export function FieldProfilePopover({

const handleModalSave = useCallback(() => {
if (modalIsBase && onBaseValueChange) {
onBaseValueChange(serializeKVPairs(modalValue));
onBaseValueChange(serializeModalValue(controlType, modalValue));
setModalOpen(false);
setModalIsBase(false);
return;
Expand All @@ -81,7 +81,7 @@ export function FieldProfilePopover({
{
principalType: modalScope.principalType,
principalId: modalScope.principalId,
value: serializeKVPairs(modalValue),
value: serializeModalValue(controlType, modalValue),
},
{
onSuccess: () => {
Expand All @@ -94,7 +94,7 @@ export function FieldProfilePopover({
},
},
);
}, [modalIsBase, modalScope, modalValue, modalMode, saveMutation, onBaseValueChange]);
}, [modalIsBase, modalScope, modalValue, modalMode, controlType, saveMutation, onBaseValueChange]);

const handleModalCancel = useCallback(() => {
setModalOpen(false);
Expand Down
16 changes: 16 additions & 0 deletions src/components/configuration/FieldRenderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,22 @@ describe('SingleFieldRenderer', () => {
expect(screen.getByDisplayValue('Authorization')).toBeInTheDocument();
expect(screen.getByDisplayValue('Bearer token')).toBeInTheDocument();
});

it('emits an empty record, not an empty array, when the last key-value row is removed', () => {
const field = createField({ key: 'headers', type: 'record' });
const onChange = vi.fn();
render(
<SingleFieldRenderer
field={field}
value={{ Authorization: 'Bearer token' }}
path="section.headers"
getValue={getValue}
onChange={onChange}
/>,
);
fireEvent.click(screen.getByRole('button', { name: 'com_ui_delete com_ui_entry 1' }));
expect(onChange).toHaveBeenCalledWith('section.headers', {});
});
});

describe('FieldRenderer with imported config values', () => {
Expand Down
7 changes: 4 additions & 3 deletions src/components/configuration/FieldRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
getControlType,
getEnumOptions,
hasDescendant,
kvPairsEditValue,
toKVPair,
isStringLikeItemType,
splitUnionTypes,
Expand All @@ -23,6 +24,7 @@ import { ListRecordField } from './fields/ListRecordField';
import { renderCollapsible } from './renderCollapsible';
import { TextareaField } from './fields/TextareaField';
import { KeyValueField } from './fields/KeyValueField';
import { cn, getSecretPreviewValue } from '@/utils';
import { NumberField } from './fields/NumberField';
import { SecretField } from './fields/SecretField';
import { ToggleField } from './fields/ToggleField';
Expand All @@ -32,7 +34,6 @@ import { ListField } from './fields/ListField';
import { CodeField } from './fields/CodeField';
import { ConfigRow } from './ConfigRow';
import { useLocalize } from '@/hooks';
import { cn, getSecretPreviewValue } from '@/utils';

function formatDefault(value: t.ConfigValue): string | null {
if (value === undefined || value === null) return null;
Expand Down Expand Up @@ -477,7 +478,7 @@ export function SingleFieldRenderer({
<KeyValueField
id={fieldId}
pairs={pairs}
onChange={(newPairs) => onChange(path, newPairs)}
onChange={(newPairs) => onChange(path, kvPairsEditValue(newPairs))}
disabled={disabled}
valueTypes={field.recordValueKVTypes}
aria-label={fieldLabel}
Expand Down Expand Up @@ -1221,7 +1222,7 @@ export function renderInlineField(
<KeyValueField
id={fieldId}
pairs={pairs}
onChange={(p) => onChange(field.key, p)}
onChange={(p) => onChange(field.key, kvPairsEditValue(p))}
disabled={disabled}
valueTypes={field.recordValueKVTypes}
aria-label={fieldLabel}
Expand Down
85 changes: 85 additions & 0 deletions src/components/configuration/ProfileValueModal.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { vi, describe, it, expect } from 'vitest';
import { render, screen, fireEvent } from '@testing-library/react';
import type * as t from '@/types';
import { ProfileValueModal, getDefaultValue } from './ProfileValueModal';
import { createField } from '@/test/fixtures';

vi.mock('@/hooks/useLocalize', () => ({
default: () => (key: string) => key,
useLocalize: () => (key: string) => key,
}));

interface ChildrenProps {
children?: React.ReactNode;
}
interface ButtonProps {
label?: string;
onClick?: () => void;
}
interface IconButtonProps {
icon: string;
onClick?: () => void;
'aria-label'?: string;
}

vi.mock('@clickhouse/click-ui', () => ({
Icon: () => null,
Button: ({ label, onClick }: ButtonProps) => <button onClick={onClick}>{label}</button>,
IconButton: ({ icon, onClick, ...props }: IconButtonProps) => (
<button onClick={onClick} aria-label={props['aria-label'] ?? icon} />
),
Select: Object.assign(({ children }: ChildrenProps) => <div>{children}</div>, {
Item: ({ children }: ChildrenProps) => <div>{children}</div>,
}),
Dialog: Object.assign(({ children }: ChildrenProps) => <div>{children}</div>, {
Content: ({ children }: ChildrenProps) => <div>{children}</div>,
}),
}));

describe('getDefaultValue', () => {
it('returns an empty record for record fields so an untouched save stays object-typed', () => {
expect(getDefaultValue('record')).toEqual({});
});

it('returns an empty array for array fields', () => {
expect(getDefaultValue('array')).toEqual([]);
});
});

describe('ProfileValueModal — record fields', () => {
function renderModal(value: t.ConfigValue, onChange: (v: t.ConfigValue) => void) {
return render(
<ProfileValueModal
open
fieldSchema={createField({ key: 'headers', type: 'record' })}
controlType="record"
value={value}
onChange={onChange}
onSave={() => {}}
onCancel={() => {}}
saving={false}
scopeName="Base configuration"
scopeType="BASE"
mode="edit"
/>,
);
}

it('emits an empty record, not an empty array, when the last row is removed', () => {
const onChange = vi.fn();
renderModal({ Authorization: 'Bearer token' }, onChange);
fireEvent.click(screen.getByRole('button', { name: 'com_ui_delete com_ui_entry 1' }));
expect(onChange).toHaveBeenCalledWith({});
});

it('keeps in-progress rows as raw pairs while editing', () => {
const onChange = vi.fn();
renderModal({ Authorization: 'Bearer token' }, onChange);
const valueInput = screen.getByLabelText('com_ui_value 1');
fireEvent.change(valueInput, { target: { value: 'Bearer {{API_KEY}}' } });
fireEvent.blur(valueInput);
expect(onChange).toHaveBeenCalledWith([
{ key: 'Authorization', value: 'Bearer {{API_KEY}}', valueType: 'string' },
]);
});
});
6 changes: 3 additions & 3 deletions src/components/configuration/ProfileValueModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useState } from 'react';
import { PrincipalType } from 'librechat-data-provider';
import { Icon, Button, Dialog } from '@clickhouse/click-ui';
import type * as t from '@/types';
import { getEnumOptions, getArrayItemType, toKVPair } from './utils';
import { getEnumOptions, getArrayItemType, kvPairsEditValue, toKVPair } from './utils';
import { KeyValueField } from './fields/KeyValueField';
import { TrashButton } from '@/components/shared';
import { getScopeTypeConfig } from '@/constants';
Expand Down Expand Up @@ -208,7 +208,7 @@ function ModalValueControl({
<KeyValueField
id="profile-kv"
pairs={pairs}
onChange={onChange}
onChange={(p) => onChange(kvPairsEditValue(p))}
aria-label={localize('com_ui_value')}
/>
);
Expand Down Expand Up @@ -318,7 +318,7 @@ export function getDefaultValue(controlType: string, fieldSchema?: t.SchemaField
return opts.length > 0 ? opts[0].value : '';
}
if (controlType === 'array') return [];
if (controlType === 'record') return [];
if (controlType === 'record') return {};
if (controlType === 'object' || controlType === 'code') return {};
return '';
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
enumerateLeafPaths,
validateMcpCrossField,
} from '../McpServersRenderer';
import { applyConfigEdit, buildSavePayload } from '../../utils';
import { createField } from '@/test/fixtures';

vi.mock('@/hooks/useLocalize', () => ({
Expand Down Expand Up @@ -1058,3 +1059,60 @@ describe('McpServersRenderer — create then edit then rename preserves nested d
expect(wholeHeadersWrite).toBeUndefined();
});
});

describe('McpServersRenderer — header KV pairs never leak into the save payload (issue #56)', () => {
function renderWithHeaders(baseRecord: Record<string, t.ConfigValue>) {
const onChange = vi.fn();
const fields = [
...fieldsForMcp(),
createField({ key: 'headers', type: 'record', isOptional: true }),
];
const props: t.FieldRendererProps = {
fields,
parentValue: baseRecord,
parentPath: 'mcpServers',
getValue: (path, fallback) => (path === 'mcpServers' ? baseRecord : fallback),
onChange,
editedValues: {},
};
return { ...render(<McpServersRenderer {...props} />), onChange };
}

const baseRecord = {
srv: {
type: 'streamable-http',
url: 'https://example.com/api/mcp',
headers: { Authorization: 'Bearer old' },
},
};

it('serializes an edited Authorization header to a plain record in the save payload', () => {
const { onChange } = renderWithHeaders(baseRecord);

fireEvent.click(screen.getByText('srv'));
const valueInput = screen.getByLabelText('com_ui_value 1');
fireEvent.change(valueInput, { target: { value: 'Bearer {{API_KEY}}' } });
fireEvent.blur(valueInput);

const headerEdits = onChange.mock.calls.filter(([p]) => p === 'mcpServers.srv.headers');
expect(headerEdits.length).toBeGreaterThan(0);
const [path, emitted] = headerEdits[headerEdits.length - 1] as [string, t.ConfigValue];

const edited = applyConfigEdit({}, path, emitted, {}, new Set(), new Set());
const { saves } = buildSavePayload(new Set([path]), edited, new Set());
expect(saves).toEqual([
{ fieldPath: 'mcpServers.srv.headers', value: { Authorization: 'Bearer {{API_KEY}}' } },
]);
});

it('emits an empty record, not an empty array, when the last header row is removed', () => {
const { onChange } = renderWithHeaders(baseRecord);

fireEvent.click(screen.getByText('srv'));
fireEvent.click(screen.getByRole('button', { name: 'com_ui_delete com_ui_entry 1' }));

const headerEdits = onChange.mock.calls.filter(([p]) => p === 'mcpServers.srv.headers');
expect(headerEdits.length).toBeGreaterThan(0);
expect(headerEdits[headerEdits.length - 1][1]).toEqual({});
});
});
66 changes: 66 additions & 0 deletions src/components/configuration/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
mergeIndexedArrayEdits,
buildSavePayload,
applyConfigEdit,
kvPairsEditValue,
serializeModalValue,
} from './utils';
import { createField } from '@/test/fixtures';
import { flattenObject } from '@/utils';
Expand Down Expand Up @@ -543,3 +545,67 @@ describe('buildSavePayload — masked secrets never reach the backend', () => {
expect(resets).toEqual(['ocr.apiKey']);
});
});

describe('kvPairsEditValue', () => {
it('passes non-empty pairs through unchanged for save-time serialization', () => {
const pairs: t.KeyValuePair[] = [
{ key: 'Authorization', value: 'Bearer {{API_KEY}}', valueType: 'string' },
];
expect(kvPairsEditValue(pairs)).toBe(pairs);
});

it('converts an emptied pair list to an empty record', () => {
expect(kvPairsEditValue([])).toEqual({});
});
});

describe('buildSavePayload — KV pairs serialize to records', () => {
it('converts header pairs edits to a plain record in the save payload', () => {
const edited: t.FlatConfigMap = {
'mcpServers.srv.headers': [
{ key: 'Authorization', value: 'Bearer {{API_KEY}}', valueType: 'string' },
],
};
const { saves } = buildSavePayload(new Set(['mcpServers.srv.headers']), edited, new Set());
expect(saves).toEqual([
{ fieldPath: 'mcpServers.srv.headers', value: { Authorization: 'Bearer {{API_KEY}}' } },
]);
});

it('converts pairs nested inside indexed array entries (azure additional params)', () => {
const edited: t.FlatConfigMap = {
'endpoints.azureOpenAI.groups.0': {
group: 'my-group',
addParams: [{ key: 'reasoning_effort', value: 'high', valueType: 'string' }],
},
};
const { saves } = buildSavePayload(
new Set(['endpoints.azureOpenAI.groups.0']),
edited,
new Set(),
);
expect(saves).toEqual([
{
fieldPath: 'endpoints.azureOpenAI.groups.0',
value: { group: 'my-group', addParams: { reasoning_effort: 'high' } },
},
]);
});
});

describe('serializeModalValue', () => {
it('serializes record-control pairs to a plain record', () => {
const pairs = [{ key: 'Authorization', value: 'Bearer {{API_KEY}}', valueType: 'string' }];
expect(serializeModalValue('record', pairs)).toEqual({ Authorization: 'Bearer {{API_KEY}}' });
});

it('passes an untouched empty record default through unchanged', () => {
expect(serializeModalValue('record', {})).toEqual({});
});

it('leaves array-control values untouched even when elements look pair-like', () => {
const value = [{ key: 'id', value: 'x', enabled: true }];
expect(serializeModalValue('array', value)).toBe(value);
expect(serializeModalValue('array-object', value)).toBe(value);
});
});
Loading