🛡️ fix: Let Background Subagents Run With HITL Enabled

src/tools/ToolNode.ts

@@ -129,10 +129,11 @@ 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';
+import { isBackgroundDenyMode } from '@/types/hitl';
 
 function stripToolApprovalReviewConfig(
   configurable: Record<string, unknown> | undefined
@@ -1949,6 +1950,8 @@ export class ToolNode<T = any> extends RunnableCallable<T, T> {
           // they dispatch — including HITL gates on `write_file` / `edit_file`.
           hookContext: {
             registry: this.hookRegistry,
+            failClosedOnHookError:
+              isBackgroundDenyMode(this.humanInTheLoop),
             runId: (config.configurable?.run_id as string | undefined) ?? '',
             threadId: config.configurable?.thread_id as string | undefined,
             agentId: this.agentId,
@@ -2541,11 +2544,18 @@ export class ToolNode<T = any> extends RunnableCallable<T, T> {
             onceReplayKey: approvalReplayKey,
             onceReplaySessionId: approvalReplaySessionId,
           }).catch((): AggregatedHookResult | undefined =>
-            approvalReviewEvidence == null
+            approvalReviewEvidence == null &&
+            !isBackgroundDenyMode(this.humanInTheLoop)
               ? undefined
               : {
                 decision: 'deny',
-                reason: 'Approval policy could not be evaluated on resume',
+                reason:
+                  isBackgroundDenyMode(this.humanInTheLoop)
+                    ? 'Approval policy could not be evaluated'
+                    : 'Approval policy could not be evaluated on resume',
+                ...(isBackgroundDenyMode(this.humanInTheLoop)
+                  ? { hasHookFailures: true }
+                  : {}),
                 additionalContexts: [],
                 injectedMessages: [],
                 errors: [],
@@ -2582,6 +2592,26 @@ export class ToolNode<T = any> extends RunnableCallable<T, T> {
           };
         }
 
+        if (
+          preResult.hasHookFailures === true &&
+          isBackgroundDenyMode(this.humanInTheLoop)
+        ) {
+          return persistOutput(
+            this.blockDirectCall({
+              call,
+              resolvedArgs,
+              reason: this.backgroundApprovalReason(
+                call.name,
+                'Approval policy could not be evaluated'
+              ),
+              hookRegistry,
+              runId,
+              threadId,
+            }),
+            effectiveCall.args as Record<string, unknown>
+          );
+        }
+
         if (preResult.decision === 'deny') {
           return persistOutput(
             this.blockDirectCall({
@@ -2621,12 +2651,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 =
+              isBackgroundDenyMode(this.humanInTheLoop)
+                ? this.backgroundApprovalReason(call.name, preResult.reason)
+                : this.resolveAskDecisionForDirectTool(
+                  preResult.reason,
+                  call.name
+                );
             return persistOutput(
               this.blockDirectCall({
                 call,
@@ -3008,6 +3040,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
@@ -3626,7 +3665,11 @@ export class ToolNode<T = any> extends RunnableCallable<T, T> {
                 matchQuery: entry.call.name,
                 onceReplayKey: approvalReplayKey,
                 onceReplaySessionId: approvalReplaySessionId,
-              }).catch((): AggregatedHookResult => HOOK_FALLBACK);
+              }).catch((): AggregatedHookResult =>
+                isBackgroundDenyMode(this.humanInTheLoop)
+                  ? { ...HOOK_FALLBACK, hasHookFailures: true }
+                  : HOOK_FALLBACK
+              );
             })
           );
 
@@ -3793,6 +3836,20 @@ export class ToolNode<T = any> extends RunnableCallable<T, T> {
           batchAdditionalContexts.push(ctx);
         }
 
+        if (
+          hookResult.hasHookFailures === true &&
+          isBackgroundDenyMode(this.humanInTheLoop)
+        ) {
+          blockEntry(
+            entry,
+            this.backgroundApprovalReason(
+              entry.call.name,
+              'Approval policy could not be evaluated'
+            )
+          );
+          continue;
+        }
+
         if (hookResult.decision === 'deny') {
           blockEntry(entry, hookResult.reason ?? 'Blocked by hook');
           continue;
@@ -3810,7 +3867,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,
+              isBackgroundDenyMode(this.humanInTheLoop)
+                ? this.backgroundApprovalReason(
+                  entry.call.name,
+                  hookResult.reason
+                )
+                : (hookResult.reason ?? 'Blocked by hook')
+            );
             continue;
           }
           /**

src/tools/__tests__/ProgrammaticToolCalling.test.ts

@@ -1842,6 +1842,47 @@ for member in team:
       expect(gate.input).toEqual({ path: '/tmp/rewritten' });
     });
 
+    it('fails closed on a broken approval hook in a background child bridge', async () => {
+      const { HookRegistry } = await import('@/hooks');
+      const ptcMod = require('../local/LocalProgrammaticToolCalling');
+      const registry = new HookRegistry();
+      registry.register('PreToolUse', {
+        internal: true,
+        hooks: [
+          async () => { throw new Error('approval service unavailable'); },
+          async () => ({ decision: 'allow' }),
+        ],
+      });
+
+      const gate = await ptcMod.applyPreToolUseHooksForBridge(
+        { registry, runId: 'background-child', failClosedOnHookError: true },
+        'write_file',
+        'call_bg_1',
+        { path: '/tmp/file' }
+      );
+      expect(gate.denyReason).toContain('Approval policy could not be evaluated');
+      expect(gate.denyReason).toContain('write_file');
+    });
+
+    it('identifies a denied background bridge tool when approval is needed', async () => {
+      const { HookRegistry } = await import('@/hooks');
+      const ptcMod = require('../local/LocalProgrammaticToolCalling');
+      const registry = new HookRegistry();
+      registry.register('PreToolUse', {
+        hooks: [async () => ({ decision: 'ask', reason: 'needs human approval' })],
+      });
+
+      const gate = await ptcMod.applyPreToolUseHooksForBridge(
+        { registry, runId: 'background-child', failClosedOnHookError: true },
+        'write_file',
+        'call_bg_ask',
+        { path: '/tmp/file' }
+      );
+      expect(gate.denyReason).toContain('Approval required for "write_file"');
+      expect(gate.denyReason).toContain('needs human approval');
+      expect(gate.denyReason).toContain('in the foreground');
+    });
+
     it('treats `ask` as fail-closed deny (HITL not reachable from bridge)', async () => {
       const { HookRegistry } = await import('@/hooks');
 

src/tools/__tests__/SubagentExecutor.test.ts

@@ -607,6 +607,189 @@ 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',
+      backgroundDeny: true,
+    });
+    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('fails closed if a background SubagentStart policy hook fails', async () => {
+    const store = new InMemorySubagentTaskStore();
+    const hooks = new HookRegistry();
+    hooks.register('SubagentStart', {
+      internal: true,
+      hooks: [async () => { throw new Error('authorization unavailable'); }],
+    });
+    const createChildGraph = jest.fn(makeNoopGraphFactory());
+    const executor = createExecutor({
+      taskConfig: { store, scopeId: 'owner:conversation' },
+      hookRegistry: hooks,
+      humanInTheLoop: { enabled: true },
+      createChildGraph,
+    });
+
+    const response = JSON.parse(
+      executor.executeInBackground({
+        description: 'Requires start authorization.',
+        subagentType: 'researcher',
+        parentToolCallId: 'call_start_policy_failure',
+      })
+    ) 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(
+      'Subagent start policy could not be evaluated'
+    );
+    expect(createChildGraph).toHaveBeenCalledTimes(1);
+  });
+
   it('keeps a detached child and its thread continuation at depth zero', async () => {
     const store = new ContinuationTaskStore();
     const childConfig = makeConfig('researcher', {