🔖 fix: Preserve Ephemeral Agent Params and Identity for ask_user_question Resume (#14254)

* fix: durable ask_user_question resume for ephemeral agents

* 🤖 refactor: Drop chat.js resume hunks in favor of shared packages/api helpers

* 🤖 fix: Normalize resume thinking param and replay modelLabel (#14253 Bugs 1&2)

* 🤖 fix: Preserve adaptive thinking display and effort across HITL resume

* 🔤 style: Sort load.spec.ts imports (repo import-order)

* 🤖 fix: Replay paused request body params on HITL resume (UI-form source of truth)

---------

Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
Jaime Hidalgo 2026-07-14 21:12:43 +02:00 committed by GitHub
parent 02a5b985e4
commit e813934731
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 357 additions and 12 deletions

View file

@ -37,7 +37,7 @@ const {
toClientPendingAction,
computeAgentRequestFingerprint,
extractDiscoveredToolsFromHistory,
sanitizeResumeModelParameters,
captureResumeModelParameters,
pickResumeContext,
getApprovalTtlMs,
isHITLEnabled,
@ -1378,19 +1378,21 @@ class AgentClient extends BaseClient {
const appConfig = this.options.req?.config;
const checkpointerCfg = appConfig?.endpoints?.[EModelEndpoint.agents]?.checkpointer;
// Persist the resolved model parameters (temperature, max tokens, custom endpoint
// params, …) so an ephemeral-agent resume continues with the SAME settings the run
// paused on. The resume payload omits them and they aren't part of the fingerprint, so
// without this the rebuilt ephemeral run falls back to defaults. (Saved agents source
// these from the DB record server-side, so this is belt-and-suspenders for them.)
// Sanitized: the resolved params are the llmConfig, which carries provider secrets
// Persist the generation params (temperature, max tokens, custom endpoint params, …)
// so an ephemeral-agent resume continues with the SAME settings the run paused on.
// The resume payload omits them and they aren't part of the fingerprint, so without
// this the rebuilt ephemeral run falls back to defaults. The paused request body is
// the primary source (UI-form, round-trips the compact-convo schema by construction);
// the resolved llmConfig fills gaps and is sanitized — it carries provider secrets
// (apiKey, credentials) and gateway config — resume re-resolves those server-side.
// (Saved agents source params from the DB record, so this is belt-and-suspenders.)
const resumeContext = pickResumeContext(this.options.req?.body);
const resolvedModelParameters = sanitizeResumeModelParameters(
const resumeModelParameters = captureResumeModelParameters(
this.options.req?.body,
this.options.agent?.model_parameters,
);
if (resolvedModelParameters) {
resumeContext.model_parameters = resolvedModelParameters;
if (resumeModelParameters) {
resumeContext.model_parameters = resumeModelParameters;
}
// Persist the question onto the paused ask tool_call's args NOW: an
// abandoned/expired/stopped pause never reaches the answer-resume stamp,

View file

@ -8,6 +8,7 @@ import {
buildPendingAction,
toClientPendingAction,
computeAgentRequestFingerprint,
captureResumeModelParameters,
sanitizeResumeModelParameters,
pickResumeContext,
applyResumeContext,
@ -356,6 +357,151 @@ describe('sanitizeResumeModelParameters', () => {
expect(sanitizeResumeModelParameters('sk-secret')).toBeUndefined();
expect(sanitizeResumeModelParameters(['sk-secret'])).toBeUndefined();
});
test('normalizes the resolved Anthropic object `thinking` back to the request-body form (#14253)', () => {
// Opus/Sonnet 4+ resolve `thinking` to a provider-format object; replaying it
// verbatim fails the compact-convo `thinking: z.boolean()` field and its
// `.catch(()=>({}))` drops model/spec → missing_model.
expect(
sanitizeResumeModelParameters({
model: 'claude-opus-4-20250514',
thinking: { type: 'enabled', budget_tokens: 2048 },
}),
).toEqual({ model: 'claude-opus-4-20250514', thinking: true, thinkingBudget: 2048 });
expect(sanitizeResumeModelParameters({ thinking: { type: 'disabled' } })).toEqual({
thinking: false,
});
// Boolean thinking (and an explicit thinkingBudget) are left untouched.
expect(sanitizeResumeModelParameters({ thinking: true, thinkingBudget: 4096 })).toEqual({
thinking: true,
thinkingBudget: 4096,
});
expect(
sanitizeResumeModelParameters({
thinking: { type: 'enabled', budget_tokens: 2048 },
thinkingBudget: 4096,
}),
).toEqual({ thinking: true, thinkingBudget: 4096 });
});
test('preserves an explicit adaptive `display` as thinkingDisplay (#14253)', () => {
// Opus 4.7+ adaptive configs carry `display`; dropping it would demote an
// explicit 'omitted' choice back to the default ('summarized') on resume.
expect(
sanitizeResumeModelParameters({ thinking: { type: 'adaptive', display: 'omitted' } }),
).toEqual({ thinking: true, thinkingDisplay: 'omitted' });
// An explicit top-level thinkingDisplay wins over the object's display.
expect(
sanitizeResumeModelParameters({
thinking: { type: 'adaptive', display: 'summarized' },
thinkingDisplay: 'omitted',
}),
).toEqual({ thinking: true, thinkingDisplay: 'omitted' });
});
test('lifts adaptive effort out of invocationKwargs.output_config (#14253)', () => {
// configureReasoning stores a non-default effort at
// invocationKwargs.output_config.effort; the request-body schema only accepts
// the top-level field, so replaying without the lift loses the effort choice.
expect(
sanitizeResumeModelParameters({
thinking: { type: 'adaptive' },
invocationKwargs: { metadata: { user_id: 'u1' }, output_config: { effort: 'max' } },
}),
).toEqual({ thinking: true, effort: 'max' });
// An existing top-level effort wins; invocationKwargs is always dropped.
expect(
sanitizeResumeModelParameters({
effort: 'low',
invocationKwargs: { output_config: { effort: 'max' } },
}),
).toEqual({ effort: 'low' });
expect(
sanitizeResumeModelParameters({ invocationKwargs: { metadata: { user_id: 'u1' } } }),
).toEqual({});
});
});
describe('captureResumeModelParameters', () => {
test('captures UI-form body params the resolved llmConfig renames or drops (#14253)', () => {
// Anthropic resolution renames maxOutputTokens → maxTokens and stop → stopSequences;
// replaying only the resolved form would silently reset those on resume.
expect(
captureResumeModelParameters(
{
text: 'hi',
maxOutputTokens: 8192,
stop: ['END'],
temperature: 0.3,
maxContextTokens: 50000,
},
{ model: 'claude-opus-4', temperature: 0.3, maxTokens: 8192, stopSequences: ['END'] },
),
).toEqual({
model: 'claude-opus-4',
temperature: 0.3,
maxTokens: 8192,
stopSequences: ['END'],
maxOutputTokens: 8192,
stop: ['END'],
maxContextTokens: 50000,
});
});
test('body values win over the normalized resolved values', () => {
expect(
captureResumeModelParameters(
{ thinking: false, effort: 'low' },
{ thinking: { type: 'adaptive' }, invocationKwargs: { output_config: { effort: 'max' } } },
),
).toEqual({ thinking: false, effort: 'low' });
});
test('resolved params still fill gaps the body lacks (normalized to UI form)', () => {
expect(
captureResumeModelParameters(
{},
{
thinking: { type: 'adaptive', display: 'omitted' },
invocationKwargs: { output_config: { effort: 'max' } },
},
),
).toEqual({ thinking: true, thinkingDisplay: 'omitted', effort: 'max' });
expect(captureResumeModelParameters({ temperature: 0.5 }, undefined)).toEqual({
temperature: 0.5,
});
});
test('only replays schema-known generation params; identity fields stay owned elsewhere', () => {
// model/spec/modelLabel/promptPrefix ride RESUME_CONTEXT_KEYS; text/files/etc.
// never reach model_parameters (parseCompactConvo strips them).
expect(
captureResumeModelParameters(
{
model: 'gpt-5',
spec: 'my-spec',
modelLabel: 'My Opus',
promptPrefix: 'be nice',
text: 'hello',
conversationId: 'c1',
top_p: 0.9,
},
undefined,
),
).toEqual({ top_p: 0.9 });
expect(captureResumeModelParameters({ text: 'hello' }, undefined)).toBeUndefined();
});
test('sanitizes sensitive keys inside captured body values', () => {
expect(
captureResumeModelParameters(
{ additionalModelRequestFields: { apiKey: 'sk-live', anthropic_beta: ['x'] } },
undefined,
),
).toEqual({ additionalModelRequestFields: { anthropic_beta: ['x'] } });
});
});
describe('computeAgentRequestFingerprint', () => {
@ -434,6 +580,8 @@ describe('pickResumeContext / applyResumeContext', () => {
timezone: 'America/New_York',
// Graph-determining: skill allowed-tools union into the tool set.
manualSkills: ['code-reviewer'],
// Graph-determining: feeds the ephemeral agent id / checkpoint namespace (#14253).
modelLabel: 'My Opus',
conversationId: 'c',
decisions: [],
actionId: 'x',
@ -447,9 +595,22 @@ describe('pickResumeContext / applyResumeContext', () => {
addedConvo: { agent_id: 'secondary' },
timezone: 'America/New_York',
manualSkills: ['code-reviewer'],
modelLabel: 'My Opus',
});
});
it('replays a dropped modelLabel so the ephemeral agent id stays stable (#14253)', () => {
// Resume/reload case: the resolved llmConfig stripped modelLabel; the server restores
// the original top-level value so parseCompactConvo re-derives the same sender/id.
const restored: Record<string, unknown> = { conversationId: 'c', actionId: 'x' };
applyResumeContext(restored, { endpoint: 'my-custom-endpoint', modelLabel: 'My Opus' });
expect(restored.modelLabel).toBe('My Opus');
// A paused turn with no modelLabel can't be made to inject one.
const injected: Record<string, unknown> = { conversationId: 'c', modelLabel: 'Spoofed' };
applyResumeContext(injected, { endpoint: 'my-custom-endpoint' });
expect('modelLabel' in injected).toBe(false);
});
it('replays a dropped manualSkills and drops a client-injected one', () => {
// Reload case: the resume client lost manualSkills; the server restores it.
const restored: Record<string, unknown> = { conversationId: 'c', actionId: 'x' };

View file

@ -1,4 +1,5 @@
import { randomUUID, createHash } from 'crypto';
import { openAIBaseSchema, googleBaseSchema, anthropicBaseSchema } from 'librechat-data-provider';
import type { Agents, TToolApprovalPolicy } from 'librechat-data-provider';
import type { ToolPolicyConfig } from '@librechat/agents';
@ -236,6 +237,16 @@ export const RESUME_CONTEXT_KEYS = [
// different skill's tools (manualSkills isn't covered by the fingerprint). Replay-only.
// (alwaysAppliedSkills is NOT here — it's resolved server-side from the DB, not req.body.)
'manualSkills',
// Graph-determining for ephemeral agents: `loadEphemeralAgent` encodes the agent id
// (and thus the LangGraph node name / HITL checkpoint namespace) from
// `sender = modelLabel ?? modelSpec.label ?? …`. `modelLabel` is stripped from the
// RESOLVED llmConfig captured at pause (sanitizeResumeModelParameters reads the
// initialized agent's model_parameters), so without replaying the original request
// value the resumed id falls back to modelSpec.label → a DIFFERENT id → the interrupt
// checkpoint (namespaced by the paused id) can't be re-entered → empty-graph resume
// (#14253). It rides top-level on req.body and flows into model_parameters via the
// build spread, so replaying it here restores the stable id. Replay-only.
'modelLabel',
] as const;
export type ResumeContext = Partial<Record<(typeof RESUME_CONTEXT_KEYS)[number], unknown>> & {
@ -328,6 +339,59 @@ function sanitizeParamValue(value: unknown, depth: number): unknown {
return value;
}
/**
* The resolved Anthropic `thinking` parameter is a provider-format object
* (`{ type: 'enabled' | 'disabled' | 'adaptive', budget_tokens? }`) for Opus/Sonnet
* 4+, but the request body and the compact-convo schema (`thinking: z.boolean()`)
* that the resume replay is validated against expects the UI form. A stray object
* fails that field, and the schema's `.catch(() => ({}))` drops the WHOLE parse
* (`model`/`spec` included), surfacing as `missing_model` on resume of a
* custom-endpoint ephemeral agent (#14253). Convert it back to
* `{ thinking: boolean, thinkingBudget?, thinkingDisplay? }` so the replayed params
* round-trip cleanly (an explicit `display: 'omitted'` choice survives too).
*/
function normalizeThinkingParam(params: Record<string, unknown>): void {
const thinking = params.thinking;
if (thinking == null || typeof thinking !== 'object' || Array.isArray(thinking)) {
return;
}
const {
type,
display,
budget_tokens: budget,
} = thinking as {
type?: unknown;
display?: unknown;
budget_tokens?: unknown;
};
params.thinking = type !== 'disabled';
if (params.thinkingBudget == null && typeof budget === 'number') {
params.thinkingBudget = budget;
}
if (params.thinkingDisplay == null && typeof display === 'string') {
params.thinkingDisplay = display;
}
}
/**
* A non-default adaptive-thinking effort resolves into
* `invocationKwargs.output_config.effort` (see `configureReasoning`), while the
* request-body schema only accepts a top-level `effort`. Lift it back so the resumed
* turn keeps the paused run's effort, and drop `invocationKwargs` entirely — it's
* resolved transport config the compact-convo schema would discard anyway.
*/
function normalizeEffortParam(params: Record<string, unknown>): void {
const kwargs = params.invocationKwargs as { output_config?: { effort?: unknown } } | undefined;
if (kwargs == null || typeof kwargs !== 'object') {
return;
}
const effort = kwargs.output_config?.effort;
if (params.effort == null && typeof effort === 'string') {
params.effort = effort;
}
delete params.invocationKwargs;
}
/**
* Strip credentials and server transport config from resolved model parameters before
* they are persisted for resume replay. The initialized agent's `model_parameters` are
@ -335,7 +399,9 @@ function sanitizeParamValue(value: unknown, depth: number): unknown {
* Google `authOptions`, Bedrock `credentials`) and gateway config (`configuration`,
* headers, base URLs). Resume re-resolves all of those server-side from env/config, so
* only the user-level generation params (temperature, max tokens, custom endpoint
* params, ) need to survive the round trip.
* params, ) need to survive the round trip. Provider-format params that conflict with
* the request-body schema on replay are normalized back to the UI form (see
* {@link normalizeThinkingParam}).
*/
export function sanitizeResumeModelParameters(
params: unknown,
@ -343,7 +409,71 @@ export function sanitizeResumeModelParameters(
if (params == null || typeof params !== 'object' || Array.isArray(params)) {
return undefined;
}
return sanitizeParamValue(params, 0) as Record<string, unknown>;
const sanitized = sanitizeParamValue(params, 0) as Record<string, unknown>;
normalizeThinkingParam(sanitized);
normalizeEffortParam(sanitized);
return sanitized;
}
/** Bedrock body params its compact schema accepts; hand-listed because
* `bedrockInputSchema` wraps the pick in a transform, hiding `.shape`. */
const BEDROCK_PARAM_KEYS = [
'region',
'system',
'maxTokens',
'reasoning_effort',
'additionalModelRequestFields',
];
/** Schema-accepted keys owned elsewhere: replayed via {@link RESUME_CONTEXT_KEYS}
* (`model`, `spec`, `promptPrefix`, `modelLabel`) or derived server-side / identity
* fields the resume request must keep as its own. */
const RESUME_PARAM_EXCLUDED = new Set([
'model',
'spec',
'iconURL',
'greeting',
'modelLabel',
'promptPrefix',
'chatProjectId',
]);
/**
* Request-body generation params worth replaying on resume: the union of the
* compact-convo schemas' fields. Only these keys can influence the rebuilt run
* `buildOptions` derives `model_parameters` from the PARSED body, and
* `parseCompactConvo` strips everything else.
*/
const RESUME_PARAM_KEYS: string[] = Array.from(
new Set(
[openAIBaseSchema, anthropicBaseSchema, googleBaseSchema]
.flatMap((schema) => Object.keys(schema.shape))
.concat(BEDROCK_PARAM_KEYS),
),
).filter((key) => !RESUME_PARAM_EXCLUDED.has(key));
/**
* Capture the model parameters to replay on resume. The paused request body is the
* primary source its fields are UI-form by construction (they already round-tripped
* `parseCompactConvo` on the original turn), so replaying them can't trip the schema.
* The resolved llmConfig only fills gaps: it's provider-format, where params are
* renamed (`maxOutputTokens` `maxTokens`, `top_p` `topP`), relocated
* (`effort` `invocationKwargs`), or retyped (`thinking` object) the schema
* silently drops or, worse, fails on them (see the `normalize*` helpers, #14253).
*/
export function captureResumeModelParameters(
body: Record<string, unknown> | undefined | null,
resolvedParams: unknown,
): Record<string, unknown> | undefined {
const captured = sanitizeResumeModelParameters(resolvedParams) ?? {};
if (body != null && typeof body === 'object') {
for (const key of RESUME_PARAM_KEYS) {
if (body[key] !== undefined) {
captured[key] = sanitizeParamValue(body[key], 1);
}
}
}
return Object.keys(captured).length > 0 ? captured : undefined;
}
/** Extract the graph-determining fields from a request body for durable replay. */

View file

@ -0,0 +1,52 @@
import type { LoadAgentDeps } from './load';
import { loadEphemeralAgent } from './load';
const deps: LoadAgentDeps = {
getAgent: async () => null,
getMCPServerTools: async () => null,
};
const baseReq = {
user: { id: 'user-1' },
config: {
modelSpecs: { list: [{ name: 'my-opus-spec', label: 'Spec Label' }] },
},
body: {},
} as unknown as Parameters<typeof loadEphemeralAgent>[0]['req'];
async function idFor(modelParameters: Record<string, unknown>) {
const agent = await loadEphemeralAgent(
{
req: baseReq,
spec: 'my-opus-spec',
endpoint: 'my-custom-endpoint',
model_parameters: modelParameters as never,
},
deps,
);
return agent?.id;
}
/**
* Documents the #14253 Bug 2 mechanism: the ephemeral agent id (LangGraph node /
* HITL checkpoint namespace) is derived from `sender = modelLabel ?? modelSpec.label`.
* When the resume drops `modelLabel`, the id drifts and the paused checkpoint can't be
* re-entered. The fix keeps `modelLabel` across resume (RESUME_CONTEXT_KEYS), so the
* original and resumed ids stay equal.
*/
describe('loadEphemeralAgent ephemeral id stability (#14253 Bug 2)', () => {
test('id changes when modelLabel is lost vs preserved', async () => {
const withLabel = await idFor({ model: 'claude-opus-4', modelLabel: 'My Opus' });
const withoutLabel = await idFor({ model: 'claude-opus-4' });
expect(withLabel).toBeTruthy();
expect(withoutLabel).toBeTruthy();
// Original turn (has modelLabel) vs a resume that dropped it → different namespace.
expect(withLabel).not.toEqual(withoutLabel);
});
test('id is stable when modelLabel is preserved across turns', async () => {
const a = await idFor({ model: 'claude-opus-4', modelLabel: 'My Opus' });
const b = await idFor({ model: 'claude-opus-4', modelLabel: 'My Opus' });
expect(a).toEqual(b);
});
});