LibreChat/api/server/controllers/agents/__tests__/responses.unit.spec.js
Danny Avila 596f806f60 🛡️ fix: Strict Opt-In Skills Activation per Agent (#12823)
* 🛡️ fix: Strict opt-in skills activation per agent

Skills were activating on every agent run that had the capability +
RBAC enabled, regardless of whether the user (ephemeral) or author
(persisted) had opted in. `scopeSkillIds(undefined)` fell through to
"full accessible catalog" whenever `agent.skills` was unset, which is
the default state for any agent created before skills existed and for
every ephemeral agent.

Activation now requires an explicit signal:
- Ephemeral agent → per-conversation skills badge toggle.
- Persisted agent → new `skills_enabled` master switch on the agent
  doc, surfaced as a toggle in the Agent Builder skills section.
  Enabled + empty/undefined allowlist = full accessible catalog;
  enabled + non-empty allowlist = narrow to those ids; disabled (or
  undefined) = no skills available, even if an allowlist is set.

Centralised the predicate in `resolveAgentScopedSkillIds` so the
primary-agent path, handoff/discovery, the subagent loop, and both
OpenAI controllers all share one source of truth. Frontend `$`
popover scope mirrors the same logic so the UI never offers skills
the backend would refuse to activate.

* test: mock resolveAgentScopedSkillIds in agent controller specs

* refactor: address review findings on skills opt-in PR

- AgentConfig: associate skills label with toggle via htmlFor for
  click/keyboard affordance; simplify Switch handler to Boolean(value).
- skills: mark scopeSkillIds as @internal so runtime callers continue
  to route through resolveAgentScopedSkillIds and inherit the activation
  predicate (ephemeral toggle, persisted skills_enabled).

* fix(agents): include skills_enabled in agent list projection

Without this field, agents loaded via the list endpoint hydrate into the
client agentsMap with skills_enabled === undefined, causing the `$`
skill popover to hide every skill on a fresh page load even when the
agent was saved with skills_enabled: true.

* fix(skills): fail closed for persisted agents during agentsMap hydration

Returning undefined while the agents map loads let the popover render the
full catalog for a persisted agent before we could read its
skills_enabled flag, so the user could pick a skill the backend would
then refuse for the turn. Match the strict opt-in contract by returning
[] until the map is authoritative.

* refactor(skills): extract skillsHintKey for readability

Replaces the nested ternary in the skills section JSX with a
pre-computed constant so the activation -> hint key mapping reads
top-down.

* refactor(skills): unflatten skillsHintKey to remove nested ternary
2026-04-25 04:02:01 -04:00

461 lines
15 KiB
JavaScript

/**
* Unit tests for Open Responses API controller
* Tests that recordCollectedUsage is called correctly for token spending
*/
const mockSpendTokens = jest.fn().mockResolvedValue({});
const mockSpendStructuredTokens = jest.fn().mockResolvedValue({});
const mockRecordCollectedUsage = jest
.fn()
.mockResolvedValue({ input_tokens: 100, output_tokens: 50 });
const mockGetBalanceConfig = jest.fn().mockReturnValue({ enabled: true });
const mockGetTransactionsConfig = jest.fn().mockReturnValue({ enabled: true });
jest.mock('nanoid', () => ({
nanoid: jest.fn(() => 'mock-nanoid-123'),
}));
jest.mock('uuid', () => ({
v4: jest.fn(() => 'mock-uuid-456'),
}));
jest.mock('@librechat/data-schemas', () => ({
logger: {
debug: jest.fn(),
error: jest.fn(),
warn: jest.fn(),
},
}));
jest.mock('@librechat/agents', () => ({
Callback: { TOOL_ERROR: 'TOOL_ERROR' },
ToolEndHandler: jest.fn(),
formatAgentMessages: jest.fn().mockReturnValue({
messages: [],
indexTokenCountMap: {},
}),
}));
jest.mock('@librechat/api', () => ({
createRun: jest.fn().mockResolvedValue({
processStream: jest.fn().mockResolvedValue(undefined),
}),
buildToolSet: jest.fn().mockReturnValue(new Set()),
scopeSkillIds: jest.fn().mockImplementation((ids) => ids),
resolveAgentScopedSkillIds: jest
.fn()
.mockImplementation(({ accessibleSkillIds }) => accessibleSkillIds),
loadSkillStates: jest.fn().mockResolvedValue({ skillStates: {}, defaultActiveOnShare: false }),
createSafeUser: jest.fn().mockReturnValue({ id: 'user-123' }),
initializeAgent: jest.fn().mockResolvedValue({
id: 'agent-123',
model: 'claude-3',
model_parameters: {},
toolRegistry: {},
edges: [],
}),
discoverConnectedAgents: jest.fn().mockResolvedValue({
agentConfigs: new Map(),
edges: [],
skippedAgentIds: new Set(),
userMCPAuthMap: undefined,
}),
getBalanceConfig: mockGetBalanceConfig,
getTransactionsConfig: mockGetTransactionsConfig,
recordCollectedUsage: mockRecordCollectedUsage,
extractManualSkills: jest.fn().mockReturnValue(undefined),
injectSkillPrimes: jest.fn().mockReturnValue({
initialMessages: [],
indexTokenCountMap: {},
inserted: 0,
insertIdx: -1,
alwaysApplyDropped: 0,
alwaysApplyDedupedFromManual: 0,
}),
createToolExecuteHandler: jest.fn().mockReturnValue({ handle: jest.fn() }),
// Responses API
writeDone: jest.fn(),
buildResponse: jest.fn().mockReturnValue({ id: 'resp_123', output: [] }),
generateResponseId: jest.fn().mockReturnValue('resp_mock-123'),
isValidationFailure: jest.fn().mockReturnValue(false),
emitResponseCreated: jest.fn(),
createResponseContext: jest.fn().mockReturnValue({ responseId: 'resp_123' }),
createResponseTracker: jest.fn().mockReturnValue({
usage: { promptTokens: 100, completionTokens: 50 },
}),
setupStreamingResponse: jest.fn(),
emitResponseInProgress: jest.fn(),
convertInputToMessages: jest.fn().mockReturnValue([]),
validateResponseRequest: jest.fn().mockReturnValue({
request: { model: 'agent-123', input: 'Hello', stream: false },
}),
buildAggregatedResponse: jest.fn().mockReturnValue({
id: 'resp_123',
status: 'completed',
output: [],
usage: { input_tokens: 100, output_tokens: 50, total_tokens: 150 },
}),
createResponseAggregator: jest.fn().mockReturnValue({
usage: { promptTokens: 100, completionTokens: 50 },
}),
sendResponsesErrorResponse: jest.fn(),
createResponsesEventHandlers: jest.fn().mockReturnValue({
handlers: {
on_message_delta: { handle: jest.fn() },
on_reasoning_delta: { handle: jest.fn() },
on_run_step: { handle: jest.fn() },
on_run_step_delta: { handle: jest.fn() },
on_chat_model_end: { handle: jest.fn() },
},
finalizeStream: jest.fn(),
}),
createAggregatorEventHandlers: jest.fn().mockReturnValue({
on_message_delta: { handle: jest.fn() },
on_reasoning_delta: { handle: jest.fn() },
on_run_step: { handle: jest.fn() },
on_run_step_delta: { handle: jest.fn() },
on_chat_model_end: { handle: jest.fn() },
}),
}));
jest.mock('~/server/services/ToolService', () => ({
loadAgentTools: jest.fn().mockResolvedValue([]),
loadToolsForExecution: jest.fn().mockResolvedValue([]),
}));
const mockGetMultiplier = jest.fn().mockReturnValue(1);
const mockGetCacheMultiplier = jest.fn().mockReturnValue(null);
jest.mock('~/server/controllers/agents/callbacks', () => {
const noop = { handle: jest.fn() };
return {
createToolEndCallback: jest.fn().mockReturnValue(jest.fn()),
createResponsesToolEndCallback: jest.fn().mockReturnValue(jest.fn()),
markSummarizationUsage: jest.fn().mockImplementation((usage) => usage),
agentLogHandlerObj: noop,
buildSummarizationHandlers: jest.fn().mockReturnValue({
on_summarize_start: noop,
on_summarize_delta: noop,
on_summarize_complete: noop,
}),
};
});
jest.mock('~/server/services/PermissionService', () => ({
findAccessibleResources: jest.fn().mockResolvedValue([]),
checkPermission: jest.fn().mockResolvedValue(true),
}));
jest.mock('~/server/controllers/ModelController', () => ({
getModelsConfig: jest.fn().mockResolvedValue({}),
}));
jest.mock('~/server/services/Files/permissions', () => ({
filterFilesByAgentAccess: jest.fn(),
}));
jest.mock('~/cache', () => ({
logViolation: jest.fn(),
}));
jest.mock('~/server/services/Files/strategies', () => ({
getStrategyFunctions: jest.fn().mockReturnValue({}),
}));
jest.mock('~/server/services/Files/Code/crud', () => ({
batchUploadCodeEnvFiles: jest.fn().mockResolvedValue({ session_id: '', files: [] }),
}));
jest.mock('~/server/services/Files/Code/process', () => ({
getSessionInfo: jest.fn().mockResolvedValue(null),
checkIfActive: jest.fn().mockReturnValue(false),
}));
const mockUpdateBalance = jest.fn().mockResolvedValue({});
const mockBulkInsertTransactions = jest.fn().mockResolvedValue(undefined);
jest.mock('~/models', () => ({
getAgent: jest.fn().mockResolvedValue({ id: 'agent-123', name: 'Test Agent' }),
getFiles: jest.fn(),
getUserKey: jest.fn(),
getMessages: jest.fn().mockResolvedValue([]),
saveMessage: jest.fn().mockResolvedValue({}),
updateFilesUsage: jest.fn(),
getUserKeyValues: jest.fn(),
getUserCodeFiles: jest.fn(),
getToolFilesByIds: jest.fn(),
getCodeGeneratedFiles: jest.fn(),
updateBalance: mockUpdateBalance,
bulkInsertTransactions: mockBulkInsertTransactions,
spendTokens: mockSpendTokens,
spendStructuredTokens: mockSpendStructuredTokens,
getMultiplier: mockGetMultiplier,
getCacheMultiplier: mockGetCacheMultiplier,
getConvoFiles: jest.fn().mockResolvedValue([]),
saveConvo: jest.fn().mockResolvedValue({}),
getConvo: jest.fn().mockResolvedValue(null),
}));
describe('createResponse controller', () => {
let createResponse;
let req, res;
beforeEach(() => {
jest.clearAllMocks();
const controller = require('../responses');
createResponse = controller.createResponse;
req = {
body: {
model: 'agent-123',
input: 'Hello',
stream: false,
},
user: { id: 'user-123' },
config: {
endpoints: {
agents: { allowedProviders: ['anthropic'] },
},
},
on: jest.fn(),
};
res = {
status: jest.fn().mockReturnThis(),
json: jest.fn(),
setHeader: jest.fn(),
flushHeaders: jest.fn(),
end: jest.fn(),
write: jest.fn(),
};
});
describe('conversation ownership validation', () => {
it('should skip ownership check when previous_response_id is not provided', async () => {
const { getConvo } = require('~/models');
await createResponse(req, res);
expect(getConvo).not.toHaveBeenCalled();
});
it('should return 400 when previous_response_id is not a string', async () => {
const { validateResponseRequest, sendResponsesErrorResponse } = require('@librechat/api');
validateResponseRequest.mockReturnValueOnce({
request: {
model: 'agent-123',
input: 'Hello',
stream: false,
previous_response_id: { $gt: '' },
},
});
await createResponse(req, res);
expect(sendResponsesErrorResponse).toHaveBeenCalledWith(
res,
400,
'previous_response_id must be a string',
'invalid_request',
);
});
it('should return 404 when conversation is not owned by user', async () => {
const { validateResponseRequest, sendResponsesErrorResponse } = require('@librechat/api');
const { getConvo } = require('~/models');
validateResponseRequest.mockReturnValueOnce({
request: {
model: 'agent-123',
input: 'Hello',
stream: false,
previous_response_id: 'resp_abc',
},
});
getConvo.mockResolvedValueOnce(null);
await createResponse(req, res);
expect(getConvo).toHaveBeenCalledWith('user-123', 'resp_abc');
expect(sendResponsesErrorResponse).toHaveBeenCalledWith(
res,
404,
'Conversation not found',
'not_found',
);
});
it('should proceed when conversation is owned by user', async () => {
const { validateResponseRequest, sendResponsesErrorResponse } = require('@librechat/api');
const { getConvo } = require('~/models');
validateResponseRequest.mockReturnValueOnce({
request: {
model: 'agent-123',
input: 'Hello',
stream: false,
previous_response_id: 'resp_abc',
},
});
getConvo.mockResolvedValueOnce({ conversationId: 'resp_abc', user: 'user-123' });
await createResponse(req, res);
expect(getConvo).toHaveBeenCalledWith('user-123', 'resp_abc');
expect(sendResponsesErrorResponse).not.toHaveBeenCalledWith(
res,
404,
expect.any(String),
expect.any(String),
);
});
it('should return 500 when getConvo throws a DB error', async () => {
const { validateResponseRequest, sendResponsesErrorResponse } = require('@librechat/api');
const { getConvo } = require('~/models');
validateResponseRequest.mockReturnValueOnce({
request: {
model: 'agent-123',
input: 'Hello',
stream: false,
previous_response_id: 'resp_abc',
},
});
getConvo.mockRejectedValueOnce(new Error('DB connection failed'));
await createResponse(req, res);
expect(sendResponsesErrorResponse).toHaveBeenCalledWith(
res,
500,
expect.any(String),
expect.any(String),
);
});
});
describe('token usage recording - non-streaming', () => {
it('should call recordCollectedUsage after successful non-streaming completion', async () => {
await createResponse(req, res);
expect(mockRecordCollectedUsage).toHaveBeenCalledTimes(1);
expect(mockRecordCollectedUsage).toHaveBeenCalledWith(
{
spendTokens: mockSpendTokens,
spendStructuredTokens: mockSpendStructuredTokens,
pricing: { getMultiplier: mockGetMultiplier, getCacheMultiplier: mockGetCacheMultiplier },
bulkWriteOps: {
insertMany: mockBulkInsertTransactions,
updateBalance: mockUpdateBalance,
},
},
expect.objectContaining({
user: 'user-123',
conversationId: expect.any(String),
collectedUsage: expect.any(Array),
context: 'message',
}),
);
});
it('should pass balance and transactions config to recordCollectedUsage', async () => {
mockGetBalanceConfig.mockReturnValue({ enabled: true, startBalance: 2000 });
mockGetTransactionsConfig.mockReturnValue({ enabled: true });
await createResponse(req, res);
expect(mockRecordCollectedUsage).toHaveBeenCalledWith(
expect.any(Object),
expect.objectContaining({
balance: { enabled: true, startBalance: 2000 },
transactions: { enabled: true },
}),
);
});
it('should pass spendTokens, spendStructuredTokens, pricing, and bulkWriteOps as dependencies', async () => {
await createResponse(req, res);
const [deps] = mockRecordCollectedUsage.mock.calls[0];
expect(deps).toHaveProperty('spendTokens', mockSpendTokens);
expect(deps).toHaveProperty('spendStructuredTokens', mockSpendStructuredTokens);
expect(deps).toHaveProperty('pricing');
expect(deps.pricing).toHaveProperty('getMultiplier', mockGetMultiplier);
expect(deps.pricing).toHaveProperty('getCacheMultiplier', mockGetCacheMultiplier);
expect(deps).toHaveProperty('bulkWriteOps');
expect(deps.bulkWriteOps).toHaveProperty('insertMany', mockBulkInsertTransactions);
expect(deps.bulkWriteOps).toHaveProperty('updateBalance', mockUpdateBalance);
});
it('should include model from primaryConfig in recordCollectedUsage params', async () => {
await createResponse(req, res);
expect(mockRecordCollectedUsage).toHaveBeenCalledWith(
expect.any(Object),
expect.objectContaining({
model: 'claude-3',
}),
);
});
});
describe('token usage recording - streaming', () => {
beforeEach(() => {
req.body.stream = true;
const api = require('@librechat/api');
api.validateResponseRequest.mockReturnValue({
request: { model: 'agent-123', input: 'Hello', stream: true },
});
});
it('should call recordCollectedUsage after successful streaming completion', async () => {
await createResponse(req, res);
expect(mockRecordCollectedUsage).toHaveBeenCalledTimes(1);
expect(mockRecordCollectedUsage).toHaveBeenCalledWith(
{
spendTokens: mockSpendTokens,
spendStructuredTokens: mockSpendStructuredTokens,
pricing: { getMultiplier: mockGetMultiplier, getCacheMultiplier: mockGetCacheMultiplier },
bulkWriteOps: {
insertMany: mockBulkInsertTransactions,
updateBalance: mockUpdateBalance,
},
},
expect.objectContaining({
user: 'user-123',
context: 'message',
}),
);
});
});
describe('collectedUsage population', () => {
it('should collect usage from on_chat_model_end events', async () => {
const api = require('@librechat/api');
api.createRun.mockImplementation(async ({ customHandlers }) => {
return {
processStream: jest.fn().mockImplementation(async () => {
customHandlers.on_chat_model_end.handle('on_chat_model_end', {
output: {
usage_metadata: {
input_tokens: 150,
output_tokens: 75,
model: 'claude-3',
},
},
});
}),
};
});
await createResponse(req, res);
expect(mockRecordCollectedUsage).toHaveBeenCalledWith(
expect.any(Object),
expect.objectContaining({
collectedUsage: expect.arrayContaining([
expect.objectContaining({
input_tokens: 150,
output_tokens: 75,
}),
]),
}),
);
});
});
});