mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
🧠 fix: Preserve Deferred Tool Schemas Across HITL Resume (#14552)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🧠 fix: Preserve deferred tool schemas across HITL resume * 🧪 test: Harden deferred tool resume regression * 📦 chore: bump @librechat/agents to v3.3.10
This commit is contained in:
parent
78ec1940a2
commit
60ca751a7f
15 changed files with 597 additions and 56 deletions
|
|
@ -1885,9 +1885,8 @@ describe('toolOutputReferences gating', () => {
|
|||
// durable checkpoint), so the in-turn `tool_search` results that mark a deferred
|
||||
// tool discovered aren't on the critical path. createRun's `discoveredToolNames`
|
||||
// input replays those names — captured at pause — so the paused deferred tool is
|
||||
// promoted back into `toolDefinitions` (and `defer_loading` flipped) and is present
|
||||
// in the rebuilt schema-only toolMap. Without it, the approved tool would be missing
|
||||
// and resume would fail with "unknown tool".
|
||||
// promoted back into `toolDefinitions` (and `defer_loading` flipped) and its schema
|
||||
// is restored to the rebuilt model binding.
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('createRun deferred-tool replay (HITL resume)', () => {
|
||||
/** Agent whose discoverable `deep_tool` lives ONLY in the registry (deferred). */
|
||||
|
|
|
|||
|
|
@ -3,12 +3,46 @@ import { ReasoningResponseKey } from 'librechat-data-provider';
|
|||
import { ToolMessage, AIMessage, HumanMessage } from '@librechat/agents/langchain/messages';
|
||||
import {
|
||||
extractDiscoveredToolsFromHistory,
|
||||
getRunDiscoveredTools,
|
||||
getReasoningKey,
|
||||
isDeepSeekReasoningProvider,
|
||||
shouldReplayReasoningContent,
|
||||
anyAgentReplaysReasoningContent,
|
||||
} from './run';
|
||||
|
||||
describe('getRunDiscoveredTools', () => {
|
||||
it('uses the run discovery snapshot instead of reconstructing it from messages', () => {
|
||||
const messages = [
|
||||
new ToolMessage({
|
||||
content: JSON.stringify({ tools: [{ name: 'save_project_mcp_linear' }] }),
|
||||
tool_call_id: 'call_1',
|
||||
name: 'tool_search',
|
||||
}),
|
||||
];
|
||||
|
||||
expect(
|
||||
getRunDiscoveredTools({
|
||||
getDiscoveredTools: () => ['save_issue_mcp_linear'],
|
||||
getRunMessages: () => messages,
|
||||
}),
|
||||
).toEqual(['save_issue_mcp_linear']);
|
||||
});
|
||||
|
||||
it('falls back to tool-search messages for agents releases without a snapshot API', () => {
|
||||
const messages = [
|
||||
new ToolMessage({
|
||||
content: JSON.stringify({ tools: [{ name: 'save_issue_mcp_linear' }] }),
|
||||
tool_call_id: 'call_1',
|
||||
name: 'tool_search',
|
||||
}),
|
||||
];
|
||||
|
||||
expect(getRunDiscoveredTools({ getRunMessages: () => messages })).toEqual([
|
||||
'save_issue_mcp_linear',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('extractDiscoveredToolsFromHistory', () => {
|
||||
it('extracts tool names from tool_search JSON output', () => {
|
||||
const toolSearchOutput = JSON.stringify({
|
||||
|
|
|
|||
|
|
@ -154,6 +154,30 @@ export function extractDiscoveredToolsFromHistory(messages: BaseMessage[]): Set<
|
|||
return discoveredTools;
|
||||
}
|
||||
|
||||
export interface RunDiscoverySnapshot {
|
||||
getDiscoveredTools?: () => string[];
|
||||
getRunMessages?: () => BaseMessage[] | undefined;
|
||||
}
|
||||
|
||||
/** Reads canonical run discovery state, with best-effort history parsing for older releases. */
|
||||
export function getRunDiscoveredTools(run: RunDiscoverySnapshot): string[] {
|
||||
if (typeof run.getDiscoveredTools === 'function') {
|
||||
const discoveredTools = run.getDiscoveredTools();
|
||||
if (Array.isArray(discoveredTools)) {
|
||||
return Array.from(new Set(discoveredTools));
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof run.getRunMessages !== 'function') {
|
||||
return [];
|
||||
}
|
||||
const messages = run.getRunMessages();
|
||||
if (!Array.isArray(messages) || messages.length === 0) {
|
||||
return [];
|
||||
}
|
||||
return Array.from(extractDiscoveredToolsFromHistory(messages));
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts skill names that were invoked in previous turns from raw message payload.
|
||||
* Scans assistant messages for tool_call content parts where name === 'skill'.
|
||||
|
|
@ -1085,9 +1109,9 @@ export async function createRun({
|
|||
* extraction. The HITL resume path rebuilds the graph with `messages: []` (state
|
||||
* comes from the durable checkpoint), so the in-turn `tool_search` results that
|
||||
* would normally mark a deferred tool discovered aren't present — without this the
|
||||
* paused tool would be absent from the rebuilt schema-only toolMap and resume would
|
||||
* fail with "unknown tool". Captured at pause via `extractDiscoveredToolsFromHistory`
|
||||
* and replayed here. Merged with (not replacing) any names extracted from `messages`.
|
||||
* paused tool's schema would be absent from the rebuilt model binding. Captured at
|
||||
* pause from canonical run state (with message parsing for older SDK releases) and
|
||||
* replayed here. Merged with (not replacing) names extracted from `messages`.
|
||||
*/
|
||||
discoveredToolNames?: string[];
|
||||
summarizationConfig?: SummarizationConfig;
|
||||
|
|
|
|||
|
|
@ -9,6 +9,12 @@ export interface ApprovalLifecycleCallbacks {
|
|||
onExpired?: (streamId: string, createdAt: number) => void;
|
||||
}
|
||||
|
||||
export interface ApprovalPauseOptions {
|
||||
discoveredTools?: string[];
|
||||
/** Generation identity observed by the interrupted run. */
|
||||
expectedCreatedAt?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* The guarded lifecycle of a run paused for human review (`requires_action`).
|
||||
*
|
||||
|
|
@ -42,20 +48,36 @@ export class ApprovalLifecycle {
|
|||
* Returns `false` when the job was not running (aborted mid-flight, gone),
|
||||
* so a late interrupt is dropped rather than pausing a dead job.
|
||||
*/
|
||||
async pause(streamId: string, pendingAction: Agents.PendingAction): Promise<boolean> {
|
||||
async pause(
|
||||
streamId: string,
|
||||
pendingAction: Agents.PendingAction,
|
||||
options?: ApprovalPauseOptions,
|
||||
): Promise<boolean> {
|
||||
const job = await this.store.getJob(streamId);
|
||||
if (!job || job.status !== 'running') {
|
||||
if (
|
||||
!job ||
|
||||
job.status !== 'running' ||
|
||||
(options?.expectedCreatedAt != null && job.createdAt !== options.expectedCreatedAt)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
const discoveredTools = options?.discoveredTools;
|
||||
const expectedCreatedAt = options?.expectedCreatedAt ?? job.createdAt;
|
||||
const ok = await this.store.transitionStatus(streamId, {
|
||||
from: 'running',
|
||||
to: 'requires_action',
|
||||
// pendingActionId is the flat mirror the atomic resolve/expire guard on.
|
||||
patch: { pendingAction, pendingActionId: pendingAction.actionId },
|
||||
expectCreatedAt: job.createdAt,
|
||||
patch: {
|
||||
pendingAction,
|
||||
pendingActionId: pendingAction.actionId,
|
||||
...(discoveredTools != null && discoveredTools.length > 0
|
||||
? { discoveredTools: [...discoveredTools] }
|
||||
: {}),
|
||||
},
|
||||
expectCreatedAt: expectedCreatedAt,
|
||||
});
|
||||
if (ok) {
|
||||
this.callbacks.onPaused?.(streamId, job.createdAt);
|
||||
this.callbacks.onPaused?.(streamId, expectedCreatedAt);
|
||||
logger.debug(
|
||||
`[ApprovalLifecycle] paused for review: ${streamId} action=${pendingAction.actionId}`,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -59,6 +59,40 @@ describe('ApprovalLifecycle via GenerationJobManager.approvals (in-memory)', ()
|
|||
}
|
||||
});
|
||||
|
||||
test('persists discovered tools in the same transition that makes the pause visible', async () => {
|
||||
const streamId = 'stream-pause-discoveries';
|
||||
await manager.createJob(streamId, 'user-1');
|
||||
|
||||
const action = buildAction(streamId);
|
||||
expect(
|
||||
await manager.approvals.pause(streamId, action, {
|
||||
discoveredTools: ['save_issue_mcp_linear'],
|
||||
}),
|
||||
).toBe(true);
|
||||
|
||||
const paused = await manager.getJob(streamId);
|
||||
expect(paused?.status).toBe('requires_action');
|
||||
expect(paused?.metadata.discoveredTools).toEqual(['save_issue_mcp_linear']);
|
||||
});
|
||||
|
||||
test('does not write a stale pause or discoveries onto a replacement job', async () => {
|
||||
const streamId = 'stream-pause-replaced';
|
||||
const original = await manager.createJob(streamId, 'user-1');
|
||||
const replacement = await manager.createJob(streamId, 'user-1');
|
||||
|
||||
expect(
|
||||
await manager.approvals.pause(streamId, buildAction(streamId), {
|
||||
discoveredTools: ['stale_tool'],
|
||||
expectedCreatedAt: original.createdAt,
|
||||
}),
|
||||
).toBe(false);
|
||||
|
||||
const liveJob = await manager.getJob(streamId);
|
||||
expect(liveJob?.createdAt).toBe(replacement.createdAt);
|
||||
expect(liveJob?.status).toBe('running');
|
||||
expect(liveJob?.metadata.discoveredTools).toBeUndefined();
|
||||
});
|
||||
|
||||
test('returns false when the job is already terminal', async () => {
|
||||
const streamId = 'stream-pause-dead';
|
||||
await manager.createJob(streamId, 'user-1');
|
||||
|
|
|
|||
|
|
@ -55,8 +55,8 @@ export interface SerializableJobData {
|
|||
/**
|
||||
* Deferred-tool names discovered (via `tool_search`) before a HITL pause, captured
|
||||
* so a resume can replay them into `createRun` — the rebuilt graph uses `messages: []`
|
||||
* (state comes from the checkpoint), so without these the paused deferred tool would be
|
||||
* absent from the schema-only toolMap and resume would fail with "unknown tool".
|
||||
* (state comes from the checkpoint), so without these the rebuilt model would lose
|
||||
* the discovered tool schemas.
|
||||
*/
|
||||
discoveredTools?: string[];
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ export interface GenerationJobMetadata {
|
|||
/**
|
||||
* Deferred-tool names discovered (via `tool_search`) before a HITL pause. A resume
|
||||
* replays these into `createRun` because the rebuilt graph uses `messages: []`, so
|
||||
* without them the paused deferred tool would be missing from the schema-only toolMap.
|
||||
* without them the rebuilt model would lose the discovered tool schemas.
|
||||
*/
|
||||
discoveredTools?: string[];
|
||||
/** See `SerializableJobData.preemptCapable`. */
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue