mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
⚡ perf: Agent List and Model Selector at Scale (#14601)
* perf: cut serial round trips from the agent list query path
The agent list was the slowest path on first page load. Three separate
problems compounded:
- `getListAgentsHandler` chained its reads: two ACL lookups, the avatar
refresh cache probe and the viewer skill scope all resolved serially
ahead of the list query, and `attachOwnerContacts` added two more hops
after it. The four independent reads now resolve together, and the
avatar refresh runs alongside the list query instead of before it -
refreshed paths reach the response through `urlCache`, not through
whatever the list query happened to read. Serial hops per request drop
from 7 to 4 on a warm cache.
- The avatar refresh loaded the user's whole accessible agent set (up to
MAX_AVATAR_REFRESH_AGENTS) to discover which entries were S3-backed.
Scoping the query to `avatar.source` means deployments on any other
file strategy match nothing instead of walking the full set.
- `fetchAllAgentPages` walked cursor pages at the server's default size
of 100, and callers consume the flattened result, so every extra page
was a serial round trip for no benefit. It now requests the server
maximum. Measured over a 2,860 agent account: 29 requests / 1.65s
before, 3 requests / 0.29s after.
Also parallelizes the conversation file reads in `initializeAgent`. The
convo file refs and the execute_code thread walk share no inputs, and the
two code-file lookups depend only on `threadFileIds`, so the chain of six
serial reads on every turn collapses to two. This one is time to first
token the user waits through.
* perf: virtualize the model selector agent list
Opening the agents submenu with a large agent set froze the tab and could
kill it outright. With ~10k accessible agents the submenu blocked for over
15 seconds and took the heap from 96MB to 911MB. Four per-row costs were
being multiplied by the full list, which rendered unwindowed:
- `useIsActiveItem` allocated a MutationObserver per row (10,016 of them
for one dropdown). Replaced with an Ariakit store subscription, which
needs no observer at all and returns a boolean so a row only re-renders
when its own active state flips.
- `useFavorites` ran per row, opening a jotai subscription, a query
subscription and a mutation each time. Hoisted to one call per endpoint.
- Each row rescanned `endpoint.models` to recover `isGlobal`, a field the
parent had already discarded from the array it was mapping. The parent
now passes it down from a lookup map.
- The list itself is now windowed above 100 rows. Ariakit's composite only
knows about mounted rows, so arrow-keying to the window edge previously
found no next item and let focus escape the nested menu, closing it;
`handleBoundaryNavigation` scrolls the next index in, waits for it to
mount, then moves the composite onto it. Navigation inside the window is
left to Ariakit.
Open drops from >15s to 96ms, mounted rows from 10,028 to ~18, DOM nodes
from 123,346 to ~1,000, and the heap no longer grows. Verified in browser:
arrow keys track 1:1 to index 238 and back, and click selection works.
* perf: serve the model selector from the shared VIEW agent query
The model selector asked for EDIT-scoped agents whenever the marketplace
is enabled, while `useAgentsMap` and `useMentions` asked for VIEW. Since
the cache key includes the params, that was two distinct entries, so first
page load ran the paginated walk twice and held two copies of the whole
agent list in memory. Measured against a 10k agent account: 22 list handler
invocations per page load, now 11.
Collapsing the two by asking for the same permission everywhere would have
changed what the selector shows - under the marketplace the EDIT scope is
what makes it "My Agents", with discovery handled by the marketplace entry.
So the list endpoint now marks each row with `isEditable`, resolved from an
ACL read folded into the existing parallel batch (no extra serial hop), and
the selector filters the shared VIEW response instead of refetching. A
VIEW-scoped list for a user with 2861 visible / 361 editable agents returns
exactly 360 rows flagged editable, matching what the EDIT query returned.
`AgentSelect` deliberately keeps its own EDIT query: it reads `skills` and
`skills_enabled`, which `sanitizeViewerSkillScope` strips from VIEW-scoped
responses. It also only mounts when the builder panel is open, so it is not
part of the first-load cost.
The field is set unconditionally rather than omitted when false so that a
client talking to an older server sees `undefined`, keeps every agent, and
degrades to showing too many rather than none.
* fix: address review findings on the agent list at scale
Three issues from review, all confirmed against the code before fixing.
Avatar refresh no longer runs alongside the list query. `updateAgent` writes
through `findOneAndUpdate` on a `timestamps: true` schema, so refreshing an
avatar advances `updatedAt` — the field `getListAgentsByAccess` sorts and
cursors on. A write landing after the first page's snapshot moved that agent
ahead of the returned cursor, dropping it from every later page and silently
truncating the caller's flattened list. This was a regression introduced when
the two were parallelized; serializing them costs nothing on the common path,
because a cache hit returns without issuing any query, so only the
once-per-30-minutes miss pays for the ordering. The new test asserts the write
lands before the list snapshot and fails against the parallel version.
The virtualized list no longer inserts a focusable grid into the combobox.
`List` spreads its props onto `Grid`, whose defaults are `role="grid"`,
`containerRole="row"` and `tabIndex={0}`; inside Ariakit's listbox that added a
tab stop ahead of any row and put grid/row semantics between the listbox and its
options. All three are now neutralized so focus and ARIA stay with the combobox
items.
The list also resets to the top when the filter changes. `Grid` keeps its scroll
offset across prop changes and clamps an out-of-range offset to
`totalRowsHeight - height`, the end of the shorter list. Scrolling deep and then
searching landed on the tail: measured at row 626 of 667 matches, with only
those rows mounted and reachable by keyboard. Keying the list on the search
value restores row 0.
* fix: declare option position and set size for the virtualized model list
Once the model list is windowed, only the mounted slice exists in the listbox,
so a screen reader infers position and total from ~19 elements instead of the
real set — announcing "3 of 19" partway through 10,014 agents.
Model rows now carry aria-posinset and aria-setsize. The marketplace entry and
any model specs share the same numbering, because they are options in the same
listbox: declaring the values on some options while leaving others to be
inferred from the DOM would make the set internally inconsistent. Both are
omitted entirely when the list is short enough to render unwindowed, where the
DOM holds every option and the implicit values are already correct.
Verified against a 10,014 agent account: the marketplace entry reports 1 of
10015, the first models 2 and 3, and after scrolling to row 4999 the leading
mounted model reports 5001 of 10015 with 19 options in the DOM.
* 🩹 fix: Address Follow-Ups on the Agent List at Scale
Corrects residual issues in the agent-list perf work, all inside its own scope.
- Forward `idOnTheSource` through `PermissionService.findAccessibleResources`
so `getUserPrincipals` skips the user-document read. The list handler resolves
three permission sets per request and each was paying its own `User.findById`;
the auth strategies already normalize the field to a value or null.
- Gate the editable-set lookup on its own predicate instead of borrowing
`canReturnSkillConfig`. The two answer unrelated questions and only coincide
today, so redefining the skill flag would have marked every agent editable.
- Log mapping failures in the list response instead of swallowing them.
- Apply the walk page size after the caller's params in `fetchAllAgentPages`.
A caller limit only changed page size, never what the flattened walk returned,
so `defaultAgentParams`' `limit: 10` would have turned one request into 301.
- Carry `isEditable` on the agent rows the create and update mutations write
into the list cache. Mutation responses omit the field, so those rows lost it.
- Document `isEditable` as list-only, ACL-derived, and fail-open on absence.
- Restore the truthiness guard on the thread walk in `initializeAgent`. Widening
it to `!= null` made an empty `parentMessageId` issue a full-conversation read
against an anchor that can never match.
- Await `getConvoFiles` directly rather than calling `.then()` on it, restoring
tolerance for synchronous test doubles.
- Correct the avatar-refresh comment: the projection was never full documents,
and the real reason to filter is that an unfiltered budget is self-reinforcing.
Tests: both new `initialize` tests and both new backend tests are
mutation-verified; the concurrency test fails under either serialization order.
* fix: preserve ACL isEditable when merging agent mutation responses
Mutation responses omit list-only isEditable. Inferring true from write
success promoted VIEW-only rows into the editable subset for MANAGE_AGENTS
callers who can PATCH agents their ACL marks non-editable.
* fix: sort imports in agent mutations test
ESLint import-order check failed on the isEditable cache-preservation test.
* 🧷 fix: Carry isEditable Onto Duplicated Agent List Rows
`useDuplicateAgentMutation` prepended the raw duplicate response to the cached
list, and mutation responses omit the list-only `isEditable` field. The row
survived the "My Agents" filter only by failing open on `undefined`, so it would
disappear the moment a consumer read the flag strictly.
Duplicating grants the caller ownership, so the new row is editable outright;
this is the create case rather than the merge case `mergeAgentListRow` handles.
Last cache write on this path that did not carry the field.
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
parent
a5b10c78cf
commit
39f5f9d846
19 changed files with 1107 additions and 179 deletions
|
|
@ -1166,6 +1166,12 @@ const getListAgentsHandler = async (req, res) => {
|
|||
requiredPermission = PermissionBits.VIEW;
|
||||
}
|
||||
const canReturnSkillConfig = hasEditBit(requiredPermission);
|
||||
/**
|
||||
* Derived from the same bit as `canReturnSkillConfig` but answering a different question:
|
||||
* skill-config exposure versus edit-permission reporting. An EDIT-scoped request matches
|
||||
* only editable agents, so it needs no second lookup to know which ones those are.
|
||||
*/
|
||||
const needsEditableLookup = !hasEditBit(requiredPermission);
|
||||
// Base filter
|
||||
const filter = {};
|
||||
|
||||
|
|
@ -1188,33 +1194,99 @@ const getListAgentsHandler = async (req, res) => {
|
|||
filter.$or = [{ name: regex }, { description: regex }];
|
||||
}
|
||||
|
||||
// Get agent IDs the user has VIEW access to via ACL
|
||||
const accessibleIds = await findAccessibleResources({
|
||||
userId,
|
||||
role: req.user.role,
|
||||
resourceType: ResourceType.AGENT,
|
||||
requiredPermissions: requiredPermission,
|
||||
});
|
||||
const cache = getLogStores(CacheKeys.S3_EXPIRY_INTERVAL);
|
||||
const refreshKey = `${userId}:agents_avatar_refresh`;
|
||||
|
||||
const publiclyAccessibleIds = await findPubliclyAccessibleResources({
|
||||
resourceType: ResourceType.AGENT,
|
||||
requiredPermissions: PermissionBits.VIEW,
|
||||
});
|
||||
/**
|
||||
* These reads share no inputs, so they resolve together rather than chaining round
|
||||
* trips ahead of the list query. The viewer skill scope and the editable set are only
|
||||
* consumed when the page is non-empty; dispatching them here trades a wasted lookup on
|
||||
* the (cheap) zero-agent path for one less serial hop on every populated page.
|
||||
*
|
||||
* `editableIds` lets a VIEW-scoped response mark which agents the caller may also edit,
|
||||
* so consumers wanting just the editable subset can filter one shared VIEW fetch rather
|
||||
* than issuing a second full paginated walk under an EDIT-scoped cache key. Requests
|
||||
* that already ask for EDIT get it for free: everything they match is editable.
|
||||
*
|
||||
* `idOnTheSource` is forwarded so `getUserPrincipals` resolves identity without reading
|
||||
* the user document; the auth strategies already normalize it to a value or null. Each
|
||||
* omission would cost this handler another `User.findById`, once per lookup.
|
||||
*/
|
||||
const { idOnTheSource } = req.user;
|
||||
const [
|
||||
accessibleIds,
|
||||
publiclyAccessibleIds,
|
||||
cachedRefreshEntry,
|
||||
accessibleSkillIds,
|
||||
editableIds,
|
||||
] = await Promise.all([
|
||||
findAccessibleResources({
|
||||
userId,
|
||||
role: req.user.role,
|
||||
idOnTheSource,
|
||||
resourceType: ResourceType.AGENT,
|
||||
requiredPermissions: requiredPermission,
|
||||
}),
|
||||
findPubliclyAccessibleResources({
|
||||
resourceType: ResourceType.AGENT,
|
||||
requiredPermissions: PermissionBits.VIEW,
|
||||
}),
|
||||
cache.get(refreshKey),
|
||||
canReturnSkillConfig
|
||||
? null
|
||||
: findAccessibleResources({
|
||||
userId,
|
||||
role: req.user.role,
|
||||
idOnTheSource,
|
||||
resourceType: ResourceType.SKILL,
|
||||
requiredPermissions: PermissionBits.VIEW,
|
||||
}),
|
||||
needsEditableLookup
|
||||
? findAccessibleResources({
|
||||
userId,
|
||||
role: req.user.role,
|
||||
idOnTheSource,
|
||||
resourceType: ResourceType.AGENT,
|
||||
requiredPermissions: PermissionBits.EDIT,
|
||||
})
|
||||
: null,
|
||||
]);
|
||||
|
||||
const isValidCachedRefresh =
|
||||
cachedRefreshEntry != null &&
|
||||
typeof cachedRefreshEntry === 'object' &&
|
||||
cachedRefreshEntry.urlCache != null;
|
||||
|
||||
/**
|
||||
* Refresh all S3 avatars for this user's accessible agent set (not only the current page)
|
||||
* This addresses page-size limits preventing refresh of agents beyond the first page
|
||||
* This addresses page-size limits preventing refresh of agents beyond the first page.
|
||||
*
|
||||
* Scoped to agents that actually carry an S3 avatar so the `MAX_AVATAR_REFRESH_AGENTS`
|
||||
* budget is spent on agents that can do work. Unfiltered, that budget is the most
|
||||
* recently updated accessible agents regardless of avatar, and because a refresh writes
|
||||
* through `updateAgent` and advances `updatedAt`, the window is self-reinforcing: an
|
||||
* S3-avatar agent ranked past the budget never enters it and its presigned URL is never
|
||||
* regenerated. The predicate is not indexed (`avatar` is `Mixed`), so this trades docs
|
||||
* examined for that coverage.
|
||||
*
|
||||
* Must settle BEFORE the list query below, and is deliberately not parallelized with
|
||||
* it. `updateAgent` writes through `findOneAndUpdate` on a `timestamps: true` schema,
|
||||
* so refreshing an avatar advances `updatedAt`, the very field
|
||||
* `getListAgentsByAccess` sorts and cursors on. A refresh landing after the first
|
||||
* page's snapshot would move that agent ahead of the returned cursor, dropping it
|
||||
* from every later page and silently truncating the caller's flattened list.
|
||||
* Serializing costs nothing on the common path: a cache hit returns below without
|
||||
* issuing any query, so only the once-per-30-minutes miss pays for the ordering.
|
||||
*/
|
||||
const cache = getLogStores(CacheKeys.S3_EXPIRY_INTERVAL);
|
||||
const refreshKey = `${userId}:agents_avatar_refresh`;
|
||||
let cachedRefresh = await cache.get(refreshKey);
|
||||
const isValidCachedRefresh =
|
||||
cachedRefresh != null && typeof cachedRefresh === 'object' && cachedRefresh.urlCache != null;
|
||||
if (!isValidCachedRefresh) {
|
||||
const resolveAvatarRefresh = async () => {
|
||||
if (isValidCachedRefresh) {
|
||||
logger.debug('[/Agents] S3 avatar refresh already checked, skipping');
|
||||
return cachedRefreshEntry;
|
||||
}
|
||||
try {
|
||||
const fullList = await db.getListAgentsByAccess({
|
||||
accessibleIds,
|
||||
otherParams: {},
|
||||
otherParams: { 'avatar.source': FileSources.s3 },
|
||||
limit: MAX_AVATAR_REFRESH_AGENTS,
|
||||
after: null,
|
||||
});
|
||||
|
|
@ -1224,14 +1296,16 @@ const getListAgentsHandler = async (req, res) => {
|
|||
refreshS3Url,
|
||||
updateAgent: db.updateAgent,
|
||||
});
|
||||
cachedRefresh = { urlCache };
|
||||
await cache.set(refreshKey, cachedRefresh, Time.THIRTY_MINUTES);
|
||||
const refreshEntry = { urlCache };
|
||||
await cache.set(refreshKey, refreshEntry, Time.THIRTY_MINUTES);
|
||||
return refreshEntry;
|
||||
} catch (err) {
|
||||
logger.error('[/Agents] Error refreshing avatars for full list: %o', err);
|
||||
return null;
|
||||
}
|
||||
} else {
|
||||
logger.debug('[/Agents] S3 avatar refresh already checked, skipping');
|
||||
}
|
||||
};
|
||||
|
||||
const cachedRefresh = await resolveAvatarRefresh();
|
||||
|
||||
// Use the new ACL-aware function
|
||||
const data = await db.getListAgentsByAccess({
|
||||
|
|
@ -1247,20 +1321,13 @@ const getListAgentsHandler = async (req, res) => {
|
|||
return res.json(data);
|
||||
}
|
||||
|
||||
let accessibleSkillSet = null;
|
||||
if (!canReturnSkillConfig) {
|
||||
const accessibleSkillIds = await findAccessibleResources({
|
||||
userId,
|
||||
role: req.user.role,
|
||||
resourceType: ResourceType.SKILL,
|
||||
requiredPermissions: PermissionBits.VIEW,
|
||||
});
|
||||
accessibleSkillSet = new Set(
|
||||
mergeDeploymentSkillIds(accessibleSkillIds).map((oid) => oid.toString()),
|
||||
);
|
||||
}
|
||||
const accessibleSkillSet = canReturnSkillConfig
|
||||
? null
|
||||
: new Set(mergeDeploymentSkillIds(accessibleSkillIds).map((oid) => oid.toString()));
|
||||
|
||||
const publicSet = new Set(publiclyAccessibleIds.map((oid) => oid.toString()));
|
||||
/** Null for EDIT-scoped requests, where every matched agent is editable by definition. */
|
||||
const editableSet = editableIds ? new Set(editableIds.map((oid) => oid.toString())) : null;
|
||||
const agentsWithContacts = await attachOwnerContacts(agents);
|
||||
|
||||
const urlCache = cachedRefresh?.urlCache;
|
||||
|
|
@ -1272,6 +1339,7 @@ const getListAgentsHandler = async (req, res) => {
|
|||
if (agent?._id && publicSet.has(agent._id.toString())) {
|
||||
agent.isPublic = true;
|
||||
}
|
||||
agent.isEditable = editableSet == null || editableSet.has(agent?._id?.toString());
|
||||
if (
|
||||
urlCache &&
|
||||
agent?.id &&
|
||||
|
|
@ -1280,9 +1348,8 @@ const getListAgentsHandler = async (req, res) => {
|
|||
) {
|
||||
agent.avatar = { ...agent.avatar, filepath: urlCache[agent.id] };
|
||||
}
|
||||
} catch (e) {
|
||||
// Silently ignore mapping errors
|
||||
void e;
|
||||
} catch (err) {
|
||||
logger.warn('[/Agents] Error mapping agent %s for list response: %o', agent?.id, err);
|
||||
}
|
||||
return agent;
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1676,6 +1676,65 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
expect(response.data[0].owner_contact).toBeUndefined();
|
||||
});
|
||||
|
||||
test('should mark isEditable per agent on a VIEW-scoped list', async () => {
|
||||
mockReq.user.id = userA.toString();
|
||||
mockReq.query = { requiredPermission: String(PermissionBits.VIEW) };
|
||||
/** VIEW reaches all three; the EDIT lookup only reaches agentA1. */
|
||||
findAccessibleResources.mockImplementation(({ resourceType, requiredPermissions }) => {
|
||||
if (resourceType === 'agent' && requiredPermissions === PermissionBits.EDIT) {
|
||||
return Promise.resolve([agentA1._id]);
|
||||
}
|
||||
if (resourceType === 'agent') {
|
||||
return Promise.resolve([agentA1._id, agentA2._id, agentA3._id]);
|
||||
}
|
||||
return Promise.resolve([]);
|
||||
});
|
||||
findPubliclyAccessibleResources.mockResolvedValue([]);
|
||||
|
||||
await getListAgentsHandler(mockReq, mockRes);
|
||||
|
||||
const byId = Object.fromEntries(
|
||||
mockRes.json.mock.calls[0][0].data.map((a) => [a.id, a.isEditable]),
|
||||
);
|
||||
expect(byId[agentA1.id]).toBe(true);
|
||||
expect(byId[agentA2.id]).toBe(false);
|
||||
expect(byId[agentA3.id]).toBe(false);
|
||||
});
|
||||
|
||||
test('should forward idOnTheSource to every ACL lookup', async () => {
|
||||
/** Without it `getUserPrincipals` reads the user document once per lookup, so the
|
||||
* handler pays an extra `User.findById` for each permission it resolves. */
|
||||
mockReq.user.id = userA.toString();
|
||||
mockReq.user.idOnTheSource = 'external-oid-1';
|
||||
findAccessibleResources.mockResolvedValue([agentA1._id]);
|
||||
findPubliclyAccessibleResources.mockResolvedValue([]);
|
||||
|
||||
await getListAgentsHandler(mockReq, mockRes);
|
||||
|
||||
expect(findAccessibleResources.mock.calls.length).toBeGreaterThan(1);
|
||||
for (const [args] of findAccessibleResources.mock.calls) {
|
||||
expect(args.idOnTheSource).toBe('external-oid-1');
|
||||
}
|
||||
});
|
||||
|
||||
test('should mark every agent editable when the request is already EDIT-scoped', async () => {
|
||||
mockReq.user.id = userA.toString();
|
||||
mockReq.query = { requiredPermission: String(PermissionBits.EDIT) };
|
||||
findAccessibleResources.mockResolvedValue([agentA1._id, agentA2._id]);
|
||||
findPubliclyAccessibleResources.mockResolvedValue([]);
|
||||
|
||||
await getListAgentsHandler(mockReq, mockRes);
|
||||
|
||||
const response = mockRes.json.mock.calls[0][0];
|
||||
expect(response.data.every((a) => a.isEditable === true)).toBe(true);
|
||||
/** No extra EDIT lookup: an EDIT-scoped match is editable by definition. */
|
||||
const editCalls = findAccessibleResources.mock.calls.filter(
|
||||
([args]) =>
|
||||
args.resourceType === 'agent' && args.requiredPermissions === PermissionBits.EDIT,
|
||||
);
|
||||
expect(editCalls).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('should return only expected safe list fields for VIEW callers', async () => {
|
||||
const hiddenSkillId = new mongoose.Types.ObjectId();
|
||||
await Agent.findByIdAndUpdate(agentA1._id, {
|
||||
|
|
@ -1721,6 +1780,7 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
'conversation_starters',
|
||||
'description',
|
||||
'id',
|
||||
'isEditable',
|
||||
'is_promoted',
|
||||
'name',
|
||||
'support_contact',
|
||||
|
|
@ -2235,6 +2295,110 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
expect(mockRes.json).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('should finish avatar writes before snapshotting the paginated list query', async () => {
|
||||
/** `updateAgent` bumps `updatedAt`, which is the field `getListAgentsByAccess`
|
||||
* sorts and cursors on. If the list query snapshots before a refresh write
|
||||
* lands, that agent jumps ahead of the returned cursor and vanishes from every
|
||||
* later page. Assert the ordering rather than the symptom, which only shows up
|
||||
* on multi-page S3 accounts under a specific interleaving. */
|
||||
const db = require('~/models');
|
||||
const order = [];
|
||||
/** Yield a macrotask so a parallelized refresh would lose the race, the way a real
|
||||
* S3 presign round trip does. */
|
||||
refreshS3Url.mockImplementation(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 5));
|
||||
order.push('avatar-write');
|
||||
return 'new-s3-path.jpg';
|
||||
});
|
||||
const realList = db.getListAgentsByAccess;
|
||||
const listSpy = jest.spyOn(db, 'getListAgentsByAccess').mockImplementation(async (params) => {
|
||||
if (params.includeSkillConfig) {
|
||||
order.push('list-query');
|
||||
return { object: 'list', data: [], has_more: false, after: null };
|
||||
}
|
||||
return realList(params);
|
||||
});
|
||||
mockCache.get.mockResolvedValue(false);
|
||||
findAccessibleResources.mockResolvedValue([agentWithS3Avatar._id]);
|
||||
findPubliclyAccessibleResources.mockResolvedValue([]);
|
||||
|
||||
const mockReq = { user: { id: userA.toString(), role: 'USER' }, query: {} };
|
||||
const mockRes = { status: jest.fn().mockReturnThis(), json: jest.fn().mockReturnThis() };
|
||||
|
||||
try {
|
||||
await getListAgentsHandler(mockReq, mockRes);
|
||||
expect(order).toContain('avatar-write');
|
||||
expect(order.indexOf('avatar-write')).toBeLessThan(order.indexOf('list-query'));
|
||||
} finally {
|
||||
listSpy.mockRestore();
|
||||
refreshS3Url.mockReset();
|
||||
}
|
||||
});
|
||||
|
||||
test('should serve the refreshed filepath in the same response on cache miss', async () => {
|
||||
const agentId = agentWithS3Avatar.id;
|
||||
mockCache.get.mockResolvedValue(false);
|
||||
findAccessibleResources.mockResolvedValue([agentWithS3Avatar._id]);
|
||||
findPubliclyAccessibleResources.mockResolvedValue([]);
|
||||
refreshS3Url.mockResolvedValue('new-s3-path.jpg');
|
||||
|
||||
const mockReq = {
|
||||
user: { id: userA.toString(), role: 'USER' },
|
||||
query: {},
|
||||
};
|
||||
const mockRes = {
|
||||
status: jest.fn().mockReturnThis(),
|
||||
json: jest.fn().mockReturnThis(),
|
||||
};
|
||||
|
||||
await getListAgentsHandler(mockReq, mockRes);
|
||||
|
||||
const responseData = mockRes.json.mock.calls[0][0];
|
||||
const agent = responseData.data.find((a) => a.id === agentId);
|
||||
/** The refresh runs alongside the list query, so the refreshed path must reach the
|
||||
* response through `urlCache` rather than through what the list query read. */
|
||||
expect(agent.avatar.filepath).toBe('new-s3-path.jpg');
|
||||
});
|
||||
|
||||
test('should scope the refresh query to S3 avatars without filtering the list query', async () => {
|
||||
const db = require('~/models');
|
||||
const listSpy = jest.spyOn(db, 'getListAgentsByAccess');
|
||||
mockCache.get.mockResolvedValue(false);
|
||||
findAccessibleResources.mockResolvedValue([agentWithLocalAvatar._id]);
|
||||
findPubliclyAccessibleResources.mockResolvedValue([]);
|
||||
|
||||
const mockReq = {
|
||||
user: { id: userA.toString(), role: 'USER' },
|
||||
query: {},
|
||||
};
|
||||
const mockRes = {
|
||||
status: jest.fn().mockReturnThis(),
|
||||
json: jest.fn().mockReturnThis(),
|
||||
};
|
||||
|
||||
try {
|
||||
await getListAgentsHandler(mockReq, mockRes);
|
||||
|
||||
/** The refresh pass must query only S3-avatar agents — `refreshListAvatars`
|
||||
* skips non-S3 entries anyway, so without this assertion the filter could
|
||||
* regress to `{}` (reloading the whole accessible set) unnoticed. */
|
||||
expect(listSpy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ otherParams: { 'avatar.source': FileSources.s3 } }),
|
||||
);
|
||||
/** The user-facing list query keeps the request filter, not the refresh scope. */
|
||||
expect(listSpy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ includeSkillConfig: true, otherParams: {} }),
|
||||
);
|
||||
|
||||
expect(refreshS3Url).not.toHaveBeenCalled();
|
||||
const responseData = mockRes.json.mock.calls[0][0];
|
||||
const agent = responseData.data.find((a) => a.id === agentWithLocalAvatar.id);
|
||||
expect(agent.avatar.filepath).toBe('local-path.jpg');
|
||||
} finally {
|
||||
listSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
test('should refresh avatars for all accessible agents (VIEW permission)', async () => {
|
||||
mockCache.get.mockResolvedValue(false);
|
||||
// User A has access to both their own agent and userB's agent
|
||||
|
|
|
|||
|
|
@ -238,11 +238,19 @@ const getResourcePermissionsMap = async ({ userId, role, resourceType, resourceI
|
|||
* @param {Object} params - Parameters for finding accessible resources
|
||||
* @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user
|
||||
* @param {string} [params.role] - Optional user role (if not provided, will query from DB)
|
||||
* @param {string|null} [params.idOnTheSource] - Optional external member id. `null` means "known to
|
||||
* be absent" (local user); only `undefined` makes `getUserPrincipals` read the user document.
|
||||
* @param {string} params.resourceType - Type of resource (e.g., 'agent')
|
||||
* @param {number} params.requiredPermissions - The minimum permission bits required (e.g., 1 for VIEW, 3 for VIEW+EDIT)
|
||||
* @returns {Promise<Array>} Array of resource IDs
|
||||
*/
|
||||
const findAccessibleResources = async ({ userId, role, resourceType, requiredPermissions }) => {
|
||||
const findAccessibleResources = async ({
|
||||
userId,
|
||||
role,
|
||||
idOnTheSource,
|
||||
resourceType,
|
||||
requiredPermissions,
|
||||
}) => {
|
||||
try {
|
||||
if (typeof requiredPermissions !== 'number' || requiredPermissions < 1) {
|
||||
throw new Error('requiredPermissions must be a positive number');
|
||||
|
|
@ -251,7 +259,7 @@ const findAccessibleResources = async ({ userId, role, resourceType, requiredPer
|
|||
validateResourceType(resourceType);
|
||||
|
||||
// Get all principals for the user (user + groups + public)
|
||||
const principalsList = await db.getUserPrincipals({ userId, role });
|
||||
const principalsList = await db.getUserPrincipals({ userId, role, idOnTheSource });
|
||||
|
||||
if (principalsList.length === 0) {
|
||||
return [];
|
||||
|
|
|
|||
|
|
@ -632,6 +632,24 @@ describe('PermissionService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
test('should forward idOnTheSource so principal resolution can skip the user lookup', async () => {
|
||||
getUserPrincipals.mockResolvedValue([
|
||||
{ principalType: PrincipalType.USER, principalId: userId },
|
||||
]);
|
||||
|
||||
await findAccessibleResources({
|
||||
userId,
|
||||
role: 'USER',
|
||||
idOnTheSource: null,
|
||||
resourceType: ResourceType.AGENT,
|
||||
requiredPermissions: 1, // VIEW
|
||||
});
|
||||
|
||||
expect(getUserPrincipals).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ idOnTheSource: null }),
|
||||
);
|
||||
});
|
||||
|
||||
test('should find resources user can view', async () => {
|
||||
// Mock getUserPrincipals to return user principal
|
||||
getUserPrincipals.mockResolvedValue([
|
||||
|
|
|
|||
|
|
@ -1,6 +1,11 @@
|
|||
import React, { createContext, useContext, useState, useMemo, useCallback } from 'react';
|
||||
import debounce from 'lodash/debounce';
|
||||
import { EModelEndpoint, isAgentsEndpoint, isAssistantsEndpoint } from 'librechat-data-provider';
|
||||
import {
|
||||
EModelEndpoint,
|
||||
PermissionBits,
|
||||
isAgentsEndpoint,
|
||||
isAssistantsEndpoint,
|
||||
} from 'librechat-data-provider';
|
||||
import type * as t from 'librechat-data-provider';
|
||||
import type { Endpoint, SelectedValues } from '~/common';
|
||||
import {
|
||||
|
|
@ -82,11 +87,27 @@ export function ModelSelectorProvider({ children, startupConfig }: ModelSelector
|
|||
}, [startupConfig, agentsMap]);
|
||||
|
||||
const permissionLevel = useAgentDefaultPermissionLevel();
|
||||
const { data: agents = null } = useListAgentsQuery(
|
||||
{ requiredPermission: permissionLevel },
|
||||
{
|
||||
select: (data) => data?.data,
|
||||
/**
|
||||
* Always query the VIEW scope so this shares one cache entry (and one paginated walk)
|
||||
* with `useAgentsMap` and `useMentions`. Asking for EDIT here spawned a second full
|
||||
* fetch under its own key, holding a duplicate copy of the whole agent list. The
|
||||
* marketplace's "my agents" framing is preserved by filtering on `isEditable`, which
|
||||
* the list endpoint resolves from the same ACL read it already performs.
|
||||
*/
|
||||
const wantsEditableOnly = permissionLevel === PermissionBits.EDIT;
|
||||
const selectAgents = useCallback(
|
||||
(data: t.AgentListResponse) => {
|
||||
const list = data?.data;
|
||||
if (!wantsEditableOnly) {
|
||||
return list;
|
||||
}
|
||||
return list?.filter((agent) => agent.isEditable !== false);
|
||||
},
|
||||
[wantsEditableOnly],
|
||||
);
|
||||
const { data: agents = null } = useListAgentsQuery(
|
||||
{ requiredPermission: PermissionBits.VIEW },
|
||||
{ select: selectAgents },
|
||||
);
|
||||
|
||||
const { mappedEndpoints, endpointRequiresUserKey } = useEndpoints({
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { useMemo } from 'react';
|
||||
import { useCallback, useMemo } from 'react';
|
||||
import { VisuallyHidden } from '@ariakit/react';
|
||||
import { Spinner, TooltipAnchor } from '@librechat/client';
|
||||
import { CheckCircle2, MousePointerClick, SettingsIcon } from 'lucide-react';
|
||||
|
|
@ -6,12 +6,13 @@ import { EModelEndpoint, isAgentsEndpoint, isAssistantsEndpoint } from 'librecha
|
|||
import type { TModelSpec } from 'librechat-data-provider';
|
||||
import type { Endpoint } from '~/common';
|
||||
import { CustomMenu as Menu, CustomMenuItem as MenuItem, CustomMenuSeparator } from '../CustomMenu';
|
||||
import { renderEndpointModels, VIRTUALIZE_THRESHOLD } from './EndpointModelItem';
|
||||
import MarketplaceItem, { marketplaceSearchMatches } from './Marketplace';
|
||||
import { filterModels, shouldRenderEndpointOption } from '../utils';
|
||||
import { useModelSelectorContext } from '../ModelSelectorContext';
|
||||
import { renderEndpointModels } from './EndpointModelItem';
|
||||
import VirtualizedModelList from './VirtualizedModelList';
|
||||
import { useFavorites, useLocalize } from '~/hooks';
|
||||
import { ModelSpecItem } from './ModelSpecItem';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { cn } from '~/utils';
|
||||
|
||||
interface EndpointItemProps {
|
||||
|
|
@ -133,21 +134,123 @@ function EndpointMenuContent({
|
|||
endpoint.showMarketplace === true && marketplaceSearchMatches(searchValue, localize);
|
||||
const hasSelectableRows = endpointSpecs.length > 0 || renderedModels.length > 0;
|
||||
|
||||
/**
|
||||
* Once the model list is windowed, the DOM no longer holds every option, so a screen
|
||||
* reader would infer position and total from the mounted slice alone. Declare them
|
||||
* explicitly across the whole listbox — mixing declared and inferred values within one
|
||||
* set is worse than either — and leave them off entirely when nothing is virtualized.
|
||||
*/
|
||||
const precedingOptionCount = (showMarketplace ? 1 : 0) + endpointSpecs.length;
|
||||
const isVirtualized = renderedModels.length > VIRTUALIZE_THRESHOLD;
|
||||
const listboxSetSize = isVirtualized ? precedingOptionCount + renderedModels.length : undefined;
|
||||
|
||||
return (
|
||||
<>
|
||||
{showMarketplace && <MarketplaceItem label={localize('com_agents_marketplace')} />}
|
||||
{showMarketplace && (
|
||||
<MarketplaceItem
|
||||
label={localize('com_agents_marketplace')}
|
||||
posInSet={isVirtualized ? 1 : undefined}
|
||||
setSize={listboxSetSize}
|
||||
/>
|
||||
)}
|
||||
{showMarketplace && hasSelectableRows && <CustomMenuSeparator />}
|
||||
{endpointSpecs.map((spec: TModelSpec) => (
|
||||
<ModelSpecItem key={spec.name} spec={spec} isSelected={selectedSpec === spec.name} />
|
||||
{endpointSpecs.map((spec: TModelSpec, specIndex: number) => (
|
||||
<ModelSpecItem
|
||||
key={spec.name}
|
||||
spec={spec}
|
||||
isSelected={selectedSpec === spec.name}
|
||||
posInSet={isVirtualized ? (showMarketplace ? 1 : 0) + specIndex + 1 : undefined}
|
||||
setSize={listboxSetSize}
|
||||
/>
|
||||
))}
|
||||
{filteredModels
|
||||
? renderEndpointModels(endpoint, endpoint.models || [], filteredModels, endpointIndex)
|
||||
: endpoint.models &&
|
||||
renderEndpointModels(endpoint, endpoint.models, undefined, endpointIndex)}
|
||||
<EndpointModels
|
||||
endpoint={endpoint}
|
||||
renderedModels={renderedModels}
|
||||
endpointIndex={endpointIndex}
|
||||
searchValue={searchValue}
|
||||
precedingOptionCount={precedingOptionCount}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Owns the model rows for one endpoint. `useFavorites` is called once here rather
|
||||
* than inside each row: it opens a jotai subscription, a React Query subscription
|
||||
* and a mutation per call site, which at agent-list scale was thousands of live
|
||||
* subscriptions for one dropdown.
|
||||
*/
|
||||
function EndpointModels({
|
||||
endpoint,
|
||||
renderedModels,
|
||||
endpointIndex,
|
||||
searchValue,
|
||||
precedingOptionCount,
|
||||
}: {
|
||||
endpoint: Endpoint;
|
||||
renderedModels: string[];
|
||||
endpointIndex: number;
|
||||
searchValue: string;
|
||||
precedingOptionCount: number;
|
||||
}) {
|
||||
const { isFavoriteModel, toggleFavoriteModel, isFavoriteAgent, toggleFavoriteAgent } =
|
||||
useFavorites();
|
||||
const isAgent = isAgentsEndpoint(endpoint.value);
|
||||
|
||||
const isFavorite = useCallback(
|
||||
(modelId: string) =>
|
||||
isAgent ? isFavoriteAgent(modelId) : isFavoriteModel(modelId, endpoint.value),
|
||||
[isAgent, isFavoriteAgent, isFavoriteModel, endpoint.value],
|
||||
);
|
||||
const onToggleFavorite = useCallback(
|
||||
(modelId: string) => {
|
||||
if (isAgent) {
|
||||
toggleFavoriteAgent(modelId);
|
||||
} else {
|
||||
toggleFavoriteModel({ model: modelId, endpoint: endpoint.value });
|
||||
}
|
||||
},
|
||||
[isAgent, toggleFavoriteAgent, toggleFavoriteModel, endpoint.value],
|
||||
);
|
||||
|
||||
const models = useMemo(() => endpoint.models ?? [], [endpoint.models]);
|
||||
const globalByName = useMemo(
|
||||
() => new Map(models.map((model) => [model.name, model.isGlobal ?? false])),
|
||||
[models],
|
||||
);
|
||||
|
||||
if (!renderedModels.length) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (renderedModels.length > VIRTUALIZE_THRESHOLD) {
|
||||
return (
|
||||
/**
|
||||
* Keyed on the filter so a new result set starts at the top. `Grid` keeps its
|
||||
* scroll offset across prop changes and, when the row count shrinks, clamps it to
|
||||
* `totalRowsHeight - height` — the END of the shorter list. Without this a user who
|
||||
* scrolled deep and then searched would land on the tail matches, with only those
|
||||
* rows mounted and therefore reachable by keyboard.
|
||||
*/
|
||||
<VirtualizedModelList
|
||||
key={searchValue}
|
||||
endpoint={endpoint}
|
||||
modelIds={renderedModels}
|
||||
globalByName={globalByName}
|
||||
isFavorite={isFavorite}
|
||||
onToggleFavorite={onToggleFavorite}
|
||||
endpointIndex={endpointIndex}
|
||||
precedingOptionCount={precedingOptionCount}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
return renderEndpointModels(endpoint, models, renderedModels, endpointIndex, {
|
||||
isFavorite,
|
||||
onToggleFavorite,
|
||||
});
|
||||
}
|
||||
|
||||
export function EndpointItem({ endpoint, endpointIndex }: EndpointItemProps) {
|
||||
const localize = useLocalize();
|
||||
const {
|
||||
|
|
|
|||
|
|
@ -3,17 +3,38 @@ import { VisuallyHidden } from '@ariakit/react';
|
|||
import { CheckCircle2, EarthIcon, Pin, PinOff } from 'lucide-react';
|
||||
import { isAgentsEndpoint, isAssistantsEndpoint } from 'librechat-data-provider';
|
||||
import type { Endpoint } from '~/common';
|
||||
import { useFavorites, useLocalize, useIsActiveItem } from '~/hooks';
|
||||
import { useModelSelectorContext } from '../ModelSelectorContext';
|
||||
import { CustomMenuItem as MenuItem } from '../CustomMenu';
|
||||
import useActiveItem from '../useActiveItem';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { cn } from '~/utils';
|
||||
|
||||
interface EndpointModelItemProps {
|
||||
modelId: string | null;
|
||||
endpoint: Endpoint;
|
||||
/** Resolved by the parent from the same array it maps, so the row does not rescan it. */
|
||||
isGlobal?: boolean;
|
||||
isFavorite: boolean;
|
||||
onToggleFavorite: (modelId: string) => void;
|
||||
/**
|
||||
* Only set when the list is virtualized. The mounted rows are then a small window over
|
||||
* a much larger set, so the position a screen reader would infer from the DOM is wrong;
|
||||
* these carry the real position and total. Left undefined otherwise, where the DOM holds
|
||||
* every option and the implicit values are already correct.
|
||||
*/
|
||||
posInSet?: number;
|
||||
setSize?: number;
|
||||
}
|
||||
|
||||
export function EndpointModelItem({ modelId, endpoint }: EndpointModelItemProps) {
|
||||
function EndpointModelItemComponent({
|
||||
modelId,
|
||||
endpoint,
|
||||
isGlobal = false,
|
||||
isFavorite,
|
||||
onToggleFavorite,
|
||||
posInSet,
|
||||
setSize,
|
||||
}: EndpointModelItemProps) {
|
||||
const localize = useLocalize();
|
||||
const { handleSelectModel, selectedValues } = useModelSelectorContext();
|
||||
const {
|
||||
|
|
@ -23,21 +44,15 @@ export function EndpointModelItem({ modelId, endpoint }: EndpointModelItemProps)
|
|||
} = selectedValues;
|
||||
const isSelected =
|
||||
!selectedSpec && selectedEndpoint === endpoint.value && selectedModel === modelId;
|
||||
const { isFavoriteModel, toggleFavoriteModel, isFavoriteAgent, toggleFavoriteAgent } =
|
||||
useFavorites();
|
||||
|
||||
const { ref: itemRef, isActive } = useIsActiveItem<HTMLDivElement>();
|
||||
const { ref: itemRef, isActive } = useActiveItem<HTMLDivElement>();
|
||||
|
||||
let isGlobal = false;
|
||||
let modelName = modelId;
|
||||
const avatarUrl = endpoint?.modelIcons?.[modelId ?? ''] || null;
|
||||
|
||||
// Use custom names if available
|
||||
if (endpoint && modelId && isAgentsEndpoint(endpoint.value) && endpoint.agentNames?.[modelId]) {
|
||||
modelName = endpoint.agentNames[modelId];
|
||||
|
||||
const modelInfo = endpoint?.models?.find((m) => m.name === modelId);
|
||||
isGlobal = modelInfo?.isGlobal ?? false;
|
||||
} else if (
|
||||
endpoint &&
|
||||
modelId &&
|
||||
|
|
@ -47,26 +62,12 @@ export function EndpointModelItem({ modelId, endpoint }: EndpointModelItemProps)
|
|||
modelName = endpoint.assistantNames[modelId];
|
||||
}
|
||||
|
||||
const isAgent = isAgentsEndpoint(endpoint.value);
|
||||
const isFavorite = isAgent
|
||||
? isFavoriteAgent(modelId ?? '')
|
||||
: isFavoriteModel(modelId ?? '', endpoint.value);
|
||||
|
||||
const handleFavoriteToggle = () => {
|
||||
const handleFavoriteClick = (e: React.MouseEvent) => {
|
||||
e.stopPropagation();
|
||||
if (!modelId) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (isAgent) {
|
||||
toggleFavoriteAgent(modelId);
|
||||
} else {
|
||||
toggleFavoriteModel({ model: modelId, endpoint: endpoint.value });
|
||||
}
|
||||
};
|
||||
|
||||
const handleFavoriteClick = (e: React.MouseEvent) => {
|
||||
e.stopPropagation();
|
||||
handleFavoriteToggle();
|
||||
onToggleFavorite(modelId);
|
||||
};
|
||||
|
||||
const renderAvatar = () => {
|
||||
|
|
@ -101,6 +102,8 @@ export function EndpointModelItem({ modelId, endpoint }: EndpointModelItemProps)
|
|||
ref={itemRef}
|
||||
onClick={() => handleSelectModel(endpoint, modelId ?? '')}
|
||||
aria-selected={isSelected || undefined}
|
||||
aria-posinset={posInSet}
|
||||
aria-setsize={setSize}
|
||||
className="group flex w-full cursor-pointer items-center justify-between rounded-lg px-2 text-sm"
|
||||
>
|
||||
<div className="flex w-full min-w-0 items-center gap-2 px-1 py-1">
|
||||
|
|
@ -141,23 +144,45 @@ export function EndpointModelItem({ modelId, endpoint }: EndpointModelItemProps)
|
|||
);
|
||||
}
|
||||
|
||||
export const EndpointModelItem = React.memo(EndpointModelItemComponent);
|
||||
|
||||
/**
|
||||
* Above this many rows the list is windowed. Below it, rendering everything keeps
|
||||
* Ariakit's composite registry complete, so arrow-key navigation and typeahead
|
||||
* reach every row — which is the behaviour virtualization has to work to preserve.
|
||||
*/
|
||||
export const VIRTUALIZE_THRESHOLD = 100;
|
||||
|
||||
export function renderEndpointModels(
|
||||
endpoint: Endpoint | null,
|
||||
models: Array<{ name: string; isGlobal?: boolean }>,
|
||||
filteredModels?: string[],
|
||||
endpointIndex?: number,
|
||||
favorites?: {
|
||||
isFavorite: (modelId: string) => boolean;
|
||||
onToggleFavorite: (modelId: string) => void;
|
||||
},
|
||||
) {
|
||||
if (!endpoint) {
|
||||
return null;
|
||||
}
|
||||
const modelsToRender = filteredModels || models.map((model) => model.name);
|
||||
const indexSuffix = endpointIndex != null ? `-${endpointIndex}` : '';
|
||||
const isFavorite = favorites?.isFavorite ?? (() => false);
|
||||
const onToggleFavorite = favorites?.onToggleFavorite ?? (() => {});
|
||||
|
||||
return modelsToRender.map(
|
||||
(modelId, modelIndex) =>
|
||||
endpoint && (
|
||||
<EndpointModelItem
|
||||
key={`${endpoint.value}${indexSuffix}-${modelId}-${modelIndex}`}
|
||||
modelId={modelId}
|
||||
endpoint={endpoint}
|
||||
/>
|
||||
),
|
||||
);
|
||||
/** `models` carries `isGlobal`; without this map each row rescanned the whole
|
||||
* array to recover it, which is quadratic in the number of agents. */
|
||||
const globalByName = new Map(models.map((model) => [model.name, model.isGlobal ?? false]));
|
||||
|
||||
return modelsToRender.map((modelId, modelIndex) => (
|
||||
<EndpointModelItem
|
||||
key={`${endpoint.value}${indexSuffix}-${modelId}-${modelIndex}`}
|
||||
modelId={modelId}
|
||||
endpoint={endpoint}
|
||||
isGlobal={globalByName.get(modelId) ?? false}
|
||||
isFavorite={isFavorite(modelId)}
|
||||
onToggleFavorite={onToggleFavorite}
|
||||
/>
|
||||
));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,9 +22,14 @@ export function marketplaceSearchMatches(searchValue: string, localize: Localize
|
|||
export default function MarketplaceItem({
|
||||
className,
|
||||
label,
|
||||
posInSet,
|
||||
setSize,
|
||||
}: {
|
||||
className?: string;
|
||||
label: string;
|
||||
/** Set when the sibling model list is virtualized; see `VirtualizedModelList`. */
|
||||
posInSet?: number;
|
||||
setSize?: number;
|
||||
}) {
|
||||
const navigate = useNavigate();
|
||||
|
||||
|
|
@ -32,6 +37,8 @@ export default function MarketplaceItem({
|
|||
<MenuItem
|
||||
onClick={() => navigate('/agents')}
|
||||
aria-label={label}
|
||||
aria-posinset={posInSet}
|
||||
aria-setsize={setSize}
|
||||
data-testid="model-selector-marketplace-item"
|
||||
className={cn(
|
||||
'flex w-full cursor-pointer items-center justify-between rounded-lg px-2 text-sm',
|
||||
|
|
|
|||
|
|
@ -12,9 +12,12 @@ import { cn } from '~/utils';
|
|||
interface ModelSpecItemProps {
|
||||
spec: TModelSpec;
|
||||
isSelected: boolean;
|
||||
/** Set when the sibling model list is virtualized; see `VirtualizedModelList`. */
|
||||
posInSet?: number;
|
||||
setSize?: number;
|
||||
}
|
||||
|
||||
export function ModelSpecItem({ spec, isSelected }: ModelSpecItemProps) {
|
||||
export function ModelSpecItem({ spec, isSelected, posInSet, setSize }: ModelSpecItemProps) {
|
||||
const localize = useLocalize();
|
||||
const { handleSelectSpec, endpointsConfig } = useModelSelectorContext();
|
||||
const { isFavoriteSpec, toggleFavoriteSpec } = useFavorites();
|
||||
|
|
@ -34,6 +37,8 @@ export function ModelSpecItem({ spec, isSelected }: ModelSpecItemProps) {
|
|||
ref={itemRef}
|
||||
onClick={() => handleSelectSpec(spec)}
|
||||
aria-selected={isSelected || undefined}
|
||||
aria-posinset={posInSet}
|
||||
aria-setsize={setSize}
|
||||
className="group flex w-full cursor-pointer items-center justify-between rounded-lg px-2 text-sm"
|
||||
>
|
||||
<div
|
||||
|
|
|
|||
|
|
@ -0,0 +1,163 @@
|
|||
import { useCallback, useEffect, useMemo, useRef } from 'react';
|
||||
import { List } from 'react-virtualized';
|
||||
import * as Ariakit from '@ariakit/react';
|
||||
import type { ListRowProps } from 'react-virtualized';
|
||||
import type { Endpoint } from '~/common';
|
||||
import { EndpointModelItem } from './EndpointModelItem';
|
||||
|
||||
/** Matches the rendered height of a `CustomMenuItem` row (px-2 py-1 around a py-1 body). */
|
||||
const ROW_HEIGHT = 36;
|
||||
const MAX_LIST_HEIGHT = 320;
|
||||
const OVERSCAN = 8;
|
||||
|
||||
interface VirtualizedModelListProps {
|
||||
endpoint: Endpoint;
|
||||
modelIds: string[];
|
||||
globalByName: Map<string, boolean>;
|
||||
isFavorite: (modelId: string) => boolean;
|
||||
onToggleFavorite: (modelId: string) => void;
|
||||
endpointIndex?: number;
|
||||
/** Count of options rendered ahead of this list in the same listbox (marketplace entry,
|
||||
* model specs), so `aria-posinset` is relative to the whole listbox and not just this list. */
|
||||
precedingOptionCount: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Windowed model list for endpoints with very large model sets (agents, mainly).
|
||||
*
|
||||
* Only the visible slice is mounted, so the per-row costs — Ariakit composite
|
||||
* registration, the active-item subscription, and ~12 DOM nodes each — stay
|
||||
* bounded no matter how many agents the user can see.
|
||||
*
|
||||
* Ariakit's composite only knows about mounted rows, so arrow-keying to the edge
|
||||
* of the window would otherwise find no next item and let focus escape the nested
|
||||
* menu, closing it. `handleBoundaryNavigation` catches that case: it scrolls the
|
||||
* next index into the window, waits for the row to mount, then moves the composite
|
||||
* onto it. Navigation inside the window is left entirely to Ariakit, whose own
|
||||
* `scrollIntoView` drives the list's scroll position.
|
||||
*/
|
||||
export default function VirtualizedModelList({
|
||||
endpoint,
|
||||
modelIds,
|
||||
globalByName,
|
||||
isFavorite,
|
||||
onToggleFavorite,
|
||||
endpointIndex,
|
||||
precedingOptionCount,
|
||||
}: VirtualizedModelListProps) {
|
||||
const listRef = useRef<List>(null);
|
||||
const containerRef = useRef<HTMLDivElement>(null);
|
||||
const combobox = Ariakit.useComboboxContext();
|
||||
const indexSuffix = endpointIndex != null ? `-${endpointIndex}` : '';
|
||||
const rowCount = modelIds.length;
|
||||
|
||||
const rowAt = useCallback(
|
||||
(index: number) =>
|
||||
containerRef.current?.querySelector<HTMLElement>(
|
||||
`[data-row-index="${index}"] [role="option"], [data-row-index="${index}"] [role="menuitem"]`,
|
||||
) ?? null,
|
||||
[],
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
if (!combobox) {
|
||||
return;
|
||||
}
|
||||
const deltaFor = (key: string) => {
|
||||
if (key === 'ArrowDown') {
|
||||
return 1;
|
||||
}
|
||||
return key === 'ArrowUp' ? -1 : 0;
|
||||
};
|
||||
const handleBoundaryNavigation = (event: KeyboardEvent) => {
|
||||
const delta = deltaFor(event.key);
|
||||
if (delta === 0) {
|
||||
return;
|
||||
}
|
||||
const activeId = combobox.getState().activeId;
|
||||
const activeRow = activeId ? document.getElementById(activeId) : null;
|
||||
const wrapper = activeRow?.closest<HTMLElement>('[data-row-index]');
|
||||
if (!wrapper || !containerRef.current?.contains(wrapper)) {
|
||||
return;
|
||||
}
|
||||
const next = Number(wrapper.dataset.rowIndex) + delta;
|
||||
/** Let Ariakit own both the in-window case and the ends of the list. */
|
||||
if (next < 0 || next >= rowCount || rowAt(next)) {
|
||||
return;
|
||||
}
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
listRef.current?.scrollToRow(next);
|
||||
requestAnimationFrame(() => {
|
||||
requestAnimationFrame(() => {
|
||||
const id = rowAt(next)?.id;
|
||||
if (id) {
|
||||
combobox.move(id);
|
||||
}
|
||||
});
|
||||
});
|
||||
};
|
||||
document.addEventListener('keydown', handleBoundaryNavigation, true);
|
||||
return () => document.removeEventListener('keydown', handleBoundaryNavigation, true);
|
||||
}, [combobox, rowCount, rowAt]);
|
||||
|
||||
const height = useMemo(
|
||||
() => Math.min(MAX_LIST_HEIGHT, Math.max(ROW_HEIGHT, rowCount * ROW_HEIGHT)),
|
||||
[rowCount],
|
||||
);
|
||||
|
||||
const rowRenderer = useCallback(
|
||||
({ index, key, style }: ListRowProps) => {
|
||||
const modelId = modelIds[index];
|
||||
return (
|
||||
<div key={key} style={style} data-row-index={index}>
|
||||
<EndpointModelItem
|
||||
modelId={modelId}
|
||||
endpoint={endpoint}
|
||||
isGlobal={globalByName.get(modelId) ?? false}
|
||||
isFavorite={isFavorite(modelId)}
|
||||
onToggleFavorite={onToggleFavorite}
|
||||
posInSet={precedingOptionCount + index + 1}
|
||||
setSize={precedingOptionCount + rowCount}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
},
|
||||
[
|
||||
endpoint,
|
||||
globalByName,
|
||||
isFavorite,
|
||||
modelIds,
|
||||
onToggleFavorite,
|
||||
precedingOptionCount,
|
||||
rowCount,
|
||||
],
|
||||
);
|
||||
|
||||
return (
|
||||
<div ref={containerRef} data-endpoint-models={`${endpoint.value}${indexSuffix}`}>
|
||||
<List
|
||||
ref={listRef}
|
||||
width={360}
|
||||
height={height}
|
||||
rowCount={rowCount}
|
||||
rowHeight={ROW_HEIGHT}
|
||||
overscanRowCount={OVERSCAN}
|
||||
rowRenderer={rowRenderer}
|
||||
className="outline-none!"
|
||||
style={{ width: '100%' }}
|
||||
/**
|
||||
* `List` spreads its props onto the underlying `Grid`, whose defaults are
|
||||
* `role="grid"`, `containerRole="row"` and `tabIndex={0}`. Left alone, that puts a
|
||||
* focusable grid between Ariakit's listbox and its options: tabbing out of the
|
||||
* search field lands on the wrapper instead of a row, where the combobox no longer
|
||||
* owns the keystroke, and the grid/row semantics fight the surrounding listbox.
|
||||
* Neutralise both so focus and ARIA stay with the combobox items.
|
||||
*/
|
||||
role="presentation"
|
||||
containerRole="presentation"
|
||||
tabIndex={-1}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
@ -26,15 +26,24 @@ jest.mock('~/components/Chat/Menus/Endpoints/CustomMenu', () => {
|
|||
|
||||
jest.mock('~/hooks', () => ({
|
||||
useLocalize: () => (key: string) => key,
|
||||
useFavorites: () => ({
|
||||
isFavoriteModel: () => false,
|
||||
toggleFavoriteModel: jest.fn(),
|
||||
isFavoriteAgent: () => false,
|
||||
toggleFavoriteAgent: jest.fn(),
|
||||
}),
|
||||
useIsActiveItem: () => ({ ref: { current: null }, isActive: false }),
|
||||
}));
|
||||
|
||||
jest.mock('~/components/Chat/Menus/Endpoints/useActiveItem', () => ({
|
||||
__esModule: true,
|
||||
default: () => ({ ref: { current: null }, isActive: false }),
|
||||
}));
|
||||
|
||||
const renderItem = (props: Partial<React.ComponentProps<typeof EndpointModelItem>> = {}) =>
|
||||
render(
|
||||
<EndpointModelItem
|
||||
modelId="claude-opus-4-6"
|
||||
endpoint={baseEndpoint}
|
||||
isFavorite={false}
|
||||
onToggleFavorite={jest.fn()}
|
||||
{...props}
|
||||
/>,
|
||||
);
|
||||
|
||||
const baseEndpoint: Endpoint = {
|
||||
value: 'anthropic',
|
||||
label: 'Anthropic',
|
||||
|
|
@ -50,7 +59,7 @@ describe('EndpointModelItem', () => {
|
|||
|
||||
it('renders checkmark when model and endpoint match with no active spec', () => {
|
||||
mockSelectedValues = { endpoint: 'anthropic', model: 'claude-opus-4-6', modelSpec: '' };
|
||||
render(<EndpointModelItem modelId="claude-opus-4-6" endpoint={baseEndpoint} />);
|
||||
renderItem();
|
||||
|
||||
const menuItem = screen.getByRole('menuitem');
|
||||
expect(menuItem).toHaveAttribute('aria-selected', 'true');
|
||||
|
|
@ -62,7 +71,7 @@ describe('EndpointModelItem', () => {
|
|||
model: 'claude-opus-4-6',
|
||||
modelSpec: 'my-anthropic-spec',
|
||||
};
|
||||
render(<EndpointModelItem modelId="claude-opus-4-6" endpoint={baseEndpoint} />);
|
||||
renderItem();
|
||||
|
||||
const menuItem = screen.getByRole('menuitem');
|
||||
expect(menuItem).not.toHaveAttribute('aria-selected');
|
||||
|
|
@ -70,7 +79,7 @@ describe('EndpointModelItem', () => {
|
|||
|
||||
it('does NOT render checkmark when model matches but endpoint differs', () => {
|
||||
mockSelectedValues = { endpoint: 'openai', model: 'claude-opus-4-6', modelSpec: '' };
|
||||
render(<EndpointModelItem modelId="claude-opus-4-6" endpoint={baseEndpoint} />);
|
||||
renderItem();
|
||||
|
||||
const menuItem = screen.getByRole('menuitem');
|
||||
expect(menuItem).not.toHaveAttribute('aria-selected');
|
||||
|
|
@ -78,7 +87,7 @@ describe('EndpointModelItem', () => {
|
|||
|
||||
it('does NOT render checkmark when endpoint matches but model differs', () => {
|
||||
mockSelectedValues = { endpoint: 'anthropic', model: 'claude-sonnet-4-5', modelSpec: '' };
|
||||
render(<EndpointModelItem modelId="claude-opus-4-6" endpoint={baseEndpoint} />);
|
||||
renderItem();
|
||||
|
||||
const menuItem = screen.getByRole('menuitem');
|
||||
expect(menuItem).not.toHaveAttribute('aria-selected');
|
||||
|
|
|
|||
33
client/src/components/Chat/Menus/Endpoints/useActiveItem.ts
Normal file
33
client/src/components/Chat/Menus/Endpoints/useActiveItem.ts
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
import { useRef } from 'react';
|
||||
import * as Ariakit from '@ariakit/react';
|
||||
import type { RefObject } from 'react';
|
||||
|
||||
/**
|
||||
* Reports whether this row is the composite's active item, read from the Ariakit
|
||||
* store rather than from a per-row `MutationObserver`.
|
||||
*
|
||||
* `useIsActiveItem` allocates one observer per mounted row, which is fine for a
|
||||
* handful of rows and ruinous for a large model list — and under virtualization
|
||||
* rows mount and unmount on every scroll frame, so the observers churn as well.
|
||||
* The store already tracks `activeId`; the selector returns a boolean so a row
|
||||
* only re-renders when its own active state flips, not on every arrow key.
|
||||
*/
|
||||
export default function useActiveItem<T extends HTMLElement = HTMLElement>(): {
|
||||
ref: RefObject<T>;
|
||||
isActive: boolean;
|
||||
} {
|
||||
const ref = useRef<T>(null);
|
||||
const combobox = Ariakit.useComboboxContext();
|
||||
const menu = Ariakit.useMenuContext();
|
||||
/** Endpoint submenus render as a combobox list; plain menus fall back to the menu store. */
|
||||
const store = combobox ?? menu;
|
||||
|
||||
const isActive =
|
||||
Ariakit.useStoreState(
|
||||
store,
|
||||
(state) =>
|
||||
state?.activeId != null && ref.current != null && state.activeId === ref.current.id,
|
||||
) ?? false;
|
||||
|
||||
return { ref, isActive };
|
||||
}
|
||||
|
|
@ -1,10 +1,14 @@
|
|||
import { createElement } from 'react';
|
||||
import { dataService, QueryKeys } from 'librechat-data-provider';
|
||||
import { act, renderHook, waitFor } from '@testing-library/react';
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
import type { Agent, GraphEdge } from 'librechat-data-provider';
|
||||
import { dataService, PermissionBits, QueryKeys } from 'librechat-data-provider';
|
||||
import type { Agent, AgentListResponse, GraphEdge } from 'librechat-data-provider';
|
||||
import type { ReactNode } from 'react';
|
||||
import { useDeleteAgentMutation } from '../mutations';
|
||||
import {
|
||||
useDeleteAgentMutation,
|
||||
useDuplicateAgentMutation,
|
||||
useUpdateAgentMutation,
|
||||
} from '../mutations';
|
||||
|
||||
jest.mock('librechat-data-provider', () => {
|
||||
const actual = jest.requireActual('librechat-data-provider');
|
||||
|
|
@ -13,11 +17,13 @@ jest.mock('librechat-data-provider', () => {
|
|||
dataService: {
|
||||
...actual.dataService,
|
||||
deleteAgent: jest.fn(),
|
||||
updateAgent: jest.fn(),
|
||||
duplicateAgent: jest.fn(),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
const createAgent = (id: string, edges: GraphEdge[] = []): Agent => ({
|
||||
const createAgent = (id: string, edges: GraphEdge[] = [], isEditable?: boolean): Agent => ({
|
||||
id,
|
||||
name: id,
|
||||
description: null,
|
||||
|
|
@ -35,6 +41,7 @@ const createAgent = (id: string, edges: GraphEdge[] = []): Agent => ({
|
|||
presence_penalty: null,
|
||||
},
|
||||
edges,
|
||||
...(isEditable !== undefined ? { isEditable } : {}),
|
||||
});
|
||||
|
||||
const createWrapper = (queryClient: QueryClient) =>
|
||||
|
|
@ -111,3 +118,88 @@ describe('useDeleteAgentMutation', () => {
|
|||
expect(queryClient.getQueryData([QueryKeys.agent, targetId, 'expanded'])).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('useUpdateAgentMutation', () => {
|
||||
it('preserves the list-cache isEditable flag after a successful update', async () => {
|
||||
/** MANAGE_AGENTS can PATCH agents the ACL marks non-editable. Mutation success must
|
||||
* not promote those VIEW rows into the editable-only "My Agents" subset. */
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: {
|
||||
queries: {
|
||||
retry: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
const agentId = 'agent_view_only';
|
||||
const listKey = [QueryKeys.agents, { requiredPermission: PermissionBits.VIEW }];
|
||||
const cachedList: AgentListResponse = {
|
||||
object: 'list',
|
||||
data: [createAgent(agentId, [], false)],
|
||||
first_id: agentId,
|
||||
last_id: agentId,
|
||||
has_more: false,
|
||||
};
|
||||
queryClient.setQueryData(listKey, cachedList);
|
||||
|
||||
const updatedAgent = createAgent(agentId);
|
||||
updatedAgent.name = 'Renamed';
|
||||
jest.mocked(dataService.updateAgent).mockResolvedValue(updatedAgent);
|
||||
|
||||
const { result } = renderHook(() => useUpdateAgentMutation(), {
|
||||
wrapper: createWrapper(queryClient),
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.mutateAsync({ agent_id: agentId, data: { name: 'Renamed' } });
|
||||
});
|
||||
|
||||
const listRes = queryClient.getQueryData<AgentListResponse>(listKey);
|
||||
expect(listRes?.data).toHaveLength(1);
|
||||
expect(listRes?.data[0]).toMatchObject({
|
||||
id: agentId,
|
||||
name: 'Renamed',
|
||||
isEditable: false,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('useDuplicateAgentMutation', () => {
|
||||
it('marks the duplicated agent editable in the list cache', async () => {
|
||||
/** Duplicating grants ownership, so the new row is editable. Without this the row
|
||||
* carries no `isEditable` and only survives the selector filter by failing open. */
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: {
|
||||
queries: {
|
||||
retry: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
const sourceId = 'agent_source';
|
||||
const duplicateId = 'agent_duplicate';
|
||||
const listKey = [QueryKeys.agents, { requiredPermission: PermissionBits.VIEW }];
|
||||
queryClient.setQueryData(listKey, {
|
||||
object: 'list',
|
||||
data: [createAgent(sourceId, [], false)],
|
||||
first_id: sourceId,
|
||||
last_id: sourceId,
|
||||
has_more: false,
|
||||
} satisfies AgentListResponse);
|
||||
|
||||
jest
|
||||
.mocked(dataService.duplicateAgent)
|
||||
.mockResolvedValue({ agent: createAgent(duplicateId), actions: [] });
|
||||
|
||||
const { result } = renderHook(() => useDuplicateAgentMutation(), {
|
||||
wrapper: createWrapper(queryClient),
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.mutateAsync({ agent_id: sourceId });
|
||||
});
|
||||
|
||||
const listRes = queryClient.getQueryData<AgentListResponse>(listKey);
|
||||
expect(listRes?.data[0]).toMatchObject({ id: duplicateId, isEditable: true });
|
||||
/** The untouched source row keeps its own ACL flag. */
|
||||
expect(listRes?.data[1]).toMatchObject({ id: sourceId, isEditable: false });
|
||||
});
|
||||
});
|
||||
|
|
|
|||
91
client/src/data-provider/Agents/__tests__/queries.test.ts
Normal file
91
client/src/data-provider/Agents/__tests__/queries.test.ts
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
import { createElement } from 'react';
|
||||
import { renderHook, waitFor } from '@testing-library/react';
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
import { dataService, QueryKeys, EModelEndpoint, PermissionBits } from 'librechat-data-provider';
|
||||
import type { AgentListResponse } from 'librechat-data-provider';
|
||||
import type { ReactNode } from 'react';
|
||||
import { defaultAgentParams, useListAgentsQuery } from '../queries';
|
||||
|
||||
jest.mock('librechat-data-provider', () => {
|
||||
const actual = jest.requireActual('librechat-data-provider');
|
||||
return {
|
||||
...actual,
|
||||
dataService: {
|
||||
...actual.dataService,
|
||||
listAgents: jest.fn(),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
const listAgents = dataService.listAgents as jest.MockedFunction<typeof dataService.listAgents>;
|
||||
|
||||
const page = (ids: string[], after: string | null): AgentListResponse =>
|
||||
({
|
||||
object: 'list',
|
||||
data: ids.map((id) => ({ id, name: id })),
|
||||
has_more: after != null,
|
||||
after,
|
||||
first_id: ids[0] ?? '',
|
||||
last_id: ids[ids.length - 1] ?? '',
|
||||
}) as unknown as AgentListResponse;
|
||||
|
||||
const createWrapper = (queryClient: QueryClient) =>
|
||||
function Wrapper({ children }: { children: ReactNode }) {
|
||||
return createElement(QueryClientProvider, { client: queryClient }, children);
|
||||
};
|
||||
|
||||
const renderListAgents = (params: Parameters<typeof useListAgentsQuery>[0]) => {
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false } },
|
||||
});
|
||||
/** The hook is gated on the agents endpoint being configured. */
|
||||
queryClient.setQueryData([QueryKeys.endpoints], { [EModelEndpoint.agents]: {} });
|
||||
return renderHook(() => useListAgentsQuery(params), {
|
||||
wrapper: createWrapper(queryClient),
|
||||
});
|
||||
};
|
||||
|
||||
describe('useListAgentsQuery', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('requests the server maximum page size so a typical agent set resolves in one round trip', async () => {
|
||||
listAgents.mockResolvedValue(page(['a', 'b'], null));
|
||||
|
||||
const { result } = renderListAgents({ requiredPermission: PermissionBits.VIEW });
|
||||
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
expect(listAgents).toHaveBeenCalledTimes(1);
|
||||
expect(listAgents).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ limit: 1000, requiredPermission: PermissionBits.VIEW }),
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps the walk page size when a caller supplies a smaller limit', async () => {
|
||||
listAgents.mockResolvedValue(page(['a'], null));
|
||||
|
||||
const { result } = renderListAgents({ limit: 10, requiredPermission: PermissionBits.VIEW });
|
||||
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
expect(listAgents).toHaveBeenCalledWith(expect.objectContaining({ limit: 1000 }));
|
||||
});
|
||||
|
||||
it('does not carry a page size in the default params', async () => {
|
||||
expect(defaultAgentParams.limit).toBeUndefined();
|
||||
});
|
||||
|
||||
it('still walks every page and flattens the result when the server returns a cursor', async () => {
|
||||
listAgents
|
||||
.mockResolvedValueOnce(page(['a', 'b'], 'cursor-1'))
|
||||
.mockResolvedValueOnce(page(['c'], null));
|
||||
|
||||
const { result } = renderListAgents({ requiredPermission: PermissionBits.VIEW });
|
||||
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
expect(listAgents).toHaveBeenCalledTimes(2);
|
||||
expect(listAgents).toHaveBeenLastCalledWith(expect.objectContaining({ cursor: 'cursor-1' }));
|
||||
expect(result.current.data?.data.map((agent) => agent.id)).toEqual(['a', 'b', 'c']);
|
||||
expect(result.current.data?.has_more).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -30,6 +30,18 @@ const hasEdgeWithAgent = (data: unknown, agentId: string): boolean => {
|
|||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Mutation responses omit list-only `isEditable`. When merging into a cached list
|
||||
* row, keep the ACL flag the list endpoint set rather than inferring it from
|
||||
* write success (`MANAGE_AGENTS` can PATCH agents the ACL marks non-editable).
|
||||
*/
|
||||
const mergeAgentListRow = (previous: t.Agent, next: t.Agent): t.Agent => {
|
||||
if (previous.isEditable === undefined) {
|
||||
return next;
|
||||
}
|
||||
return { ...next, isEditable: previous.isEditable };
|
||||
};
|
||||
|
||||
/**
|
||||
* Create a new agent
|
||||
*/
|
||||
|
|
@ -47,7 +59,12 @@ export const useCreateAgentMutation = (
|
|||
if (!listRes) {
|
||||
return options?.onSuccess?.(newAgent, variables, context);
|
||||
}
|
||||
const currentAgents = [newAgent, ...JSON.parse(JSON.stringify(listRes.data))];
|
||||
/** The create succeeded, so the caller can edit it. Mutation responses carry no
|
||||
* `isEditable`; without this the cached row loses the field the list sets. */
|
||||
const currentAgents = [
|
||||
{ ...newAgent, isEditable: true },
|
||||
...JSON.parse(JSON.stringify(listRes.data)),
|
||||
];
|
||||
|
||||
queryClient.setQueryData<t.AgentListResponse>([QueryKeys.agents, key], {
|
||||
...listRes,
|
||||
|
|
@ -94,7 +111,7 @@ export const useUpdateAgentMutation = (
|
|||
...listRes,
|
||||
data: listRes.data.map((agent) => {
|
||||
if (agent.id === variables.agent_id) {
|
||||
return updatedAgent;
|
||||
return mergeAgentListRow(agent, updatedAgent);
|
||||
}
|
||||
return agent;
|
||||
}),
|
||||
|
|
@ -186,7 +203,9 @@ export const useDuplicateAgentMutation = (
|
|||
keys.forEach((key) => {
|
||||
const listRes = queryClient.getQueryData<t.AgentListResponse>([QueryKeys.agents, key]);
|
||||
if (listRes) {
|
||||
const currentAgents = [agent, ...listRes.data];
|
||||
/** Duplicating grants the caller ownership, so the new row is editable.
|
||||
* The response omits list-only `isEditable`; see `mergeAgentListRow`. */
|
||||
const currentAgents = [{ ...agent, isEditable: true }, ...listRes.data];
|
||||
queryClient.setQueryData<t.AgentListResponse>([QueryKeys.agents, key], {
|
||||
...listRes,
|
||||
data: currentAgents,
|
||||
|
|
@ -235,7 +254,7 @@ export const useUploadAgentAvatarMutation = (
|
|||
...listRes,
|
||||
data: listRes.data.map((agent) => {
|
||||
if (agent.id === variables.agent_id) {
|
||||
return updatedAgent;
|
||||
return mergeAgentListRow(agent, updatedAgent);
|
||||
}
|
||||
return agent;
|
||||
}),
|
||||
|
|
@ -286,7 +305,7 @@ export const useUpdateAgentAction = (
|
|||
...listRes,
|
||||
data: listRes.data.map((agent) => {
|
||||
if (agent.id === variables.agent_id) {
|
||||
return updatedAgent;
|
||||
return mergeAgentListRow(agent, updatedAgent);
|
||||
}
|
||||
return agent;
|
||||
}),
|
||||
|
|
@ -422,7 +441,7 @@ export const useRevertAgentVersionMutation = (
|
|||
...listRes,
|
||||
data: listRes.data.map((agent) => {
|
||||
if (agent.id === variables.agent_id) {
|
||||
return revertedAgent;
|
||||
return mergeAgentListRow(agent, revertedAgent);
|
||||
}
|
||||
return agent;
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -12,10 +12,18 @@ import { isEphemeralAgent } from '~/common';
|
|||
* AGENTS
|
||||
*/
|
||||
export const defaultAgentParams: t.AgentListParams = {
|
||||
limit: 10,
|
||||
requiredPermission: PermissionBits.EDIT,
|
||||
};
|
||||
|
||||
/**
|
||||
* Page size for the internal pagination walk. Callers consume the flattened result, so
|
||||
* every page costs a serial round trip with no benefit: request the server's maximum
|
||||
* (`getListAgentsByAccess` caps at 1000) so realistic agent sets resolve in one request.
|
||||
* Kept out of the query key, and applied last so a caller-supplied `limit` cannot shrink
|
||||
* it: this is a transport detail, and a caller limit never bounds what the walk returns.
|
||||
*/
|
||||
const WALK_PAGE_SIZE = 1000;
|
||||
|
||||
/** Walk the cursor pagination and return all pages flattened into one `AgentListResponse`. */
|
||||
async function fetchAllAgentPages(params: t.AgentListParams): Promise<t.AgentListResponse> {
|
||||
const pages: t.AgentListResponse[] = [];
|
||||
|
|
@ -24,6 +32,7 @@ async function fetchAllAgentPages(params: t.AgentListParams): Promise<t.AgentLis
|
|||
const page = await dataService.listAgents({
|
||||
...params,
|
||||
...(cursor ? { cursor } : {}),
|
||||
limit: WALK_PAGE_SIZE,
|
||||
});
|
||||
pages.push(page);
|
||||
cursor = page.after;
|
||||
|
|
|
|||
|
|
@ -2264,6 +2264,86 @@ describe('initializeAgent — code-generated file thread filter (regression)', (
|
|||
* empty-guard is exercised by data-schemas tests. */
|
||||
expect(getUserCodeFiles).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips the thread walk when parentMessageId is an empty string', async () => {
|
||||
/* An empty anchor can never match a parent chain, so walking the
|
||||
* conversation only buys an unbounded read whose result is discarded.
|
||||
* `req.body.parentMessageId` reaches this layer unnormalized. */
|
||||
const { agent, req, res, loadTools, db } = setupExecuteCodeAgent();
|
||||
|
||||
const getMessages = jest.fn().mockResolvedValue([]);
|
||||
const getConvoFiles = jest.fn().mockResolvedValue([]);
|
||||
|
||||
await initializeAgent(
|
||||
{
|
||||
req,
|
||||
res,
|
||||
agent,
|
||||
loadTools,
|
||||
endpointOption: { endpoint: EModelEndpoint.agents },
|
||||
conversationId: 'conv-1',
|
||||
parentMessageId: '',
|
||||
allowedProviders: new Set([Providers.OPENAI]),
|
||||
isInitialAgent: true,
|
||||
codeEnvAvailable: true,
|
||||
},
|
||||
{ ...db, getMessages, getConvoFiles },
|
||||
);
|
||||
|
||||
expect(getMessages).not.toHaveBeenCalled();
|
||||
expect(mockGetThreadData).not.toHaveBeenCalled();
|
||||
/* The conversation read is unconditional and must survive the guard. */
|
||||
expect(getConvoFiles).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('dispatches the convo-file read and the thread walk concurrently', async () => {
|
||||
/* Both reads gate the model call, so serializing them costs
|
||||
* time-to-first-token on every turn. Holding BOTH unresolved is what
|
||||
* makes this fail under either ordering: whichever runs first blocks,
|
||||
* and the second is never dispatched.
|
||||
*
|
||||
* DELETE this test, do not repair it, if the thread walk ever gains a
|
||||
* data dependency on the convo file ids — serializing becomes correct. */
|
||||
const { agent, req, res, loadTools, db } = setupExecuteCodeAgent();
|
||||
|
||||
let releaseConvoFiles!: (fileIds: string[]) => void;
|
||||
let releaseMessages!: (messages: Array<{ messageId: string }>) => void;
|
||||
const getConvoFiles = jest
|
||||
.fn()
|
||||
.mockReturnValue(new Promise<string[]>((resolve) => (releaseConvoFiles = resolve)));
|
||||
const getMessages = jest
|
||||
.fn()
|
||||
.mockReturnValue(
|
||||
new Promise<Array<{ messageId: string }>>((resolve) => (releaseMessages = resolve)),
|
||||
);
|
||||
|
||||
const initialized = initializeAgent(
|
||||
{
|
||||
req,
|
||||
res,
|
||||
agent,
|
||||
loadTools,
|
||||
endpointOption: { endpoint: EModelEndpoint.agents },
|
||||
conversationId: 'conv-1',
|
||||
parentMessageId: 'msgN',
|
||||
allowedProviders: new Set([Providers.OPENAI]),
|
||||
isInitialAgent: true,
|
||||
codeEnvAvailable: true,
|
||||
},
|
||||
{ ...db, getConvoFiles, getMessages },
|
||||
);
|
||||
|
||||
/* Drain pending microtasks so the mocked chain runs up to the first
|
||||
* genuinely-pending await. */
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
expect(getConvoFiles).toHaveBeenCalledTimes(1);
|
||||
expect(getMessages).toHaveBeenCalledTimes(1);
|
||||
|
||||
releaseConvoFiles([]);
|
||||
releaseMessages([]);
|
||||
await initialized;
|
||||
});
|
||||
});
|
||||
|
||||
describe('initializeAgent — run-scoped MCP tool definitions', () => {
|
||||
|
|
|
|||
|
|
@ -707,7 +707,6 @@ export async function initializeAgent(
|
|||
* on handoff agents would fail to find previously attached files.
|
||||
*/
|
||||
if (conversationId != null && resendFiles) {
|
||||
const fileIds = (await db.getConvoFiles(conversationId)) ?? [];
|
||||
const toolResourceSet = new Set<EToolResources>();
|
||||
for (const tool of agent.tools ?? []) {
|
||||
if (EToolResources[tool as keyof typeof EToolResources]) {
|
||||
|
|
@ -715,74 +714,76 @@ export async function initializeAgent(
|
|||
}
|
||||
}
|
||||
|
||||
const toolFiles = requestFileOwnerScope
|
||||
? ((await db.getToolFilesByIds(
|
||||
fileIds,
|
||||
toolResourceSet,
|
||||
requestFileOwnerScope,
|
||||
)) as IMongoFile[])
|
||||
: [];
|
||||
const getThreadMessages = db.getMessages;
|
||||
/** Falsy anchors cannot match a parent chain, so they get no walk. */
|
||||
const threadAnchor =
|
||||
parentMessageId && parentMessageId !== Constants.NO_PARENT ? parentMessageId : null;
|
||||
const needsThreadWalk =
|
||||
toolResourceSet.has(EToolResources.execute_code) &&
|
||||
threadAnchor != null &&
|
||||
getThreadMessages != null;
|
||||
|
||||
/**
|
||||
* The conversation's file refs and the thread walk share no inputs, so they resolve
|
||||
* together. Both gate the model call, and this runs on every turn — each serialized
|
||||
* round trip here is time-to-first-token the user waits through.
|
||||
*
|
||||
* Thread walk selects only the fields traversal needs. Both `files` (user uploads)
|
||||
* and `attachments` (code-execution outputs from `processCodeOutput`) carry the
|
||||
* `file_id` refs the next turn must prime — selecting only `files` silently drops
|
||||
* every code-output ref.
|
||||
*/
|
||||
const [convoFileIds, threadMessages] = await Promise.all([
|
||||
db.getConvoFiles(conversationId),
|
||||
needsThreadWalk && getThreadMessages
|
||||
? getThreadMessages({ conversationId }, 'messageId parentMessageId files attachments')
|
||||
: null,
|
||||
]);
|
||||
const fileIds = convoFileIds ?? [];
|
||||
|
||||
/** Walk the parent chain and collect file_ids referenced by
|
||||
* any message in the thread (`messages.files[].file_id` +
|
||||
* `messages.attachments[].file_id`). Used as the primary
|
||||
* anchor for both `getCodeGeneratedFiles` and
|
||||
* `getUserCodeFiles` — message ids no longer needed at
|
||||
* this layer. */
|
||||
const threadFileIds =
|
||||
threadMessages && threadMessages.length > 0
|
||||
? getThreadData(threadMessages, threadAnchor).fileIds
|
||||
: undefined;
|
||||
|
||||
/**
|
||||
* Retrieve execute_code files filtered to the current thread.
|
||||
* This includes both code-generated files and user-uploaded execute_code files.
|
||||
*
|
||||
* Code-generated and user-uploaded execute_code files share the same primary anchor:
|
||||
* file_ids referenced by messages in the current thread. The two queries differ only
|
||||
* by `context` (`execute_code` for generated outputs, others for uploads). Anchoring
|
||||
* both on `threadFileIds` reaches files regardless of which sibling first generated
|
||||
* them — see `getCodeGeneratedFiles` for the branched-conversation rationale.
|
||||
*/
|
||||
let codeGeneratedFiles: IMongoFile[] = [];
|
||||
let userCodeFiles: IMongoFile[] = [];
|
||||
|
||||
if (toolResourceSet.has(EToolResources.execute_code)) {
|
||||
let threadFileIds: string[] | undefined;
|
||||
|
||||
if (parentMessageId && parentMessageId !== Constants.NO_PARENT && db.getMessages) {
|
||||
/** Only select fields needed for thread traversal. Both
|
||||
* `files` (user uploads) and `attachments` (code-execution
|
||||
* outputs from `processCodeOutput`) carry the `file_id`
|
||||
* refs the next turn must prime — selecting only `files`
|
||||
* silently drops every code-output ref. */
|
||||
const messages = await db.getMessages(
|
||||
{ conversationId },
|
||||
'messageId parentMessageId files attachments',
|
||||
);
|
||||
if (messages && messages.length > 0) {
|
||||
/** Walk the parent chain and collect file_ids referenced by
|
||||
* any message in the thread (`messages.files[].file_id` +
|
||||
* `messages.attachments[].file_id`). Used as the primary
|
||||
* anchor for both `getCodeGeneratedFiles` and
|
||||
* `getUserCodeFiles` — message ids no longer needed at
|
||||
* this layer. */
|
||||
threadFileIds = getThreadData(messages, parentMessageId).fileIds;
|
||||
}
|
||||
}
|
||||
|
||||
/** Code-generated and user-uploaded execute_code files share the
|
||||
* same primary anchor: file_ids referenced by messages in the
|
||||
* current thread. The two queries differ only by `context`
|
||||
* (`execute_code` for generated outputs, others for uploads).
|
||||
* Anchoring both on `threadFileIds` reaches files regardless of
|
||||
* which sibling first generated them — see `getCodeGeneratedFiles`
|
||||
* for the branched-conversation rationale. */
|
||||
if (db.getCodeGeneratedFiles) {
|
||||
codeGeneratedFiles = requestFileOwnerScope
|
||||
? ((await db.getCodeGeneratedFiles(
|
||||
conversationId,
|
||||
threadFileIds,
|
||||
requestFileOwnerScope,
|
||||
)) as IMongoFile[])
|
||||
: [];
|
||||
}
|
||||
|
||||
if (
|
||||
db.getUserCodeFiles &&
|
||||
requestFileOwnerScope &&
|
||||
threadFileIds &&
|
||||
threadFileIds.length > 0
|
||||
) {
|
||||
userCodeFiles = (await db.getUserCodeFiles(
|
||||
threadFileIds,
|
||||
requestFileOwnerScope,
|
||||
)) as IMongoFile[];
|
||||
}
|
||||
}
|
||||
const wantsCodeFiles = toolResourceSet.has(EToolResources.execute_code);
|
||||
const [toolFiles, codeGeneratedFiles, userCodeFiles] = await Promise.all([
|
||||
requestFileOwnerScope
|
||||
? (db.getToolFilesByIds(fileIds, toolResourceSet, requestFileOwnerScope) as Promise<
|
||||
IMongoFile[]
|
||||
>)
|
||||
: ([] as IMongoFile[]),
|
||||
wantsCodeFiles && db.getCodeGeneratedFiles && requestFileOwnerScope
|
||||
? (db.getCodeGeneratedFiles(
|
||||
conversationId,
|
||||
threadFileIds,
|
||||
requestFileOwnerScope,
|
||||
) as Promise<IMongoFile[]>)
|
||||
: ([] as IMongoFile[]),
|
||||
wantsCodeFiles &&
|
||||
db.getUserCodeFiles &&
|
||||
requestFileOwnerScope &&
|
||||
threadFileIds &&
|
||||
threadFileIds.length > 0
|
||||
? (db.getUserCodeFiles(threadFileIds, requestFileOwnerScope) as Promise<IMongoFile[]>)
|
||||
: ([] as IMongoFile[]),
|
||||
]);
|
||||
|
||||
const allToolFiles = toolFiles.concat(codeGeneratedFiles, userCodeFiles);
|
||||
if (requestFiles.length || allToolFiles.length) {
|
||||
|
|
|
|||
|
|
@ -304,6 +304,19 @@ export type Agent = {
|
|||
artifacts?: ArtifactModes;
|
||||
recursion_limit?: number;
|
||||
isPublic?: boolean;
|
||||
/**
|
||||
* Whether the requesting user holds EDIT on this agent, so a single VIEW-scoped fetch can
|
||||
* serve consumers that only need the editable subset instead of issuing a second full
|
||||
* paginated walk under an EDIT-scoped cache key.
|
||||
*
|
||||
* Set by the list endpoint only; single-agent responses omit it. Treat absence as unknown
|
||||
* and fail open (`isEditable !== false`), never as `false`, since a client on an older
|
||||
* server would otherwise see an empty list rather than too many rows.
|
||||
*
|
||||
* Reflects the caller's ACL grant. The `MANAGE_AGENTS` capability bypasses ACL on write,
|
||||
* so a capability holder can edit agents this flag reports as not editable.
|
||||
*/
|
||||
isEditable?: boolean;
|
||||
version?: number;
|
||||
category?: string;
|
||||
support_contact?: SupportContact;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue