Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions src/tools/ToolNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ import {
resolveLocalExecutionTools,
} from '@/tools/local';
import { stripCodeSessionFileSummary } from '@/tools/CodeSessionFileSummary';
import { formatToolErrorContent } from '@/tools/toolErrorContent';
import { Constants, GraphEvents, CODE_EXECUTION_TOOLS } from '@/common';
import { formatToolErrorContent } from '@/tools/toolErrorContent';
import { PreparedSubagentError } from '@/tools/preparedSubagents';
import { attachRunStepResumeState } from '@/tools/runStepResume';

Expand Down Expand Up @@ -2621,12 +2621,14 @@ export class ToolNode<T = any> extends RunnableCallable<T, T> {

if (preResult.decision === 'ask' || reviewedApproval != null) {
if (this.humanInTheLoop?.enabled !== true) {
// Fail-closed: no HITL UI configured, so we can't actually
// ask. Logged once via the existing helper.
const reason = this.resolveAskDecisionForDirectTool(
preResult.reason,
call.name
);
// Fail closed when there is no foreground approval channel.
const reason =
this.humanInTheLoop?.backgroundPausePolicy === 'deny'
? this.backgroundApprovalReason(call.name, preResult.reason)
: this.resolveAskDecisionForDirectTool(
preResult.reason,
call.name
);
return persistOutput(
this.blockDirectCall({
call,
Expand Down Expand Up @@ -3008,6 +3010,13 @@ export class ToolNode<T = any> extends RunnableCallable<T, T> {
* LangGraph `interrupt()` instead — see `runDirectToolWithLifecycleHooks`.
*/
private askDirectWarningEmitted = false;
private backgroundApprovalReason(toolName: string, reason?: string): string {
return (
`Approval required for "${toolName}"; unavailable in a background subagent. ` +
`Ask the parent to run it in the foreground.${reason == null ? '' : ` Reason: ${reason}`}`
);
}

private resolveAskDecisionForDirectTool(
reason: string | undefined,
toolName: string
Expand Down Expand Up @@ -3810,7 +3819,15 @@ export class ToolNode<T = any> extends RunnableCallable<T, T> {
* JSDoc for the full rationale and the migration plan.
*/
if (this.humanInTheLoop?.enabled !== true) {
blockEntry(entry, hookResult.reason ?? 'Blocked by hook');
blockEntry(
entry,
this.humanInTheLoop?.backgroundPausePolicy === 'deny'
? this.backgroundApprovalReason(
entry.call.name,
hookResult.reason
)
: (hookResult.reason ?? 'Blocked by hook')
);
continue;
}
/**
Expand Down
153 changes: 153 additions & 0 deletions src/tools/__tests__/SubagentExecutor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,159 @@ describe('SubagentExecutor', () => {
throw new Error(`Timed out waiting for background task ${taskId}.`);
}

it('preserves the legacy background HITL rejection when requested', () => {
const store = new InMemorySubagentTaskStore();
const executor = createExecutor({
taskConfig: { store, scopeId: 'owner:conversation' },
humanInTheLoop: { enabled: true, backgroundPausePolicy: 'reject' },
});

const response = JSON.parse(
executor.executeInBackground({
description: 'Blocked by policy.',
subagentType: 'researcher',
parentToolCallId: 'call_reject',
})
) as { status: string; message: string };

expect(response.status).toBe('rejected');
expect(response.message).toContain(
'does not support human-in-the-loop pauses'
);
expect(store.list('owner:conversation')).toEqual([]);
});

it('runs HITL background tasks under deny, omits question tools, and reports blocked calls', async () => {
const store = new InMemorySubagentTaskStore();
const questionTool = { name: 'ask_user_question' } as NonNullable<
AgentInputs['graphTools']
>[number];
const normalTool = { name: 'search' } as NonNullable<
AgentInputs['graphTools']
>[number];
const inputs: AgentInputs = {
...makeChildInputs(),
tools: [questionTool, normalTool],
toolMap: new Map([
['ask_user_question', questionTool],
['search', normalTool],
]),
toolRegistry: new Map([
['ask_user_question', { name: 'ask_user_question' }],
['search', { name: 'search' }],
]),
graphTools: [questionTool, normalTool],
toolDefinitions: [{ name: 'ask_user_question' }, { name: 'search' }],
};
const observedInputs: StandardGraphInput[] = [];
const observedGraphs: StandardGraph[] = [];
const { factory } = makeStubGraphFactory({
messages: [
new ToolMessage({
name: 'delete_file',
tool_call_id: 'call_delete',
status: 'error',
content:
'Blocked: Approval required for "delete_file"; unavailable in a background subagent. Ask the parent to run it in the foreground.',
}),
new AIMessage('Could not finish that operation.'),
],
});
const handlers = new HandlerRegistry();
handlers.register(GraphEvents.ON_TOOL_EXECUTE, {
handle: (): void => undefined,
});
const executor = createExecutor({
configs: new Map([
['researcher', makeConfig('researcher', { agentInputs: inputs })],
]),
humanInTheLoop: { enabled: true },
taskConfig: { store, scopeId: 'owner:conversation' },
parentHandlerRegistry: handlers,
createChildGraph: (input): StandardGraph => {
observedInputs.push(input);
const graph = factory();
graph.eagerEventToolExecution = { enabled: true };
observedGraphs.push(graph);
return graph;
},
});

const response = JSON.parse(
executor.executeInBackground({
description: 'Try the task.',
subagentType: 'researcher',
parentToolCallId: 'call_deny',
})
) as { background_task_id: string; status: string };
expect(response.status).toBe('running');
await waitForTask(
store,
response.background_task_id,
(status) => status === 'completed'
);

expect(observedGraphs[0].humanInTheLoop).toEqual({
enabled: false,
backgroundPausePolicy: 'deny',
});
expect(observedGraphs[0].eagerEventToolExecution).toBeUndefined();
const child = observedInputs[0].agents[0];
expect(
child.tools?.map((tool) => ('name' in tool ? tool.name : ''))
).toEqual(['search']);
expect(child.graphTools?.map((tool) => tool.name)).toEqual(['search']);
expect(child.toolDefinitions?.map((tool) => tool.name)).toEqual(['search']);
expect([...child.toolMap!.keys()]).toEqual(['search']);
expect([...child.toolRegistry!.keys()]).toEqual(['search']);
expect(inputs.graphTools).toHaveLength(2);
expect(inputs.toolMap).toHaveProperty('size', 2);
expect(inputs.toolRegistry).toHaveProperty('size', 2);
expect(
store.claim('owner:conversation', response.background_task_id)
).toMatchObject({
status: 'completed',
result: expect.stringContaining(
'Background approval denied for: delete_file'
),
});
});

it('fails closed if a detached child unexpectedly raises an interrupt', async () => {
const store = new InMemorySubagentTaskStore();
const executor = createExecutor({
humanInTheLoop: { enabled: true },
taskConfig: { store, scopeId: 'owner:conversation' },
createChildGraph: makeThrowingGraphFactory(
new GraphInterrupt([
{
id: 'unexpected-question',
value: {
type: 'ask_user_question',
question: { question: 'Proceed?' },
},
},
])
),
});

const response = JSON.parse(
executor.executeInBackground({
description: 'Unexpected pause.',
subagentType: 'researcher',
parentToolCallId: 'call_unexpected',
})
) as { background_task_id: string };
await waitForTask(
store,
response.background_task_id,
(status) => status === 'error'
);
expect(
store.get('owner:conversation', response.background_task_id)?.error
).toContain('cannot pause for human input');
});

it('keeps a detached child and its thread continuation at depth zero', async () => {
const store = new ContinuationTaskStore();
const childConfig = makeConfig('researcher', {
Expand Down
77 changes: 75 additions & 2 deletions src/tools/__tests__/hitl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@ import type {
StopHookOutput,
UserPromptSubmitHookOutput,
} from '@/hooks';
import type * as t from '@/types';
import type { ReplayableSubagentTool } from '@/tools/subagent/SubagentReplay';
import type * as t from '@/types';
import {
TOOL_APPROVAL_REVIEW_CONFIG_KEY,
createToolApprovalReviewEvidence,
} from '@/hitl/approvalReview';
import { TOOL_BATCH_REPLAY_KEY, getToolBatchReplayState } from '../toolBatchReplay';
import {
SUBAGENT_REPLAY_CONTROLLER,
} from '@/tools/subagent/SubagentReplay';
Expand All @@ -50,7 +51,6 @@ import { HookRegistry, createToolPolicyHook } from '@/hooks';
import * as events from '@/utils/events';
import { askUserQuestion } from '@/hitl';
import { ToolNode } from '../ToolNode';
import { TOOL_BATCH_REPLAY_KEY, getToolBatchReplayState } from '../toolBatchReplay';

async function flushAsyncWork(): Promise<void> {
await Promise.resolve();
Expand Down Expand Up @@ -676,6 +676,79 @@ describe('ToolNode HITL — `ask` decision raises interrupt() when humanInTheLoo
});
});

describe('ToolNode HITL — background deny policy', () => {
it('blocks event-driven calls requesting approval with actionable feedback', async () => {
mockEventDispatch([]);
const dispatch = jest.spyOn(events, 'safeDispatchCustomEvent');
const node = new ToolNode({
tools: [createSchemaStub('write_file')],
eventDrivenMode: true,
agentId: 'agent-x',
toolCallStepIds: new Map([['call_write', 'step_write']]),
hookRegistry: makeHookRegistry('ask', 'write requires approval'),
humanInTheLoop: { enabled: false, backgroundPausePolicy: 'deny' },
});
const graph = buildHITLGraph(node, [
{ id: 'call_write', name: 'write_file', args: { command: 'write' } },
]);

const result = (await graph.invoke(
{ messages: [] },
{
configurable: { thread_id: 'background-deny-event' },
}
)) as { messages: BaseMessage[] };
const toolMessages = result.messages.filter(
(message): message is ToolMessage => message instanceof ToolMessage
);
expect(isInterrupted(result)).toBe(false);
expect(toolMessages).toHaveLength(1);
expect(toolMessages[0].status).toBe('error');
expect(toolMessages[0].content).toContain(
'Approval required for "write_file"'
);
expect(toolMessages[0].content).toContain('write requires approval');
expect(
dispatch.mock.calls.filter(([event]) => event === 'on_tool_execute')
).toHaveLength(0);
});

it('blocks a direct tool requesting approval before its body runs', async () => {
const execute = jest.fn(async () => 'should not run');
const writeTool = tool(execute, {
name: 'write_file',
description: 'Writes files',
schema: z.object({ command: z.string() }),
});
const node = new ToolNode({
tools: [writeTool],
toolMap: new Map([['write_file', writeTool]]),
hookRegistry: makeHookRegistry('ask'),
humanInTheLoop: { enabled: false, backgroundPausePolicy: 'deny' },
});
const graph = buildHITLGraph(node, [
{ id: 'call_write', name: 'write_file', args: { command: 'write' } },
]);

const result = (await graph.invoke(
{ messages: [] },
{
configurable: { thread_id: 'background-deny-direct' },
}
)) as { messages: BaseMessage[] };
const toolMessages = result.messages.filter(
(message): message is ToolMessage => message instanceof ToolMessage
);
expect(isInterrupted(result)).toBe(false);
expect(toolMessages).toHaveLength(1);
expect(toolMessages[0].status).toBe('error');
expect(toolMessages[0].content).toContain(
'Approval required for "write_file"'
);
expect(execute).not.toHaveBeenCalled();
});
});

describe('ToolNode HITL — opt-out (`humanInTheLoop: { enabled: false }`) is fail-closed', () => {
afterEach(() => {
jest.restoreAllMocks();
Expand Down
Loading