🧭 refactor: Align HITL Surface with Agents SDK Permissions Model

Reshapes Slice A on top of the agents SDK's now-landed HITL surface
(`createToolPolicyHook`, discriminated `HumanInterruptPayload`, `'bypass'`
mode naming). Host stops reimplementing evaluation logic and becomes a
config mapper + payload wrapper.

Schema (data-provider):
- `toolApproval` shape now mirrors SDK `ToolPolicyConfig` 1:1:
  `mode: 'default' | 'dontAsk' | 'bypass'`, plus `allow` / `deny` / `ask`
  glob lists and an optional `reason` template. `enabled` is the
  LibreChat-only admin kill switch.
- `'bypass'` (not `'bypassPermissions'`) — matches the SDK's surface.

Types (`Agents.*` namespace):
- `HumanInterruptType` extended to `'tool_approval' | 'ask_user_question'`.
- `HumanInterruptPayload` is now a discriminated union — `tool_approval`
  carries `action_requests` + `review_configs`; `ask_user_question`
  carries a free-form question with optional curated options.
- New: `AskUserQuestionRequest`, `AskUserQuestionOption`,
  `AskUserQuestionResolution`.
- `ToolApprovalDecision` (string union) renamed to
  `ToolApprovalDecisionType` to free the `Decision` name for the SDK's
  discriminated object union later.
- `ToolApprovalResolution` gains `reason?` and `scope?: 'once' | 'session'
  | 'always'` so route signatures stabilize before persistence lands.

Policy module (`packages/api/src/agents/hitl/policy.ts`):
- Drop `decideToolApproval` / `requiresApproval` / `ToolRef` — the SDK's
  `createToolPolicyHook` handles full evaluation
  (`deny → bypass → allow → ask → dontAsk → fallthrough(ask)`).
- Add `isHITLEnabled(policy)` — the kill-switch predicate that gates the
  SDK's `humanInTheLoop: { enabled: false }` opt-out in Slice B.
- Add `mapToolApprovalPolicy(policy)` — strips `enabled`, returns a
  `ToolPolicyConfig` to feed `createToolPolicyHook`. Structural mirror of
  the SDK type so this compiles before the SDK upgrade ships.
- Reshape `buildPendingAction(payload, ctx)` to wrap any
  `HumanInterruptPayload` with job context — accepts SDK output directly.
- Add `buildToolApprovalPayload(...)` and `buildAskUserQuestionPayload(...)`
  helpers for synthesizing payloads in tests / pre-SDK flows.

Tests:
- 22 new unit tests covering the mapper, predicate, and payload builders;
  20 → 27 total pass across policy + manager-lifecycle suites.
This commit is contained in:
Danny Avila 2026-05-04 05:18:51 +09:00
parent b2008931b1
commit 2335ae2805
5 changed files with 371 additions and 194 deletions

View file

@ -1,125 +1,198 @@
import type { TToolApprovalPolicy } from 'librechat-data-provider';
import { decideToolApproval, requiresApproval, buildPendingAction } from './policy';
import type { Agents, TToolApprovalPolicy } from 'librechat-data-provider';
import {
isHITLEnabled,
mapToolApprovalPolicy,
buildToolApprovalPayload,
buildAskUserQuestionPayload,
buildPendingAction,
} from './policy';
describe('decideToolApproval', () => {
test('returns "allow" when no policy is configured', () => {
expect(decideToolApproval(undefined, { name: 'shell' })).toBe('allow');
describe('isHITLEnabled', () => {
test('default-on when no policy configured (SDK default)', () => {
expect(isHITLEnabled(undefined)).toBe(true);
});
test('returns the configured default when no rule matches', () => {
const policy: TToolApprovalPolicy = { default: 'ask' };
expect(decideToolApproval(policy, { name: 'unmapped' })).toBe('ask');
test('default-on when policy is configured but `enabled` is omitted', () => {
expect(isHITLEnabled({})).toBe(true);
expect(isHITLEnabled({ mode: 'default', allow: ['read_*'] })).toBe(true);
});
test('falls back to "allow" when default is omitted', () => {
const policy: TToolApprovalPolicy = { required: ['shell'] };
expect(decideToolApproval(policy, { name: 'web_search' })).toBe('allow');
test('explicit false is the only off signal', () => {
expect(isHITLEnabled({ enabled: false })).toBe(false);
});
test('returns "ask" when tool is in required list', () => {
const policy: TToolApprovalPolicy = { required: ['shell', 'execute_code'] };
expect(decideToolApproval(policy, { name: 'execute_code' })).toBe('ask');
});
test('returns "allow" for tools in excluded list, even when default is "ask"', () => {
const policy: TToolApprovalPolicy = { default: 'ask', excluded: ['web_search'] };
expect(decideToolApproval(policy, { name: 'web_search' })).toBe('allow');
});
test('excluded wins over required when a tool appears in both (defensive)', () => {
const policy: TToolApprovalPolicy = {
default: 'allow',
required: ['shell'],
excluded: ['shell'],
};
expect(decideToolApproval(policy, { name: 'shell' })).toBe('allow');
});
test('returns the default when tool name is missing', () => {
const policy: TToolApprovalPolicy = { default: 'ask', required: ['shell'] };
expect(decideToolApproval(policy, {})).toBe('ask');
test('explicit true is on', () => {
expect(isHITLEnabled({ enabled: true })).toBe(true);
});
});
describe('requiresApproval', () => {
test('true only when decision is "ask"', () => {
const policy: TToolApprovalPolicy = { required: ['shell'] };
expect(requiresApproval(policy, { name: 'shell' })).toBe(true);
expect(requiresApproval(policy, { name: 'web_search' })).toBe(false);
expect(requiresApproval(undefined, { name: 'shell' })).toBe(false);
describe('mapToolApprovalPolicy', () => {
test('returns undefined when no policy is configured', () => {
expect(mapToolApprovalPolicy(undefined)).toBeUndefined();
});
test('returns undefined when policy is empty after stripping enabled', () => {
expect(mapToolApprovalPolicy({ enabled: true })).toBeUndefined();
expect(mapToolApprovalPolicy({ enabled: false })).toBeUndefined();
});
test('returns undefined when only empty arrays are present', () => {
expect(mapToolApprovalPolicy({ allow: [], deny: [], ask: [] })).toBeUndefined();
});
test('passes through mode/allow/deny/ask/reason verbatim', () => {
const policy: TToolApprovalPolicy = {
mode: 'dontAsk',
allow: ['read_*', 'mcp:github:*'],
deny: ['delete_*'],
ask: ['execute_*'],
reason: 'Tool {tool} requires review',
};
expect(mapToolApprovalPolicy(policy)).toEqual({
mode: 'dontAsk',
allow: ['read_*', 'mcp:github:*'],
deny: ['delete_*'],
ask: ['execute_*'],
reason: 'Tool {tool} requires review',
});
});
test('strips enabled regardless of value (LibreChat-only field)', () => {
expect(mapToolApprovalPolicy({ enabled: false, mode: 'bypass' })).toEqual({
mode: 'bypass',
});
expect(mapToolApprovalPolicy({ enabled: true, allow: ['read_*'] })).toEqual({
allow: ['read_*'],
});
});
test('omits empty list fields from the output', () => {
expect(mapToolApprovalPolicy({ mode: 'default', allow: [], deny: ['rm'] })).toEqual({
mode: 'default',
deny: ['rm'],
});
});
});
describe('buildToolApprovalPayload', () => {
const calls = [
{
name: 'shell',
arguments: { command: 'ls' },
tool_call_id: 'call_abc',
description: 'List files',
},
];
test('produces a tool_approval-discriminated payload', () => {
const payload = buildToolApprovalPayload(calls);
expect(payload.type).toBe('tool_approval');
expect(payload.action_requests).toEqual([
{
name: 'shell',
arguments: { command: 'ls' },
tool_call_id: 'call_abc',
description: 'List files',
},
]);
});
test("default decisions exclude 'respond' (reserved for AskUserQuestion semantics)", () => {
const payload = buildToolApprovalPayload(calls);
expect(payload.review_configs[0].allowed_decisions).toEqual(['approve', 'reject', 'edit']);
});
test('respects per-tool decision overrides', () => {
const payload = buildToolApprovalPayload(calls, {
shell: ['approve', 'reject'],
});
expect(payload.review_configs[0].allowed_decisions).toEqual(['approve', 'reject']);
});
test('produces one review_config per call, in order', () => {
const payload = buildToolApprovalPayload([
{ name: 'a', arguments: {}, tool_call_id: '1' },
{ name: 'b', arguments: {}, tool_call_id: '2' },
]);
expect(payload.review_configs.map((r) => r.action_name)).toEqual(['a', 'b']);
});
});
describe('buildAskUserQuestionPayload', () => {
test('produces an ask_user_question-discriminated payload', () => {
const payload = buildAskUserQuestionPayload({
question: 'Which environment?',
options: [
{ label: 'Staging', value: 'staging' },
{ label: 'Production', value: 'production' },
],
});
expect(payload.type).toBe('ask_user_question');
expect(payload.question.question).toBe('Which environment?');
expect(payload.question.options).toHaveLength(2);
});
test('options are optional', () => {
const payload = buildAskUserQuestionPayload({ question: 'Free-form?' });
expect(payload.question.options).toBeUndefined();
});
});
describe('buildPendingAction', () => {
const baseInput = {
const ctx = {
streamId: 'stream-1',
conversationId: 'conv-1',
runId: 'run-1',
responseMessageId: 'msg-1',
toolCalls: [
{
name: 'shell',
arguments: { command: 'ls' },
tool_call_id: 'call_abc',
description: 'List files',
},
],
};
test('produces a payload mirroring LangChain HumanInterrupt shape', () => {
const action = buildPendingAction(baseInput);
expect(action.payload.type).toBe('tool_approval');
expect(action.payload.action_requests).toEqual([
{
name: 'shell',
arguments: { command: 'ls' },
tool_call_id: 'call_abc',
description: 'List files',
},
]);
expect(action.payload.review_configs).toEqual([
{ action_name: 'shell', allowed_decisions: ['approve', 'reject', 'edit'] },
]);
const toolApprovalPayload: Agents.ToolApprovalInterruptPayload = {
type: 'tool_approval',
action_requests: [{ name: 'shell', arguments: { command: 'ls' }, tool_call_id: 'call_abc' }],
review_configs: [{ action_name: 'shell', allowed_decisions: ['approve', 'reject'] }],
};
test('wraps a tool_approval payload with job context', () => {
const action = buildPendingAction(toolApprovalPayload, ctx);
expect(action.streamId).toBe('stream-1');
expect(action.conversationId).toBe('conv-1');
expect(action.runId).toBe('run-1');
expect(action.responseMessageId).toBe('msg-1');
expect(action.payload).toBe(toolApprovalPayload);
expect(typeof action.createdAt).toBe('number');
});
test('respects per-tool decision overrides', () => {
const action = buildPendingAction({
...baseInput,
decisionsByToolName: { shell: ['approve', 'reject'] },
});
expect(action.payload.review_configs[0].allowed_decisions).toEqual(['approve', 'reject']);
test('wraps an ask_user_question payload with the same envelope', () => {
const askPayload: Agents.AskUserQuestionInterruptPayload = {
type: 'ask_user_question',
question: { question: 'Which env?' },
};
const action = buildPendingAction(askPayload, ctx);
expect(action.payload.type).toBe('ask_user_question');
});
test('generates a uuid actionId by default', () => {
const a = buildPendingAction(baseInput);
const b = buildPendingAction(baseInput);
const a = buildPendingAction(toolApprovalPayload, ctx);
const b = buildPendingAction(toolApprovalPayload, ctx);
expect(a.actionId).not.toBe(b.actionId);
expect(a.actionId).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i);
});
test('honours an explicit actionId', () => {
const action = buildPendingAction({ ...baseInput, actionId: 'fixed-id' });
const action = buildPendingAction(toolApprovalPayload, { ...ctx, actionId: 'fixed-id' });
expect(action.actionId).toBe('fixed-id');
});
test('sets expiresAt only when ttlMs is provided', () => {
const without = buildPendingAction(baseInput);
const without = buildPendingAction(toolApprovalPayload, ctx);
expect(without.expiresAt).toBeUndefined();
const ttl = 5_000;
const before = Date.now();
const withTtl = buildPendingAction({ ...baseInput, ttlMs: ttl });
const withTtl = buildPendingAction(toolApprovalPayload, { ...ctx, ttlMs: ttl });
const after = Date.now();
expect(withTtl.expiresAt).toBeDefined();
expect(withTtl.expiresAt).toBeGreaterThanOrEqual(before + ttl);
expect(withTtl.expiresAt).toBeLessThanOrEqual(after + ttl);
});
test('preserves stream/conversation/run identifiers', () => {
const action = buildPendingAction(baseInput);
expect(action.streamId).toBe('stream-1');
expect(action.conversationId).toBe('conv-1');
expect(action.runId).toBe('run-1');
expect(action.responseMessageId).toBe('msg-1');
});
});

View file

@ -1,76 +1,125 @@
import { randomUUID } from 'crypto';
import type { Agents, TToolApprovalPolicy, ToolApprovalDecision } from 'librechat-data-provider';
import type { Agents, TToolApprovalPolicy } from 'librechat-data-provider';
/**
* Default decision set offered to the user for a paused tool call.
* Default decisions offered to the user for a paused tool call.
*
* `approve` runs the tool as-is, `reject` blocks it with a rejection message,
* `edit` re-runs it with user-supplied arguments. `respond` is reserved for
* the future ask-user-question flow and intentionally NOT in the default set.
* `'respond'` is intentionally NOT in the default set: it represents the agent
* substituting a synthetic tool result, which is rarely the right ergonomic for
* a stock approval prompt. Hosts that want it can pass an override.
*/
const DEFAULT_REVIEW_DECISIONS: Agents.ToolApprovalDecision[] = ['approve', 'reject', 'edit'];
const DEFAULT_REVIEW_DECISIONS: Agents.ToolApprovalDecisionType[] = ['approve', 'reject', 'edit'];
/**
* Tool reference accepted by the policy resolver.
* Loosened from `Agents.ToolCall` so callers can pass minimal shapes
* (e.g. SDK hook payloads, MCP-derived names) without re-typing.
* Structural mirror of `@librechat/agents`'s `ToolPolicyConfig`.
*
* Defined here (rather than imported) so this module compiles before the SDK
* version that ships `createToolPolicyHook` is published. When the SDK is
* pinned, callers in Slice B can `import type { ToolPolicyConfig }` and
* the structural identity holds.
*/
export interface ToolRef {
name?: string;
export interface ToolPolicyConfig {
mode?: 'default' | 'dontAsk' | 'bypass';
allow?: string[];
deny?: string[];
ask?: string[];
reason?: string;
}
/**
* Decide whether a tool call requires human approval, denial, or can run as-is.
* Whether the HITL machinery should run for this policy.
*
* Resolution order:
* 1. `excluded` (always allow) wins over everything else.
* 2. `required` (always ask) wins over the default.
* 3. Falls back to `policy.default`, which itself defaults to `'allow'`.
*
* Returns `'allow'` when no policy is configured or the tool name is missing.
* `false` is the LibreChat-only admin kill switch it disables the SDK
* checkpointer fallback and skips installing the policy hook entirely.
* Users wanting "stop asking me" should use `mode: 'bypass'` instead, which
* keeps the machinery in place but auto-approves.
*/
export function decideToolApproval(
export function isHITLEnabled(policy: TToolApprovalPolicy | undefined): boolean {
return policy?.enabled !== false;
}
/**
* Map a LibreChat tool-approval policy to the SDK's `ToolPolicyConfig`.
*
* Returns `undefined` when there's nothing to configure (so the SDK's own
* defaults apply). The `enabled` field is LibreChat-only and stripped here
* it's consumed separately via {@link isHITLEnabled} to gate the SDK opt-out.
*/
export function mapToolApprovalPolicy(
policy: TToolApprovalPolicy | undefined,
tool: ToolRef,
): ToolApprovalDecision {
): ToolPolicyConfig | undefined {
if (!policy) {
return 'allow';
return undefined;
}
const name = tool.name;
const fallback = policy.default ?? 'allow';
if (!name) {
return fallback;
const config: ToolPolicyConfig = {};
if (policy.mode) {
config.mode = policy.mode;
}
if (policy.excluded?.includes(name)) {
return 'allow';
if (policy.allow && policy.allow.length > 0) {
config.allow = policy.allow;
}
if (policy.required?.includes(name)) {
return 'ask';
if (policy.deny && policy.deny.length > 0) {
config.deny = policy.deny;
}
return fallback;
if (policy.ask && policy.ask.length > 0) {
config.ask = policy.ask;
}
if (policy.reason) {
config.reason = policy.reason;
}
return Object.keys(config).length > 0 ? config : undefined;
}
/** Convenience wrapper. True when the tool call should be paused for human review. */
export function requiresApproval(policy: TToolApprovalPolicy | undefined, tool: ToolRef): boolean {
return decideToolApproval(policy, tool) === 'ask';
/** Tool-call shape consumed by {@link buildToolApprovalPayload}. */
export interface ToolApprovalCallInput {
name: string;
arguments: string | Record<string, unknown>;
tool_call_id: string;
description?: string;
}
/** Input shape for {@link buildPendingAction}. */
export interface BuildPendingActionInput {
/**
* Build a tool-approval interrupt payload from one or more paused tool calls.
*
* Mirrors the SDK's `ToolApprovalInterruptPayload` shape so this can be used
* to synthesize payloads in tests, or by the host before the SDK upgrade ships.
*/
export function buildToolApprovalPayload(
toolCalls: ToolApprovalCallInput[],
decisionsByToolName?: Record<string, Agents.ToolApprovalDecisionType[]>,
): Agents.ToolApprovalInterruptPayload {
return {
type: 'tool_approval',
action_requests: toolCalls.map((tc) => ({
name: tc.name,
arguments: tc.arguments,
tool_call_id: tc.tool_call_id,
description: tc.description,
})),
review_configs: toolCalls.map((tc) => ({
action_name: tc.name,
allowed_decisions: decisionsByToolName?.[tc.name] ?? DEFAULT_REVIEW_DECISIONS,
})),
};
}
/** Build an ask-user-question interrupt payload. */
export function buildAskUserQuestionPayload(
question: Agents.AskUserQuestionRequest,
): Agents.AskUserQuestionInterruptPayload {
return {
type: 'ask_user_question',
question,
};
}
/** Job-context fields wrapped around a {@link Agents.HumanInterruptPayload}. */
export interface PendingActionContext {
streamId: string;
conversationId?: string;
/** Stable per-turn identifier (e.g. responseMessageId or LangGraph checkpoint_ns). */
runId?: string;
responseMessageId?: string;
/** One entry per tool call awaiting review in this interrupt. */
toolCalls: Array<{
name: string;
arguments: string | Record<string, unknown>;
tool_call_id: string;
description?: string;
}>;
/** Override decisions per tool name. Falls back to {@link DEFAULT_REVIEW_DECISIONS}. */
decisionsByToolName?: Record<string, Agents.ToolApprovalDecision[]>;
/** Optional TTL (ms). When set, `expiresAt = createdAt + ttlMs`. */
ttlMs?: number;
/** Override actionId; defaults to a fresh uuid. */
@ -78,36 +127,25 @@ export interface BuildPendingActionInput {
}
/**
* Build a {@link Agents.PendingAction} record from one or more paused tool calls.
* Wrap a HumanInterruptPayload (from the SDK or synthesized locally) as a
* {@link Agents.PendingAction} record persisted with the job.
*
* The resulting `payload` mirrors LangChain HITL middleware's `HumanInterrupt` shape
* (`action_requests` + `review_configs`) so it can be forwarded directly when the
* SDK adopts native HITL primitives.
* Accepts both interrupt categories (`tool_approval` and `ask_user_question`)
* via the discriminated union the host doesn't need to branch.
*/
export function buildPendingAction(input: BuildPendingActionInput): Agents.PendingAction {
export function buildPendingAction(
payload: Agents.HumanInterruptPayload,
ctx: PendingActionContext,
): Agents.PendingAction {
const createdAt = Date.now();
const action_requests: Agents.ToolApprovalRequest[] = input.toolCalls.map((tc) => ({
name: tc.name,
arguments: tc.arguments,
tool_call_id: tc.tool_call_id,
description: tc.description,
}));
const review_configs: Agents.ToolReviewConfig[] = input.toolCalls.map((tc) => ({
action_name: tc.name,
allowed_decisions: input.decisionsByToolName?.[tc.name] ?? DEFAULT_REVIEW_DECISIONS,
}));
return {
actionId: input.actionId ?? randomUUID(),
streamId: input.streamId,
conversationId: input.conversationId,
runId: input.runId,
responseMessageId: input.responseMessageId,
payload: {
type: 'tool_approval',
action_requests,
review_configs,
},
actionId: ctx.actionId ?? randomUUID(),
streamId: ctx.streamId,
conversationId: ctx.conversationId,
runId: ctx.runId,
responseMessageId: ctx.responseMessageId,
payload,
createdAt,
expiresAt: input.ttlMs ? createdAt + input.ttlMs : undefined,
expiresAt: ctx.ttlMs ? createdAt + ctx.ttlMs : undefined,
};
}

View file

@ -2,7 +2,7 @@ import type { Agents } from 'librechat-data-provider';
import { InMemoryEventTransport } from '~/stream/implementations/InMemoryEventTransport';
import { InMemoryJobStore } from '~/stream/implementations/InMemoryJobStore';
import { GenerationJobManagerClass } from '~/stream/GenerationJobManager';
import { buildPendingAction } from '~/agents/hitl/policy';
import { buildPendingAction, buildToolApprovalPayload } from '~/agents/hitl/policy';
jest.spyOn(console, 'log').mockImplementation();
@ -25,12 +25,14 @@ describe('GenerationJobManager pending-action lifecycle (in-memory)', () => {
});
function buildAction(streamId: string, overrides: Partial<Agents.PendingAction> = {}) {
const action = buildPendingAction({
const payload = buildToolApprovalPayload([
{ name: 'shell', arguments: { command: 'ls' }, tool_call_id: 'call_abc' },
]);
const action = buildPendingAction(payload, {
streamId,
conversationId: streamId,
runId: 'run-1',
responseMessageId: 'msg-1',
toolCalls: [{ name: 'shell', arguments: { command: 'ls' }, tool_call_id: 'call_abc' }],
});
return { ...action, ...overrides };
}
@ -48,7 +50,10 @@ describe('GenerationJobManager pending-action lifecycle (in-memory)', () => {
const pending = await manager.getPendingAction(streamId);
expect(pending).not.toBeNull();
expect(pending?.actionId).toBe(action.actionId);
expect(pending?.payload.action_requests[0].name).toBe('shell');
expect(pending?.payload.type).toBe('tool_approval');
if (pending?.payload.type === 'tool_approval') {
expect(pending.payload.action_requests[0].name).toBe('shell');
}
});
test('getPendingAction returns null for jobs not in requires_action', async () => {