mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
fix: adversarial-review findings — in-graph execution, orphan prunes, endpoint scoping, real kill switch
Pre-PR multi-agent review confirmed 5 defects in the initial commit; all fixed: 1. CRITICAL — the tool never paused on the real agents endpoint: production loads tools definitions-only, flipping the SDK ToolNode to event-driven dispatch, and the host ON_TOOL_EXECUTE handler runs outside the Pregel task frame (under runOutsideTracing), where interrupt() throws and becomes an error ToolMessage. Reworked: the ask tool never rides toolDefinitions/ toolRegistry — on HITL-capable top-level agents a real instance is supplied via AgentInputs.graphTools (agents#289, requires @librechat/agents > 3.2.57), the SDK's in-graph direct-tool seam; new production-shape e2e pins the event-driven mode end to end. 2. CRITICAL — ask-only runs left orphaned interrupted checkpoints (silent context duplication on every later turn): both orphan prunes were gated on toolApproval.enabled. The pre-turn prune now also fires for ask-capable agents (exported agentRequestsAskUserQuestion), and the abort-route prune fires when the aborted job carries a pendingAction. 3. MAJOR — self-spawned subagents bypassed the strip (self config resolves from the parent's _sourceInputs): fixed SDK-side (buildChildInputs clears graphTools) and the tool is now never present on child surfaces host-side. 4. MINOR — the manifest entry leaked into the Assistants tools dialog and the legacy plugins endpoint, where tools execute with no run to pause: new agentsOnly manifest flag, scoped out of both listings. 5. MINOR — filteredTools/includedTools only hid the tool from the dialog: now enforced at run build (strip + no checkpointer), making the admin filter a real kill switch for already-saved agents.
This commit is contained in:
Danny Avila
parent
27358c47c5
commit
381dbb1753
8 changed files with 349 additions and 21 deletions
|
|
@ -95,6 +95,7 @@
|
|||
"pluginKey": "ask_user_question",
|
||||
"description": "Let the agent pause mid-run to ask you a clarifying question and wait for your answer.",
|
||||
"icon": "assets/ask-user-question.svg",
|
||||
"agentsOnly": true,
|
||||
"authConfig": []
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -22,6 +22,11 @@ const getAvailablePluginsController = async (req, res) => {
|
|||
/** includedTools takes precedence — filteredTools ignored when both are set. */
|
||||
const plugins = [];
|
||||
for (const plugin of uniquePlugins) {
|
||||
/** Agents-runtime-only tools (e.g. ask_user_question) never work on the
|
||||
* legacy plugins endpoint — no run to pause, no resume surface. */
|
||||
if (plugin.agentsOnly === true) {
|
||||
continue;
|
||||
}
|
||||
if (includeSet.size > 0) {
|
||||
if (!includeSet.has(plugin.pluginKey)) {
|
||||
continue;
|
||||
|
|
@ -66,8 +71,21 @@ const getAvailableTools = async (req, res) => {
|
|||
const toolDefKeysList = toolDefinitions ? Object.keys(toolDefinitions) : null;
|
||||
const toolDefKeys = toolDefKeysList ? new Set(toolDefKeysList) : null;
|
||||
|
||||
/**
|
||||
* `getAvailableTools` serves BOTH tool dialogs — /api/agents/tools and
|
||||
* /api/assistants/tools. Tools flagged `agentsOnly` in the manifest (e.g.
|
||||
* ask_user_question, which pauses an agents run via a LangGraph interrupt)
|
||||
* cannot work on the assistants runtime: it executes tools directly with no
|
||||
* run to pause and no resume surface, so attaching one there guarantees a
|
||||
* permanent tool error. Scope them out of the assistants listing by route.
|
||||
*/
|
||||
const isAssistantsRoute = req.baseUrl?.includes('/assistants') === true;
|
||||
|
||||
const toolsOutput = [];
|
||||
for (const plugin of uniquePlugins) {
|
||||
if (plugin.agentsOnly === true && isAssistantsRoute) {
|
||||
continue;
|
||||
}
|
||||
const isToolDefined = toolDefKeys?.has(plugin.pluginKey) === true;
|
||||
const isToolkit =
|
||||
plugin.toolkit === true &&
|
||||
|
|
|
|||
|
|
@ -90,6 +90,18 @@ describe('PluginController', () => {
|
|||
expect(responseData[0].authenticated).toBeUndefined();
|
||||
});
|
||||
|
||||
it('excludes agentsOnly plugins from the legacy plugins endpoint (no run to pause)', async () => {
|
||||
require('~/app/clients/tools').availableTools.push(
|
||||
{ name: 'Ask User', pluginKey: 'ask_user_question', description: 'q', agentsOnly: true },
|
||||
{ name: 'Plugin2', pluginKey: 'key2', description: 'Second' },
|
||||
);
|
||||
|
||||
await getAvailablePluginsController(mockReq, mockRes);
|
||||
|
||||
const responseData = mockRes.json.mock.calls[0][0];
|
||||
expect(responseData.map((p) => p.pluginKey)).toEqual(['key2']);
|
||||
});
|
||||
|
||||
it('should filter plugins based on includedTools', async () => {
|
||||
const mockPlugins = [
|
||||
{ name: 'Plugin1', pluginKey: 'key1', description: 'First' },
|
||||
|
|
@ -153,6 +165,36 @@ describe('PluginController', () => {
|
|||
});
|
||||
|
||||
describe('getAvailableTools', () => {
|
||||
it('scopes agentsOnly plugins out of the ASSISTANTS listing but keeps them for agents', async () => {
|
||||
const cached = {
|
||||
ask_user_question: {
|
||||
type: 'function',
|
||||
function: { name: 'ask_user_question', description: 'q', parameters: {} },
|
||||
},
|
||||
};
|
||||
require('~/app/clients/tools').availableTools.push({
|
||||
name: 'Ask User',
|
||||
pluginKey: 'ask_user_question',
|
||||
description: 'q',
|
||||
agentsOnly: true,
|
||||
});
|
||||
|
||||
// Agents route: listed.
|
||||
getCachedTools.mockResolvedValueOnce(cached);
|
||||
mockReq.baseUrl = '/api/agents/tools';
|
||||
await getAvailableTools(mockReq, mockRes);
|
||||
expect(mockRes.json.mock.calls[0][0].map((t) => t.pluginKey)).toContain('ask_user_question');
|
||||
|
||||
// Assistants route: the runtime executes tools with no run to pause — excluded.
|
||||
mockRes.json.mockClear();
|
||||
getCachedTools.mockResolvedValueOnce(cached);
|
||||
mockReq.baseUrl = '/api/assistants/v2/tools';
|
||||
await getAvailableTools(mockReq, mockRes);
|
||||
expect(mockRes.json.mock.calls[0][0].map((t) => t.pluginKey)).not.toContain(
|
||||
'ask_user_question',
|
||||
);
|
||||
});
|
||||
|
||||
it('should use filterUniquePlugins to deduplicate combined tools', async () => {
|
||||
const mockUserTools = {
|
||||
'user-tool': {
|
||||
|
|
|
|||
|
|
@ -121,6 +121,44 @@ async function buildAskRun({ saver, responses, toolCalls, runId }) {
|
|||
return run;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a REAL run in the PRODUCTION shape: the agents endpoint loads tools
|
||||
* definitions-only, so the run is EVENT-DRIVEN (`toolDefinitions` non-empty flips
|
||||
* the SDK ToolNode to event dispatch) and the ask tool rides `graphTools` — the
|
||||
* SDK's in-graph direct-tool seam (agents#289, > 3.2.57) — because an event-
|
||||
* dispatched tool body executes in the host handler outside the Pregel task
|
||||
* frame, where `interrupt()` throws instead of pausing. This is the mode
|
||||
* `createRun` produces via `buildAgentInput`; the traditional-mode harness above
|
||||
* covers runs with zero toolDefinitions.
|
||||
*/
|
||||
async function buildAskRunEventMode({ saver, responses, toolCalls, runId }) {
|
||||
const run = await Run.create({
|
||||
runId,
|
||||
graphConfig: {
|
||||
type: 'standard',
|
||||
agents: [
|
||||
{
|
||||
agentId: 'agent-ask-event',
|
||||
provider: Providers.OPENAI,
|
||||
clientOptions: { model: 'gpt-4o-mini', streaming: true, streamUsage: false },
|
||||
instructions: 'You are a helpful assistant.',
|
||||
maxContextTokens: 8000,
|
||||
toolDefinitions: [{ name: 'dummy_event_tool', description: 'host-executed event tool' }],
|
||||
graphTools: [askTool],
|
||||
},
|
||||
],
|
||||
compileOptions: { checkpointer: saver },
|
||||
},
|
||||
returnContent: true,
|
||||
customHandlers: {},
|
||||
tokenCounter: (text) => String(text ?? '').length,
|
||||
indexTokenCountMap: {},
|
||||
eagerEventToolExecution: { enabled: true, excludeToolNames: [ASK_TOOL] },
|
||||
});
|
||||
run.Graph.overrideModel = new FakeChatModel({ responses, toolCalls });
|
||||
return run;
|
||||
}
|
||||
|
||||
const runConfig = (conversationId) => ({
|
||||
runName: 'AgentRun',
|
||||
configurable: { thread_id: conversationId, user_id: USER_ID },
|
||||
|
|
@ -308,6 +346,101 @@ describe('ask_user_question lifecycle (full wiring, approval policy disabled)',
|
|||
expect(await checkpointCounts(conversationId)).toEqual({ checkpoints: 0, writes: 0 });
|
||||
});
|
||||
|
||||
test('EVENT-DRIVEN mode (production shape): the graphTools ask tool pauses and resumes over the REAL /resume controller', async () => {
|
||||
const conversationId = `ask-e2e-event-${Date.now()}`;
|
||||
const responseMessageId = 'resp-ask-event-1';
|
||||
|
||||
const run = await buildAskRunEventMode({
|
||||
saver,
|
||||
responses: ['Let me check with you.'],
|
||||
toolCalls: [
|
||||
{
|
||||
name: ASK_TOOL,
|
||||
args: { question: 'Proceed with the migration?' },
|
||||
id: 'tc_ask_ev1',
|
||||
type: 'tool_call',
|
||||
},
|
||||
],
|
||||
runId: responseMessageId,
|
||||
});
|
||||
await run.processStream(
|
||||
{ messages: [new HumanMessage('run the migration')] },
|
||||
runConfig(conversationId),
|
||||
);
|
||||
|
||||
const interrupt = run.getInterrupt();
|
||||
expect(interrupt?.payload?.type).toBe('ask_user_question');
|
||||
expect(interrupt.payload.question).toEqual({ question: 'Proceed with the migration?' });
|
||||
expect(bodyRuns).toBe(1);
|
||||
expect((await checkpointCounts(conversationId)).checkpoints).toBeGreaterThan(0);
|
||||
|
||||
await GenerationJobManager.createJob(conversationId, USER_ID, conversationId);
|
||||
await GenerationJobManager.updateMetadata(conversationId, {
|
||||
endpoint: 'agents',
|
||||
agent_id: 'agent-ask-e2e',
|
||||
responseMessageId,
|
||||
});
|
||||
const pendingAction = buildPendingAction(interrupt.payload, {
|
||||
streamId: conversationId,
|
||||
conversationId,
|
||||
runId: responseMessageId,
|
||||
responseMessageId,
|
||||
ttlMs: 60_000,
|
||||
});
|
||||
expect(await GenerationJobManager.approvals.pause(conversationId, pendingAction)).toBe(true);
|
||||
|
||||
const thinClient = {
|
||||
contentParts: [],
|
||||
artifactPromises: [],
|
||||
conversationId,
|
||||
responseMessageId,
|
||||
pendingApproval: null,
|
||||
async resumeCompletion({ resumeValue, abortController }) {
|
||||
const resumed = await buildAskRunEventMode({
|
||||
saver,
|
||||
responses: ['Migration underway.'],
|
||||
runId: responseMessageId,
|
||||
});
|
||||
await resumed.resume(resumeValue, {
|
||||
...runConfig(conversationId),
|
||||
signal: (abortController ?? new AbortController()).signal,
|
||||
});
|
||||
this.contentParts.push({ type: 'text', text: 'Migration underway.' });
|
||||
return resumed;
|
||||
},
|
||||
};
|
||||
const initializeClient = jest.fn(async () => ({ client: thinClient }));
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
req.user = { id: USER_ID };
|
||||
req.config = { endpoints: { agents: { checkpointer: MONGO_CFG } }, interfaceConfig: {} };
|
||||
next();
|
||||
});
|
||||
app.post('/api/agents/chat/resume', (req, res, next) =>
|
||||
ResumeAgentController(req, res, next, initializeClient, jest.fn()),
|
||||
);
|
||||
|
||||
const response = await request(app).post('/api/agents/chat/resume').send({
|
||||
conversationId,
|
||||
actionId: pendingAction.actionId,
|
||||
agent_id: 'agent-ask-e2e',
|
||||
endpoint: 'agents',
|
||||
answer: 'yes, proceed',
|
||||
});
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
await waitFor(async () => {
|
||||
const liveJob = await GenerationJobManager.getJob(conversationId);
|
||||
return liveJob?.status !== 'requires_action' && liveJob?.status !== 'running';
|
||||
});
|
||||
|
||||
expect(bodyRuns).toBe(2);
|
||||
expect(resolvedAnswers).toEqual(['yes, proceed']);
|
||||
await waitFor(async () => (await checkpointCounts(conversationId)).checkpoints === 0);
|
||||
});
|
||||
|
||||
test('a second question raised after resume re-pauses with a fresh ask_user_question interrupt', async () => {
|
||||
const conversationId = `ask-e2e-seq-${Date.now()}`;
|
||||
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ const {
|
|||
getApprovalTtlMs,
|
||||
isHITLEnabled,
|
||||
deleteAgentCheckpoint,
|
||||
agentRequestsAskUserQuestion,
|
||||
getRequestMemories,
|
||||
createMemoryProcessor,
|
||||
agentHasInlineMemoryTools,
|
||||
|
|
@ -1625,7 +1626,14 @@ class AgentClient extends BaseClient {
|
|||
// a Redis flag) can go stale across replicas/restarts and skip the prune
|
||||
// exactly when an orphan exists, while these are two indexed, usually-empty
|
||||
// deleteMany ops — correctness over a micro-optimization.
|
||||
if (streamId && isHITLEnabled(agentsEConfig?.toolApproval)) {
|
||||
// The gate mirrors createRun's checkpointer condition: the approval policy
|
||||
// OR an ask_user_question-capable agent (which attaches a checkpointer
|
||||
// WITHOUT the approval policy) — an ask pause abandoned via job replacement
|
||||
// or Stop would otherwise rehydrate here and silently duplicate context.
|
||||
if (
|
||||
streamId &&
|
||||
(isHITLEnabled(agentsEConfig?.toolApproval) || agents.some(agentRequestsAskUserQuestion))
|
||||
) {
|
||||
await deleteAgentCheckpoint(this.conversationId, agentsEConfig?.checkpointer);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -298,9 +298,12 @@ router.post('/chat/abort', configMiddleware, async (req, res) => {
|
|||
// HITL: prune the durable checkpoint of a run aborted while paused, so a new turn
|
||||
// in this conversation can't rehydrate the stale interrupt before the Mongo TTL
|
||||
// reclaims it (thread_id is the stable conversationId). Idempotent / no-op when
|
||||
// HITL is off or nothing was written.
|
||||
// HITL is off or nothing was written. The pendingAction check covers ask-only
|
||||
// pauses (ask_user_question attaches a checkpointer WITHOUT the approval policy):
|
||||
// a job aborted while paused still carries its pendingAction in metadata, which is
|
||||
// exactly the case whose checkpoint would otherwise go stale.
|
||||
const agentsCfg = req.config?.endpoints?.agents;
|
||||
if (isHITLEnabled(agentsCfg?.toolApproval)) {
|
||||
if (isHITLEnabled(agentsCfg?.toolApproval) || job.metadata?.pendingAction != null) {
|
||||
await deleteAgentCheckpoint(jobStreamId, agentsCfg?.checkpointer).catch((err) =>
|
||||
logger.error(`[AgentStream] Failed to prune checkpoint on abort: ${jobStreamId}`, err),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -2056,8 +2056,11 @@ describe('ask_user_question run wiring', () => {
|
|||
expect(config).not.toHaveProperty('humanInTheLoop');
|
||||
expect(config).not.toHaveProperty('hooks');
|
||||
expect(getCheckpointer(config)).toBeDefined();
|
||||
// The tool itself survives on the HITL-capable path.
|
||||
expect((firstAgent(config).tools as Array<{ name: string }>).map((t) => t.name)).toContain(ASK);
|
||||
const agent = firstAgent(config);
|
||||
// The tool rides the in-graph direct path (graphTools) — never the
|
||||
// event-dispatched surfaces, where interrupt() cannot pause the run.
|
||||
expect((agent.graphTools as Array<{ name: string }>).map((t) => t.name)).toEqual([ASK]);
|
||||
expect((agent.tools as Array<{ name: string }>).map((t) => t.name)).not.toContain(ASK);
|
||||
});
|
||||
|
||||
it('detects the tool via toolRegistry / toolDefinitions too', async () => {
|
||||
|
|
@ -2115,15 +2118,18 @@ describe('ask_user_question run wiring', () => {
|
|||
subagentAgentConfigs: [child],
|
||||
});
|
||||
const config = await runAndGetConfig(parent, { hitlCapable: true });
|
||||
// Parent keeps the tool (and gets the checkpointer)…
|
||||
expect((firstAgent(config).tools as Array<{ name: string }>).map((t) => t.name)).toContain(ASK);
|
||||
// Parent keeps the tool — as an in-graph direct tool — and gets the checkpointer…
|
||||
expect((firstAgent(config).graphTools as Array<{ name: string }>).map((t) => t.name)).toEqual([
|
||||
ASK,
|
||||
]);
|
||||
expect(getCheckpointer(config)).toBeDefined();
|
||||
// …the child copy is stripped everywhere.
|
||||
// …the child copy is stripped everywhere, with no graphTools replacement.
|
||||
const subagentConfigs = firstAgent(config).subagentConfigs as Array<{
|
||||
agentInputs: Record<string, unknown>;
|
||||
}>;
|
||||
expect(subagentConfigs).toHaveLength(1);
|
||||
const childInputs = subagentConfigs[0].agentInputs;
|
||||
expect(childInputs.graphTools).toBeUndefined();
|
||||
expect((childInputs.tools as Array<{ name: string }>).map((t) => t.name)).not.toContain(ASK);
|
||||
expect((childInputs.toolDefinitions as Array<{ name: string }>).map((d) => d.name)).toEqual([]);
|
||||
expect((childInputs.toolRegistry as Map<string, unknown>).has(ASK)).toBe(false);
|
||||
|
|
@ -2145,6 +2151,72 @@ describe('ask_user_question run wiring', () => {
|
|||
expect(eager.excludeToolNames).toContain(ASK);
|
||||
});
|
||||
|
||||
it('admin filteredTools is a real kill switch: strips the tool and blocks the checkpointer even on an HITL-capable run', async () => {
|
||||
const filteredConfig = {
|
||||
...(plainAppConfig as unknown as Record<string, unknown>),
|
||||
filteredTools: [ASK],
|
||||
} as unknown as AppConfig;
|
||||
await createRun({
|
||||
agents: [
|
||||
makeAgent({
|
||||
tools: [askToolInstance],
|
||||
toolDefinitions: [{ name: ASK }],
|
||||
toolRegistry: new Map([[ASK, { name: ASK }]]),
|
||||
}),
|
||||
] as never,
|
||||
signal: new AbortController().signal,
|
||||
appConfig: filteredConfig,
|
||||
streaming: true,
|
||||
streamUsage: true,
|
||||
hitlCapable: true,
|
||||
});
|
||||
const config = (Run.create as jest.Mock).mock.calls[0][0] as Record<string, unknown>;
|
||||
expect(getCheckpointer(config)).toBeUndefined();
|
||||
const agent = firstAgent(config);
|
||||
expect((agent.tools as Array<{ name: string }>).map((t) => t.name)).toEqual([]);
|
||||
expect((agent.toolDefinitions as Array<{ name: string }>).map((d) => d.name)).toEqual([]);
|
||||
expect((agent.toolRegistry as Map<string, unknown>).has(ASK)).toBe(false);
|
||||
});
|
||||
|
||||
it('an includedTools allowlist disables the tool unless listed (allowlist precedence)', async () => {
|
||||
const withoutTool = {
|
||||
...(plainAppConfig as unknown as Record<string, unknown>),
|
||||
includedTools: ['calculator'],
|
||||
} as unknown as AppConfig;
|
||||
await createRun({
|
||||
agents: [makeAgent({ tools: [askToolInstance] })] as never,
|
||||
signal: new AbortController().signal,
|
||||
appConfig: withoutTool,
|
||||
streaming: true,
|
||||
streamUsage: true,
|
||||
hitlCapable: true,
|
||||
});
|
||||
let config = (Run.create as jest.Mock).mock.calls[0][0] as Record<string, unknown>;
|
||||
expect(getCheckpointer(config)).toBeUndefined();
|
||||
expect((firstAgent(config).tools as Array<{ name: string }>).map((t) => t.name)).toEqual([]);
|
||||
|
||||
jest.clearAllMocks();
|
||||
const withTool = {
|
||||
...(plainAppConfig as unknown as Record<string, unknown>),
|
||||
// includedTools wins over filteredTools — same precedence as loadAndFormatTools.
|
||||
includedTools: [ASK],
|
||||
filteredTools: [ASK],
|
||||
} as unknown as AppConfig;
|
||||
await createRun({
|
||||
agents: [makeAgent({ tools: [askToolInstance] })] as never,
|
||||
signal: new AbortController().signal,
|
||||
appConfig: withTool,
|
||||
streaming: true,
|
||||
streamUsage: true,
|
||||
hitlCapable: true,
|
||||
});
|
||||
config = (Run.create as jest.Mock).mock.calls[0][0] as Record<string, unknown>;
|
||||
expect(getCheckpointer(config)).toBeDefined();
|
||||
expect((firstAgent(config).graphTools as Array<{ name: string }>).map((t) => t.name)).toEqual([
|
||||
ASK,
|
||||
]);
|
||||
});
|
||||
|
||||
it('composes with the approval policy: both humanInTheLoop and the checkpointer attach', async () => {
|
||||
const approvalConfig = {
|
||||
config: {},
|
||||
|
|
|
|||
|
|
@ -34,8 +34,11 @@ import type { BaseMessage } from '@librechat/agents/langchain/messages';
|
|||
import type { AppConfig, IUser } from '@librechat/data-schemas';
|
||||
import type { SubagentUsageEvent } from '~/agents/usage';
|
||||
import type * as t from '~/types';
|
||||
import {
|
||||
ASK_USER_QUESTION_TOOL_NAME,
|
||||
createAskUserQuestionTool,
|
||||
} from '~/agents/hitl/askUserQuestionTool';
|
||||
import { getLLMConfig as getAnthropicLLMConfig } from '~/endpoints/anthropic/llm';
|
||||
import { ASK_USER_QUESTION_TOOL_NAME } from '~/agents/hitl/askUserQuestionTool';
|
||||
import { CREATE_FILE_TOOL_NAME, EDIT_FILE_TOOL_NAME } from '~/agents/tools';
|
||||
import { getProviderConfig } from '~/endpoints/config/providers';
|
||||
import { resolveToolApprovalPolicy } from '~/agents/hitl/policy';
|
||||
|
|
@ -738,8 +741,16 @@ function anyAgentHasCodeEnv(agents: RunAgent[]): boolean {
|
|||
* Checked against TOP-LEVEL agents only (not subagents — the tool is stripped from
|
||||
* child configs in `buildAgentInput`, since a child graph executing outside the parent
|
||||
* run's stream cannot pause the parent).
|
||||
*
|
||||
* Exported for AgentClient's pre-turn orphan-checkpoint prune gate: the prune must
|
||||
* fire whenever THIS turn may attach a checkpointer, which since the ask tool is no
|
||||
* longer coupled to `toolApproval.enabled` includes ask-capable runs.
|
||||
*/
|
||||
function agentRequestsAskUserQuestion(agent: RunAgent): boolean {
|
||||
export function agentRequestsAskUserQuestion(agent: {
|
||||
tools?: unknown[];
|
||||
toolRegistry?: Map<string, unknown>;
|
||||
toolDefinitions?: Array<{ name: string }>;
|
||||
}): boolean {
|
||||
return (
|
||||
agent.tools?.some(
|
||||
(tool) => (tool as { name?: string } | undefined)?.name === ASK_USER_QUESTION_TOOL_NAME,
|
||||
|
|
@ -749,6 +760,22 @@ function agentRequestsAskUserQuestion(agent: RunAgent): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the admin tool filter (`includedTools` allowlist, else `filteredTools`
|
||||
* exclude list — same precedence as `loadAndFormatTools`) disables
|
||||
* `ask_user_question`. Enforced at RUN BUILD, not just in the tools-dialog listing:
|
||||
* agents saved before an admin filtered the tool out would otherwise keep exposing
|
||||
* it to the model, attaching checkpointers, and pausing runs — for a run-pausing
|
||||
* tool the filter must be an actual kill switch.
|
||||
*/
|
||||
function isAskUserQuestionAdminDisabled(appConfig?: AppConfig): boolean {
|
||||
const included = appConfig?.includedTools;
|
||||
if (included != null && included.length > 0) {
|
||||
return !included.includes(ASK_USER_QUESTION_TOOL_NAME);
|
||||
}
|
||||
return appConfig?.filteredTools?.includes(ASK_USER_QUESTION_TOOL_NAME) === true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether any agent reachable in the run — primary, handoff/parallel, or a
|
||||
* nested subagent — opts into cross-turn `reasoning_content` reconstruction.
|
||||
|
|
@ -990,6 +1017,9 @@ export async function createRun({
|
|||
}
|
||||
}
|
||||
|
||||
/** Admin kill switch for the ask tool — see {@link isAskUserQuestionAdminDisabled}. */
|
||||
const askToolAdminDisabled = isAskUserQuestionAdminDisabled(appConfig);
|
||||
|
||||
const buildAgentInput = (agent: RunAgent, opts: { isSubagent?: boolean } = {}): AgentInputs => {
|
||||
const isSubagent = opts.isSubagent === true;
|
||||
const provider =
|
||||
|
|
@ -1112,17 +1142,24 @@ export async function createRun({
|
|||
}
|
||||
|
||||
/**
|
||||
* `ask_user_question` can only pause on runs whose caller implements the HITL
|
||||
* pause/resume lifecycle, so strip it fail-closed everywhere else: non-HITL
|
||||
* callers (the OpenAI-compatible + Responses controllers) have no pending-action
|
||||
* surface or resume endpoint, and subagent child graphs execute outside the
|
||||
* parent run's stream, so an interrupt raised inside one could not pause the
|
||||
* parent (v1: stripped; revisit with subagent HITL). Clone-before-mutate,
|
||||
* matching the registry-clone discipline above — `agent.toolRegistry` may be
|
||||
* shared with other builds of the same agent.
|
||||
* `ask_user_question` pauses via a LangGraph `interrupt()` raised from its own
|
||||
* tool body, so it must execute IN-PROCESS inside the graph's ToolNode — the
|
||||
* event-dispatched path runs tool bodies in the host handler outside the Pregel
|
||||
* task frame, where `interrupt()` throws and becomes an error ToolMessage. The
|
||||
* tool therefore never rides the schema-only `toolDefinitions`/`toolRegistry`
|
||||
* surfaces: on every path it is REMOVED from them (clone-before-mutate,
|
||||
* matching the registry-clone discipline above), and on the one path where it
|
||||
* can actually work — an HITL-capable caller's top-level agent, with the admin
|
||||
* filter allowing it — a real instance is supplied via `graphTools`, the SDK's
|
||||
* in-graph direct-tool seam (bound to the model, executed inside the task
|
||||
* frame; requires `@librechat/agents` > 3.2.57, older versions ignore the
|
||||
* field). Everywhere else (OpenAI-compatible + Responses controllers with no
|
||||
* resume surface, subagent child graphs that compile without a checkpointer,
|
||||
* admin-disabled) it is stripped fail-closed with no replacement.
|
||||
*/
|
||||
let tools = agent.tools;
|
||||
if ((!hitlCapable || isSubagent) && agentRequestsAskUserQuestion(agent)) {
|
||||
let askGraphTools: GenericTool[] | undefined;
|
||||
if (agentRequestsAskUserQuestion(agent)) {
|
||||
tools = tools?.filter(
|
||||
(tool) => (tool as { name?: string } | undefined)?.name !== ASK_USER_QUESTION_TOOL_NAME,
|
||||
);
|
||||
|
|
@ -1131,6 +1168,9 @@ export async function createRun({
|
|||
toolRegistry = new Map(toolRegistry);
|
||||
toolRegistry.delete(ASK_USER_QUESTION_TOOL_NAME);
|
||||
}
|
||||
if (hitlCapable && !isSubagent && !askToolAdminDisabled) {
|
||||
askGraphTools = [createAskUserQuestionTool() as unknown as GenericTool];
|
||||
}
|
||||
}
|
||||
|
||||
const effectiveMaxContextTokens = computeEffectiveMaxContextTokens(
|
||||
|
|
@ -1140,7 +1180,7 @@ export async function createRun({
|
|||
);
|
||||
|
||||
const reasoningKey = getReasoningKey(provider, llmConfig, agent.endpoint, agent.reasoningKey);
|
||||
return {
|
||||
const agentInput: AgentInputs = {
|
||||
provider,
|
||||
reasoningKey,
|
||||
toolDefinitions,
|
||||
|
|
@ -1161,6 +1201,16 @@ export async function createRun({
|
|||
contextPruningConfig: summarization.contextPruning,
|
||||
maxToolResultChars: agent.maxToolResultChars,
|
||||
};
|
||||
if (askGraphTools) {
|
||||
/**
|
||||
* Typed structurally — not as `AgentInputs['graphTools']` — because the
|
||||
* field ships in `@librechat/agents` > 3.2.57 (agents#289); older SDK
|
||||
* versions ignore it at runtime (the tool is then simply absent, never
|
||||
* broken). Inline the field in the literal once the dependency is bumped.
|
||||
*/
|
||||
(agentInput as AgentInputs & { graphTools?: GenericTool[] }).graphTools = askGraphTools;
|
||||
}
|
||||
return agentInput;
|
||||
};
|
||||
|
||||
const agentInputs: AgentInputs[] = [];
|
||||
|
|
@ -1253,7 +1303,8 @@ export async function createRun({
|
|||
* above, so a checkpointer here always has a resume surface. The LazyMongoSaver only
|
||||
* persists when a run actually pauses, so attaching it is near-zero overhead.
|
||||
*/
|
||||
const asksUserQuestions = hitlCapable && agents.some(agentRequestsAskUserQuestion);
|
||||
const asksUserQuestions =
|
||||
hitlCapable && !askToolAdminDisabled && agents.some(agentRequestsAskUserQuestion);
|
||||
if (hitl || asksUserQuestions) {
|
||||
const checkpointer = await getAgentCheckpointer(agentsEndpointConfig?.checkpointer);
|
||||
graphConfig.compileOptions = { ...graphConfig.compileOptions, checkpointer };
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue