mirror of
https://github.com/danny-avila/LibreChat.git
synced 2025-12-17 00:40:14 +01:00
🔧 fix: Agent Versioning with Action Hashing and OAuth Redirect (#7627)
* 🔧 chore: Update navigateFallbackDenylist in Vite config to include API routes * 🔧 fix: Update redirect_uri in createActionTool to use DOMAIN_SERVER instead of DOMAIN_CLIENT * 🔧 feat: Enhance Agent Versioning with Action Metadata Hashing - Added support for generating a hash of action metadata to detect changes and manage agent versioning. - Updated `updateAgent` function to include an optional `forceVersion` parameter for version creation. - Modified `isDuplicateVersion` to compare action metadata hashes. - Updated related tests to validate new versioning behavior with action changes. - Refactored agent update logic to ensure proper tracking of user updates and version history.
This commit is contained in:
Danny Avila
GitHub
parent
fb88ac00c6
commit
442976c74f
6 changed files with 213 additions and 31 deletions
|
|
@ -1,6 +1,7 @@
|
|||
const mongoose = require('mongoose');
|
||||
const crypto = require('node:crypto');
|
||||
const { agentSchema } = require('@librechat/data-schemas');
|
||||
const { SystemRoles, Tools } = require('librechat-data-provider');
|
||||
const { SystemRoles, Tools, actionDelimiter } = require('librechat-data-provider');
|
||||
const { GLOBAL_PROJECT_NAME, EPHEMERAL_AGENT_ID, mcp_delimiter } =
|
||||
require('librechat-data-provider').Constants;
|
||||
const { CONFIG_STORE, STARTUP_CONFIG } = require('librechat-data-provider').CacheKeys;
|
||||
|
|
@ -11,6 +12,8 @@ const {
|
|||
removeAgentFromAllProjects,
|
||||
} = require('./Project');
|
||||
const getLogStores = require('~/cache/getLogStores');
|
||||
const { getActions } = require('./Action');
|
||||
const { logger } = require('~/config');
|
||||
|
||||
const Agent = mongoose.model('agent', agentSchema);
|
||||
|
||||
|
|
@ -149,10 +152,12 @@ const loadAgent = async ({ req, agent_id, endpoint, model_parameters }) => {
|
|||
/**
|
||||
* Check if a version already exists in the versions array, excluding timestamp and author fields
|
||||
* @param {Object} updateData - The update data to compare
|
||||
* @param {Object} currentData - The current agent data
|
||||
* @param {Array} versions - The existing versions array
|
||||
* @param {string} [actionsHash] - Hash of current action metadata
|
||||
* @returns {Object|null} - The matching version if found, null otherwise
|
||||
*/
|
||||
const isDuplicateVersion = (updateData, currentData, versions) => {
|
||||
const isDuplicateVersion = (updateData, currentData, versions, actionsHash = null) => {
|
||||
if (!versions || versions.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
|
@ -169,17 +174,22 @@ const isDuplicateVersion = (updateData, currentData, versions) => {
|
|||
'__v',
|
||||
'agent_ids',
|
||||
'versions',
|
||||
'actionsHash', // Exclude actionsHash from direct comparison
|
||||
];
|
||||
|
||||
const { $push, $pull, $addToSet, ...directUpdates } = updateData;
|
||||
|
||||
if (Object.keys(directUpdates).length === 0) {
|
||||
if (Object.keys(directUpdates).length === 0 && !actionsHash) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const wouldBeVersion = { ...currentData, ...directUpdates };
|
||||
const lastVersion = versions[versions.length - 1];
|
||||
|
||||
if (actionsHash && lastVersion.actionsHash !== actionsHash) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const allFields = new Set([...Object.keys(wouldBeVersion), ...Object.keys(lastVersion)]);
|
||||
|
||||
const importantFields = Array.from(allFields).filter((field) => !excludeFields.includes(field));
|
||||
|
|
@ -249,21 +259,58 @@ const isDuplicateVersion = (updateData, currentData, versions) => {
|
|||
* @param {string} searchParameter.id - The ID of the agent to update.
|
||||
* @param {string} [searchParameter.author] - The user ID of the agent's author.
|
||||
* @param {Object} updateData - An object containing the properties to update.
|
||||
* @param {string} [updatingUserId] - The ID of the user performing the update (used for tracking non-author updates).
|
||||
* @param {Object} [options] - Optional configuration object.
|
||||
* @param {string} [options.updatingUserId] - The ID of the user performing the update (used for tracking non-author updates).
|
||||
* @param {boolean} [options.forceVersion] - Force creation of a new version even if no fields changed.
|
||||
* @returns {Promise<Agent>} The updated or newly created agent document as a plain object.
|
||||
* @throws {Error} If the update would create a duplicate version
|
||||
*/
|
||||
const updateAgent = async (searchParameter, updateData, updatingUserId = null) => {
|
||||
const options = { new: true, upsert: false };
|
||||
const updateAgent = async (searchParameter, updateData, options = {}) => {
|
||||
const { updatingUserId = null, forceVersion = false } = options;
|
||||
const mongoOptions = { new: true, upsert: false };
|
||||
|
||||
const currentAgent = await Agent.findOne(searchParameter);
|
||||
if (currentAgent) {
|
||||
const { __v, _id, id, versions, author, ...versionData } = currentAgent.toObject();
|
||||
const { $push, $pull, $addToSet, ...directUpdates } = updateData;
|
||||
|
||||
if (Object.keys(directUpdates).length > 0 && versions && versions.length > 0) {
|
||||
const duplicateVersion = isDuplicateVersion(updateData, versionData, versions);
|
||||
if (duplicateVersion) {
|
||||
let actionsHash = null;
|
||||
|
||||
// Generate actions hash if agent has actions
|
||||
if (currentAgent.actions && currentAgent.actions.length > 0) {
|
||||
// Extract action IDs from the format "domain_action_id"
|
||||
const actionIds = currentAgent.actions
|
||||
.map((action) => {
|
||||
const parts = action.split(actionDelimiter);
|
||||
return parts[1]; // Get just the action ID part
|
||||
})
|
||||
.filter(Boolean);
|
||||
|
||||
if (actionIds.length > 0) {
|
||||
try {
|
||||
const actions = await getActions(
|
||||
{
|
||||
action_id: { $in: actionIds },
|
||||
},
|
||||
true,
|
||||
); // Include sensitive data for hash
|
||||
|
||||
actionsHash = await generateActionMetadataHash(currentAgent.actions, actions);
|
||||
} catch (error) {
|
||||
logger.error('Error fetching actions for hash generation:', error);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const shouldCreateVersion =
|
||||
forceVersion ||
|
||||
(versions &&
|
||||
versions.length > 0 &&
|
||||
(Object.keys(directUpdates).length > 0 || $push || $pull || $addToSet));
|
||||
|
||||
if (shouldCreateVersion) {
|
||||
const duplicateVersion = isDuplicateVersion(updateData, versionData, versions, actionsHash);
|
||||
if (duplicateVersion && !forceVersion) {
|
||||
const error = new Error(
|
||||
'Duplicate version: This would create a version identical to an existing one',
|
||||
);
|
||||
|
|
@ -284,18 +331,25 @@ const updateAgent = async (searchParameter, updateData, updatingUserId = null) =
|
|||
updatedAt: new Date(),
|
||||
};
|
||||
|
||||
// Include actions hash in version if available
|
||||
if (actionsHash) {
|
||||
versionEntry.actionsHash = actionsHash;
|
||||
}
|
||||
|
||||
// Always store updatedBy field to track who made the change
|
||||
if (updatingUserId) {
|
||||
versionEntry.updatedBy = new mongoose.Types.ObjectId(updatingUserId);
|
||||
}
|
||||
|
||||
updateData.$push = {
|
||||
...($push || {}),
|
||||
versions: versionEntry,
|
||||
};
|
||||
if (shouldCreateVersion || forceVersion) {
|
||||
updateData.$push = {
|
||||
...($push || {}),
|
||||
versions: versionEntry,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
return Agent.findOneAndUpdate(searchParameter, updateData, options).lean();
|
||||
return Agent.findOneAndUpdate(searchParameter, updateData, mongoOptions).lean();
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -333,7 +387,9 @@ const addAgentResourceFile = async ({ req, agent_id, tool_resource, file_id }) =
|
|||
},
|
||||
};
|
||||
|
||||
const updatedAgent = await updateAgent(searchParameter, updateData, req?.user?.id);
|
||||
const updatedAgent = await updateAgent(searchParameter, updateData, {
|
||||
updatingUserId: req?.user?.id,
|
||||
});
|
||||
if (updatedAgent) {
|
||||
return updatedAgent;
|
||||
} else {
|
||||
|
|
@ -497,7 +553,7 @@ const updateAgentProjects = async ({ user, agentId, projectIds, removeProjectIds
|
|||
delete updateQuery.author;
|
||||
}
|
||||
|
||||
const updatedAgent = await updateAgent(updateQuery, updateOps, user.id);
|
||||
const updatedAgent = await updateAgent(updateQuery, updateOps, { updatingUserId: user.id });
|
||||
if (updatedAgent) {
|
||||
return updatedAgent;
|
||||
}
|
||||
|
|
@ -548,6 +604,63 @@ const revertAgentVersion = async (searchParameter, versionIndex) => {
|
|||
return Agent.findOneAndUpdate(searchParameter, updateData, { new: true }).lean();
|
||||
};
|
||||
|
||||
/**
|
||||
* Generates a hash of action metadata for version comparison
|
||||
* @param {string[]} actionIds - Array of action IDs in format "domain_action_id"
|
||||
* @param {Action[]} actions - Array of action documents
|
||||
* @returns {Promise<string>} - SHA256 hash of the action metadata
|
||||
*/
|
||||
const generateActionMetadataHash = async (actionIds, actions) => {
|
||||
if (!actionIds || actionIds.length === 0) {
|
||||
return '';
|
||||
}
|
||||
|
||||
// Create a map of action_id to metadata for quick lookup
|
||||
const actionMap = new Map();
|
||||
actions.forEach((action) => {
|
||||
actionMap.set(action.action_id, action.metadata);
|
||||
});
|
||||
|
||||
// Sort action IDs for consistent hashing
|
||||
const sortedActionIds = [...actionIds].sort();
|
||||
|
||||
// Build a deterministic string representation of all action metadata
|
||||
const metadataString = sortedActionIds
|
||||
.map((actionFullId) => {
|
||||
// Extract just the action_id part (after the delimiter)
|
||||
const parts = actionFullId.split(actionDelimiter);
|
||||
const actionId = parts[1];
|
||||
|
||||
const metadata = actionMap.get(actionId);
|
||||
if (!metadata) {
|
||||
return `${actionId}:null`;
|
||||
}
|
||||
|
||||
// Sort metadata keys for deterministic output
|
||||
const sortedKeys = Object.keys(metadata).sort();
|
||||
const metadataStr = sortedKeys
|
||||
.map((key) => `${key}:${JSON.stringify(metadata[key])}`)
|
||||
.join(',');
|
||||
return `${actionId}:{${metadataStr}}`;
|
||||
})
|
||||
.join(';');
|
||||
|
||||
// Use Web Crypto API to generate hash
|
||||
const encoder = new TextEncoder();
|
||||
const data = encoder.encode(metadataString);
|
||||
const hashBuffer = await crypto.webcrypto.subtle.digest('SHA-256', data);
|
||||
const hashArray = Array.from(new Uint8Array(hashBuffer));
|
||||
const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join('');
|
||||
|
||||
return hashHex;
|
||||
};
|
||||
|
||||
/**
|
||||
* Load a default agent based on the endpoint
|
||||
* @param {string} endpoint
|
||||
* @returns {Agent | null}
|
||||
*/
|
||||
|
||||
module.exports = {
|
||||
Agent,
|
||||
getAgent,
|
||||
|
|
@ -556,8 +669,9 @@ module.exports = {
|
|||
updateAgent,
|
||||
deleteAgent,
|
||||
getListAgents,
|
||||
revertAgentVersion,
|
||||
updateAgentProjects,
|
||||
addAgentResourceFile,
|
||||
removeAgentResourceFiles,
|
||||
revertAgentVersion,
|
||||
generateActionMetadataHash,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -903,7 +903,7 @@ describe('Agent Version History', () => {
|
|||
const updatedAgent = await updateAgent(
|
||||
{ id: agentId },
|
||||
{ name: 'Updated Agent', description: 'Updated description' },
|
||||
updatingUser.toString(),
|
||||
{ updatingUserId: updatingUser.toString() },
|
||||
);
|
||||
|
||||
expect(updatedAgent.versions).toHaveLength(2);
|
||||
|
|
@ -927,7 +927,7 @@ describe('Agent Version History', () => {
|
|||
const updatedAgent = await updateAgent(
|
||||
{ id: agentId },
|
||||
{ name: 'Updated Agent', description: 'Updated description' },
|
||||
originalAuthor.toString(),
|
||||
{ updatingUserId: originalAuthor.toString() },
|
||||
);
|
||||
|
||||
expect(updatedAgent.versions).toHaveLength(2);
|
||||
|
|
@ -955,28 +955,28 @@ describe('Agent Version History', () => {
|
|||
await updateAgent(
|
||||
{ id: agentId },
|
||||
{ name: 'Updated by User 1', description: 'First update' },
|
||||
user1.toString(),
|
||||
{ updatingUserId: user1.toString() },
|
||||
);
|
||||
|
||||
// Original author makes an update
|
||||
await updateAgent(
|
||||
{ id: agentId },
|
||||
{ description: 'Updated by original author' },
|
||||
originalAuthor.toString(),
|
||||
{ updatingUserId: originalAuthor.toString() },
|
||||
);
|
||||
|
||||
// User 2 makes an update
|
||||
await updateAgent(
|
||||
{ id: agentId },
|
||||
{ name: 'Updated by User 2', model: 'new-model' },
|
||||
user2.toString(),
|
||||
{ updatingUserId: user2.toString() },
|
||||
);
|
||||
|
||||
// User 3 makes an update
|
||||
const finalAgent = await updateAgent(
|
||||
{ id: agentId },
|
||||
{ description: 'Final update by User 3' },
|
||||
user3.toString(),
|
||||
{ updatingUserId: user3.toString() },
|
||||
);
|
||||
|
||||
expect(finalAgent.versions).toHaveLength(5);
|
||||
|
|
@ -1012,7 +1012,7 @@ describe('Agent Version History', () => {
|
|||
await updateAgent(
|
||||
{ id: agentId },
|
||||
{ name: 'Updated Agent', description: 'Updated description' },
|
||||
updatingUser.toString(),
|
||||
{ updatingUserId: updatingUser.toString() },
|
||||
);
|
||||
|
||||
const { revertAgentVersion } = require('./Agent');
|
||||
|
|
@ -1022,4 +1022,55 @@ describe('Agent Version History', () => {
|
|||
expect(revertedAgent.name).toBe('Original Agent');
|
||||
expect(revertedAgent.description).toBe('Original description');
|
||||
});
|
||||
|
||||
test('should detect action metadata changes and force version update', async () => {
|
||||
const agentId = `agent_${uuidv4()}`;
|
||||
const authorId = new mongoose.Types.ObjectId();
|
||||
const actionId = 'testActionId123';
|
||||
|
||||
// Create agent with actions
|
||||
await createAgent({
|
||||
id: agentId,
|
||||
name: 'Agent with Actions',
|
||||
provider: 'test',
|
||||
model: 'test-model',
|
||||
author: authorId,
|
||||
actions: [`test.com_action_${actionId}`],
|
||||
tools: ['listEvents_action_test.com', 'createEvent_action_test.com'],
|
||||
});
|
||||
|
||||
// First update with forceVersion should create a version
|
||||
const firstUpdate = await updateAgent(
|
||||
{ id: agentId },
|
||||
{ tools: ['listEvents_action_test.com', 'createEvent_action_test.com'] },
|
||||
{ updatingUserId: authorId.toString(), forceVersion: true },
|
||||
);
|
||||
|
||||
expect(firstUpdate.versions).toHaveLength(2);
|
||||
|
||||
// Second update with same data but forceVersion should still create a version
|
||||
const secondUpdate = await updateAgent(
|
||||
{ id: agentId },
|
||||
{ tools: ['listEvents_action_test.com', 'createEvent_action_test.com'] },
|
||||
{ updatingUserId: authorId.toString(), forceVersion: true },
|
||||
);
|
||||
|
||||
expect(secondUpdate.versions).toHaveLength(3);
|
||||
|
||||
// Update without forceVersion and no changes should not create a version
|
||||
let error;
|
||||
try {
|
||||
await updateAgent(
|
||||
{ id: agentId },
|
||||
{ tools: ['listEvents_action_test.com', 'createEvent_action_test.com'] },
|
||||
{ updatingUserId: authorId.toString(), forceVersion: false },
|
||||
);
|
||||
} catch (e) {
|
||||
error = e;
|
||||
}
|
||||
|
||||
expect(error).toBeDefined();
|
||||
expect(error.message).toContain('Duplicate version');
|
||||
expect(error.statusCode).toBe(409);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -111,7 +111,7 @@ const getAgentHandler = async (req, res) => {
|
|||
const originalUrl = agent.avatar.filepath;
|
||||
agent.avatar.filepath = await refreshS3Url(agent.avatar);
|
||||
if (originalUrl !== agent.avatar.filepath) {
|
||||
await updateAgent({ id }, { avatar: agent.avatar }, req.user.id);
|
||||
await updateAgent({ id }, { avatar: agent.avatar }, { updatingUserId: req.user.id });
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -170,7 +170,7 @@ const updateAgentHandler = async (req, res) => {
|
|||
|
||||
let updatedAgent =
|
||||
Object.keys(updateData).length > 0
|
||||
? await updateAgent({ id }, updateData, req.user.id)
|
||||
? await updateAgent({ id }, updateData, { updatingUserId: req.user.id })
|
||||
: existingAgent;
|
||||
|
||||
if (projectIds || removeProjectIds) {
|
||||
|
|
@ -407,7 +407,11 @@ const uploadAgentAvatarHandler = async (req, res) => {
|
|||
},
|
||||
};
|
||||
|
||||
promises.push(await updateAgent({ id: agent_id, author: req.user.id }, data, req.user.id));
|
||||
promises.push(
|
||||
await updateAgent({ id: agent_id, author: req.user.id }, data, {
|
||||
updatingUserId: req.user.id,
|
||||
}),
|
||||
);
|
||||
|
||||
const resolved = await Promise.all(promises);
|
||||
res.status(201).json(resolved[0]);
|
||||
|
|
|
|||
|
|
@ -107,7 +107,15 @@ router.post('/:agent_id', async (req, res) => {
|
|||
.filter((tool) => !(tool && (tool.includes(domain) || tool.includes(action_id))))
|
||||
.concat(functions.map((tool) => `${tool.function.name}${actionDelimiter}${domain}`));
|
||||
|
||||
const updatedAgent = await updateAgent(agentQuery, { tools, actions }, req.user.id);
|
||||
// Force version update since actions are changing
|
||||
const updatedAgent = await updateAgent(
|
||||
agentQuery,
|
||||
{ tools, actions },
|
||||
{
|
||||
updatingUserId: req.user.id,
|
||||
forceVersion: true,
|
||||
},
|
||||
);
|
||||
|
||||
// Only update user field for new actions
|
||||
const actionUpdateData = { metadata, agent_id };
|
||||
|
|
@ -172,7 +180,12 @@ router.delete('/:agent_id/:action_id', async (req, res) => {
|
|||
|
||||
const updatedTools = tools.filter((tool) => !(tool && tool.includes(domain)));
|
||||
|
||||
await updateAgent(agentQuery, { tools: updatedTools, actions: updatedActions }, req.user.id);
|
||||
// Force version update since actions are being removed
|
||||
await updateAgent(
|
||||
agentQuery,
|
||||
{ tools: updatedTools, actions: updatedActions },
|
||||
{ updatingUserId: req.user.id, forceVersion: true },
|
||||
);
|
||||
// If admin, can delete any action, otherwise only user's actions
|
||||
const actionQuery = admin ? { action_id } : { action_id, user: req.user.id };
|
||||
await deleteAction(actionQuery);
|
||||
|
|
|
|||
|
|
@ -207,7 +207,7 @@ async function createActionTool({
|
|||
state: stateToken,
|
||||
userId: userId,
|
||||
client_url: metadata.auth.client_url,
|
||||
redirect_uri: `${process.env.DOMAIN_CLIENT}/api/actions/${action_id}/oauth/callback`,
|
||||
redirect_uri: `${process.env.DOMAIN_SERVER}/api/actions/${action_id}/oauth/callback`,
|
||||
/** Encrypted values */
|
||||
encrypted_oauth_client_id: encrypted.oauth_client_id,
|
||||
encrypted_oauth_client_secret: encrypted.oauth_client_secret,
|
||||
|
|
|
|||
|
|
@ -49,7 +49,7 @@ export default defineConfig(({ command }) => ({
|
|||
],
|
||||
globIgnores: ['images/**/*', '**/*.map'],
|
||||
maximumFileSizeToCacheInBytes: 4 * 1024 * 1024,
|
||||
navigateFallbackDenylist: [/^\/oauth/],
|
||||
navigateFallbackDenylist: [/^\/oauth/, /^\/api/],
|
||||
},
|
||||
includeAssets: [],
|
||||
manifest: {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue