diff --git a/api/package.json b/api/package.json index e588bfb8eaf..3cdb8cb3b31 100644 --- a/api/package.json +++ b/api/package.json @@ -46,7 +46,7 @@ "@azure/storage-blob": "^12.30.0", "@google/genai": "^2.8.0", "@keyv/redis": "5.1.6", - "@librechat/agents": "^3.9.6", + "@librechat/agents": "^3.9.7", "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", diff --git a/api/server/controllers/agents/client.test.js b/api/server/controllers/agents/client.test.js index a5725c12dd9..a6a3991c421 100644 --- a/api/server/controllers/agents/client.test.js +++ b/api/server/controllers/agents/client.test.js @@ -3172,6 +3172,102 @@ describe('AgentClient - startup telemetry', () => { errorSpy.mockRestore(); }); + it.each([ + [ + 'closed', + 'SocketError', + ErrorTypes.MODEL_STREAM_CLOSED, + 'stream_closed', + 'The model provider closed the connection before the response finished. Try again.', + ], + [ + 'stalled', + 'BodyTimeoutError', + ErrorTypes.MODEL_STREAM_STALLED, + 'stream_stalled', + 'The model provider stopped sending the response, and the request timed out. Try again.', + ], + ])( + 'keeps partial content and a safe %s model error in the agent turn', + async (kind, causeName, type, errorType, prose) => { + jest.clearAllMocks(); + const { logger } = require('@librechat/data-schemas'); + const { errors: undiciErrors } = require('undici'); + const privateValue = 'PRIVATE-TRANSPORT-DIAGNOSTIC'; + const cause = new undiciErrors[causeName](privateValue); + const providerError = new TypeError('terminated', { cause }); + const errorSpy = jest.spyOn(logger, 'error').mockImplementation(() => logger); + mockCreateRun.mockImplementation(async (options) => { + const tracker = options.modelCallbacks.find( + (callback) => callback.name === 'librechat-upstream-model-error-tracker', + ); + return { + Graph: null, + processStream: jest.fn(async () => { + tracker.handleLLMError(providerError); + throw new Error('graph failed', { cause: providerError }); + }), + getCalibrationRatio: jest.fn(() => 0), + }; + }); + mockIsHITLEnabled.mockReturnValue(false); + const partial = { type: ContentTypes.TEXT, [ContentTypes.TEXT]: 'Partial findings' }; + const client = new AgentClient({ + req: { + user: { id: 'user-123' }, + body: {}, + config: { + endpoints: { [EModelEndpoint.agents]: {} }, + filters: { messages: { pii: {} } }, + }, + _resumableStreamId: `conversation-stream-${kind}`, + }, + res: {}, + agent: { + id: 'agent-123', + endpoint: EModelEndpoint.openAI, + provider: EModelEndpoint.openAI, + model_parameters: { model: 'gpt-4' }, + hide_sequential_outputs: false, + }, + endpointTokenConfig: {}, + eventHandlers: {}, + contentParts: [partial], + collectedUsage: [], + artifactPromises: [], + }); + client.conversationId = `conversation-stream-${kind}`; + client.responseMessageId = `response-stream-${kind}`; + client.parentMessageId = `parent-stream-${kind}`; + client.recordCollectedUsage = jest.fn().mockResolvedValue(); + + try { + await client.chatCompletion({ payload: [] }); + + expect(client.contentParts).toEqual( + expect.arrayContaining([ + partial, + { + type: ContentTypes.ERROR, + [ContentTypes.ERROR]: `${prose}\n${JSON.stringify({ type })}`, + }, + ]), + ); + expect(JSON.stringify(client.contentParts)).not.toContain(privateValue); + expect(client.recordCollectedUsage).toHaveBeenCalledWith( + expect.objectContaining({ context: 'message' }), + ); + expect(errorSpy).toHaveBeenCalledWith( + '[api/server/controllers/agents/client.js #sendCompletion] Upstream model error', + expect.objectContaining({ errorType }), + ); + expect(JSON.stringify(errorSpy.mock.calls)).not.toContain(privateValue); + } finally { + errorSpy.mockRestore(); + } + }, + ); + /** A compaction's only record of having been one is the marker on the part it * produced, and Compact runs on whatever leaf the branch ends with. Without * the marker on the failure, a compaction that failed on a user leaf keeps a diff --git a/api/server/services/Files/Code/process.js b/api/server/services/Files/Code/process.js index 10348d71af3..6371b1c40fe 100644 --- a/api/server/services/Files/Code/process.js +++ b/api/server/services/Files/Code/process.js @@ -1088,10 +1088,14 @@ async function readWorkspaceFile({ req, signal, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, }) { return executeWorkspaceTool({ baseURL: codeApiBaseUrl, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, /** Minted per admission attempt: a queued call outlives one token TTL. */ authHeaders: async () => ({ ...(await getCodeApiAuthHeaders(req, bridgeWorkerId)), @@ -1137,10 +1141,14 @@ async function searchWorkspace({ req, signal, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, }) { return executeWorkspaceTool({ baseURL: codeApiBaseUrl, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, /** Minted per admission attempt: a queued call outlives one token TTL. */ authHeaders: async () => ({ ...(await getCodeApiAuthHeaders(req, bridgeWorkerId)), @@ -1186,10 +1194,14 @@ async function listWorkspaceFiles({ req, signal, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, }) { return executeWorkspaceTool({ baseURL: codeApiBaseUrl, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, /** Minted per admission attempt: a queued call outlives one token TTL. */ authHeaders: async () => ({ ...(await getCodeApiAuthHeaders(req, bridgeWorkerId)), @@ -1221,10 +1233,14 @@ async function writeWorkspaceFile({ req, signal, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, }) { return executeWorkspaceTool({ baseURL: codeApiBaseUrl, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, /** Minted per admission attempt: a queued call outlives one token TTL. */ authHeaders: async () => ({ ...(await getCodeApiAuthHeaders(req, bridgeWorkerId)), @@ -1256,10 +1272,14 @@ async function editWorkspaceFile({ req, signal, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, }) { return executeWorkspaceTool({ baseURL: codeApiBaseUrl, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, /** Minted per admission attempt: a queued call outlives one token TTL. */ authHeaders: async () => ({ ...(await getCodeApiAuthHeaders(req, bridgeWorkerId)), @@ -1290,10 +1310,14 @@ async function previewWorkspaceEdit({ req, signal, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, }) { return executeWorkspaceTool({ baseURL: codeApiBaseUrl, maxQueueWaitMs, + maxRequestTimeoutMs, + deadlineAtMs, /** Minted per admission attempt: a queued call outlives one token TTL. */ authHeaders: async () => ({ ...(await getCodeApiAuthHeaders(req, bridgeWorkerId)), diff --git a/api/server/services/Files/Code/process.spec.js b/api/server/services/Files/Code/process.spec.js index 4714f570a1d..875368196fd 100644 --- a/api/server/services/Files/Code/process.spec.js +++ b/api/server/services/Files/Code/process.spec.js @@ -2061,6 +2061,8 @@ describe('Code Process', () => { req: mockReq, signal: controller.signal, maxQueueWaitMs: 0, + maxRequestTimeoutMs: 125_000, + deadlineAtMs: 160_000, }), ).resolves.toBe(result); @@ -2081,6 +2083,8 @@ describe('Code Process', () => { baseURL: 'https://attached-code.example.com/v1', authHeaders: expect.any(Function), maxQueueWaitMs: 0, + maxRequestTimeoutMs: 125_000, + deadlineAtMs: 160_000, request: { protocolVersion: 1, operation: 'read_file', @@ -2140,6 +2144,8 @@ describe('Code Process', () => { baseURL: 'https://attached-code.example.com/v1', authHeaders: expect.any(Function), maxQueueWaitMs: 0, + maxRequestTimeoutMs: undefined, + deadlineAtMs: undefined, request: { protocolVersion: 1, operation: 'search_text', @@ -2198,6 +2204,8 @@ describe('Code Process', () => { baseURL: 'https://attached-code.example.com/v1', authHeaders: expect.any(Function), maxQueueWaitMs: 0, + maxRequestTimeoutMs: undefined, + deadlineAtMs: undefined, request: { protocolVersion: 1, operation: 'list_files', @@ -2258,6 +2266,8 @@ describe('Code Process', () => { baseURL: 'https://attached-code.example.com/v1', authHeaders: expect.any(Function), maxQueueWaitMs: 0, + maxRequestTimeoutMs: undefined, + deadlineAtMs: undefined, request: { protocolVersion: 1, operation: 'write_file', @@ -2296,11 +2306,15 @@ describe('Code Process', () => { bridgeWorkerId: 'worker-user-1', req: mockReq, expected_base_sha256: 'a'.repeat(64), + maxRequestTimeoutMs: 125_000, + deadlineAtMs: 160_000, }), ).resolves.toBe(result); expect(mockExecuteWorkspaceTool).toHaveBeenCalledWith( expect.objectContaining({ + maxRequestTimeoutMs: 125_000, + deadlineAtMs: 160_000, request: { protocolVersion: 1, operation: 'edit_file', @@ -2338,11 +2352,15 @@ describe('Code Process', () => { codeApiBaseUrl: 'https://attached-code.example.com/v1', executionProfile: 'stateful', req: mockReq, + maxRequestTimeoutMs: 125_000, + deadlineAtMs: 160_000, }), ).resolves.toBe(result); expect(mockExecuteWorkspaceTool).toHaveBeenCalledWith( expect.objectContaining({ + maxRequestTimeoutMs: 125_000, + deadlineAtMs: 160_000, request: { protocolVersion: 1, operation: 'preview_edit', diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index 6edf562a055..61750ac9828 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -842,6 +842,8 @@ async function reconnectServer({ upstreamTokenProviderResolver, recoveryPolicy, oboIdentityContext, + streamId, + jobCreatedAt, forceNew: true, returnOnOAuth: false, connectionTimeout: Time.THIRTY_SECONDS, @@ -1386,6 +1388,16 @@ function createToolInstance({ ); } + // The schedule service checks the typed cause and verified job identity before + // recording a durable tool failure; other tool errors are a cheap no-op. + await require('~/server/services/Schedules').recordMCPToolAuthFailure({ + error, + streamId, + jobCreatedAt, + userId, + serverName, + }); + /** Carries the actionable re-auth message; the substring heuristic below would misreport it as an OAuth configuration problem */ if ( error instanceof OpenIDReauthRequiredError || diff --git a/api/server/services/MCP.spec.js b/api/server/services/MCP.spec.js index 11b5328e092..487a50730b9 100644 --- a/api/server/services/MCP.spec.js +++ b/api/server/services/MCP.spec.js @@ -87,6 +87,10 @@ jest.mock('./Tools/mcp', () => ({ reinitMCPServer: jest.fn(), })); +jest.mock('~/server/services/Schedules', () => ({ + recordMCPToolAuthFailure: jest.fn(async () => true), +})); + jest.mock('./GraphTokenService', () => ({ getGraphApiToken: jest.fn(), })); @@ -1544,6 +1548,44 @@ describe('User parameter passing tests', () => { }); describe('createMCPTool', () => { + it('records a typed OBO failure against the scheduled generation before returning an error', async () => { + const user = { id: 'scheduled-owner', role: 'USER' }; + const missing = new Error('Unattended provider not configured'); + const error = Object.assign(new Error('MCP tool error'), { cause: missing }); + const receipt = require('~/server/services/Schedules').recordMCPToolAuthFailure; + require('~/models').getRoleByName.mockResolvedValue({ + permissions: { [PermissionTypes.MCP_SERVERS]: { [Permissions.USE]: true } }, + }); + mockGetMCPManager.mockReturnValue({ callTool: jest.fn().mockRejectedValue(error) }); + const tool = await createMCPTool({ + user, + toolKey: `test-tool${D}test-server`, + provider: 'openai', + streamId: 'scheduled-conversation', + jobCreatedAt: 42, + config: { type: 'streamable-http', url: 'https://mcp.example.com' }, + availableTools: { + [`test-tool${D}test-server`]: { + function: { description: 'Test MCP', parameters: { type: 'object', properties: {} } }, + }, + }, + }); + await expect( + tool.func({}, undefined, { + configurable: { user }, + metadata: { provider: 'openai', thread_id: 'scheduled-conversation', run_id: 'run-1' }, + toolCall: {}, + }), + ).rejects.toThrow(); + expect(receipt).toHaveBeenCalledWith({ + error, + streamId: 'scheduled-conversation', + jobCreatedAt: 42, + userId: 'scheduled-owner', + serverName: 'test-server', + }); + }); + it('keeps shared OAuth recovery alive when one tool caller aborts', async () => { const mockUser = { id: 'shared-recovery-user', role: 'USER' }; const mockRes = { write: jest.fn(), flush: jest.fn() }; @@ -2783,6 +2825,8 @@ describe('User parameter passing tests', () => { serverName: 'server1', provider: 'anthropic', userMCPAuthMap: {}, + streamId: 'scheduled-stream', + jobCreatedAt: 42, }); // Verify all calls to reinitMCPServer had the user @@ -2790,6 +2834,8 @@ describe('User parameter passing tests', () => { reinitCalls.forEach((call) => { expect(call.user).toBe(mockUser); expect(call.user.id).toBe('user-001'); + expect(call.streamId).toBe('scheduled-stream'); + expect(call.jobCreatedAt).toBe(42); }); }); @@ -2817,12 +2863,16 @@ describe('User parameter passing tests', () => { provider: 'google', userMCPAuthMap: {}, availableTools: undefined, // Force reinit + streamId: 'resumed-stream', + jobCreatedAt: 43, }); // Verify the call to reinitMCPServer had the user expect(reinitCalls.length).toBe(1); expect(reinitCalls[0].user).toBe(mockUser); expect(reinitCalls[0].user.id).toBe('user-002'); + expect(reinitCalls[0].streamId).toBe('resumed-stream'); + expect(reinitCalls[0].jobCreatedAt).toBe(43); }); }); diff --git a/api/server/services/Schedules/index.js b/api/server/services/Schedules/index.js index 50944efd019..2968ddfc590 100644 --- a/api/server/services/Schedules/index.js +++ b/api/server/services/Schedules/index.js @@ -84,6 +84,11 @@ module.exports = { getLimits: invoke('getLimits'), fireScheduleNow: invoke('fireScheduleNow'), recordScheduleOutcome: invoke('recordScheduleOutcome'), + recordMCPToolAuthFailure: (input) => + require('@librechat/api').recordScheduledMCPToolAuthFailure( + input, + () => getService().recordMCPToolAuthFailure, + ), beginScheduledStop: invoke('beginScheduledStop'), acknowledgeScheduledStopPersistence: invoke('acknowledgeScheduledStopPersistence'), claimScheduleResume: invoke('claimScheduleResume'), diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 62a1cb04204..f215d60cda3 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -53,6 +53,7 @@ const { createRepositoryInstructionLoader, resolveAttachedWorkspaceCommandTimeoutMax, resolveAttachedWorkspaceQueueWaitMs, + resolveAttachedWorkspaceRequestTimeoutMs, createContextProgrammaticBashTool, resolveCodeExecutionContext, resolveCodeExecutionWorkspaceContext, @@ -1192,6 +1193,8 @@ async function loadToolDefinitionsWrapper({ const result = await reinitMCPServer({ signal, user: req.user, + streamId, + jobCreatedAt, oauthStart, flowManager, serverName, @@ -1223,6 +1226,8 @@ async function loadToolDefinitionsWrapper({ const result = await reinitMCPServer({ signal, user: req.user, + streamId, + jobCreatedAt, forceNew: true, oauthStart, flowManager, @@ -1375,6 +1380,8 @@ async function loadToolDefinitionsWrapper({ const result = await reinitMCPServer({ signal, user: req.user, + streamId, + jobCreatedAt, serverName, configServers, userMCPAuthMap, @@ -2308,6 +2315,9 @@ async function loadToolsForExecution({ maxQueueWaitMs: resolveAttachedWorkspaceQueueWaitMs( codeExecutionContext.codeEnvironmentConfigSchema, ), + maxRequestTimeoutMs: resolveAttachedWorkspaceRequestTimeoutMs( + codeExecutionContext.codeEnvironmentConfigSchema, + ), }) : createBashExecutionTool({ authHeaders, diff --git a/api/server/services/Tools/mcp.js b/api/server/services/Tools/mcp.js index aea58c87289..762c9a3e56c 100644 --- a/api/server/services/Tools/mcp.js +++ b/api/server/services/Tools/mcp.js @@ -9,6 +9,7 @@ const { getMissingRuntimeBodyPlaceholderFields, isMCPInitializationError, prepareMCPAuthorizationMutation, + recordScheduledMCPToolAuthFailure, } = require('@librechat/api'); const { CacheKeys, Constants } = require('librechat-data-provider'); const { getMCPManager, getMCPServersRegistry, getFlowStateManager } = require('~/config'); @@ -127,6 +128,8 @@ async function loadMCPServerCatalogs({ * @param {import('@librechat/api').RequestScopedMCPConnectionStore} [params.requestScopedConnections] * @param {Record>} [params.userMCPAuthMap] * @param {import('@librechat/api').MCPServerCatalogRecoveryPolicy} [params.recoveryPolicy] + * @param {string | null} [params.streamId] - Owning generation's stream ID, if resumable. + * @param {number} [params.jobCreatedAt] - Owning generation epoch. */ async function reinitMCPServer({ user, @@ -147,6 +150,8 @@ async function reinitMCPServer({ oboIdentityContext, oauthEnd, recoveryPolicy, + streamId, + jobCreatedAt, }) { /** @type {MCPConnection | null} */ let connection = null; @@ -452,6 +457,10 @@ async function reinitMCPServer({ return result; } catch (error) { if (isMCPInitializationError(error, signal)) { + await recordScheduledMCPToolAuthFailure( + { error, streamId, jobCreatedAt, userId: user?.id, serverName }, + () => require('~/server/services/Schedules').recordMCPToolAuthFailure, + ); throw error; } logger.error('[MCP Reinitialize] Error loading MCP tools; servers may still be initializing'); diff --git a/api/server/services/Tools/mcp.spec.js b/api/server/services/Tools/mcp.spec.js index bc50be0fcca..9e5def26224 100644 --- a/api/server/services/Tools/mcp.spec.js +++ b/api/server/services/Tools/mcp.spec.js @@ -58,6 +58,9 @@ jest.mock('~/server/services/GraphTokenService', () => ({ jest.mock('~/cache', () => ({ getLogStores: jest.fn(() => ({})), })); +jest.mock('~/server/services/Schedules', () => ({ + recordMCPToolAuthFailure: jest.fn(async () => true), +})); const { reinitMCPServer, loadMCPServerCatalogs } = require('./mcp'); @@ -536,6 +539,75 @@ describe('reinitMCPServer — recovery of a server that failed inspection', () = }); }); +describe('scheduled MCP connection initialization', () => { + beforeEach(() => jest.clearAllMocks()); + + it('records a typed missing OBO provider before any tool instance exists', async () => { + const { OboTokenResolutionError } = require('@librechat/api'); + const failure = new OboTokenResolutionError('missing_upstream_provider', 'Provider missing'); + const receipt = require('~/server/services/Schedules').recordMCPToolAuthFailure; + mockGetConnection.mockRejectedValueOnce(failure); + + await expect( + reinitMCPServer({ + user: { id: 'owner' }, + serverName: 'Graph', + serverConfig: { + type: 'streamable-http', + url: 'https://mcp.example.com', + source: 'yaml', + obo: { scopes: 'api://graph/.default' }, + }, + streamId: 'scheduled-conversation', + jobCreatedAt: 42, + }), + ).rejects.toBe(failure); + expect(receipt).toHaveBeenCalledTimes(1); + expect(receipt).toHaveBeenCalledWith({ + error: failure, + streamId: 'scheduled-conversation', + jobCreatedAt: 42, + userId: 'owner', + serverName: 'Graph', + }); + }); + + it('preserves the connection error when the receipt store fails', async () => { + const { OboTokenResolutionError } = require('@librechat/api'); + const failure = new OboTokenResolutionError('missing_upstream_provider', 'Provider missing'); + const receipt = require('~/server/services/Schedules').recordMCPToolAuthFailure; + receipt.mockRejectedValueOnce(new Error('Mongo unavailable')); + mockGetConnection.mockRejectedValueOnce(failure); + + await expect( + reinitMCPServer({ + user: { id: 'owner' }, + serverName: 'Graph', + serverConfig: { type: 'streamable-http', url: 'https://mcp.example.com', source: 'yaml' }, + streamId: 'scheduled-conversation', + jobCreatedAt: 42, + }), + ).rejects.toBe(failure); + }); + + it('does not construct the schedule facade for unrelated typed OBO failures', async () => { + const { OboTokenResolutionError } = require('@librechat/api'); + const failure = new OboTokenResolutionError('session_refresh_failed', 'Retry later', true); + const receipt = require('~/server/services/Schedules').recordMCPToolAuthFailure; + mockGetConnection.mockRejectedValueOnce(failure); + await expect( + reinitMCPServer({ + user: { id: 'owner' }, + serverName: 'Graph', + serverConfig: { type: 'streamable-http', url: 'https://mcp.example.com', source: 'yaml' }, + streamId: 'scheduled-conversation', + jobCreatedAt: 42, + }), + ).rejects.toBe(failure); + expect(receipt).not.toHaveBeenCalled(); + }); +}); + describe('reinitMCPServer — direct bearer authentication outcomes', () => { it('preserves a typed rejection instead of reducing it to a generic result', async () => { const { MCPAuthenticationRejectedError } = require('@librechat/api'); diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index f57a506b07f..dd388f232dc 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -1767,6 +1767,9 @@ describe('ToolService - Action Capability Gating', () => { expect(emittedStreamId).toBe(streamId); expect(options).toEqual({ expectedCreatedAt: jobCreatedAt }); } + for (const [reinitInput] of reinitMCPServer.mock.calls) { + expect(reinitInput).toEqual(expect.objectContaining({ streamId, jobCreatedAt })); + } }); it('should not expose cached MCP tool definitions when the registry lookup fails', async () => { @@ -2307,6 +2310,8 @@ describe('ToolService - Action Capability Gating', () => { definitionsOnly: true, signal, upstreamTokenProvider: scheduledProvider, + streamId: 'scheduled-stream', + jobCreatedAt: 42, }); expect(reinitMCPServer).toHaveBeenCalledWith( @@ -2314,6 +2319,8 @@ describe('ToolService - Action Capability Gating', () => { serverName, forceNew: true, upstreamTokenProvider: scheduledProvider, + streamId: 'scheduled-stream', + jobCreatedAt: 42, }), ); }); diff --git a/client/src/components/Conversations/Conversations.tsx b/client/src/components/Conversations/Conversations.tsx index dae11862ef4..259a55b678c 100644 --- a/client/src/components/Conversations/Conversations.tsx +++ b/client/src/components/Conversations/Conversations.tsx @@ -284,18 +284,19 @@ const Conversations: FC = ({ [rawConversations], ); - /** The pinned section above carries pins, so they stay out of these groups — except in - * the archive, which that section does not cover: an archived pin would otherwise be - * absent from the sidebar entirely rather than merely further down it. */ + /** The pinned section carries pins except in the archive or during search, when it is + * hidden. Keep matching pins in the Chats results instead of showing an empty list. */ + const includePinned = isArchivedView || !!search.query; const datedConversations = useMemo( () => groupConversations(filteredConversations, { field: sort.field, direction: sort.direction, - includePinned: isArchivedView, + includePinned, }), - [filteredConversations, isArchivedView, sort.direction, sort.field], + [filteredConversations, includePinned, sort.direction, sort.field], ); + /** The archive keeps its server order, while search still promotes active matches. */ const groupedConversations = useMemo( () => groupConversationsWithRunning(datedConversations, activeJobIds, { @@ -306,8 +307,8 @@ const Conversations: FC = ({ [datedConversations, activeJobIds, isArchivedView, sort.direction, sort.field], ); - /* Pins are stripped from the date groups. An all-pin page leaves the - virtual list with no rows, so onRowsRendered never fires and later + /* Outside search, pins are stripped from the date groups. An all-pin page leaves + the virtual list with no rows, so onRowsRendered never fires and later unpinned chats stay unreachable. Ask for another page only when the conversations input actually changes; a failed fetchNextPage leaves the same array and must not loop. */ diff --git a/client/src/components/Conversations/__tests__/Conversations.test.tsx b/client/src/components/Conversations/__tests__/Conversations.test.tsx index 5916901cea9..53bdb3a412f 100644 --- a/client/src/components/Conversations/__tests__/Conversations.test.tsx +++ b/client/src/components/Conversations/__tests__/Conversations.test.tsx @@ -215,8 +215,42 @@ describe('Conversations: pinned chats live in PinnedSection', () => { ); it('does not render a pinned header inside the chats list', () => { - const { queryByText } = renderConversations([pinnedConvo]); + const { queryByText, queryByTestId } = renderConversations([pinnedConvo]); expect(queryByText('com_ui_pinned')).not.toBeInTheDocument(); + expect(queryByTestId('convo')).not.toBeInTheDocument(); + }); + + it('shows pinned search matches in Chats while the Pinned section is hidden', () => { + renderConversations([pinnedConvo, pinnedConvo], 'Pinned Chat'); + + expect(screen.getAllByTestId('convo')).toHaveLength(1); + expect(screen.getByTestId('convo')).toHaveTextContent('Pinned Chat'); + expect(screen.queryByText('com_ui_no_search_results')).not.toBeInTheDocument(); + }); + + it('keeps running matches promoted when search includes pinned chats', () => { + mockActiveJobIds = ['running']; + const running = { + ...pinnedConvo, + conversationId: 'running', + title: 'Running match', + pinned: false, + updatedAt: '2026-01-01T00:00:00.000Z', + } as TConversation; + + try { + renderConversations([pinnedConvo, running], 'match'); + + expect(screen.getAllByTestId('convo').map((row) => row.textContent)).toEqual([ + 'Running match', + 'Pinned Chat', + ]); + expect( + screen.getByRole('heading', { name: 'com_a11y_chats_running_section' }), + ).toBeInTheDocument(); + } finally { + mockActiveJobIds = []; + } }); it('does not render a duplicate new chat button in the chats header', () => { diff --git a/client/src/components/Messages/Content/Error/registry.ts b/client/src/components/Messages/Content/Error/registry.ts index 1fc44b1a95a..3f878637a03 100644 --- a/client/src/components/Messages/Content/Error/registry.ts +++ b/client/src/components/Messages/Content/Error/registry.ts @@ -29,6 +29,8 @@ export const errorCopy: Record = { [ErrorTypes.STREAM_EXPIRED]: 'com_error_stream_expired', [ErrorTypes.MODEL_NOT_FOUND]: 'com_error_model_not_found', [ErrorTypes.MODEL_RATE_LIMIT]: 'com_error_model_rate_limit', + [ErrorTypes.MODEL_STREAM_CLOSED]: 'com_error_model_stream_closed', + [ErrorTypes.MODEL_STREAM_STALLED]: 'com_error_model_stream_stalled', [ErrorTypes.COMPACTION_FAILED]: 'com_error_compaction_failed', [ErrorTypes.AUTH_FAILED]: 'com_error_auth_failed', [ErrorTypes.AUTH_RATE_LIMITED]: 'com_error_auth_rate_limited', diff --git a/client/src/components/Messages/Content/__tests__/Error.spec.tsx b/client/src/components/Messages/Content/__tests__/Error.spec.tsx index 944bbc62f83..de0c0772f4f 100644 --- a/client/src/components/Messages/Content/__tests__/Error.spec.tsx +++ b/client/src/components/Messages/Content/__tests__/Error.spec.tsx @@ -198,6 +198,43 @@ describe('Error — reader-facing provider and fallback copy', () => { expectReadable(); }); + it.each([ + [ + ErrorTypes.MODEL_STREAM_CLOSED, + 'com_error_model_stream_closed', + 'The model provider closed the connection before the response finished. Try again.', + ], + [ + ErrorTypes.MODEL_STREAM_STALLED, + 'com_error_model_stream_stalled', + 'The model provider stopped sending the response, and the request timed out. Try again.', + ], + ])( + 'localizes a %s error even when it carries older-client fallback prose', + (type, key, prose) => { + renderError(`${prose}\n${JSON.stringify({ type })}`, providerMessage); + + expect(screen.getByText(catalog[key])).toBeInTheDocument(); + expect(screen.queryByText(prose)).not.toBeInTheDocument(); + expect(screen.queryByText(/terminated/i)).not.toBeInTheDocument(); + expectReadable(); + }, + ); + + it('keeps fallback prose when an older client cannot recognize the server error type', () => { + const prose = + 'The model provider closed the connection before the response finished. Try again.'; + renderError( + `${prose}\n${JSON.stringify({ type: 'newer_model_stream_failure' })}`, + providerMessage, + ); + + expect(screen.getByText(prose)).toBeInTheDocument(); + expect(screen.queryByText(catalog.com_error_unknown)).not.toBeInTheDocument(); + expect(document.body.textContent).not.toContain('{'); + expectReadable(); + }); + it('keeps provider text for a LangChain code without localized copy, minus the URL', () => { const raw = 'An error occurred while processing the request: could not parse output\n\nTroubleshooting URL: https://docs.langchain.com/oss/javascript/langchain/errors/OUTPUT_PARSING_FAILURE/\n'; diff --git a/client/src/components/SidePanel/Schedules/ScheduleCard.tsx b/client/src/components/SidePanel/Schedules/ScheduleCard.tsx index 31ecb3cefcc..fb979853d85 100644 --- a/client/src/components/SidePanel/Schedules/ScheduleCard.tsx +++ b/client/src/components/SidePanel/Schedules/ScheduleCard.tsx @@ -17,18 +17,19 @@ import { import type { TSchedule, ScheduleRunStatus, ScheduleDisabledReason } from 'librechat-data-provider'; import type { ImmediateScheduleMCPFailure } from './errors'; import type { TranslationKeys } from '~/hooks'; -import { - useGetAgentByIdQuery, - useDeleteScheduleMutation, - useUpdateScheduleMutation, - useRunScheduleNowMutation, -} from '~/data-provider'; import { scheduleMCPErrorMessage, scheduleMCPErrorOutcomes, scheduleLastRunKey, scheduleMCPCardOutcomes, + scheduleDisabledMCPLabel, } from './errors'; +import { + useGetAgentByIdQuery, + useDeleteScheduleMutation, + useUpdateScheduleMutation, + useRunScheduleNowMutation, +} from '~/data-provider'; import { useLocalize, useHasAccess, useClockFormat, useWeekStart } from '~/hooks'; import ScheduleMCPRecovery from './ScheduleMCPRecovery'; import { useAgentsMapContext } from '~/Providers'; @@ -75,6 +76,7 @@ export default function ScheduleCard({ schedule, projectName }: ScheduleCardProp const [immediateMCPFailure, setImmediateMCPFailure] = useState(null); const mcpOutcomes = scheduleMCPCardOutcomes(schedule, immediateMCPFailure); + const disabledMCPLabel = scheduleDisabledMCPLabel(schedule.disabledReason, mcpOutcomes); const { i18n } = useTranslation(); const { showToast } = useToastContext(); const agentsMap = useAgentsMapContext(); @@ -293,7 +295,9 @@ export default function ScheduleCard({ schedule, projectName }: ScheduleCardProp {localize(statusChip.label)} ))} {schedule.disabledReason != null && ( - {localize(DISABLED_REASON_LABELS[schedule.disabledReason])} + + {localize(disabledMCPLabel ?? DISABLED_REASON_LABELS[schedule.disabledReason])} + )} )} diff --git a/client/src/components/SidePanel/Schedules/ScheduleMCPRecovery.tsx b/client/src/components/SidePanel/Schedules/ScheduleMCPRecovery.tsx index 91ae6b8c444..17721a6ab6c 100644 --- a/client/src/components/SidePanel/Schedules/ScheduleMCPRecovery.tsx +++ b/client/src/components/SidePanel/Schedules/ScheduleMCPRecovery.tsx @@ -1,5 +1,5 @@ import type { ScheduleMCPOutcome } from 'librechat-data-provider'; -import { MCP_STATUS_LABELS, scheduleMCPNeedsAgentRecovery } from './errors'; +import { scheduleMCPStatusLabel, scheduleMCPNeedsAgentRecovery } from './errors'; import { useLocalize } from '~/hooks'; export default function ScheduleMCPRecovery({ @@ -28,7 +28,7 @@ export default function ScheduleMCPRecovery({ className="flex flex-wrap items-baseline gap-x-2 text-xs text-text-secondary" >

- {item.server} ({ownerLabel}): {localize(MCP_STATUS_LABELS[item.status])} + {item.server} ({ownerLabel}): {localize(scheduleMCPStatusLabel(item))}

{scheduleMCPNeedsAgentRecovery([item]) && ( + ); + } + if (isSearchLoading) { + body =
; + } + return
{body}
; }); return { __esModule: true, Conversations: ConversationsStub }; }); @@ -139,9 +166,11 @@ import ConversationsSection from '../ConversationsSection'; import store from '~/store'; let setStreamTick: SetterOrUpdater; +let setSearchState: SetterOrUpdater; function TickController() { setStreamTick = useSetRecoilState(streamTickAtom); + setSearchState = useSetRecoilState(store.search); return null; } @@ -169,10 +198,22 @@ const settleRenders = async () => { } }; -const renderSection = () => +const renderSection = (searchQuery = '') => render( - + { + if (searchQuery) { + set(store.search, { + query: searchQuery, + debouncedQuery: searchQuery, + enabled: true, + isTyping: false, + isSearching: false, + }); + } + }} + > @@ -243,6 +284,153 @@ describe('ConversationsSection streaming re-renders', () => { ); }); +describe('ConversationsSection search refetch', () => { + it('does not display old matches as unfiltered chats while clearing the search', async () => { + const previousData = mockConversationsResult.data; + mockConversationsResult.data = { + pages: [ + { + conversations: [{ conversationId: 'chat-1', title: 'Old search match' }], + nextCursor: null, + }, + ], + }; + + try { + renderSection('draft'); + await settleRenders(); + expect(screen.getByText('Old search match')).toBeInTheDocument(); + + act(() => { + setSearchState({ + query: '', + debouncedQuery: 'draft', + enabled: true, + isTyping: true, + isSearching: false, + }); + }); + + expect(screen.getByTestId('projects-stub')).toBeInTheDocument(); + expect(screen.queryByText('Old search match')).not.toBeInTheDocument(); + expect(screen.getByTestId('search-spinner')).toBeInTheDocument(); + } finally { + mockConversationsResult.data = previousData; + } + }); + + it('shows progress while retrying a failed cached search with no results', async () => { + const previousData = mockConversationsResult.data; + mockConversationsResult.isError = true; + + try { + renderSection('draft'); + await settleRenders(); + expect(screen.getByRole('button', { name: 'com_ui_retry' })).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'com_ui_retry' })); + expect(mockConversationsResult.refetch).toHaveBeenCalledTimes(1); + + act(() => { + mockConversationsResult.isFetching = true; + setStreamTick((prev) => prev + 1); + }); + + expect(screen.getByTestId('search-spinner')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'com_ui_retry' })).not.toBeInTheDocument(); + + act(() => { + mockConversationsResult.isFetching = false; + mockConversationsResult.isError = false; + mockConversationsResult.data = { + pages: [ + { + conversations: [{ conversationId: 'chat-1', title: 'Found match' }], + nextCursor: null, + }, + ], + }; + setStreamTick((prev) => prev + 1); + }); + + expect(screen.getByText('Found match')).toBeInTheDocument(); + expect(screen.queryByTestId('search-spinner')).not.toBeInTheDocument(); + } finally { + mockConversationsResult.data = previousData; + mockConversationsResult.isError = false; + mockConversationsResult.isFetching = false; + mockConversationsResult.refetch.mockClear(); + } + }); + + it('shows loading for an uncached search', async () => { + const previousData = mockConversationsResult.data; + mockConversationsResult.data = undefined; + mockConversationsResult.isLoading = true; + + try { + renderSection('draft'); + await settleRenders(); + expect(screen.getByTestId('search-spinner')).toBeInTheDocument(); + } finally { + mockConversationsResult.data = previousData; + mockConversationsResult.isLoading = false; + } + }); + + it('does not show results from the previous search while the next one loads', async () => { + const previousData = mockConversationsResult.data; + mockConversationsResult.data = { + pages: [ + { + conversations: [{ conversationId: 'chat-1', title: 'Previous match' }], + nextCursor: null, + }, + ], + }; + mockConversationsResult.isPreviousData = true; + + try { + renderSection('new term'); + await settleRenders(); + expect(screen.getByTestId('search-spinner')).toBeInTheDocument(); + expect(screen.queryByText('Previous match')).not.toBeInTheDocument(); + } finally { + mockConversationsResult.data = previousData; + mockConversationsResult.isPreviousData = false; + } + }); + + it('keeps cached results visible when a message triggers a background list refetch', async () => { + const previousData = mockConversationsResult.data; + mockConversationsResult.data = { + pages: [ + { + conversations: [{ conversationId: 'chat-1', title: 'Matching chat' }], + nextCursor: null, + }, + ], + }; + + try { + renderSection('draft'); + await settleRenders(); + expect(screen.getByText('Matching chat')).toBeInTheDocument(); + + act(() => { + mockConversationsResult.isFetching = true; + setStreamTick((prev) => prev + 1); + }); + + expect(screen.getByText('Matching chat')).toBeInTheDocument(); + expect(screen.queryByTestId('search-spinner')).not.toBeInTheDocument(); + } finally { + mockConversationsResult.isFetching = false; + mockConversationsResult.data = previousData; + } + }); +}); + describe('ConversationsSection shared scroll surface', () => { /** Searching swaps what the one surface holds — Projects and Pinned leave, * the chats become results — and a position kept from the previous contents diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 2ae5ecd7f34..47b7bfb1ad5 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -64,11 +64,13 @@ "com_ui_code_workspace_default_description": "New chats use this workspace. Without a default, reuse your last available choice for this agent and machine in this browser. Existing chats keep their workspace.", "com_ui_schedule_mcp_ready": "Ready for scheduled chats", "com_ui_schedule_mcp_reauth": "Reconnect this MCP server before enabling the schedule.", + "com_ui_schedule_mcp_unattended_auth": "Unattended access is not configured for this MCP server. Ask your administrator to set up a renewable credential provider; reconnecting in chat will not fix this.", "com_ui_schedule_mcp_configuration": "Configure or remove this MCP server before enabling the schedule.", "com_ui_schedule_mcp_permission": "An administrator must restore your MCP server permission.", "com_ui_schedule_mcp_unavailable": "MCP server unavailable. Try again later.", "com_ui_schedule_mcp_open_agent": "Open agent to reconnect MCPs", "com_ui_schedule_disabled_mcp_reauth": "Disabled: MCP reauthentication required", + "com_ui_schedule_disabled_mcp_unattended_auth": "Disabled: unattended MCP access not configured", "com_ui_schedule_disabled_mcp_configuration": "Disabled: MCP configuration required", "com_ui_schedule_disabled_mcp_permission": "Disabled: MCP permission revoked", "com_ui_code_approval_mode": "Code approval mode", @@ -571,6 +573,8 @@ "com_error_missing_model": "No model selected for {{0}}. Please select a model and try again.", "com_error_model_not_found": "The selected model is unavailable from this provider. Select a different model or ask an administrator to verify the provider configuration.", "com_error_model_rate_limit": "The provider declined this request for exceeding a rate or spending limit. Wait a moment and try again, or ask an administrator to review the provider's limits.", + "com_error_model_stream_closed": "The model provider closed the connection before the response finished. Anything already received was kept. Try again; long reasoning runs are the most likely to be cut off.", + "com_error_model_stream_stalled": "The model provider stopped sending the response, and the request timed out waiting for it. Anything already received was kept. Try again, or ask an administrator to check the provider.", "com_error_models_not_loaded": "Models configuration could not be loaded. Please refresh the page and try again.", "com_error_moderation": "This message was flagged by the moderation system, so it was not sent. Edit the message or start a new conversation.", "com_error_no_base_url": "No base URL found. Please provide one and try again.", diff --git a/config/create-error-convo.js b/config/create-error-convo.js index 20e3ff3ab02..f9db4f271c2 100644 --- a/config/create-error-convo.js +++ b/config/create-error-convo.js @@ -189,6 +189,16 @@ const ERROR_CASES = [ payloadCase('Stream expired before the client attached', { type: ErrorTypes.STREAM_EXPIRED }), payloadCase('Model not served by this provider', { type: ErrorTypes.MODEL_NOT_FOUND }), payloadCase('Provider rate or spend limit', { type: ErrorTypes.MODEL_RATE_LIMIT }), + { + label: 'Provider closed the stream mid-response', + text: `The model provider closed the connection before the response finished. Try again.\n${JSON.stringify({ type: ErrorTypes.MODEL_STREAM_CLOSED })}`, + covers: ErrorTypes.MODEL_STREAM_CLOSED, + }, + { + label: 'Provider stream stalled past the response timeout', + text: `The model provider stopped sending the response, and the request timed out. Try again.\n${JSON.stringify({ type: ErrorTypes.MODEL_STREAM_STALLED })}`, + covers: ErrorTypes.MODEL_STREAM_STALLED, + }, { label: 'Upstream model error with status (server prefix + JSON)', text: `The model provider failed and the run could not recover.\n${JSON.stringify({ diff --git a/librechat.example.yaml b/librechat.example.yaml index 7836c55405b..2f8caedec6d 100644 --- a/librechat.example.yaml +++ b/librechat.example.yaml @@ -720,6 +720,17 @@ endpoints: # # Maximum Bash execution budget the model may request per call. # # Omit to preserve the historical 30-second limit. Hard cap: 300000. # maxCommandTimeoutMs: 120000 + # # Optional total HTTP budget per workspace tool call, including + # # admission, retries, worst-case execution, settlement, and delivery. + # # Do NOT enable until every LibreChat API replica recognizes this + # # strict config field, Code API honors X-LibreChat-Workspace-Queue-Wait-Ms, + # # and a non-mutating timed canary verifies the live CDN, load balancer, + # # ingress, and caller can all outlast this value. The Nginx example + # # (120s) cannot establish the live limit. Omission keeps 30s admission. + # # With 125s total, a read can queue <=90s, a default command <=85s, + # # a 90s command <=25s. Commands >115s cannot fit; the 120s command + # # ceiling above requires a larger VERIFIED total budget to be usable. + # # maxRequestTimeoutMs: 125000 # permissions: # fileWrite: # allowed: [allow, ask, deny] diff --git a/package-lock.json b/package-lock.json index 36921d7d969..3f11d56f451 100644 --- a/package-lock.json +++ b/package-lock.json @@ -64,7 +64,7 @@ "@azure/storage-blob": "^12.30.0", "@google/genai": "^2.8.0", "@keyv/redis": "5.1.6", - "@librechat/agents": "^3.9.6", + "@librechat/agents": "^3.9.7", "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", @@ -10622,9 +10622,9 @@ } }, "node_modules/@librechat/agents": { - "version": "3.9.6", - "resolved": "https://registry.npmjs.org/@librechat/agents/-/agents-3.9.6.tgz", - "integrity": "sha512-sHKqt4f12+jfSybbi1hizvMm6gB/yYpwcS0+miad+ZZoBP3xNeuaOXIILW9TO9WwQYlVRbGImS6IF89qd8BSXw==", + "version": "3.9.7", + "resolved": "https://registry.npmjs.org/@librechat/agents/-/agents-3.9.7.tgz", + "integrity": "sha512-r3JSh7ACjKVn/ZzDlXtRU/l16L19pxjlPzcTxa+h+Myol3qLDdx/ldmSiNpSFMUQcGDcCxE07A+ZJWhChcaBZg==", "license": "MIT", "dependencies": { "@anthropic-ai/sdk": "^0.115.0", @@ -42449,7 +42449,7 @@ "@azure/storage-blob": "^12.30.0", "@google/genai": "^2.8.0", "@keyv/redis": "5.1.6", - "@librechat/agents": "^3.9.6", + "@librechat/agents": "^3.9.7", "@librechat/data-schemas": "*", "@modelcontextprotocol/sdk": "^1.30.0", "@opentelemetry/api": "^1.9.0", diff --git a/packages/api/package.json b/packages/api/package.json index 7fc2ae30b7f..bf9bb076194 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -120,7 +120,7 @@ "@azure/storage-blob": "^12.30.0", "@google/genai": "^2.8.0", "@keyv/redis": "5.1.6", - "@librechat/agents": "^3.9.6", + "@librechat/agents": "^3.9.7", "@librechat/data-schemas": "*", "@modelcontextprotocol/sdk": "^1.30.0", "@opentelemetry/api": "^1.9.0", diff --git a/packages/api/src/agents/errors.spec.ts b/packages/api/src/agents/errors.spec.ts index bc621f2baf3..3726c154f90 100644 --- a/packages/api/src/agents/errors.spec.ts +++ b/packages/api/src/agents/errors.spec.ts @@ -1,5 +1,10 @@ +import { createServer } from 'node:http'; +import { Agent, errors, fetch } from 'undici'; import { ErrorTypes } from 'librechat-data-provider'; +import { ChatOpenAI } from '@librechat/agents/llm/openai'; import { GraphRecursionError } from '@langchain/langgraph'; +import type { AddressInfo } from 'node:net'; +import type { ModelErrorTrackerCallback } from './failures/tracker'; import { GENERIC_PROVIDER_ERROR, getLangChainErrorCode, @@ -11,8 +16,11 @@ import { AGENT_ATTACHMENT_LIMIT_EXCEEDED, AGENT_EXPECTED_MCP_TOOLS_UNAVAILABLE, isStepLimitError, + getModelStreamFailure, + resolveModelStreamError, } from './errors'; import { MCPAuthenticationRejectedError, MCPAuthenticationRefreshError } from '~/mcp/errors'; +import { createTerminalRunErrorObserver } from './failures/terminal'; import { OboTokenResolutionError } from '~/mcp/oauth/obo'; import { OpenIDReauthRequiredError } from '~/utils/oidc'; @@ -306,3 +314,221 @@ describe('getAgentErrorMetadata', () => { expect(getAgentErrorMetadata({ status })).toEqual({}); }); }); + +type StreamEnding = 'close' | 'stall' | 'abort' | 'headers' | 'close-before-headers'; + +interface DyingStreamClient { + baseURL: string; + fetch: typeof globalThis.fetch; + signal: AbortSignal; +} + +/** Returns the real transport error from a provider failure before or after response headers. */ +async function readDyingStream( + ending: StreamEnding, + read: (client: DyingStreamClient) => Promise, + firstEvent = 'data: {"delta":"Hel"}\n\n', +): Promise { + const controller = new AbortController(); + const server = createServer((_req, res) => { + if (ending === 'headers') { + return; + } + if (ending === 'close-before-headers') { + setTimeout(() => res.socket?.destroy(), 20); + return; + } + res.writeHead(200, { 'content-type': 'text/event-stream' }); + res.write(firstEvent); + if (ending === 'close') { + setTimeout(() => res.socket?.destroy(), 20); + } + if (ending === 'abort') { + setTimeout(() => controller.abort(), 20); + } + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as AddressInfo; + const dispatcher = new Agent({ bodyTimeout: 150, headersTimeout: 150 }); + const client: DyingStreamClient = { + baseURL: `http://127.0.0.1:${port}/v1`, + signal: controller.signal, + fetch: ((input: string, init?: object) => + fetch(input, { ...init, dispatcher })) as unknown as typeof globalThis.fetch, + }; + try { + await read(client); + return undefined; + } catch (error) { + return error; + } finally { + await dispatcher.destroy(); + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + } +} + +const readRawBody = async ({ baseURL, fetch: read, signal }: DyingStreamClient) => { + const response = await read(baseURL, { signal }); + await response.text(); +}; + +const CHAT_COMPLETION_EVENT = `data: ${JSON.stringify({ + id: 'chatcmpl-1', + object: 'chat.completion.chunk', + created: 0, + model: 'test-model', + choices: [{ index: 0, delta: { role: 'assistant', content: 'Hel' }, finish_reason: null }], +})}\n\n`; + +const RESPONSES_EVENT = `event: response.output_text.delta\ndata: ${JSON.stringify({ + type: 'response.output_text.delta', + item_id: 'msg_1', + output_index: 0, + content_index: 0, + delta: 'Hel', + sequence_number: 1, +})}\n\n`; + +function streamThroughChatOpenAI( + useResponsesApi: boolean, + modelCallback?: ModelErrorTrackerCallback, +) { + return async ({ baseURL, fetch: read, signal }: DyingStreamClient) => { + const model = new ChatOpenAI({ + apiKey: 'test-key', + model: 'test-model', + streaming: true, + useResponsesApi, + maxRetries: 0, + ...(modelCallback != null ? { callbacks: [modelCallback] } : {}), + configuration: { baseURL, fetch: read }, + } as never); + for await (const _chunk of await model.stream('hi', { signal })) { + /* drain */ + } + }; +} + +describe('model stream failures', () => { + it('classifies a provider closing the connection mid-stream', async () => { + const error = await readDyingStream('close', readRawBody); + + expect(error).toBeInstanceOf(TypeError); + expect(getModelStreamFailure(error)).toBe('closed'); + expect(resolveModelStreamError(error)).toBe( + 'The model provider closed the connection before the response finished. Try again.\n' + + JSON.stringify({ type: ErrorTypes.MODEL_STREAM_CLOSED }), + ); + }); + + it('classifies a provider that goes silent past the body timeout', async () => { + const error = await readDyingStream('stall', readRawBody); + + expect(error).toBeInstanceOf(TypeError); + expect(getModelStreamFailure(error)).toBe('stalled'); + expect(resolveModelStreamError(error)).toBe( + 'The model provider stopped sending the response, and the request timed out. Try again.\n' + + JSON.stringify({ type: ErrorTypes.MODEL_STREAM_STALLED }), + ); + }); + + it('still classifies the transport failure when an outer layer rethrows it', async () => { + const error = await readDyingStream('stall', readRawBody); + + expect(getModelStreamFailure(new Error('graph failed', { cause: error }))).toBe('stalled'); + }); + + it('prefers a body timeout over a socket error found earlier in the cause chain', () => { + const socket = new TypeError('terminated', { + cause: new errors.SocketError('other side closed'), + }); + const timeout = Object.assign(new errors.BodyTimeoutError(), { cause: socket }); + + expect(getModelStreamFailure(new Error('graph failed', { cause: timeout }))).toBe('stalled'); + }); + + it('leaves a cancelled stream to the cancellation path', async () => { + const error = await readDyingStream('abort', readRawBody); + + expect(error).toBeDefined(); + expect(getModelStreamFailure(error)).toBeUndefined(); + }); + + it.each([ + ['Chat Completions', false, CHAT_COMPLETION_EVENT], + ['Responses', true, RESPONSES_EVENT], + ])( + 'tracks and classifies a real %s model stream for each ending', + async (_api, responses, event) => { + const observer = createTerminalRunErrorObserver({ + logger: { error: jest.fn() }, + source: '[Agent API]', + }); + const read = streamThroughChatOpenAI(responses, observer.modelCallback); + const closed = await readDyingStream('close', read, event); + const stalled = await readDyingStream('stall', read, event); + const aborted = await readDyingStream('abort', read, event); + + expect(getModelStreamFailure(closed)).toBe('closed'); + expect(observer.getUserFacingError(closed, () => 'untracked')).toBe( + resolveModelStreamError(closed), + ); + expect(getModelStreamFailure(stalled)).toBe('stalled'); + expect(observer.getUserFacingError(stalled, () => 'untracked')).toBe( + resolveModelStreamError(stalled), + ); + expect(aborted).toBeDefined(); + expect(getModelStreamFailure(aborted)).toBeUndefined(); + }, + ); + + it.each([ + ['waiting for headers', 'headers'], + ['a socket closing before headers', 'close-before-headers'], + ] as const)('does not mistake %s for a failed response body', async (_label, ending) => { + const raw = await readDyingStream(ending, readRawBody); + expect(raw).toBeDefined(); + expect(getModelStreamFailure(raw)).toBeUndefined(); + + for (const responses of [false, true]) { + const modelError = await readDyingStream(ending, streamThroughChatOpenAI(responses)); + expect(modelError).toBeDefined(); + expect(getModelStreamFailure(modelError)).toBeUndefined(); + } + }); + + it.each([ + ['a provider rejection', Object.assign(new Error('500 upstream failed'), { status: 500 })], + ['prose that happens to say terminated', new Error('terminated')], + ['a bare terminated TypeError', new TypeError('terminated')], + ['a socket error before body streaming', new errors.SocketError('other side closed')], + [ + 'a socket close wrapped by a pre-response fetch failure', + new TypeError('fetch failed', { cause: new errors.SocketError('other side closed') }), + ], + [ + 'a header timeout', + new TypeError('fetch failed', { cause: new errors.HeadersTimeoutError() }), + ], + ['a non-error value', 'terminated'], + ['nothing', undefined], + ])('does not classify %s', (_label, error) => { + expect(getModelStreamFailure(error)).toBeUndefined(); + expect(resolveModelStreamError(error)).toBeUndefined(); + }); + + it('reads the cause chain without trusting hostile accessors', () => { + const hostile = Object.defineProperty( + new TypeError('terminated', { cause: new errors.BodyTimeoutError() }), + 'code', + { + get() { + throw new Error('accessor exploded'); + }, + }, + ); + + expect(getModelStreamFailure(hostile)).toBe('stalled'); + }); +}); diff --git a/packages/api/src/agents/errors.ts b/packages/api/src/agents/errors.ts index 38777e76563..37f6d1b2071 100644 --- a/packages/api/src/agents/errors.ts +++ b/packages/api/src/agents/errors.ts @@ -102,6 +102,67 @@ export function resolveLangChainError(error: unknown): string | undefined { return type == null ? undefined : JSON.stringify({ type }); } +export type ModelStreamFailure = 'closed' | 'stalled'; + +/** A timeout while reading a response body, not while waiting for its headers. */ +const STALLED_TRANSPORT_CODES = new Set(['UND_ERR_BODY_TIMEOUT']); +const STALLED_TRANSPORT_NAMES = new Set(['BodyTimeoutError']); +/** Socket errors only describe a mid-response close when Fetch also reports body termination. */ +const CLOSED_TRANSPORT_CODES = new Set(['UND_ERR_SOCKET', 'ECONNRESET', 'EPIPE']); + +/** + * How a model response died in transit, or `undefined` for any other failure. + * + * Fetch reports both a provider hanging up mid-stream and our own body timeout as a bare + * `TypeError: terminated`; only the undici error in its `cause` tells them apart. A stall wins + * over a close found deeper in the chain, since the timeout is what ended the request. + */ +export function getModelStreamFailure(error: unknown): ModelStreamFailure | undefined { + let current = error; + let terminated = false; + let closed = false; + for (let depth = 0; depth < MAX_CAUSE_DEPTH && current != null; depth++) { + if (typeof current !== 'object') { + break; + } + const code = readErrorProperty(current, 'code'); + const name = readErrorProperty(current, 'name'); + if ( + (typeof code === 'string' && STALLED_TRANSPORT_CODES.has(code)) || + (typeof name === 'string' && STALLED_TRANSPORT_NAMES.has(name)) + ) { + return 'stalled'; + } + if (typeof code === 'string' && CLOSED_TRANSPORT_CODES.has(code)) { + closed = true; + } + if (name === 'TypeError' && readErrorProperty(current, 'message') === 'terminated') { + terminated = true; + } + current = readErrorProperty(current, 'cause'); + } + return closed && terminated ? 'closed' : undefined; +} + +const MODEL_STREAM_ERROR_TYPES: Record = { + closed: ErrorTypes.MODEL_STREAM_CLOSED, + stalled: ErrorTypes.MODEL_STREAM_STALLED, +}; + +/** Safe fallback for an older client that cannot localize these new error types yet. */ +const MODEL_STREAM_FALLBACK: Record = { + closed: 'The model provider closed the connection before the response finished. Try again.', + stalled: 'The model provider stopped sending the response, and the request timed out. Try again.', +}; + +/** Typed payload for new clients, with useful prose when the client bundle predates these types. */ +export function resolveModelStreamError(error: unknown): string | undefined { + const failure = getModelStreamFailure(error); + return failure == null + ? undefined + : `${MODEL_STREAM_FALLBACK[failure]}\n${JSON.stringify({ type: MODEL_STREAM_ERROR_TYPES[failure] })}`; +} + /** * Provider failure text for OpenAI-compatible responses, which carry raw strings rather than the * typed payloads the LibreChat client localizes. diff --git a/packages/api/src/agents/failures/terminal.spec.ts b/packages/api/src/agents/failures/terminal.spec.ts index 18421f9a3f2..d0e1308afcd 100644 --- a/packages/api/src/agents/failures/terminal.spec.ts +++ b/packages/api/src/agents/failures/terminal.spec.ts @@ -1,3 +1,4 @@ +import { errors } from 'undici'; import { createTerminalRunErrorObserver, getUpstreamModelErrorMetadata } from './terminal'; describe('terminal agent-run error logging', () => { @@ -217,13 +218,111 @@ describe('terminal agent-run error logging', () => { ); }); + it.each([ + [ + 'stalled', + new errors.BodyTimeoutError(), + 'model_stream_stalled', + 'The model provider stopped sending the response, and the request timed out. Try again.', + ], + [ + 'closed', + new errors.SocketError('other side closed'), + 'model_stream_closed', + 'The model provider closed the connection before the response finished. Try again.', + ], + ])( + 'names a %s model stream instead of the bare transport error', + (failure, cause, type, prose) => { + const logger = { error: jest.fn() }; + const observer = createTerminalRunErrorObserver({ + logger, + source: '[Agent API]', + protectionEnabled: false, + }); + const transportError = new TypeError('terminated', { cause }); + observer.modelCallback.handleLLMError(transportError); + const terminalError = new Error('graph failed', { cause: transportError }); + + expect(observer.getUserFacingError(terminalError, () => 'fallback')).toBe( + `${prose}\n${JSON.stringify({ type })}`, + ); + observer.log(terminalError); + expect(logger.error).toHaveBeenCalledWith( + '[Agent API] Upstream model error', + expect.objectContaining({ errorType: `stream_${failure}` }), + ); + }, + ); + + it.each([ + ['a response header timeout', new errors.HeadersTimeoutError()], + ['a pre-response socket close', new errors.SocketError('other side closed')], + ])('keeps %s on the generic upstream path', (_label, cause) => { + const logger = { error: jest.fn() }; + const observer = createTerminalRunErrorObserver({ + logger, + source: '[Agent API]', + protectionEnabled: true, + }); + const transportError = new TypeError('fetch failed', { cause }); + observer.modelCallback.handleLLMError(transportError); + + expect(observer.getUserFacingError(transportError, () => 'fallback')).toBe( + 'The model provider could not complete this request.\n' + + JSON.stringify({ type: 'upstream_model_error' }), + ); + observer.log(transportError); + expect(logger.error).toHaveBeenCalledWith( + '[Agent API] Upstream model error', + expect.objectContaining({ errorType: '_OTHER' }), + ); + }); + + it.each([ + [200, 'stream_stalled', 'model_stream_stalled'], + [502, '502', 'upstream_model_error'], + ])('keeps HTTP %i and a body timeout in their correct categories', (status, errorType, type) => { + const logger = { error: jest.fn() }; + const observer = createTerminalRunErrorObserver({ + logger, + source: '[Agent API]', + protectionEnabled: true, + }); + const failure = Object.assign( + new TypeError('terminated', { cause: new errors.BodyTimeoutError() }), + { response: { status } }, + ); + observer.modelCallback.handleLLMError(failure); + + const userError = observer.getUserFacingError(failure, () => 'fallback'); + expect(userError).toContain(JSON.stringify({ type, ...(status >= 400 ? { status } : {}) })); + expect(userError).not.toContain('terminated'); + observer.log(failure); + expect(logger.error).toHaveBeenCalledWith( + '[Agent API] Upstream model error', + expect.objectContaining({ errorType, status }), + ); + }); + + it('keeps an untracked transport failure off the upstream model path', () => { + const observer = createTerminalRunErrorObserver({ + logger: { error: jest.fn() }, + source: '[Agent API]', + protectionEnabled: false, + }); + const transportError = new TypeError('terminated', { cause: new errors.BodyTimeoutError() }); + + expect(observer.getUserFacingError(transportError, () => 'fallback')).toBe('fallback'); + }); + it('preserves a more specific localized model classification', () => { const observer = createTerminalRunErrorObserver({ logger: { error: jest.fn() }, source: '[Agent API]', protectionEnabled: false, }); - const providerError = new Error('provider failed'); + const providerError = new TypeError('terminated', { cause: new errors.BodyTimeoutError() }); const terminalError = Object.assign(new Error('rate limited', { cause: providerError }), { lc_error_code: 'MODEL_RATE_LIMIT', }); diff --git a/packages/api/src/agents/failures/terminal.ts b/packages/api/src/agents/failures/terminal.ts index ce2fcf5909e..79e054ad8d8 100644 --- a/packages/api/src/agents/failures/terminal.ts +++ b/packages/api/src/agents/failures/terminal.ts @@ -1,8 +1,13 @@ import { ErrorTypes } from 'librechat-data-provider'; import type { SafeErrorMetadata } from '../../utils/errors'; import type { ModelErrorTrackerCallback } from './tracker'; +import { + resolveLangChainError, + getModelStreamFailure, + resolveModelStreamError, + getProviderErrorMessage, +} from '../errors'; import { getSafeErrorMetadata, isOwnedAbortError } from '../../utils/errors'; -import { getProviderErrorMessage, resolveLangChainError } from '../errors'; import { traceIdForMessage } from '../../langfuse/trace'; import { createModelErrorTracker } from './tracker'; @@ -41,6 +46,18 @@ export function isAgentRunCancellation(error: unknown, signal?: AbortSignal): bo return isOwnedAbortError(error, signal); } +/** Preserve rejection status; a successful status can still precede a broken response body. */ +function getUpstreamErrorType(error: unknown, status?: number): string { + if (status != null && status >= 400) { + return String(status); + } + const streamFailure = getModelStreamFailure(error); + if (streamFailure != null) { + return `stream_${streamFailure}`; + } + return status == null ? UNKNOWN_UPSTREAM_MODEL_ERROR_TYPE : String(status); +} + export function getUpstreamModelErrorMetadata( error: unknown, responseMessageId?: string, @@ -50,8 +67,7 @@ export function getUpstreamModelErrorMetadata( ...safeMetadata, errorCode: UPSTREAM_MODEL_ERROR_CODE, errorOrigin: UPSTREAM_MODEL_ERROR_ORIGIN, - errorType: - safeMetadata.status != null ? String(safeMetadata.status) : UNKNOWN_UPSTREAM_MODEL_ERROR_TYPE, + errorType: getUpstreamErrorType(error, safeMetadata.status), ...(typeof responseMessageId === 'string' && responseMessageId !== '' ? { traceId: traceIdForMessage(responseMessageId) } : {}), @@ -89,13 +105,17 @@ export function createTerminalRunErrorObserver({ return fallback(); } + const { status } = getSafeErrorMetadata(upstreamModelError); const classifiedError = - safelyResolveLangChainError(error) ?? safelyResolveLangChainError(upstreamModelError); + safelyResolveLangChainError(error) ?? + safelyResolveLangChainError(upstreamModelError) ?? + (status == null || status < 400 + ? (resolveModelStreamError(upstreamModelError) ?? resolveModelStreamError(error)) + : undefined); if (classifiedError != null) { return classifiedError; } - const { status } = getSafeErrorMetadata(upstreamModelError); /** Unclassified: the provider's own explanation is the only account of what happened, and a * rejection from a gateway or proxy carries it as the whole point of the 400. The status * headlines it either way, so a deployment withholding provider text loses no taxonomy. */ diff --git a/packages/api/src/agents/handlers.spec.ts b/packages/api/src/agents/handlers.spec.ts index c79e694fd67..afb806d0f82 100644 --- a/packages/api/src/agents/handlers.spec.ts +++ b/packages/api/src/agents/handlers.spec.ts @@ -5610,7 +5610,9 @@ describe('createToolExecuteHandler', () => { workspaceInstanceId: 'a'.repeat(64), operations: TEST_ATTACHED_WORKSPACE_OPERATIONS, }, - codeEnvironmentConfigSchema: { limits: { maxQueueWaitMs: 0 } }, + codeEnvironmentConfigSchema: { + limits: { maxQueueWaitMs: 0, maxRequestTimeoutMs: 125_000 }, + }, bridgeWorkerId: 'user-worker', }, }, @@ -5642,6 +5644,8 @@ describe('createToolExecuteHandler', () => { workspace_id: 'project-a', workspace_instance_id: 'a'.repeat(64), maxQueueWaitMs: 0, + maxRequestTimeoutMs: 125_000, + deadlineAtMs: expect.any(Number), codeApiBaseUrl: 'https://code.example.com', executionProfile: 'stateful', bridgeWorkerId: 'user-worker', @@ -5845,7 +5849,8 @@ describe('createToolExecuteHandler', () => { ])( 'shares a protected edit retry horizon ($budget ms, preview $elapsed ms)', async ({ budget, elapsed, remaining }) => { - let nowMs = Date.now(); + const startedAt = Date.now(); + let nowMs = startedAt; jest.spyOn(Date, 'now').mockImplementation(() => nowMs); const previewWorkspaceEdit = jest.fn(async () => { nowMs += elapsed; @@ -5899,7 +5904,9 @@ describe('createToolExecuteHandler', () => { executionProfile: 'stateful', statefulSessions: true, environmentType: 'attached', - codeEnvironmentConfigSchema: { limits: { maxQueueWaitMs: budget } }, + codeEnvironmentConfigSchema: { + limits: { maxQueueWaitMs: budget, maxRequestTimeoutMs: 125_000 }, + }, bridgeWorkerId: 'user-worker', }, }, @@ -5918,7 +5925,11 @@ describe('createToolExecuteHandler', () => { ]); expect(previewWorkspaceEdit).toHaveBeenCalledWith( - expect.objectContaining({ maxQueueWaitMs: budget }), + expect.objectContaining({ + maxQueueWaitMs: budget, + maxRequestTimeoutMs: 125_000, + deadlineAtMs: startedAt + 125_000, + }), ); if (remaining == null) { expect(result.status).toBe('error'); @@ -5931,6 +5942,8 @@ describe('createToolExecuteHandler', () => { expect.objectContaining({ expected_base_sha256: 'b'.repeat(64), maxQueueWaitMs: remaining, + maxRequestTimeoutMs: 125_000, + deadlineAtMs: startedAt + 125_000, }), ); }, @@ -6626,7 +6639,9 @@ describe('createToolExecuteHandler', () => { workspaceInstanceId: 'b'.repeat(64), operations: TEST_ATTACHED_WORKSPACE_OPERATIONS, }, - codeEnvironmentConfigSchema: { limits: { maxQueueWaitMs: 0 } }, + codeEnvironmentConfigSchema: { + limits: { maxQueueWaitMs: 0, maxRequestTimeoutMs: 125_000 }, + }, bridgeWorkerId: 'personal-worker-1', statefulSessions: true, }, @@ -6647,6 +6662,7 @@ describe('createToolExecuteHandler', () => { workspace_id: 'project-a', workspace_instance_id: 'b'.repeat(64), maxQueueWaitMs: 0, + maxRequestTimeoutMs: 125_000, start_line: 1, max_lines: 200, codeApiBaseUrl: 'https://code.example.com/v1', @@ -6930,6 +6946,7 @@ describe('createToolExecuteHandler', () => { executionProfile: 'stateful', environmentType: 'attached', bridgeWorkerId: 'personal-worker-1', + codeEnvironmentConfigSchema: { limits: { maxRequestTimeoutMs: 125_000 } }, statefulSessions: true, }, searchWorkspace, @@ -6954,6 +6971,7 @@ describe('createToolExecuteHandler', () => { query: 'needle', workspace_id: 'project-a', maxQueueWaitMs: 300000, + maxRequestTimeoutMs: 125_000, path: 'src', max_results: 20, codeApiBaseUrl: 'https://code.example.com/v1', @@ -7084,6 +7102,7 @@ describe('createToolExecuteHandler', () => { executionProfile: 'stateful', environmentType: 'attached', bridgeWorkerId: 'personal-worker-1', + codeEnvironmentConfigSchema: { limits: { maxRequestTimeoutMs: 125_000 } }, statefulSessions: true, }, listWorkspaceFiles, @@ -7107,6 +7126,7 @@ describe('createToolExecuteHandler', () => { expect(listWorkspaceFiles).toHaveBeenCalledWith({ workspace_id: 'project-a', maxQueueWaitMs: 300000, + maxRequestTimeoutMs: 125_000, path: 'src', after_path: 'src/app.ts', max_results: 20, diff --git a/packages/api/src/agents/handlers.ts b/packages/api/src/agents/handlers.ts index 4438b0d0c4e..7eb99e4e4dc 100644 --- a/packages/api/src/agents/handlers.ts +++ b/packages/api/src/agents/handlers.ts @@ -119,6 +119,10 @@ import { WORKSPACE_EDIT_MAX_COUNT, WORKSPACE_WRITE_MAX_BYTES, } from '~/code/workspace'; +import { + resolveAttachedWorkspaceQueueWaitMs, + resolveAttachedWorkspaceRequestTimeoutMs, +} from '~/code/command'; import { hasIntentArg, stripIntentArg, @@ -127,7 +131,6 @@ import { } from './intent'; import { buildSkillPrimeMessage, isSkillFilePath, SKILL_FILE_PREFIX } from './skills'; import { resolveCallerCapabilityProjectionSnapshot } from './callerCapabilities'; -import { resolveAttachedWorkspaceQueueWaitMs } from '~/code/command'; import { BACKGROUND_TOOL_INVOCATION_CONFIG_KEY } from './invocation'; import { mergeCodeFilesIntoContext } from './codeFilesSession'; import { toolValidationFeedback } from './validationFeedback'; @@ -568,6 +571,8 @@ export interface ToolExecuteOptions { req?: ServerRequest; signal?: AbortSignal; maxQueueWaitMs?: number; + maxRequestTimeoutMs?: number; + deadlineAtMs?: number; }) => Promise; /** Searches literal text within an attached worker's logical workspace. */ searchWorkspace?: (params: { @@ -582,6 +587,8 @@ export interface ToolExecuteOptions { req?: ServerRequest; signal?: AbortSignal; maxQueueWaitMs?: number; + maxRequestTimeoutMs?: number; + deadlineAtMs?: number; }) => Promise; /** Lists relative file paths within an attached worker's logical workspace. */ listWorkspaceFiles?: (params: { @@ -596,6 +603,8 @@ export interface ToolExecuteOptions { req?: ServerRequest; signal?: AbortSignal; maxQueueWaitMs?: number; + maxRequestTimeoutMs?: number; + deadlineAtMs?: number; }) => Promise; /** Writes a UTF-8 file within an attached worker's logical workspace. */ writeWorkspaceFile?: (params: { @@ -610,6 +619,8 @@ export interface ToolExecuteOptions { req?: ServerRequest; signal?: AbortSignal; maxQueueWaitMs?: number; + maxRequestTimeoutMs?: number; + deadlineAtMs?: number; }) => Promise; /** Previews exact replacements without mutating an attached worker workspace. */ previewWorkspaceEdit?: (params: { @@ -623,6 +634,8 @@ export interface ToolExecuteOptions { req?: ServerRequest; signal?: AbortSignal; maxQueueWaitMs?: number; + maxRequestTimeoutMs?: number; + deadlineAtMs?: number; }) => Promise; /** Applies exact replacements atomically within an attached worker workspace. */ editWorkspaceFile?: (params: { @@ -637,6 +650,8 @@ export interface ToolExecuteOptions { req?: ServerRequest; signal?: AbortSignal; maxQueueWaitMs?: number; + maxRequestTimeoutMs?: number; + deadlineAtMs?: number; }) => Promise; /** * Reads a code-execution sandbox file by shelling `cat` through the @@ -2462,9 +2477,7 @@ async function handleWorkspaceFileRead( start_line: startLine, max_lines: maxLines, codeApiBaseUrl: codeExecutionContext.baseUrl, - maxQueueWaitMs: resolveAttachedWorkspaceQueueWaitMs( - codeExecutionContext.codeEnvironmentConfigSchema, - ), + ...attachedWorkspaceRequestLimits(codeExecutionContext), executionProfile: codeExecutionContext.executionProfile, ...(codeExecutionContext.bridgeWorkerId ? { bridgeWorkerId: codeExecutionContext.bridgeWorkerId } @@ -2570,9 +2583,7 @@ async function handleWorkspaceSearchCall( ...(typeof args.path === 'string' && args.path.length > 0 ? { path: args.path } : {}), max_results: Number(maxResults), codeApiBaseUrl: codeExecutionContext.baseUrl, - maxQueueWaitMs: resolveAttachedWorkspaceQueueWaitMs( - codeExecutionContext.codeEnvironmentConfigSchema, - ), + ...attachedWorkspaceRequestLimits(codeExecutionContext), executionProfile: codeExecutionContext.executionProfile, ...(codeExecutionContext.bridgeWorkerId ? { bridgeWorkerId: codeExecutionContext.bridgeWorkerId } @@ -2663,9 +2674,7 @@ async function handleWorkspaceListCall( : {}), max_results: Number(maxResults), codeApiBaseUrl: codeExecutionContext.baseUrl, - maxQueueWaitMs: resolveAttachedWorkspaceQueueWaitMs( - codeExecutionContext.codeEnvironmentConfigSchema, - ), + ...attachedWorkspaceRequestLimits(codeExecutionContext), executionProfile: codeExecutionContext.executionProfile, ...(codeExecutionContext.bridgeWorkerId ? { bridgeWorkerId: codeExecutionContext.bridgeWorkerId } @@ -3910,6 +3919,18 @@ function attachedWorkspaceAuthoringPath( return pathError ? errorResult(tc, pathError) : { filePath: relativePath }; } +function attachedWorkspaceRequestLimits(codeExecutionContext: CodeExecutionContext): { + maxQueueWaitMs: number; + maxRequestTimeoutMs?: number; +} { + const config = codeExecutionContext.codeEnvironmentConfigSchema; + const maxRequestTimeoutMs = resolveAttachedWorkspaceRequestTimeoutMs(config); + return { + maxQueueWaitMs: resolveAttachedWorkspaceQueueWaitMs(config), + ...(maxRequestTimeoutMs == null ? {} : { maxRequestTimeoutMs }), + }; +} + function attachedWorkspaceMutationParams( codeExecutionContext: CodeExecutionContext, workspaceId: string, @@ -3924,16 +3945,20 @@ function attachedWorkspaceMutationParams( req?: ServerRequest; signal?: AbortSignal; maxQueueWaitMs: number; + maxRequestTimeoutMs?: number; + deadlineAtMs?: number; } { + const limits = attachedWorkspaceRequestLimits(codeExecutionContext); return { workspace_id: workspaceId, ...(codeExecutionContext.codeWorkspace?.workspaceInstanceId ? { workspace_instance_id: codeExecutionContext.codeWorkspace.workspaceInstanceId } : {}), codeApiBaseUrl: codeExecutionContext.baseUrl, - maxQueueWaitMs: resolveAttachedWorkspaceQueueWaitMs( - codeExecutionContext.codeEnvironmentConfigSchema, - ), + ...limits, + ...(limits.maxRequestTimeoutMs == null + ? {} + : { deadlineAtMs: Date.now() + limits.maxRequestTimeoutMs }), executionProfile: codeExecutionContext.executionProfile, ...(codeExecutionContext.bridgeWorkerId ? { bridgeWorkerId: codeExecutionContext.bridgeWorkerId } diff --git a/packages/api/src/code/command.spec.ts b/packages/api/src/code/command.spec.ts index 0cd2dd12f8f..4ba032bbde9 100644 --- a/packages/api/src/code/command.spec.ts +++ b/packages/api/src/code/command.spec.ts @@ -11,6 +11,7 @@ import { resolveAttachedWorkspaceCommandTimeoutMax, resolveAttachedWorkspaceProgrammaticTimeout, resolveAttachedWorkspaceQueueWaitMs, + resolveAttachedWorkspaceRequestTimeoutMs, } from './command'; import { BACKGROUND_TOOL_INVOCATION_CONFIG_KEY } from '~/agents/invocation'; @@ -376,7 +377,7 @@ describe('createAttachedWorkspaceBashTool', () => { expect(foregroundRequest).toMatchObject({ timeoutMs: 30_000 }); }); - test('resolves the administrator-configured admission budget', () => { + test('resolves the administrator-configured retry horizon', () => { expect(resolveAttachedWorkspaceQueueWaitMs()).toBe(5 * 60_000); expect(resolveAttachedWorkspaceQueueWaitMs({ limits: { maxQueueWaitMs: 30_000 } })).toBe( 30_000, @@ -387,6 +388,37 @@ describe('createAttachedWorkspaceBashTool', () => { ); }); + test('opts Bash into a verified HTTP limit without changing execution or disabling the initial attempt', async () => { + jest.spyOn(Date, 'now').mockReturnValue(1_000); + expect(resolveAttachedWorkspaceRequestTimeoutMs()).toBeUndefined(); + expect( + resolveAttachedWorkspaceRequestTimeoutMs({ limits: { maxRequestTimeoutMs: 125_000 } }), + ).toBe(125_000); + expect( + resolveAttachedWorkspaceRequestTimeoutMs({ limits: { maxRequestTimeoutMs: 610_001 } }), + ).toBeUndefined(); + const fetchImpl: CodeBridgeFetch = jest.fn(async () => commandResponse()); + const bashTool = createAttachedWorkspaceBashTool({ + baseUrl: 'https://code.example.com/v1', + authHeaders: () => ({}), + workspaceId: 'project-a', + maxTimeoutMs: 90_000, + maxQueueWaitMs: 0, + maxRequestTimeoutMs: 125_000, + fetchImpl, + }); + + await bashTool.func({ command: 'sleep 90', timeoutMs: 90_000 }, undefined, {}); + await bashTool.func({ command: 'pwd' }, undefined, {}); + + const [, long] = (fetchImpl as jest.Mock).mock.calls[0]; + const [, short] = (fetchImpl as jest.Mock).mock.calls[1]; + expect(JSON.parse(long.body).timeoutMs).toBe(90_000); + expect(JSON.parse(short.body).timeoutMs).toBe(30_000); + expect(long.headers['X-LibreChat-Workspace-Queue-Wait-Ms']).toBe('25000'); + expect(short.headers['X-LibreChat-Workspace-Queue-Wait-Ms']).toBe('85000'); + }); + test('lowers the omitted timeout when the deployment ceiling is below 30 seconds', async () => { const fetchImpl: CodeBridgeFetch = jest.fn(async () => commandResponse()); const bashTool = createAttachedWorkspaceBashTool({ diff --git a/packages/api/src/code/command.ts b/packages/api/src/code/command.ts index e3ea3b3fd2d..2ec67f9885b 100644 --- a/packages/api/src/code/command.ts +++ b/packages/api/src/code/command.ts @@ -1,5 +1,6 @@ import { logger } from '@librechat/data-schemas'; import { tool } from '@librechat/agents/langchain/tools'; +import { CODE_ENVIRONMENT_REQUEST_TIMEOUT_HARD_MAX_MS } from 'librechat-data-provider'; import { BashExecutionToolDefinition, BashToolOutputReferencesGuide, @@ -140,6 +141,21 @@ export function resolveAttachedWorkspaceQueueWaitMs( return Math.min(WORKSPACE_QUEUE_MAX_WAIT_MS, configured); } +export function resolveAttachedWorkspaceRequestTimeoutMs( + configSchema?: CodeEnvironmentUserConfigSchema, +): number | undefined { + const configured = configSchema?.limits?.maxRequestTimeoutMs; + if ( + configured == null || + !Number.isSafeInteger(configured) || + configured < 1 || + configured > CODE_ENVIRONMENT_REQUEST_TIMEOUT_HARD_MAX_MS + ) { + return undefined; + } + return configured; +} + export function buildAttachedWorkspaceBashSchema( maxTimeoutMs: number = WORKSPACE_COMMAND_DEFAULT_TIMEOUT_MS, environment?: CodeWorkspaceDescriptor['environment'], @@ -286,6 +302,7 @@ export function createAttachedWorkspaceBashTool({ gitIdentity, maxTimeoutMs = WORKSPACE_COMMAND_DEFAULT_TIMEOUT_MS, maxQueueWaitMs, + maxRequestTimeoutMs, fetchImpl, }: { baseUrl: string; @@ -296,8 +313,10 @@ export function createAttachedWorkspaceBashTool({ gitIdentity?: AgentGitIdentity | null; /** Effective admin/upstream ceiling already intersected with the protocol hard cap. */ maxTimeoutMs?: number; - /** Deployment admission budget; omitted keeps the built-in default. */ + /** Retry horizon across typed queue expirations, not an admission budget. */ maxQueueWaitMs?: number; + /** Verified total HTTP budget; omission keeps the legacy per-attempt timeout. */ + maxRequestTimeoutMs?: number; fetchImpl?: CodeBridgeFetch; }): DynamicStructuredTool { const effectiveMaxTimeoutMs = normalizeAttachedWorkspaceCommandTimeoutMax(maxTimeoutMs); @@ -376,6 +395,7 @@ export function createAttachedWorkspaceBashTool({ signal, fetchImpl, ...(maxQueueWaitMs == null ? {} : { maxQueueWaitMs }), + ...(maxRequestTimeoutMs == null ? {} : { maxRequestTimeoutMs }), }); if (result.operation !== 'execute_command') { throw new Error('Attached workspace returned an unexpected command result.'); diff --git a/packages/api/src/code/workspace.spec.ts b/packages/api/src/code/workspace.spec.ts index 84b02735a73..149f5bd4daa 100644 --- a/packages/api/src/code/workspace.spec.ts +++ b/packages/api/src/code/workspace.spec.ts @@ -242,6 +242,37 @@ describe('workspace admission feedback', () => { }, ); + test('cancels a stalled credential refresh without opening a workspace admission window', async () => { + const controller = new AbortController(); + const reason = new DOMException('Chat stopped', 'AbortError'); + const fetchImpl = jest.fn(); + let entered!: () => void; + const started = new Promise((resolve) => { + entered = resolve; + }); + const operation = executeWorkspaceTool({ + baseURL: 'https://code.example.com/v1', + authHeaders: async () => { + entered(); + return new Promise>(() => {}); + }, + request: { + protocolVersion: 1, + operation: 'edit_file', + workspaceId: 'primary', + path: 'src/app.ts', + edits: [{ oldText: 'a', newText: 'b' }], + }, + signal: controller.signal, + maxRequestTimeoutMs: 125_000, + fetchImpl, + }); + await started; + controller.abort(reason); + await expect(operation).rejects.toBe(reason); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + test('does not dispatch a retry if credential refresh spends the remaining queue budget', async () => { const now = jest.spyOn(Date, 'now').mockReturnValue(1000); const authHeaders = jest @@ -438,6 +469,287 @@ describe('executeWorkspaceTool', () => { ).rejects.toMatchObject({ upstreamStatus: 504 }); expect(timeout).toHaveBeenCalledWith(budget); expect(fetchImpl.mock.calls[0][1].body).toBe(JSON.stringify(request)); + const admissionMs = Number( + fetchImpl.mock.calls[0][1].headers['X-LibreChat-Workspace-Queue-Wait-Ms'], + ); + expect(admissionMs).toBeGreaterThan(0); + expect(admissionMs).toBeLessThanOrEqual(30_000); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + test.each<[WorkspaceToolRequest, number, number]>([ + [ + { protocolVersion: 1, operation: 'read_file', workspaceId: 'primary', path: 'README.md' }, + 125_000, + 90_000, + ], + [ + { protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'pwd' }, + 125_000, + 85_000, + ], + [ + { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'primary', + command: 'pwd', + timeoutMs: 90_000, + }, + 125_000, + 25_000, + ], + [ + { protocolVersion: 1, operation: 'read_file', workspaceId: 'primary', path: 'README.md' }, + 60_000, + 25_000, + ], + [ + { protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'pwd' }, + 60_000, + 20_000, + ], + [ + { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'primary', + command: 'pwd', + timeoutMs: 300_000, + }, + 610_000, + 300_000, + ], + ])( + 'advertises only the remaining queue time for %j under a %i ms limit', + async (request, maxRequestTimeoutMs, allowanceMs) => { + jest.spyOn(Date, 'now').mockReturnValue(1_000); + const timeout = jest.spyOn(AbortSignal, 'timeout'); + const fetchImpl = jest + .fn() + .mockResolvedValue( + new Response(JSON.stringify({ code: 'ASSIGNMENT_EXPIRED' }), { status: 504 }), + ); + await expect( + executeWorkspaceTool({ + baseURL: 'https://code.example.com/v1', + authHeaders: {}, + request, + maxRequestTimeoutMs, + fetchImpl, + }), + ).rejects.toMatchObject({ upstreamStatus: 504 }); + expect(timeout).toHaveBeenCalledWith(maxRequestTimeoutMs); + expect(fetchImpl.mock.calls[0][1].headers['X-LibreChat-Workspace-Queue-Wait-Ms']).toBe( + String(allowanceMs), + ); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }, + ); + + test('accepts a result after 31 simulated seconds of admission with an explicit longer budget', async () => { + const now = jest.spyOn(Date, 'now').mockReturnValue(1_000); + const fetchImpl: CodeBridgeFetch = jest.fn(async (_url, init) => { + expect((init?.headers as Record)['X-LibreChat-Workspace-Queue-Wait-Ms']).toBe( + '90000', + ); + now.mockReturnValue(32_000); + expect(init?.signal?.aborted).toBe(false); + return Response.json({ + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'primary', + path: 'README.md', + content: 'ready', + startLine: 1, + endLine: 1, + truncated: false, + }); + }); + await expect( + executeWorkspaceTool({ + baseURL: 'https://code.example.com/v1', + authHeaders: {}, + request: { + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'primary', + path: 'README.md', + }, + maxRequestTimeoutMs: 125_000, + fetchImpl, + }), + ).resolves.toMatchObject({ content: 'ready' }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + test('uses the earlier caller deadline rather than assuming the abort signal reveals it', async () => { + jest.spyOn(Date, 'now').mockReturnValue(1_000); + const timeout = jest.spyOn(AbortSignal, 'timeout'); + const fetchImpl = jest.fn().mockResolvedValue(new Response('{}', { status: 504 })); + await expect( + executeWorkspaceTool({ + baseURL: 'https://code.example.com/v1', + authHeaders: {}, + request: { + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'primary', + path: 'README.md', + }, + maxRequestTimeoutMs: 125_000, + deadlineAtMs: 61_000, + fetchImpl, + }), + ).rejects.toMatchObject({ upstreamStatus: 504 }); + expect(timeout).toHaveBeenCalledWith(60_000); + expect(fetchImpl.mock.calls[0][1].headers['X-LibreChat-Workspace-Queue-Wait-Ms']).toBe('25000'); + }); + + test('subtracts credential acquisition from the allowance before dispatching', async () => { + const now = jest.spyOn(Date, 'now').mockReturnValue(1_000); + const fetchImpl = jest.fn().mockResolvedValue(new Response('{}', { status: 504 })); + await expect( + executeWorkspaceTool({ + baseURL: 'https://code.example.com/v1', + authHeaders: async () => { + now.mockReturnValue(20_000); + return {}; + }, + request: { + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'primary', + path: 'README.md', + }, + maxRequestTimeoutMs: 125_000, + fetchImpl, + }), + ).rejects.toMatchObject({ upstreamStatus: 504 }); + expect(fetchImpl.mock.calls[0][1].headers['X-LibreChat-Workspace-Queue-Wait-Ms']).toBe('71000'); + }); + + test.each<[WorkspaceToolRequest, number]>([ + [ + { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'primary', + command: 'pwd', + timeoutMs: 300_000, + }, + 125_000, + ], + [ + { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'primary', + command: 'pwd', + timeoutMs: 90_000, + }, + 60_000, + ], + ])( + 'does not dispatch when %j cannot fit within the %i ms ingress limit', + async (request, maxRequestTimeoutMs) => { + const fetchImpl = jest.fn(); + await expect( + executeWorkspaceTool({ + baseURL: 'https://code.example.com/v1', + authHeaders: {}, + request, + maxRequestTimeoutMs, + fetchImpl, + }), + ).rejects.toMatchObject({ + reason: 'insufficient_time', + message: expect.stringContaining('not started'), + }); + expect(fetchImpl).not.toHaveBeenCalled(); + }, + ); + + test('keeps the retry horizon separate from one longer admission attempt', async () => { + jest.spyOn(Date, 'now').mockReturnValue(1_000); + const fetchImpl = jest + .fn() + .mockResolvedValue( + new Response(JSON.stringify({ code: 'WORKSPACE_QUEUE_TIMEOUT' }), { status: 503 }), + ); + await expect( + executeWorkspaceTool({ + baseURL: 'https://code.example.com/v1', + authHeaders: {}, + request: { + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'primary', + path: 'README.md', + }, + maxRequestTimeoutMs: 125_000, + maxQueueWaitMs: 0, + fetchImpl, + }), + ).rejects.toThrow('The operation was not started'); + expect(fetchImpl.mock.calls[0][1].headers['X-LibreChat-Workspace-Queue-Wait-Ms']).toBe('90000'); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + test('keeps Retry-After capped at 30 seconds even when one admission may wait longer', async () => { + jest.useFakeTimers(); + try { + const fetchImpl = jest + .fn() + .mockResolvedValueOnce( + new Response(JSON.stringify({ code: 'WORKSPACE_QUEUE_TIMEOUT' }), { + status: 503, + headers: { 'Retry-After': '300' }, + }), + ) + .mockResolvedValueOnce(new Response('{}', { status: 504 })); + const result = executeWorkspaceTool({ + baseURL: 'https://code.example.com/v1', + authHeaders: {}, + request: { + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'primary', + path: 'README.md', + }, + maxRequestTimeoutMs: 125_000, + fetchImpl, + }).catch((error: WorkspaceToolHttpError) => error); + await jest.advanceTimersByTimeAsync(29_999); + expect(fetchImpl).toHaveBeenCalledTimes(1); + await jest.advanceTimersByTimeAsync(1); + await expect(result).resolves.toMatchObject({ upstreamStatus: 504 }); + expect(fetchImpl.mock.calls[1][1].headers['X-LibreChat-Workspace-Queue-Wait-Ms']).toBe( + '60000', + ); + } finally { + jest.useRealTimers(); + } + }); + + test('never retries a post-dispatch timeout with an unknown mutation outcome', async () => { + const fetchImpl = jest + .fn() + .mockRejectedValue(new DOMException('response deadline', 'TimeoutError')); + await expect( + executeWorkspaceTool({ + baseURL: 'https://code.example.com/v1', + authHeaders: {}, + request: { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'primary', + command: 'touch marker', + }, + maxRequestTimeoutMs: 125_000, + fetchImpl, + }), + ).rejects.toMatchObject({ reason: 'timeout' }); expect(fetchImpl).toHaveBeenCalledTimes(1); }); diff --git a/packages/api/src/code/workspace.ts b/packages/api/src/code/workspace.ts index 3de4d8ab5ff..c39d4002f90 100644 --- a/packages/api/src/code/workspace.ts +++ b/packages/api/src/code/workspace.ts @@ -1,4 +1,8 @@ -import { CODE_ENVIRONMENT_QUEUE_WAIT_DEFAULT_MS } from 'librechat-data-provider'; +import { + CODE_ENVIRONMENT_ADMISSION_MAX_MS, + CODE_ENVIRONMENT_QUEUE_WAIT_DEFAULT_MS, + CODE_ENVIRONMENT_REQUEST_TIMEOUT_HARD_MAX_MS, +} from 'librechat-data-provider'; import type { CodeBridgeFetch } from './bridge'; const WORKSPACE_TOOL_TIMEOUT_MS = 30_000; @@ -19,8 +23,9 @@ const DEFAULT_COMMAND_OUTPUT_BYTES = 256 * 1024; const MAX_COMMAND_OUTPUT_BYTES = 1024 * 1024; const MAX_COMMAND_SIGNAL_LENGTH = 32; const WORKSPACE_COMMAND_TRANSPORT_GRACE_MS = 5_000; -/** Matches Code API's bounded admission wait and command settlement allowance. */ +/** Legacy admission window and Retry-After cap, independent of the retry horizon. */ const WORKSPACE_QUEUE_TIMEOUT_MS = 30_000; +const WORKSPACE_QUEUE_WAIT_HEADER = 'X-LibreChat-Workspace-Queue-Wait-Ms'; /** Compatibility export; the deployment schema owns the default and hard cap. */ export const WORKSPACE_QUEUE_MAX_WAIT_MS: number = CODE_ENVIRONMENT_QUEUE_WAIT_DEFAULT_MS; const WORKSPACE_QUEUE_RETRY_DELAY_MS = 1_000; @@ -263,16 +268,25 @@ export type WorkspaceToolResult = export class WorkspaceToolHttpError extends Error { constructor( - public readonly reason: 'rejected' | 'invalid' | 'timeout' | 'failed', + public readonly reason: 'rejected' | 'invalid' | 'timeout' | 'failed' | 'insufficient_time', public readonly upstreamStatus?: number, public readonly upstreamBody?: string, public readonly upstreamBodyTruncated = false, ) { - super( - (reason === 'rejected' && + let message = `Workspace tool request ${reason}`; + if ( + reason === 'rejected' && isWorkspaceAdmissionTimeout(upstreamStatus, upstreamBody, upstreamBodyTruncated) - ? 'Workspace capacity was unavailable before the queue deadline. The operation was not started. Wait for active work to finish or select an independent workspace on a machine with available capacity.' - : `Workspace tool request ${reason}`) + + ) { + message = + 'Workspace capacity was unavailable before the queue deadline. The operation was not started. Wait for active work to finish or select an independent workspace on a machine with available capacity.'; + } + if (reason === 'insufficient_time') { + message = + 'Workspace execution cannot fit within the remaining HTTP budget. The operation was not started.'; + } + super( + message + (upstreamStatus == null ? '' : ` (upstreamStatus: ${upstreamStatus})`) + (upstreamBody ? `; upstreamBody: ${JSON.stringify(upstreamBody)}` : '') + (upstreamBodyTruncated ? ' [body truncated or incomplete]' : ''), @@ -731,13 +745,19 @@ function isValidResult( ); } -function getWorkspaceToolTimeoutMs(request: WorkspaceToolRequest): number { - const executionBudgetMs = - request.operation === 'execute_command' - ? (request.timeoutMs ?? WORKSPACE_COMMAND_DEFAULT_TIMEOUT_MS) + +function getWorkspaceExecutionBudgetMs(request: WorkspaceToolRequest): number { + return request.operation === 'execute_command' + ? (request.timeoutMs ?? WORKSPACE_COMMAND_DEFAULT_TIMEOUT_MS) + WORKSPACE_COMMAND_SETTLEMENT_GRACE_MS - : WORKSPACE_TOOL_TIMEOUT_MS; - return WORKSPACE_QUEUE_TIMEOUT_MS + executionBudgetMs + WORKSPACE_COMMAND_TRANSPORT_GRACE_MS; + : WORKSPACE_TOOL_TIMEOUT_MS; +} + +function getWorkspaceToolTimeoutMs(request: WorkspaceToolRequest): number { + return ( + WORKSPACE_QUEUE_TIMEOUT_MS + + getWorkspaceExecutionBudgetMs(request) + + WORKSPACE_COMMAND_TRANSPORT_GRACE_MS + ); } /** @@ -749,6 +769,31 @@ export type WorkspaceToolAuthHeaders = | Record | (() => Promise> | Record); +function getWorkspaceAuthHeaders( + supplier: () => Promise> | Record, + signal: AbortSignal, +): Promise> { + signal.throwIfAborted(); + return new Promise((resolve, reject) => { + const abort = () => reject(signal.reason); + signal.addEventListener('abort', abort, { once: true }); + if (signal.aborted) { + abort(); + return; + } + try { + void Promise.resolve(supplier()) + .then(resolve, reject) + .finally(() => { + signal.removeEventListener('abort', abort); + }); + } catch (error) { + signal.removeEventListener('abort', abort); + reject(error); + } + }); +} + export async function executeWorkspaceTool({ baseURL, authHeaders, @@ -756,6 +801,8 @@ export async function executeWorkspaceTool({ signal, fetchImpl = fetch, maxQueueWaitMs = WORKSPACE_QUEUE_MAX_WAIT_MS, + maxRequestTimeoutMs, + deadlineAtMs, }: { baseURL: string; authHeaders: WorkspaceToolAuthHeaders; @@ -763,34 +810,81 @@ export async function executeWorkspaceTool({ signal?: AbortSignal; fetchImpl?: CodeBridgeFetch; maxQueueWaitMs?: number; + /** End-to-end HTTP budget for this call. Omission keeps legacy per-attempt timeouts. */ + maxRequestTimeoutMs?: number; + /** Optional earlier caller deadline; a signal alone has no remaining-time value. */ + deadlineAtMs?: number; }): Promise { if ( !isValidRequest(request) || !Number.isSafeInteger(maxQueueWaitMs) || maxQueueWaitMs < 0 || - maxQueueWaitMs > WORKSPACE_QUEUE_MAX_WAIT_MS + maxQueueWaitMs > WORKSPACE_QUEUE_MAX_WAIT_MS || + (maxRequestTimeoutMs !== undefined && + (!Number.isSafeInteger(maxRequestTimeoutMs) || + maxRequestTimeoutMs < 1 || + maxRequestTimeoutMs > CODE_ENVIRONMENT_REQUEST_TIMEOUT_HARD_MAX_MS)) || + (deadlineAtMs !== undefined && (!Number.isSafeInteger(deadlineAtMs) || deadlineAtMs < 1)) ) { throw new WorkspaceToolHttpError('invalid'); } + const executionBudgetMs = getWorkspaceExecutionBudgetMs(request); + const completionReserveMs = executionBudgetMs + WORKSPACE_COMMAND_TRANSPORT_GRACE_MS; + const perAttemptTimeoutMs = maxRequestTimeoutMs ?? getWorkspaceToolTimeoutMs(request); + const callerDeadlineAt = Math.min( + deadlineAtMs ?? Infinity, + maxRequestTimeoutMs == null ? Infinity : Date.now() + maxRequestTimeoutMs, + ); const queueDeadlineAt = Date.now() + maxQueueWaitMs; + const retryDeadlineAt = Math.min(queueDeadlineAt, callerDeadlineAt - completionReserveMs); const body = JSON.stringify(request); let lastAdmissionRejection: WorkspaceToolHttpError | undefined; while (true) { try { signal?.throwIfAborted(); - /** The endpoint replies only after execution settles. Capping this by the - * retry horizon would also abort already-admitted commands, with an unknown - * mutation outcome. Server admission and execution retain separate budgets. */ - const timeoutSignal = AbortSignal.timeout(getWorkspaceToolTimeoutMs(request)); + /** Admission and retries cannot consume the reserved execution or delivery time. */ + const attemptTimeoutMs = Math.floor( + Math.min(perAttemptTimeoutMs, callerDeadlineAt - Date.now()), + ); + if (attemptTimeoutMs <= completionReserveMs) { + throw lastAdmissionRejection ?? new WorkspaceToolHttpError('insufficient_time'); + } + const attemptStartedAt = Date.now(); + const timeoutSignal = AbortSignal.timeout(attemptTimeoutMs); const requestSignal = signal != null && typeof AbortSignal.any === 'function' ? AbortSignal.any([signal, timeoutSignal]) : timeoutSignal; - const attemptHeaders = typeof authHeaders === 'function' ? await authHeaders() : authHeaders; - requestSignal.throwIfAborted(); - if (lastAdmissionRejection && Date.now() >= queueDeadlineAt) { + let attemptHeaders: Record; + try { + attemptHeaders = + typeof authHeaders === 'function' + ? await getWorkspaceAuthHeaders(authHeaders, requestSignal) + : authHeaders; + } catch (error) { + if (timeoutSignal.aborted && signal?.aborted !== true) { + throw lastAdmissionRejection ?? new WorkspaceToolHttpError('insufficient_time'); + } + throw error; + } + signal?.throwIfAborted(); + if (timeoutSignal.aborted) { + throw lastAdmissionRejection ?? new WorkspaceToolHttpError('insufficient_time'); + } + if (lastAdmissionRejection && Date.now() >= retryDeadlineAt) { throw lastAdmissionRejection; } + const remainingMs = Math.min( + attemptTimeoutMs - (Date.now() - attemptStartedAt), + callerDeadlineAt - Date.now(), + ); + const queueAllowanceMs = Math.min( + CODE_ENVIRONMENT_ADMISSION_MAX_MS, + Math.floor(remainingMs - completionReserveMs), + ); + if (queueAllowanceMs < 1) { + throw lastAdmissionRejection ?? new WorkspaceToolHttpError('insufficient_time'); + } const response = await fetchImpl( `${baseURL.trim().replace(/\/+$/, '')}/workspace-tools/execute`, { @@ -798,6 +892,7 @@ export async function executeWorkspaceTool({ headers: { ...attemptHeaders, 'Content-Type': 'application/json', + [WORKSPACE_QUEUE_WAIT_HEADER]: String(queueAllowanceMs), }, body, redirect: 'error', @@ -816,13 +911,14 @@ export async function executeWorkspaceTool({ } const delayMs = Math.min( workspaceAdmissionRetryDelay(response.headers.get('Retry-After')), - queueDeadlineAt - Date.now(), + retryDeadlineAt - Date.now(), ); lastAdmissionRejection = rejection; + if (delayMs <= 0) throw rejection; await waitForWorkspaceAdmission(delayMs, signal); - /** A clamped delay can land exactly on the deadline: the budget is spent, - * so never open another admission window that could still be admitted. */ - if (Date.now() >= queueDeadlineAt) { + /** A clamped delay can land exactly on either deadline. Never open + * another admission window without the full execution reserve. */ + if (Date.now() >= retryDeadlineAt) { throw rejection; } continue; @@ -840,7 +936,7 @@ export async function executeWorkspaceTool({ ) { throw error; } - if (error instanceof Error && error.name === 'TimeoutError') { + if (isRecord(error) && error.name === 'TimeoutError') { throw new WorkspaceToolHttpError('timeout'); } throw new WorkspaceToolHttpError('failed'); diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index 913a0000295..3f6006fa393 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -682,10 +682,9 @@ export class MCPConnectionFactory { ); } if (!this.upstreamTokenProvider) { - throw new Error( - `${this.logPrefix} Internal: upstreamTokenProvider not plumbed for OBO connection. ` + - 'OBO requires a live upstream-token closure; the caller must construct one via ' + - 'createOpenIDSessionTokenProvider() and forward it through the MCP connection options.', + throw new OboTokenResolutionError( + 'missing_upstream_provider', + 'No upstream credential provider is configured for this OBO MCP connection.', ); } @@ -733,6 +732,8 @@ export class MCPConnectionFactory { recoveryHint = 'Please retry.'; } else if (error.reason === 'exchange_failed') { recoveryHint = 'Re-authenticate the user or verify the configured OBO scopes and retry.'; + } else if (error.reason === 'missing_upstream_provider') { + recoveryHint = 'Configure a renewable upstream credential provider before retrying.'; } return new OboTokenResolutionError( diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index 016af28aa4e..54c17f73e07 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -62,6 +62,8 @@ function createOboToolCallErrorMessage( failureSuffix = 'Re-authenticate the user or verify the configured OBO scopes and retry.'; } else if (error.reason === 'session_refresh_failed') { failureSuffix = 'Please sign in again.'; + } else if (error.reason === 'missing_upstream_provider') { + failureSuffix = 'Configure a renewable upstream credential provider before retrying.'; } return `${logPrefix} ${error.userMessage} Cannot execute tool ${toolName}. ${failureSuffix}`; @@ -1323,11 +1325,16 @@ Please follow these instructions when using tools from the respective MCP server ); } if (!oboUpstreamTokenProvider) { - throw new McpError( - ErrorCode.InternalError, - `${logPrefix} Internal: upstreamTokenProvider not plumbed for OBO tool call. ` + - 'OBO requires a live upstream-token closure; the caller must construct one via ' + - 'createOpenIDSessionTokenProvider() and forward it through callTool().', + const missing = new OboTokenResolutionError( + 'missing_upstream_provider', + 'No upstream credential provider is configured for this OBO MCP tool.', + ); + throw Object.assign( + new McpError( + ErrorCode.InternalError, + createOboToolCallErrorMessage(logPrefix, toolName, missing), + ), + { cause: missing }, ); } const oboTrusted = oboTrustChecker @@ -1361,10 +1368,14 @@ Please follow these instructions when using tools from the respective MCP server ); } catch (error) { if (error instanceof OboTokenResolutionError) { - throw new McpError( + const failure = new McpError( ErrorCode.InternalError, createOboToolCallErrorMessage(logPrefix, toolName, error), ); + if (error.reason === 'missing_upstream_provider') { + throw Object.assign(failure, { cause: error }); + } + throw failure; } throw error; } diff --git a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts index af050c348e7..2e70d1a82b2 100644 --- a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts +++ b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts @@ -6426,11 +6426,22 @@ describe('MCPConnectionFactory', () => { expect(upstreamTokenProviderResolver).not.toHaveBeenCalled(); }); - it('throws an internal error when upstreamTokenProvider is omitted on an OBO connection', async () => { + it('identifies missing upstream credentials before exchanging an OBO token', async () => { const { resolveOboToken } = jest.requireMock('~/mcp/oauth') as { resolveOboToken: jest.Mock; }; resolveOboToken.mockClear(); + const OboError = OboTokenResolutionError as unknown as jest.Mock; + OboError.mockImplementation((reason: string, userMessage: string, retryable = false) => { + const error = new Error(userMessage); + Object.setPrototypeOf(error, OboError.prototype); + return Object.assign(error, { + name: 'OboTokenResolutionError', + reason, + retryable, + userMessage, + }); + }); const oboTokenResolver = jest.fn(); await expect( @@ -6450,7 +6461,11 @@ describe('MCPConnectionFactory', () => { /** upstreamTokenProvider intentionally omitted */ }, ), - ).rejects.toThrow(/upstreamTokenProvider not plumbed/); + ).rejects.toMatchObject({ + name: 'OboTokenResolutionError', + reason: 'missing_upstream_provider', + retryable: false, + }); expect(resolveOboToken).not.toHaveBeenCalled(); }); diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index 3970263c129..c6369c41e57 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -3224,7 +3224,7 @@ describe('MCPManager', () => { /** upstreamTokenProvider intentionally omitted */ }), ).rejects.toMatchObject({ - message: expect.stringContaining('upstreamTokenProvider not plumbed'), + cause: expect.objectContaining({ reason: 'missing_upstream_provider', retryable: false }), }); expect(mockResolveOboToken).not.toHaveBeenCalled(); }); diff --git a/packages/api/src/mcp/oauth/obo.spec.ts b/packages/api/src/mcp/oauth/obo.spec.ts index daa1572641d..2cde171d2aa 100644 --- a/packages/api/src/mcp/oauth/obo.spec.ts +++ b/packages/api/src/mcp/oauth/obo.spec.ts @@ -192,7 +192,7 @@ describe('resolveOboToken', () => { mockResolver, createLazyOboUpstreamTokenProvider(lookup), ), - ).rejects.toMatchObject({ reason: 'session_refresh_failed', retryable: false }); + ).rejects.toMatchObject({ reason: 'missing_upstream_provider', retryable: false }); expect(mockExtractOpenIDTokenInfo).not.toHaveBeenCalled(); expect(mockResolver).not.toHaveBeenCalled(); }); diff --git a/packages/api/src/mcp/oauth/obo.ts b/packages/api/src/mcp/oauth/obo.ts index 32fca35f2e9..5e3d091ea89 100644 --- a/packages/api/src/mcp/oauth/obo.ts +++ b/packages/api/src/mcp/oauth/obo.ts @@ -117,7 +117,10 @@ export function createLazyOboUpstreamTokenProvider( effectiveSignal?.throwIfAborted(); if (!provider) { pending = undefined; - throw new Error('Renewable upstream credentials are unavailable.'); + throw new OboTokenResolutionError( + 'missing_upstream_provider', + 'No renewable upstream credential provider is available for this MCP server.', + ); } return await detachOnAbort( provider({ ...options, signal: effectiveSignal }), @@ -130,6 +133,7 @@ export function createLazyOboUpstreamTokenProvider( } export type OboTokenResolutionReason = + | 'missing_upstream_provider' | 'missing_upstream_token' | 'missing_upstream_access_token' | 'empty_exchange_response' @@ -299,6 +303,9 @@ export async function resolveOboToken( liveTokens = await upstreamTokenProvider(); } catch (error) { if (isAbortError(error)) throw error; + if (error instanceof OboTokenResolutionError && error.reason === 'missing_upstream_provider') { + throw error; + } logger.error('[OBO] Upstream session refresh failed:', error); const retryable = isRetryableOboExchangeError(error); throw new OboTokenResolutionError( diff --git a/packages/api/src/schedules/README.md b/packages/api/src/schedules/README.md index e155a4b4024..678625c6983 100644 --- a/packages/api/src/schedules/README.md +++ b/packages/api/src/schedules/README.md @@ -23,13 +23,18 @@ Supported authentication is determined by a successful unattended connection: Browser-only credentials, interactive OpenID bearer/OBO sources, missing user variables, and OAuth grants needing renewed consent cannot be assumed available. -Reconnect or configure the server in an interactive agent chat, or remove it from the -agent, then enable the schedule. A browser connection alone does not prove readiness; -enabling always reruns the unattended check. +An OBO server without a host-owned renewable upstream-token provider reports +`mcp_configuration_missing` with `detail: 'unattended_auth_required'` (a +backward-compatible subtype); reconnecting in a browser cannot make it ready. +The operator must configure a properly authorized unattended provider, or the +owner must remove the server from the agent. Other servers with missing user +credentials can be reconnected or configured in an interactive agent chat. +A browser connection alone does not prove readiness; enabling always reruns +the unattended check. -`mcp_reauth_required`, `mcp_configuration_missing`, and `mcp_permission_denied` stop a -scheduled occurrence and disable the schedule immediately. The permission status tells -the owner that an administrator must restore MCP use access. `mcp_unavailable` counts +`mcp_reauth_required`, `mcp_configuration_missing`, and `mcp_permission_denied` +stop a scheduled occurrence and disable the schedule immediately. The permission status tells the owner that an administrator must +restore MCP use access. `mcp_unavailable` counts toward the existing configured consecutive-failure threshold. Credential-store and configuration-store outages are transient; they must never be treated as proof of missing credentials. Failure records contain only server names and resolution statuses, never exception diff --git a/packages/api/src/schedules/erasure.spec.ts b/packages/api/src/schedules/erasure.spec.ts index 0617201ad69..a47269d3382 100644 --- a/packages/api/src/schedules/erasure.spec.ts +++ b/packages/api/src/schedules/erasure.spec.ts @@ -24,6 +24,9 @@ async function sweepOnce(options: { eraseScheduleIfDrained: jest.fn(async () => false), markEraseAttempted: jest.fn(async () => undefined), getRunsForReconciliation: jest.fn(async () => []), + getUnbookkeptRuns: jest.fn(async () => []), + finalizeBookkeeping: jest.fn(async () => undefined), + markRunsReconciled: jest.fn(async () => undefined), }; const sweep = startScheduleErasureSweep({ methods: methods as unknown as ScheduleMethods, @@ -118,6 +121,9 @@ describe('topology-safe dead-delivery convergence', () => { getRunsForReconciliation: jest.fn(async () => [ oldRun({ deliveryKey: 'dk-1', ...options.run } as Partial), ]), + getUnbookkeptRuns: jest.fn(async () => []), + finalizeBookkeeping: jest.fn(async () => undefined), + markRunsReconciled: jest.fn(async () => undefined), }; const getTriggerDelivery = jest.fn(async () => options.delivery ?? null); const clearReconciledJob = jest.fn(async () => undefined); @@ -328,6 +334,64 @@ describe('topology-safe dead-delivery convergence', () => { }); }); +describe('permanent MCP bookkeeping recovery without an armed scheduler', () => { + beforeEach(() => { + jest.useFakeTimers(); + jest.spyOn(Math, 'random').mockReturnValue(0); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + it('replays a half-bookkept MCP run without applying ordinary failure policy', async () => { + const mcp = [ + { + server: 'Graph', + status: 'mcp_configuration_missing' as const, + detail: 'unattended_auth_required' as const, + }, + ]; + const permanent = oldRun({ status: 'error', error: 'MCP unavailable', mcp }); + const ordinary = oldRun({ + scheduleId: 'schedule-2', + status: 'error', + error: 'Temporary upstream failure', + }); + const methods = { + getDeletingSchedules: jest.fn(async () => []), + getActiveRunsForSchedule: jest.fn(async () => []), + eraseScheduleIfDrained: jest.fn(async () => false), + markEraseAttempted: jest.fn(async () => undefined), + getRunsForReconciliation: jest.fn(async () => []), + recordRunOutcome: jest.fn(async () => undefined), + getUnbookkeptRuns: jest.fn(async () => [permanent, ordinary]), + finalizeBookkeeping: jest.fn(async () => undefined), + markRunsReconciled: jest.fn(async () => undefined), + }; + const sweep = startScheduleErasureSweep({ + methods: methods as unknown as ScheduleMethods, + getJobStatus: jest.fn(async () => null), + getTriggerDelivery: jest.fn(async () => null), + clearReconciledJob: jest.fn(async () => undefined), + canInferOwnerDeathFromMissingJob: false, + }); + await jest.advanceTimersByTimeAsync(5 * 60_000); + sweep.stop(); + expect(methods.finalizeBookkeeping).toHaveBeenCalledTimes(1); + expect(methods.finalizeBookkeeping).toHaveBeenCalledWith( + expect.objectContaining({ + scheduleId: 'schedule-1', + status: 'error', + mcp, + autoDisableAfterFailures: Number.MAX_SAFE_INTEGER, + }), + ); + expect(methods.markRunsReconciled).toHaveBeenCalledWith([permanent, ordinary]); + }); +}); + describe('dead-delivery certainty fence', () => { beforeEach(() => { jest.useFakeTimers(); @@ -350,6 +414,9 @@ describe('dead-delivery certainty fence', () => { eraseScheduleIfDrained: jest.fn(async () => false), markEraseAttempted: jest.fn(async () => undefined), getRunsForReconciliation: jest.fn(async () => [oldRun({ deliveryKey: 'dk-1' } as never)]), + getUnbookkeptRuns: jest.fn(async () => []), + finalizeBookkeeping: jest.fn(async () => undefined), + markRunsReconciled: jest.fn(async () => undefined), }; const sweep = startScheduleErasureSweep({ methods: methods as unknown as ScheduleMethods, diff --git a/packages/api/src/schedules/erasure.ts b/packages/api/src/schedules/erasure.ts index 37dc92ea013..de210244593 100644 --- a/packages/api/src/schedules/erasure.ts +++ b/packages/api/src/schedules/erasure.ts @@ -1,4 +1,5 @@ import { logger, runAsSystem } from '@librechat/data-schemas'; +import { getScheduleMCPDisabledReason } from 'librechat-data-provider'; import type { ScheduleMethods, IScheduleRun } from '@librechat/data-schemas'; import type { JobState, ScheduleEngineDeps } from './types'; import { @@ -39,6 +40,9 @@ export interface ScheduleErasureDeps { | 'markEraseAttempted' | 'getActiveRunsForSchedule' | 'getRunsForReconciliation' + | 'getUnbookkeptRuns' + | 'markRunsReconciled' + | 'finalizeBookkeeping' | 'recordRunOutcome' >; /** Job state at a run's conversationId; null = confirmed absent, throw = unknown. */ @@ -229,8 +233,9 @@ async function settleFromObservedJob( * is observed (a shared store shows the real generation; a process-local store can only be * showing this process's own), and a `dead` delivery is shared state proving no generation * owns the reservation. This never claims, fires, or advances, and defers anything fenced - * by an in-flight abort or resume hand-off. Auto-disable policy is deliberately NOT applied - * (the armed engine owns that): the run settles and frees its slot, the streak is untouched. + * by an in-flight abort or resume hand-off. Generic failures retain the fallback's + * no-auto-disable policy; a durable MCP configuration receipt still disables via the + * run-row transition, including when a retained job originally ended as a success. */ async function settleStrandedRuns(deps: ScheduleErasureDeps): Promise { const runs = await deps.methods.getRunsForReconciliation( @@ -297,8 +302,35 @@ async function settleStrandedRuns(deps: ScheduleErasureDeps): Promise { } } +/** Replays a crashed permanent-MCP terminal projection where no engine is armed. */ +async function replayUnbookkeptMCPRuns(deps: ScheduleErasureDeps): Promise { + const runs = await deps.methods.getUnbookkeptRuns( + new Date(Date.now() - STRANDED_RUN_MIN_AGE_MS), + SWEEP_BATCH, + ); + for (const run of runs) { + if (run.status !== 'error' || !getScheduleMCPDisabledReason(run.mcp)) { + continue; + } + try { + await deps.methods.finalizeBookkeeping({ + scheduleId: run.scheduleId, + scheduledFor: run.scheduledFor, + status: 'error', + conversationId: run.conversationId, + error: run.error, + mcp: run.mcp, + autoDisableAfterFailures: Number.MAX_SAFE_INTEGER, + }); + } catch (error) { + logger.warn(`[schedules] MCP bookkeeping replay failed for ${run.scheduleId}:`, error); + } + } + await deps.methods.markRunsReconciled(runs); +} + /** - * Erases soft-deleted schedules once they drain — and NOTHING else. + * Maintains erasure and positively evidenced stranded-run convergence without arming a scheduler. * * A `deleting` row is normally erased by whichever actor first observes it drained: the * delete request, or the terminal outcome write (erase-on-settle). Both are single @@ -307,11 +339,9 @@ async function settleStrandedRuns(deps: ScheduleErasureDeps): Promise { * retry it (the row is hidden from their list). In the standard entrypoint the * reconciler retries it; the clustered entrypoint runs no engine, so nothing does. * - * This is deliberately NOT the engine: it never claims, leases, fires, advances, or - * reconciles a run, so running it in every replica of a clustered deployment is safe and - * changes nothing about v1's single-process scheduling. It only re-drives - * `eraseScheduleIfDrained`, which re-checks drained-ness itself (no active run, no live - * lease) and is idempotent — concurrent sweepers race harmlessly. + * This is deliberately NOT the engine: it never claims, leases, fires or advances. + * Every convergence write is idempotent and guarded by the run or schedule's current + * state, so concurrent clustered sweepers can re-drive cleanup safely. */ export function startScheduleErasureSweep(deps: ScheduleErasureDeps): ScheduleErasureSweep { let stopped = false; @@ -340,6 +370,9 @@ export function startScheduleErasureSweep(deps: ScheduleErasureDeps): ScheduleEr await settleStrandedRuns(deps).catch((err) => logger.warn('[schedules] stranded-run convergence pass failed:', err), ); + await replayUnbookkeptMCPRuns(deps).catch((err) => + logger.warn('[schedules] MCP bookkeeping replay pass failed:', err), + ); }); } catch (err) { logger.error('[schedules] erasure sweep failed:', err); diff --git a/packages/api/src/schedules/mcp.spec.ts b/packages/api/src/schedules/mcp.spec.ts index da647d57af6..b491e8882f1 100644 --- a/packages/api/src/schedules/mcp.spec.ts +++ b/packages/api/src/schedules/mcp.spec.ts @@ -152,6 +152,43 @@ it('lazily resolves an upstream token provider for an OBO preflight', async () = ); }); +it('reports missing unattended OBO credentials without offering a browser reconnect', async () => { + const { check, deps } = setup(); + deps.getServerConfigs = jest.fn(async () => ({ + docs: { ...server, obo: { scopes: 'api://mcp/.default' } }, + })); + deps.resolveUpstreamTokenProvider = jest.fn(async () => undefined); + deps.connect = jest.fn(async (options) => { + const provider = createLazyOboUpstreamTokenProvider( + options.upstreamTokenProviderResolver!, + options.signal, + { mcpServer: options.serverName, scopes: options.serverConfig!.obo!.scopes }, + ); + await provider(); + throw new Error('A missing provider must never connect'); + }); + + await expect(check('agent', principal, { scheduleId: 'schedule' })).rejects.toMatchObject({ + code: 'mcp_configuration_missing', + outcomes: [ + { server: 'docs', status: 'mcp_configuration_missing', detail: 'unattended_auth_required' }, + ], + }); + expect(deps.resolveUpstreamTokenProvider).toHaveBeenCalledWith( + expect.objectContaining({ id: 'owner' }), + { + signal: undefined, + context: { + scheduleId: 'schedule', + ownerId: 'owner', + agentId: 'agent', + invocationMode: 'delegated', + }, + target: { mcpServer: 'docs', scopes: 'api://mcp/.default' }, + }, + ); +}); + it('does not resolve upstream credentials for non-OBO servers', async () => { const { check, deps } = setup(); deps.resolveUpstreamTokenProvider = jest.fn(async () => { @@ -271,6 +308,19 @@ it('classifies a transport outage as retryable', async () => { }); }); +it('classifies absent OBO provider wiring as a permanent unattended-auth setup failure', async () => { + const { check, deps } = setup(); + deps.connect = async () => { + throw new OboTokenResolutionError('missing_upstream_provider', 'Internal provider detail'); + }; + + await expect(check('agent', principal)).rejects.toMatchObject({ + code: 'mcp_configuration_missing', + message: + 'mcp_configuration_missing: [{"server":"docs","status":"mcp_configuration_missing","detail":"unattended_auth_required"}]', + }); +}); + it('classifies a permanent OBO credential failure as requiring reauthentication', async () => { const { check, deps } = setup(); deps.connect = async () => { diff --git a/packages/api/src/schedules/mcp.ts b/packages/api/src/schedules/mcp.ts index 37d3a344894..7697f133c2f 100644 --- a/packages/api/src/schedules/mcp.ts +++ b/packages/api/src/schedules/mcp.ts @@ -751,6 +751,17 @@ export function createScheduleMCPPreflight(deps: ScheduleMCPDeps): ScheduleMCPPr missingOwners.size > 0 ? missingOwners : undefined, ); } catch (error) { + if ( + error instanceof OboTokenResolutionError && + error.reason === 'missing_upstream_provider' + ) { + return outcomesForOwners(server, 'mcp_configuration_missing').map( + (outcome) => ({ + ...outcome, + detail: 'unattended_auth_required' as const, + }), + ); + } return outcomesForOwners( server, reauth || diff --git a/packages/api/src/schedules/service.spec.ts b/packages/api/src/schedules/service.spec.ts index 0c616ac7d32..5251ddd7ced 100644 --- a/packages/api/src/schedules/service.spec.ts +++ b/packages/api/src/schedules/service.spec.ts @@ -1,7 +1,9 @@ import { logger } from '@librechat/data-schemas'; +import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js'; import type { SchedulesServiceDeps } from './service'; +import { createSchedulesService, recordScheduledMCPToolAuthFailure } from './service'; +import { OboTokenResolutionError } from '../mcp/oauth/obo'; import { isShutdownInProgress } from '../app/shutdown'; -import { createSchedulesService } from './service'; /** Swappable per test: null keeps the no-job-store harness the drain tests rely on. */ let mockJobStore: { getJob: jest.Mock; deleteJob?: jest.Mock } | null = null; @@ -55,6 +57,7 @@ function makeService( countActiveRuns: jest.fn(async () => 0), requestRunAbort: jest.fn(async () => true), getScheduleRunAbortState: jest.fn(async () => null), + recordMCPToolAuthFailure: jest.fn(async () => true), markRunAbortPersisted: jest.fn(async () => undefined), recordRunOutcome, }; @@ -724,6 +727,195 @@ describe('erase-on-settle', () => { }); }); +describe('scheduled OBO tool failure settlement', () => { + const occurrence = '2026-09-09T12:00:00.000Z'; + const failure = { + server: 'Graph', + status: 'mcp_configuration_missing' as const, + detail: 'unattended_auth_required' as const, + }; + + function setup() { + const service = makeService(jest.fn, [string]>(async () => [])); + const methods = service.engineDeps.methods as unknown as { + getScheduleById: jest.Mock; + eraseScheduleIfDrained: jest.Mock; + getScheduleRunAbortState: jest.Mock; + recordMCPToolAuthFailure: jest.Mock; + recordRunOutcome: jest.Mock; + }; + methods.getScheduleById = jest.fn(async () => null); + methods.eraseScheduleIfDrained = jest.fn(async () => false); + methods.getScheduleRunAbortState = jest.fn(async () => ({ status: 'started', mcp: [failure] })); + const store = { + getJob: jest.fn(async () => ({ + createdAt: 42, + scheduleId: 's1', + scheduledFor: occurrence, + conversationId: 'c1', + userId: 'owner', + tenantId: 'tenant-1', + status: 'running', + })), + }; + mockJobStore = store; + return { service, methods, store }; + } + + it('does not initialize the schedule service for unrelated MCP errors', async () => { + const getRecorder = jest.fn(); + await expect( + recordScheduledMCPToolAuthFailure( + { + error: new Error('Ordinary MCP failure'), + streamId: 'c1', + jobCreatedAt: 42, + userId: 'owner', + serverName: 'Graph', + }, + getRecorder, + ), + ).resolves.toBe(false); + expect(getRecorder).not.toHaveBeenCalled(); + }); + + it.each(['job lookup', 'run write'] as const)( + 'keeps receipt persistence best-effort when the %s fails', + async (failurePoint) => { + const { service, methods, store } = setup(); + const persistenceError = new Error('Storage temporarily unavailable'); + if (failurePoint === 'job lookup') { + store.getJob.mockRejectedValueOnce(persistenceError); + } else { + methods.recordMCPToolAuthFailure.mockRejectedValueOnce(persistenceError); + } + const warn = jest.spyOn(logger, 'warn').mockImplementation(() => logger); + const missing = new OboTokenResolutionError('missing_upstream_provider', 'No credentials'); + const input = { + error: missing, + streamId: 'c1', + jobCreatedAt: 42, + userId: 'owner', + serverName: 'Graph', + }; + + await expect( + recordScheduledMCPToolAuthFailure(input, () => service.recordMCPToolAuthFailure), + ).resolves.toBe(false); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('could not persist'), + persistenceError, + ); + warn.mockRestore(); + }, + ); + + it('records a typed tool failure only for the matching scheduled generation and owner', async () => { + const { service, methods, store } = setup(); + const source = new OboTokenResolutionError('missing_upstream_provider', 'No credentials'); + const error = Object.assign(new McpError(ErrorCode.InternalError, 'OBO failed'), { + cause: source, + }); + const input = { error, streamId: 'c1', jobCreatedAt: 42, userId: 'owner', serverName: 'Graph' }; + await expect(service.recordMCPToolAuthFailure(input)).resolves.toBe(true); + expect(methods.recordMCPToolAuthFailure).toHaveBeenCalledWith({ + scheduleId: 's1', + scheduledFor: new Date(occurrence), + conversationId: 'c1', + tenantId: 'tenant-1', + server: 'Graph', + }); + expect(store.getJob).toHaveBeenCalledWith('c1'); + + for (const wrong of [{ jobCreatedAt: 43 }, { userId: 'another' }]) { + await expect(service.recordMCPToolAuthFailure({ ...input, ...wrong })).resolves.toBe(false); + } + await expect( + service.recordMCPToolAuthFailure({ ...input, error: new Error('No credentials') }), + ).resolves.toBe(false); + store.getJob.mockResolvedValueOnce({ + createdAt: 42, + userId: 'owner', + scheduleId: '', + scheduledFor: occurrence, + conversationId: 'c1', + tenantId: 'tenant-1', + status: 'running', + }); + await expect(service.recordMCPToolAuthFailure(input)).resolves.toBe(false); + expect(methods.recordMCPToolAuthFailure).toHaveBeenCalledTimes(1); + }); + + it('settles a resumed run with missing MCP credentials as an error with old-client-readable guidance', async () => { + const { service, methods } = setup(); + methods.getScheduleRunAbortState.mockResolvedValue({ status: 'started', mcp: [failure] }); + await service.recordScheduleOutcome({ + scheduleId: 's1', + scheduledFor: occurrence, + status: 'requires_action', + }); + expect(methods.getScheduleRunAbortState).not.toHaveBeenCalled(); + await expect( + service.recordScheduleOutcome({ + scheduleId: 's1', + scheduledFor: occurrence, + status: 'success', + conversationId: 'c1', + streamId: 'c1', + jobCreatedAt: 42, + }), + ).resolves.toBe(true); + expect(methods.recordRunOutcome).toHaveBeenCalledWith( + expect.objectContaining({ + status: 'error', + mcp: [failure], + error: 'MCP unattended authorization unavailable', + }), + ); + expect( + jest.requireMock('../stream/GenerationJobManager').GenerationJobManager.updateMetadata, + ).toHaveBeenCalledWith('c1', expect.objectContaining({ scheduleOutcome: 'error' }), 42); + }); + + it('retains an error rather than a balance skip for a known missing OBO provider', async () => { + const { service, methods } = setup(); + await service.recordScheduleOutcome({ + scheduleId: 's1', + scheduledFor: occurrence, + status: 'skipped_balance', + conversationId: 'c1', + streamId: 'c1', + jobCreatedAt: 42, + }); + expect(methods.recordRunOutcome).toHaveBeenLastCalledWith( + expect.objectContaining({ + status: 'error', + error: 'MCP unattended authorization unavailable', + mcp: [failure], + }), + ); + expect( + jest.requireMock('../stream/GenerationJobManager').GenerationJobManager.updateMetadata, + ).toHaveBeenCalledWith('c1', expect.objectContaining({ scheduleOutcome: 'error' }), 42); + }); + + it('keeps ordinary completion and missing-run outcomes unchanged', async () => { + const { service, methods } = setup(); + methods.getScheduleRunAbortState.mockResolvedValueOnce({ + status: 'started', + mcp: [{ server: 'Graph', status: 'ready' }], + }); + await service.recordScheduleOutcome({ + scheduleId: 's1', + scheduledFor: occurrence, + status: 'success', + }); + expect(methods.recordRunOutcome).toHaveBeenLastCalledWith( + expect.objectContaining({ status: 'success' }), + ); + }); +}); + describe('interactive Stop persistence barrier', () => { function outcomeService() { const service = makeService(jest.fn, [string]>().mockResolvedValue([])); diff --git a/packages/api/src/schedules/service.ts b/packages/api/src/schedules/service.ts index 4c3fb6d7c37..63715062f91 100644 --- a/packages/api/src/schedules/service.ts +++ b/packages/api/src/schedules/service.ts @@ -2,6 +2,7 @@ import { logger, runAsSystem, tenantStorage, isRuntimeDisabled } from '@librecha import { getRefillEligibilityDate, Permissions, PermissionTypes } from 'librechat-data-provider'; import type { ScheduleMethods, AppConfig, IBalance, IChatProject } from '@librechat/data-schemas'; import type { TCheckpointerConfig } from 'librechat-data-provider'; +import type { ScheduleMCPOutcome } from 'librechat-data-provider'; import type { Types } from 'mongoose'; import type { ScheduleEngineDeps, @@ -34,6 +35,7 @@ import { GenerationJobManager } from '../stream/GenerationJobManager'; import { isStopConfirmed } from '../stream/interfaces/IJobStore'; import { buildBalanceUpdateFields } from '../middleware/balance'; import { getAppConfigOptionsFromUser } from '../app/service'; +import { OboTokenResolutionError } from '../mcp/oauth/obo'; import { isShutdownInProgress } from '../app/shutdown'; import { startScheduleErasureSweep } from './erasure'; import { getBalanceConfig } from '../app/config'; @@ -193,6 +195,13 @@ export interface SchedulesService { options?: { signal?: AbortSignal }, ) => Promise; recordScheduleOutcome: (input: RecordScheduleOutcomeInput) => Promise; + recordMCPToolAuthFailure: (input: { + error: unknown; + streamId?: string; + jobCreatedAt?: number; + userId?: string; + serverName: string; + }) => Promise; /** * Stamps a scheduled run's interactive Stop BEFORE the abort is signalled, so the owner * settlement barrier, reconciliation, and schedule/account deletion hold off settling or @@ -273,14 +282,37 @@ export interface SchedulesService { * the topology cannot be shown safe (process-local job store with no single-process * assertion) — see isTopologySafeToArm. */ initializeScheduleEngine: () => Promise | undefined>; - /** Starts erasure-ONLY maintenance for an entrypoint that never arms the engine (the + /** Starts bounded maintenance for an entrypoint that never arms the engine (the * clustered worker). Idempotent per process and a no-op once the full engine is armed. - * Arms nothing else — no claims, firing, cadence advancement, or absence-based - * reconciliation — and refuses to infer owner death from a process-local missing job - * (isTopologySafeToArm gates that). See startScheduleErasureSweep. */ + * Replays terminal permanent-MCP bookkeeping but never claims, fires, advances, or + * infers owner death from a process-local missing job. See startScheduleErasureSweep. */ initializeScheduleErasureSweep: () => void; } +/** Screening happens before the CJS facade constructs any schedule dependencies. */ +export async function recordScheduledMCPToolAuthFailure( + input: Parameters[0], + getRecorder: () => SchedulesService['recordMCPToolAuthFailure'], +): Promise { + const cause = input.error instanceof Error ? input.error.cause : undefined; + const missing = input.error instanceof OboTokenResolutionError ? input.error : cause; + if ( + !(missing instanceof OboTokenResolutionError) || + missing.reason !== 'missing_upstream_provider' || + !input.streamId || + input.jobCreatedAt == null || + !input.userId + ) { + return false; + } + try { + return await getRecorder()(input); + } catch (error) { + logger.warn('[schedules] could not persist MCP authorization failure receipt:', error); + return false; + } +} + /** Test-only overrides for the service's bounded waits (drains, barriers). */ export interface ScheduleServiceTimings { drainTimeoutMs?: number; @@ -752,12 +784,10 @@ export function createSchedulesService( } /** - * The clustered entrypoint's ONLY schedule maintenance. Exposes the same erasure sweep - * the standard entrypoint falls back to, so a soft-deleted row whose delete/terminal - * erase-on-settle attempts missed still drains instead of retaining the owner's prompt - * forever. It shares startErasureFallback's idempotent startup guard, the sweep's own - * shutdown registration, and the topology-fenced owner-death policy — and arms nothing - * else, so running it in every clustered replica changes nothing about v1 scheduling. + * The clustered entrypoint's ONLY schedule maintenance. The fallback erases soft-deleted + * rows and converges terminal evidence, including permanent MCP bookkeeping interrupted + * between the run-row transition and card projection. It shares startErasureFallback's + * idempotent startup guard and topology-fenced owner-death policy, and never fires work. */ function initializeScheduleErasureSweep(): void { startErasureFallback(); @@ -811,6 +841,43 @@ export function createSchedulesService( } } + async function recordMCPToolAuthFailure({ + error, + streamId, + jobCreatedAt, + userId, + serverName, + }: Parameters[0]): Promise { + const cause = error instanceof Error ? error.cause : undefined; + const missing = error instanceof OboTokenResolutionError ? error : cause; + if ( + !(missing instanceof OboTokenResolutionError) || + missing.reason !== 'missing_upstream_provider' || + !streamId || + jobCreatedAt == null || + !userId + ) { + return false; + } + const job = await GenerationJobManager.getJobStore()?.getJob(streamId); + if ( + job?.createdAt !== jobCreatedAt || + job.userId !== userId || + !job.scheduleId || + !job.scheduledFor || + !job.conversationId + ) { + return false; + } + return methods.recordMCPToolAuthFailure({ + scheduleId: job.scheduleId, + scheduledFor: new Date(job.scheduledFor), + conversationId: job.conversationId, + ...(job.tenantId ? { tenantId: job.tenantId } : {}), + server: serverName, + }); + } + const OUTCOME_RETRY_ATTEMPTS = 3; /** @@ -829,7 +896,10 @@ export function createSchedulesService( * marker, a non-stop source, or a stamp gone stale (its route presumed dead) proceeds * immediately, so the stale-owner timeout remains the bounded recovery path. */ - async function waitForStopPersistence(scheduleId: string, scheduledFor: Date): Promise { + async function waitForStopPersistence( + scheduleId: string, + scheduledFor: Date, + ): Promise { const deadline = Date.now() + STOP_BARRIER_TIMEOUT_MS; for (;;) { const state = await methods.getScheduleRunAbortState(scheduleId, scheduledFor); @@ -841,7 +911,7 @@ export function createSchedulesService( ) { // Cleared to settle: no Stop owns the run, it acknowledged, or its owner is past // the stale cutoff and is presumed dead (the bounded recovery path). - return true; + return state?.mcp?.filter((item) => item.detail === 'unattended_auth_required') ?? []; } if (Date.now() >= deadline) { // Still an UNACKNOWLEDGED, FRESH Stop. The poll budget expiring proves nothing about @@ -850,7 +920,7 @@ export function createSchedulesService( // its deletion/erasure barriers) while beforePublish may still be writing. DEFER // instead: the acknowledgement settles it, or the stale-owner cutoff authorizes a // later attempt. - return false; + return null; } await new Promise((resolve) => setTimeout(resolve, STOP_BARRIER_POLL_MS)); } @@ -871,18 +941,26 @@ export function createSchedulesService( return true; } const terminal = status !== 'requires_action'; + let mcp: ScheduleMCPOutcome[] = []; if (terminal) { // Honor an in-flight interactive Stop's persistence before terminalizing. A deferral // is NOT a failure to record — the run is deliberately left active/preserved — but it // must report "not settled" so callers with durable retry (the approval-expiry host // action, reconciliation) re-drive it rather than assuming the outcome landed. - if (!(await waitForStopPersistence(scheduleId, new Date(scheduledFor)))) { + const observed = await waitForStopPersistence(scheduleId, new Date(scheduledFor)); + if (observed == null) { logger.info( `[schedules] deferring terminal settlement for ${scheduleId}: interactive Stop persistence is still unacknowledged`, ); return false; } + mcp = observed; } + const missingAuth = + mcp.length > 0 && + (status === 'success' || status === 'error' || status === 'skipped_balance'); + const effectiveStatus = missingAuth ? 'error' : status; + const effectiveError = missingAuth ? 'MCP unattended authorization unavailable' : error; if (terminal && streamId && jobCreatedAt != null) { try { await GenerationJobManager.updateMetadata( @@ -890,13 +968,13 @@ export function createSchedulesService( { preserveForScheduleReconcile: true, scheduleOutcome: - status === 'success' || - status === 'error' || - status === 'interrupted' || - status === 'skipped_balance' - ? status + effectiveStatus === 'success' || + effectiveStatus === 'error' || + effectiveStatus === 'interrupted' || + effectiveStatus === 'skipped_balance' + ? effectiveStatus : 'error', - ...(error ? { scheduleOutcomeError: error } : {}), + ...(effectiveError ? { scheduleOutcomeError: effectiveError } : {}), }, jobCreatedAt, ); @@ -915,13 +993,14 @@ export function createSchedulesService( await methods.recordRunOutcome({ scheduleId, scheduledFor: new Date(scheduledFor), - status, + status: effectiveStatus, clearConversationId, conversationId, ...(status === 'requires_action' && checkpointNamespace != null ? { checkpointNamespace } : {}), - error, + error: effectiveError, + ...(mcp.length > 0 ? { mcp } : {}), autoDisableAfterFailures: limits.autoDisableAfterFailures, balanceSkipDisableThreshold: BALANCE_SKIP_DISABLE_THRESHOLD, }); @@ -1716,6 +1795,7 @@ export function createSchedulesService( engineDeps, fireScheduleNow, recordScheduleOutcome, + recordMCPToolAuthFailure, beginScheduledStop, acknowledgeScheduledStopPersistence, claimScheduleResume, diff --git a/packages/data-provider/src/config.spec.ts b/packages/data-provider/src/config.spec.ts index 5e0eead7ced..2dd82eaa88a 100644 --- a/packages/data-provider/src/config.spec.ts +++ b/packages/data-provider/src/config.spec.ts @@ -561,6 +561,24 @@ describe('attached code environment user config schema', () => { }, ); + it.each([60_000, 125_000, 610_000])( + 'accepts a bounded %i ms workspace HTTP limit', + (maxRequestTimeoutMs) => { + expect(codeEnvironmentUserConfigSchema.parse({ limits: { maxRequestTimeoutMs } })).toEqual({ + limits: { maxRequestTimeoutMs }, + }); + }, + ); + + it.each([0, -1, 0.5, 610_001, NaN, Infinity])( + 'rejects an invalid workspace HTTP limit of %s', + (maxRequestTimeoutMs) => { + expect( + codeEnvironmentUserConfigSchema.safeParse({ limits: { maxRequestTimeoutMs } }).success, + ).toBe(false); + }, + ); + it('keeps an omitted admission budget backward compatible', () => { expect(codeEnvironmentUserConfigSchema.parse({ limits: {} })).toEqual({ limits: {} }); }); @@ -584,7 +602,7 @@ describe('attached code environment user config schema', () => { fileWrite: { allowed: ['allow', 'ask', 'deny'], default: 'ask' }, commandExecution: { allowed: ['ask', 'deny'], default: 'ask' }, }, - limits: { maxCommandTimeoutMs: 120000 }, + limits: { maxCommandTimeoutMs: 120000, maxRequestTimeoutMs: 125_000 }, }, }, ], @@ -597,6 +615,19 @@ describe('attached code environment user config schema', () => { throw new Error(result.error.toString()); } expect(result.success).toBe(true); + expect(result.data).toMatchObject({ + endpoints: { + agents: { + statefulCodeSessions: { + environments: [ + { + configSchema: { limits: { maxRequestTimeoutMs: 125_000 } }, + }, + ], + }, + }, + }, + }); }); it('rejects an attached command timeout above the protocol hard cap', () => { diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 7afb9cb2808..21288b3dda4 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -1205,6 +1205,11 @@ export const CODE_ENVIRONMENT_COMMAND_TIMEOUT_HARD_MAX_MS = 5 * 60_000; * initial admission wait or an already-admitted operation's execution budget. */ export const CODE_ENVIRONMENT_QUEUE_WAIT_DEFAULT_MS = 5 * 60_000; +/** Code API's per-request admission ceiling, independent of the retry horizon. */ +export const CODE_ENVIRONMENT_ADMISSION_MAX_MS = 5 * 60_000; +/** Maximum opt-in HTTP budget: five minutes of admission and execution plus ten seconds for settlement and delivery. */ +export const CODE_ENVIRONMENT_REQUEST_TIMEOUT_HARD_MAX_MS = + CODE_ENVIRONMENT_ADMISSION_MAX_MS + CODE_ENVIRONMENT_COMMAND_TIMEOUT_HARD_MAX_MS + 10_000; /** * Typed user-tunable surface for one attached code environment. Omitted fields @@ -1240,6 +1245,17 @@ export const codeEnvironmentUserConfigSchema = z .min(0) .max(CODE_ENVIRONMENT_QUEUE_WAIT_DEFAULT_MS) .optional(), + /** Total HTTP budget for one workspace tool call, including retries, + * execution, settlement, and delivery. Only set this after verifying + * the shortest timeout on the actual Code API path and updating Code API + * to honor per-request queue allowances. Omission keeps the 30-second + * per-attempt admission budget. */ + maxRequestTimeoutMs: z + .number() + .int() + .min(1) + .max(CODE_ENVIRONMENT_REQUEST_TIMEOUT_HARD_MAX_MS) + .optional(), }) .strict() .optional(), @@ -3913,6 +3929,14 @@ export enum ErrorTypes { * Provider throttled or refused the request for exceeding a rate/spend allowance */ MODEL_RATE_LIMIT = 'model_rate_limit', + /** + * Provider accepted the request, then closed the connection before the response finished + */ + MODEL_STREAM_CLOSED = 'model_stream_closed', + /** + * Provider accepted the request, then sent nothing for longer than the model response timeout + */ + MODEL_STREAM_STALLED = 'model_stream_stalled', /** * An agent model provider failed and the run could not recover. */ diff --git a/packages/data-provider/src/types/schedules.spec.ts b/packages/data-provider/src/types/schedules.spec.ts new file mode 100644 index 00000000000..3514d733068 --- /dev/null +++ b/packages/data-provider/src/types/schedules.spec.ts @@ -0,0 +1,34 @@ +import { z } from 'zod'; +import { + getScheduleMCPDisabledReason, + readScheduleMCPOutcomes, + scheduleMCPOutcomeSchema, +} from './schedules'; + +it('preserves the previous wire status while providing an optional unattended-auth diagnosis', () => { + const outcome = { + server: 'Graph', + status: 'mcp_configuration_missing' as const, + detail: 'unattended_auth_required' as const, + }; + // Represents the schema used by an open tab from before this feature. + const previousClientSchema = z.object({ + server: z.string(), + agentId: z.string().optional(), + status: z.enum([ + 'ready', + 'mcp_reauth_required', + 'mcp_configuration_missing', + 'mcp_permission_denied', + 'mcp_unavailable', + ]), + }); + expect(previousClientSchema.array().parse([outcome])).toEqual([ + { server: 'Graph', status: 'mcp_configuration_missing' }, + ]); + expect(scheduleMCPOutcomeSchema.parse(outcome)).toEqual(outcome); + expect(getScheduleMCPDisabledReason([outcome])).toBe('mcp_configuration_missing'); + expect( + readScheduleMCPOutcomes(`mcp_configuration_missing: ${JSON.stringify([outcome])}`), + ).toEqual([outcome]); +}); diff --git a/packages/data-provider/src/types/schedules.ts b/packages/data-provider/src/types/schedules.ts index 1575c21e303..b62f492d690 100644 --- a/packages/data-provider/src/types/schedules.ts +++ b/packages/data-provider/src/types/schedules.ts @@ -209,6 +209,8 @@ export const scheduleMCPOutcomeSchema = z.object({ /** Agent whose selected tool requires this server. Used to open the correct * recovery chat when the requirement belongs to a handoff or subagent. */ agentId: z.string().optional(), + /** Additional diagnosis; older clients ignore unknown keys and retain the known status. */ + detail: z.enum(['unattended_auth_required']).optional(), status: z.enum([ 'ready', 'mcp_reauth_required', diff --git a/packages/data-schemas/src/methods/schedule.methods.spec.ts b/packages/data-schemas/src/methods/schedule.methods.spec.ts index 4f3f0126376..dc4faee158f 100644 --- a/packages/data-schemas/src/methods/schedule.methods.spec.ts +++ b/packages/data-schemas/src/methods/schedule.methods.spec.ts @@ -3042,6 +3042,223 @@ describe('erasure sweep rotation and idempotency-key lookup', () => { }); }); +describe('scheduled MCP tool failure receipt', () => { + it('records only the matching active run and preserves the backwards-compatible failure', async () => { + const schedule = await methods.createSchedule(scheduleData()); + const scheduledFor = new Date('2026-09-09T14:00:00Z'); + await methods.insertScheduleRun(runData(schedule, { scheduledFor, conversationId: 'c1' })); + const input = { scheduleId: schedule.id, scheduledFor, conversationId: 'c1', server: 'Graph' }; + expect(await methods.recordMCPToolAuthFailure({ ...input, conversationId: 'another' })).toBe( + false, + ); + expect(await methods.recordMCPToolAuthFailure({ ...input, tenantId: 'other-tenant' })).toBe( + false, + ); + expect(await methods.recordMCPToolAuthFailure(input)).toBe(true); + expect(await methods.recordMCPToolAuthFailure(input)).toBe(true); + const run = await methods.getScheduleRunAbortState(schedule.id, scheduledFor); + expect(run?.mcp).toEqual([ + { server: 'Graph', status: 'mcp_configuration_missing', detail: 'unattended_auth_required' }, + ]); + await methods.recordRunOutcome({ + scheduleId: schedule.id, + scheduledFor, + status: 'error', + mcp: run!.mcp, + error: `mcp_configuration_missing: ${JSON.stringify(run!.mcp)}`, + autoDisableAfterFailures: 5, + }); + const after = await getSchedule(schedule.id); + expect(after.enabled).toBe(false); + expect(after.disabledReason).toBe('mcp_configuration_missing'); + expect(after.lastRun?.mcp).toEqual(run!.mcp); + expect(await methods.recordMCPToolAuthFailure(input)).toBe(false); + }); +}); + +describe('scheduled MCP receipt settlement', () => { + const failure = { + server: 'Graph', + status: 'mcp_configuration_missing' as const, + detail: 'unattended_auth_required' as const, + }; + const ready = { server: 'Graph', status: 'ready' as const }; + + it.each(['success', 'skipped_balance', 'error'] as const)( + 'keeps a persisted receipt through post-enqueue writes and settles %s as a permanent failure', + async (status) => { + const schedule = await methods.createSchedule(scheduleData()); + const scheduledFor = new Date('2026-09-09T15:00:00Z'); + const input = { + scheduleId: schedule.id, + scheduledFor, + conversationId: 'c1', + server: 'Graph', + }; + await methods.insertScheduleRun( + runData(schedule, { + scheduledFor, + conversationId: 'c1', + configRevision: schedule.configRevision, + mcp: [ready], + }), + ); + + expect(await methods.recordMCPToolAuthFailure(input)).toBe(true); + await methods.setRunFireDetails(schedule.id, scheduledFor, { + conversationId: 'c1', + mcp: [ready], + droppedFileIds: ['missing-file'], + }); + expect((await getRun(schedule.id, scheduledFor)).mcp).toEqual([ready, failure]); + + await methods.recordRunOutcome({ + scheduleId: schedule.id, + scheduledFor, + status, + conversationId: 'c1', + ...(status === 'success' ? { mcp: [ready] } : {}), + ...(status === 'error' ? { error: 'Run ended in error' } : {}), + autoDisableAfterFailures: Number.MAX_SAFE_INTEGER, + balanceSkipDisableThreshold: 3, + }); + const result = await getRun(schedule.id, scheduledFor); + const card = await getSchedule(schedule.id); + expect(result).toMatchObject({ status: 'error', bookkept: true, mcp: [ready, failure] }); + expect(card.lastRun).toMatchObject({ status: 'error', mcp: [ready, failure] }); + expect(card.enabled).toBe(false); + expect(card.disabledReason).toBe('mcp_configuration_missing'); + expect(card.failureCount).toBe(1); + expect(card.balanceSkipCount).toBe(0); + expect(card.lastRun?.error).toBe( + status === 'error' ? 'Run ended in error' : 'MCP unattended authorization unavailable', + ); + await methods.recordRunOutcome({ + scheduleId: schedule.id, + scheduledFor, + status: 'success', + autoDisableAfterFailures: Number.MAX_SAFE_INTEGER, + }); + expect((await getSchedule(schedule.id)).failureCount).toBe(1); + expect(await methods.recordMCPToolAuthFailure(input)).toBe(false); + }, + ); + + it('lets the receipt and terminal settlement linearize in either order', async () => { + const schedule = await methods.createSchedule(scheduleData()); + const scheduledFor = new Date('2026-09-09T16:00:00Z'); + await methods.insertScheduleRun(runData(schedule, { scheduledFor, conversationId: 'c1' })); + const [recorded] = await Promise.all([ + methods.recordMCPToolAuthFailure({ + scheduleId: schedule.id, + scheduledFor, + conversationId: 'c1', + server: 'Graph', + }), + methods.recordRunOutcome({ + scheduleId: schedule.id, + scheduledFor, + status: 'success', + autoDisableAfterFailures: Number.MAX_SAFE_INTEGER, + }), + ]); + const run = await getRun(schedule.id, scheduledFor); + const card = await getSchedule(schedule.id); + expect(run.status).toBe(recorded ? 'error' : 'success'); + expect(card.disabledReason).toBe(recorded ? 'mcp_configuration_missing' : undefined); + expect(card.lastRun?.status).toBe(run.status); + }); + + it('does not restore a cleared conversation ID from late fire bookkeeping', async () => { + const schedule = await methods.createSchedule(scheduleData()); + const scheduledFor = new Date('2026-09-09T17:00:00Z'); + await methods.insertScheduleRun(runData(schedule, { scheduledFor, conversationId: 'c1' })); + await methods.recordRunOutcome({ + scheduleId: schedule.id, + scheduledFor, + status: 'interrupted', + clearConversationId: true, + autoDisableAfterFailures: 5, + }); + await methods.setRunFireDetails(schedule.id, scheduledFor, { + conversationId: 'c1', + mcp: [ready], + }); + const result = await getRun(schedule.id, scheduledFor); + expect(result.conversationId).toBeUndefined(); + expect(result.mcp).toBeUndefined(); + }); + + it('fences an old receipt from disabling a schedule the owner has edited', async () => { + const schedule = await methods.createSchedule(scheduleData()); + const scheduledFor = new Date('2026-09-09T18:00:00Z'); + await methods.insertScheduleRun( + runData(schedule, { scheduledFor, conversationId: 'c1', configRevision: 0 }), + ); + await methods.recordMCPToolAuthFailure({ + scheduleId: schedule.id, + scheduledFor, + conversationId: 'c1', + server: 'Graph', + }); + await Schedule.updateOne({ id: schedule.id }, { $inc: { configRevision: 1 } }); + + await methods.recordRunOutcome({ + scheduleId: schedule.id, + scheduledFor, + status: 'success', + autoDisableAfterFailures: 5, + }); + expect((await getRun(schedule.id, scheduledFor)).status).toBe('error'); + expect(await getSchedule(schedule.id)).toMatchObject({ enabled: true, failureCount: 0 }); + expect((await getSchedule(schedule.id)).lastRun).toBeUndefined(); + }); + + it('replays a receipt when schedule bookkeeping fails after the run terminalizes', async () => { + const schedule = await methods.createSchedule(scheduleData()); + const scheduledFor = new Date('2026-09-09T19:00:00Z'); + await methods.insertScheduleRun(runData(schedule, { scheduledFor, conversationId: 'c1' })); + await methods.recordMCPToolAuthFailure({ + scheduleId: schedule.id, + scheduledFor, + conversationId: 'c1', + server: 'Graph', + }); + const firstWrite = jest + .spyOn(Schedule, 'updateOne') + .mockRejectedValueOnce(new Error('DB unavailable')); + await expect( + methods.recordRunOutcome({ + scheduleId: schedule.id, + scheduledFor, + status: 'success', + autoDisableAfterFailures: Number.MAX_SAFE_INTEGER, + }), + ).rejects.toThrow('DB unavailable'); + firstWrite.mockRestore(); + const pending = await getRun(schedule.id, scheduledFor); + expect(pending).toMatchObject({ status: 'error', bookkept: false, mcp: [failure] }); + + const replay = { + scheduleId: schedule.id, + scheduledFor, + status: 'error' as const, + conversationId: 'c1', + error: pending.error, + mcp: pending.mcp, + autoDisableAfterFailures: Number.MAX_SAFE_INTEGER, + }; + await methods.finalizeBookkeeping(replay); + await methods.finalizeBookkeeping(replay); + expect((await getRun(schedule.id, scheduledFor)).bookkept).toBe(true); + expect(await getSchedule(schedule.id)).toMatchObject({ + enabled: false, + disabledReason: 'mcp_configuration_missing', + failureCount: 1, + }); + }); +}); + describe('scheduled MCP failure policy', () => { it.each([ 'mcp_reauth_required', diff --git a/packages/data-schemas/src/methods/schedule.ts b/packages/data-schemas/src/methods/schedule.ts index e5ed0240f4a..dc6e29b631b 100644 --- a/packages/data-schemas/src/methods/schedule.ts +++ b/packages/data-schemas/src/methods/schedule.ts @@ -266,8 +266,16 @@ export type ScheduleMethods = { scheduledFor: Date, ) => Promise | null>; + /** Records an exact live generation's missing OBO authorization before the tool returns. */ + recordMCPToolAuthFailure: (input: { + scheduleId: string; + scheduledFor: Date; + conversationId: string; + tenantId?: string; + server: string; + }) => Promise; markRunResumeClaimed: ( scheduleId: string, scheduledFor: Date, @@ -1037,14 +1045,14 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche scheduledFor: Date, ): Promise | null> { return ScheduleRun() .findOne({ scheduleId, scheduledFor }) - .select('status abortRequestedAt abortSource abortPersistedAt') + .select('status abortRequestedAt abortSource abortPersistedAt mcp') .lean | null>(); } @@ -1115,6 +1123,31 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche ); } + async function recordMCPToolAuthFailure({ + scheduleId, + scheduledFor, + conversationId, + tenantId, + server, + }: Parameters[0]): Promise { + const updated = await ScheduleRun().updateOne( + { + scheduleId, + scheduledFor, + conversationId, + ...(tenantId ? { tenantId } : { tenantId: { $exists: false } }), + status: { $in: ['started', 'requires_action'] }, + }, + { + $addToSet: { + mcp: { server, status: 'mcp_configuration_missing', detail: 'unattended_auth_required' }, + }, + }, + { timestamps: false }, + ); + return (updated.matchedCount ?? 0) > 0; + } + /** Count of in-flight scheduled runs (across all schedules) for the fire cap. */ async function countActiveRuns(): Promise { return ScheduleRun().countDocuments({ status: 'started' }); @@ -1463,39 +1496,77 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche // TERMINAL: flip the run row (match-guarded), then apply bookkeeping. `bookkept` is // set false at the flip and true only after bookkeeping lands, so a crash between is // re-applied by the reconciler (getUnbookkeptRuns) while countedFor keeps counters idempotent. - const settled = await ScheduleRun() + const runFilter = { + scheduleId: params.scheduleId, + scheduledFor: params.scheduledFor, + status: { $in: ['started', 'requires_action'] }, + }; + const canOverride = + params.status === 'success' || + params.status === 'error' || + params.status === 'skipped_balance'; + const incomingFailure = + canOverride && params.mcp?.some((item) => item.detail === 'unattended_auth_required'); + const authError = + params.status === 'error' && params.error + ? params.error + : 'MCP unattended authorization unavailable'; + const terminalUpdate = ( + status: RecordRunOutcomeParams['status'], + error: string | undefined, + includeInputMcp: boolean, + ) => ({ + $set: { + status, + bookkept: false, + settledAt: firedAt, + ...(params.conversationId && !params.clearConversationId + ? { conversationId: params.conversationId } + : {}), + ...(error ? { error } : {}), + ...(includeInputMcp && params.mcp ? { mcp: params.mcp } : {}), + ...(params.durationMs != null ? { durationMs: params.durationMs } : {}), + }, + // Only terminal settlement releases the global capacity slot, not an abort request. + $unset: { + capacitySlot: 1, + admissionOnly: 1, + ...(params.clearConversationId ? { conversationId: 1 } : {}), + }, + }); + // The absence check is part of the row update, not a stale read. A concurrent + // receipt either wins first (the fallback below records an error) or loses to + // terminalization (and can no longer stamp this run). + let settled = await ScheduleRun() .findOneAndUpdate( { - scheduleId: params.scheduleId, - scheduledFor: params.scheduledFor, - status: { $in: ['started', 'requires_action'] }, - }, - { - $set: { - status: params.status, - bookkept: false, - settledAt: firedAt, - ...(params.conversationId && !params.clearConversationId - ? { conversationId: params.conversationId } - : {}), - ...(params.error ? { error: params.error } : {}), - ...(params.mcp ? { mcp: params.mcp } : {}), - ...(params.durationMs != null ? { durationMs: params.durationMs } : {}), - }, - // SETTLEMENT: a terminal outcome is the generation owner confirming the run - // actually stopped, so this is the ONLY place the global capacity slot is - // released. An abort request alone does not free it (see requestRunAbort). - $unset: { - capacitySlot: 1, - admissionOnly: 1, - ...(params.clearConversationId ? { conversationId: 1 } : {}), - }, + ...runFilter, + ...(canOverride ? { 'mcp.detail': { $ne: 'unattended_auth_required' } } : {}), }, + terminalUpdate( + incomingFailure ? 'error' : params.status, + incomingFailure ? authError : params.error, + true, + ), { new: false }, ) .lean(); - // No-match guard: never touch schedule bookkeeping without a matching run - // (protects against a spoofed scheduleId on a normal chat). + let effectiveParams = incomingFailure + ? { ...params, status: 'error' as const, error: authError } + : params; + if (settled == null && canOverride) { + settled = await ScheduleRun() + .findOneAndUpdate( + { ...runFilter, 'mcp.detail': 'unattended_auth_required' }, + terminalUpdate('error', authError, false), + { new: false }, + ) + .lean(); + if (settled != null) { + effectiveParams = { ...params, status: 'error', error: authError, mcp: settled.mcp }; + } + } + // No-match guard: never touch schedule bookkeeping without a matching run. if (settled == null) { return; } @@ -1503,7 +1574,7 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche // passed in by each caller. Callers only say "this occurrence reached status X" and // structurally cannot forget a token — which is exactly how the reconcile and // balance-skip paths previously shipped unfenced. - if (params.status === 'skipped_balance') { + if (effectiveParams.status === 'skipped_balance') { await applyBalanceSkipBookkeeping({ scheduleId: params.scheduleId, scheduledFor: params.scheduledFor, @@ -1514,7 +1585,7 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche }); } else { await applyTerminalBookkeeping({ - ...params, + ...effectiveParams, firedAt: settled.firedAt ?? firedAt, expectConfigRevision: settled.configRevision, }); @@ -1597,13 +1668,13 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche details: { conversationId: string; droppedFileIds?: string[]; mcp?: IScheduleRun['mcp'] }, ): Promise { await ScheduleRun().updateOne( - { scheduleId, scheduledFor }, + { scheduleId, scheduledFor, conversationId: details.conversationId }, { $set: { conversationId: details.conversationId, - ...(details.mcp ? { mcp: details.mcp } : {}), ...(details.droppedFileIds?.length ? { droppedFileIds: details.droppedFileIds } : {}), }, + ...(details.mcp?.length ? { $addToSet: { mcp: { $each: details.mcp } } } : {}), }, ); } @@ -2148,6 +2219,7 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche persistResolvedProject, getScheduleRunProject, getScheduleRunAbortState, + recordMCPToolAuthFailure, markRunResumeClaimed, releaseRunResumeClaim, markRunAbortPersisted, diff --git a/packages/data-schemas/src/schema/schedule.ts b/packages/data-schemas/src/schema/schedule.ts index 0990cad5b99..e52cf958636 100644 --- a/packages/data-schemas/src/schema/schedule.ts +++ b/packages/data-schemas/src/schema/schedule.ts @@ -232,6 +232,7 @@ const scheduleSchema: Schema = new Schema( _id: false, server: { type: String, required: true }, agentId: { type: String }, + detail: { type: String, enum: ['unattended_auth_required'] }, status: { type: String, required: true, diff --git a/packages/data-schemas/src/schema/scheduleRun.ts b/packages/data-schemas/src/schema/scheduleRun.ts index b51a105a602..ba685bee995 100644 --- a/packages/data-schemas/src/schema/scheduleRun.ts +++ b/packages/data-schemas/src/schema/scheduleRun.ts @@ -71,6 +71,7 @@ const scheduleRunSchema: Schema = new Schema( _id: false, server: { type: String, required: true }, agentId: { type: String }, + detail: { type: String, enum: ['unattended_auth_required'] }, status: { type: String, required: true,