From bb45bca5ff7ef7d2cd2b2f2e98812d96a5971b01 Mon Sep 17 00:00:00 2001 From: Guillaume Jacquart Date: Tue, 13 Oct 2026 12:19:29 +0000 Subject: [PATCH] feat(editor): Show the policy reason when an agent tool call is refused (#40673) Co-authored-by: Claude Opus 5.5 (1M context) --- .../tools/__tests__/node-tool-factory.test.ts | 55 +++++++ .../modules/agents/tools/node-tool-factory.ts | 8 + .../modules/agents/tools/policy-refusal.ts | 20 +++ .../__tests__/ephemeral-node-executor.test.ts | 14 +- .../node-execution/ephemeral-node-executor.ts | 37 ++++- .../__tests__/policy-violation.error.test.ts | 43 ++++- .../cli/src/policy/policy-violation.error.ts | 15 ++ .../N8nAiActivityStep/AiActivityStep.test.ts | 48 ++++++ .../N8nAiActivityStep/AiActivityStep.vue | 15 +- .../frontend/@n8n/i18n/src/locales/en.json | 1 + .../__tests__/AgentChatToolSteps.test.ts | 149 +++++++++++++++++- .../__tests__/useAgentChatStream.test.ts | 36 +++++ .../components/AgentChatToolPolicyRefusal.vue | 57 +++++++ .../agents/components/AgentChatToolSteps.vue | 36 ++++- .../useToolPolicyRefusalText.test.ts | 122 ++++++++++++++ .../agents/composables/useAgentChatStream.ts | 6 +- .../composables/useToolPolicyRefusalText.ts | 50 ++++++ .../messageMappers.toolPolicyRefusal.test.ts | 60 +++++++ .../__tests__/toolPolicyRefusal.test.ts | 27 ++++ .../ai/shared/agentsChat/messageMappers.ts | 6 +- .../ai/shared/agentsChat/toolPolicyRefusal.ts | 18 +++ 21 files changed, 800 insertions(+), 23 deletions(-) create mode 100644 packages/cli/src/modules/agents/tools/policy-refusal.ts create mode 100644 packages/frontend/editor-ui/src/features/agents/components/AgentChatToolPolicyRefusal.vue create mode 100644 packages/frontend/editor-ui/src/features/agents/composables/__tests__/useToolPolicyRefusalText.test.ts create mode 100644 packages/frontend/editor-ui/src/features/agents/composables/useToolPolicyRefusalText.ts create mode 100644 packages/frontend/editor-ui/src/features/ai/shared/agentsChat/__tests__/messageMappers.toolPolicyRefusal.test.ts create mode 100644 packages/frontend/editor-ui/src/features/ai/shared/agentsChat/__tests__/toolPolicyRefusal.test.ts create mode 100644 packages/frontend/editor-ui/src/features/ai/shared/agentsChat/toolPolicyRefusal.ts diff --git a/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts b/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts index c362a22bb72..a75ba8710ce 100644 --- a/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts +++ b/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts @@ -1,12 +1,25 @@ import type { AgentJsonToolConfig } from '@n8n/api-types'; +import type { PolicyViolation } from '@n8n/decorators'; import { Container } from '@n8n/di'; +import { hasPolicyRefusalMarker } from 'n8n-workflow'; import { z } from 'zod'; import type { EphemeralNodeExecutor } from '@/node-execution'; import { NodeTypes } from '@/node-types'; +import type { NonEmptyViolations } from '@/policy/policy-violation.error'; import { resolveNodeTool } from '../node-tool-factory'; +const violation = (overrides: Partial = {}): PolicyViolation => ({ + kind: 'node-type-unavailable', + checkId: 'node-type-availability', + message: 'The node type n8n-nodes-base.googleDrive is not available on this instance', + subject: 'n8n-nodes-base.googleDrive', + subjectType: 'nodeType', + scope: 'instance', + ...overrides, +}); + // The node-tool-factory imports the DI `Container` to look up NodeTypes inside // `resolveInputSchema` (for auto-seeding a `{ input: string }` schema on // native tools). In the unit-test scope the container isn't registered and @@ -212,6 +225,48 @@ describe('resolveNodeTool → tool name sanitization', () => { }); }); +describe('resolveNodeTool → policy refusal', () => { + it('returns a policy refusal in the serialized refusal shape instead of throwing', async () => { + const violations: NonEmptyViolations = [violation()]; + const executeInline = vi.fn().mockResolvedValue({ + status: 'error', + data: [], + error: 'Node type is not permitted for agent tool execution', + violations, + }); + + const tool = await resolveNodeTool(baseToolSchema, { + executor: { executeInline } as unknown as EphemeralNodeExecutor, + projectId: 'p1', + }); + + const output = await tool.handler!({}, {} as never); + expect(output).toEqual({ + isPolicyRefusal: true, + message: 'Node type is not permitted for agent tool execution', + violations, + instruction: expect.stringMatching(/do not retry/i), + }); + expect(output).toMatchObject({ instruction: expect.stringMatching(/tell the user/i) }); + expect(hasPolicyRefusalMarker(output)).toBe(true); + }); + + it('throws for an ordinary executor error with no violations, as before', async () => { + const executeInline = vi.fn().mockResolvedValue({ + status: 'error', + data: [], + error: 'boom', + }); + + const tool = await resolveNodeTool(baseToolSchema, { + executor: { executeInline } as unknown as EphemeralNodeExecutor, + projectId: 'p1', + }); + + await expect(tool.handler!({}, {} as never)).rejects.toThrow('boom'); + }); +}); + describe('toExecutorCredentials (credential mapping at execution)', () => { type NodeCredentials = Extract['node']['credentials']; diff --git a/packages/cli/src/modules/agents/tools/node-tool-factory.ts b/packages/cli/src/modules/agents/tools/node-tool-factory.ts index 29fcb49ee26..5c8460144a2 100644 --- a/packages/cli/src/modules/agents/tools/node-tool-factory.ts +++ b/packages/cli/src/modules/agents/tools/node-tool-factory.ts @@ -17,6 +17,7 @@ import { resolveToolNodeType, type EphemeralNodeExecutor } from '@/node-executio import { NodeTypes } from '@/node-types'; import type { InstrumentToolAdditionalData } from '../agent-runtime-instrumentation'; +import { toPolicyRefusalResult } from './policy-refusal'; type NodeToolInputSchema = JSONSchema7 | z.ZodType; @@ -164,6 +165,13 @@ export async function resolveNodeTool( projectId: ctx.projectId, ...instrumentedExecution, }); + // A refusal is returned, not thrown: thrown errors reach the chat as plain text. + if (result.status === 'error' && result.violations) { + return toPolicyRefusalResult( + result.error ?? result.violations[0].message, + result.violations, + ); + } // Throw on the executor's structured error so the agent runtime // flags the tool-result with `isError: true` and the recorder // marks the timeline entry as a failed call. Returning the error diff --git a/packages/cli/src/modules/agents/tools/policy-refusal.ts b/packages/cli/src/modules/agents/tools/policy-refusal.ts new file mode 100644 index 00000000000..c842bfec851 --- /dev/null +++ b/packages/cli/src/modules/agents/tools/policy-refusal.ts @@ -0,0 +1,20 @@ +import type { NonEmptyViolations } from '@/policy/policy-violation.error'; + +const POLICY_REFUSAL_INSTRUCTION = + 'An administrator policy refused this tool call. Do not retry it or work around it. ' + + 'Tell the user which action you could not do and why, then continue with the rest of the request.'; + +/** Same fields as a serialized `PolicyViolationError`, so existing refusal readers apply. */ +export interface ToolPolicyRefusalResult { + isPolicyRefusal: true; + message: string; + violations: NonEmptyViolations; + instruction: string; +} + +export function toPolicyRefusalResult( + message: string, + violations: NonEmptyViolations, +): ToolPolicyRefusalResult { + return { isPolicyRefusal: true, message, violations, instruction: POLICY_REFUSAL_INSTRUCTION }; +} diff --git a/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts b/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts index add50eb08a7..db465c8c6aa 100644 --- a/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts +++ b/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts @@ -498,6 +498,7 @@ describe('EphemeralNodeExecutor', () => { expect(result.status).toBe('error'); expect(result.error).toBe('upstream 500'); + expect(result).not.toHaveProperty('violations'); }, ); @@ -662,6 +663,7 @@ describe('EphemeralNodeExecutor', () => { expect(result.status).toBe('error'); expect(result.error).toBe('upstream 500'); + expect(result).not.toHaveProperty('violations'); }, ); @@ -771,13 +773,14 @@ describe('EphemeralNodeExecutor', () => { ); }); - it('returns a tool error and never runs a blocked node', async () => { + it("returns a tool error carrying the refusal's violations, and never runs a blocked node", async () => { const execute = vi.fn(); nodeTypes.getByNameAndVersion.mockReturnValue({ description: toolDescription, execute, } as unknown as INodeType); - policyEnforcementService.enforceWorkflowStart.mockRejectedValueOnce(blocked()); + const refusal = blocked(); + policyEnforcementService.enforceWorkflowStart.mockRejectedValueOnce(refusal); const result = await executor.executeInline({ nodeType: 'n8n-nodes-base.dateTimeTool', @@ -791,16 +794,18 @@ describe('EphemeralNodeExecutor', () => { status: 'error', data: [], error: expect.stringContaining('is blocked by an instance policy'), + violations: refusal.violations, }); expect(execute).not.toHaveBeenCalled(); }); - it('never runs supplyData for a blocked native tool node', async () => { + it("never runs supplyData for a blocked native tool node, and carries the refusal's violations", async () => { const supplyData = vi.fn(); nodeTypes.getByNameAndVersion.mockReturnValue( mockNodeType({ description: toolDescription, supplyData }), ); - policyEnforcementService.enforceWorkflowStart.mockRejectedValueOnce(blocked()); + const refusal = blocked(); + policyEnforcementService.enforceWorkflowStart.mockRejectedValueOnce(refusal); const result = await executor.executeInline({ nodeType: '@n8n/n8n-nodes-langchain.toolWikipedia', @@ -811,6 +816,7 @@ describe('EphemeralNodeExecutor', () => { }); expect(result.status).toBe('error'); + expect(result.violations).toEqual(refusal.violations); expect(supplyData).not.toHaveBeenCalled(); }); diff --git a/packages/cli/src/node-execution/ephemeral-node-executor.ts b/packages/cli/src/node-execution/ephemeral-node-executor.ts index b4c4475fc02..de041eee786 100644 --- a/packages/cli/src/node-execution/ephemeral-node-executor.ts +++ b/packages/cli/src/node-execution/ephemeral-node-executor.ts @@ -28,6 +28,7 @@ import { v4 as uuid } from 'uuid'; import { NodeTypes } from '@/node-types'; import { PolicyEnforcementService } from '@/policy/policy-enforcement.service'; +import { findPolicyViolations, type NonEmptyViolations } from '@/policy/policy-violation.error'; import { withExpressionIsolate } from '@/utils'; import { getBase } from '@/workflow-execute-additional-data'; @@ -71,6 +72,23 @@ export interface NodeExecutionResult { status: 'success' | 'error'; data: INodeExecutionData[]; error?: string; + /** Set when a policy refused the run. */ + violations?: NonEmptyViolations; +} + +type SupplyDataToolOutcome = + | { ok: true; value: T } + | { ok: false; error: string; violations?: NonEmptyViolations }; + +/** The `{ status: 'error' }` result for a caught error, keeping a refusal's violations. */ +function errorResult(error: unknown): NodeExecutionResult { + const violations = findPolicyViolations(error); + return { + status: 'error', + data: [], + error: getErrorMessage(error), + ...(violations && { violations }), + }; } /** @@ -392,9 +410,9 @@ export class EphemeralNodeExecutor { ); return executionResult; } catch (error) { - const message = getErrorMessage(error); - this.logger.debug('Node execution failed', { nodeType: tool.nodeType, error: message }); - return { status: 'error', data: [], error: message }; + const result = errorResult(error); + this.logger.debug('Node execution failed', { nodeType: tool.nodeType, error: result.error }); + return result; } } @@ -514,7 +532,7 @@ export class EphemeralNodeExecutor { tool: EphemeralWorkflowToolLike, inputItems: INodeExecutionData[], onTool: (response: LangChainToolType | StructuredToolkit) => Promise | T, - ): Promise<{ ok: true; value: T } | { ok: false; error: string }> { + ): Promise> { const closeFunctions: CloseFunction[] = []; try { @@ -560,8 +578,8 @@ export class EphemeralNodeExecutor { }; }); } catch (error) { - const message = getErrorMessage(error); - return { ok: false, error: message }; + const violations = findPolicyViolations(error); + return { ok: false, error: getErrorMessage(error), ...(violations && { violations }) }; } finally { for (const closeFunction of closeFunctions) { try { @@ -611,7 +629,12 @@ export class EphemeralNodeExecutor { nodeType: tool.nodeType, error: result.error, }); - return { status: 'error', data: [], error: result.error }; + return { + status: 'error', + data: [], + error: result.error, + ...(result.violations && { violations: result.violations }), + }; } return { diff --git a/packages/cli/src/policy/__tests__/policy-violation.error.test.ts b/packages/cli/src/policy/__tests__/policy-violation.error.test.ts index ce3099a48ae..96cd822d4fe 100644 --- a/packages/cli/src/policy/__tests__/policy-violation.error.test.ts +++ b/packages/cli/src/policy/__tests__/policy-violation.error.test.ts @@ -1,10 +1,13 @@ import type { PolicyViolation } from '@n8n/decorators'; -import { UserError } from 'n8n-workflow'; +import { NodeOperationError, UserError } from 'n8n-workflow'; +import type { INode } from 'n8n-workflow'; +import { mock } from 'vitest-mock-extended'; import { classifyRestError, RestErrorKind } from '@n8n/backend-services'; import { serializeInternalRestError } from '@n8n/backend-services'; import { + findPolicyViolations, isPolicyRefusal, PolicyViolationError, type NonEmptyViolations, @@ -122,4 +125,42 @@ describe('PolicyViolationError', () => { expect(isPolicyRefusal(value)).toBe(false); }); }); + + describe('findPolicyViolations', () => { + function wrapWithCauses(innermost: Error, wrapperCount: number): Error { + let current = innermost; + for (let i = 0; i < wrapperCount; i++) { + current = new Error(`wrapper ${i}`, { cause: current }); + } + return current; + } + + const violations: NonEmptyViolations = [violation()]; + + it('returns the violations of a direct refusal', () => { + expect(findPolicyViolations(new PolicyViolationError(violations))).toEqual(violations); + }); + + it('returns the violations of a refusal a node error wraps', () => { + const nodeError = new NodeOperationError(mock(), new PolicyViolationError(violations)); + + expect(findPolicyViolations(nodeError)).toEqual(violations); + }); + + it('walks at most five errors down the cause chain', () => { + const refusal = new PolicyViolationError(violations); + + expect(findPolicyViolations(wrapWithCauses(refusal, 4))).toEqual(violations); + expect(findPolicyViolations(wrapWithCauses(refusal, 5))).toBeUndefined(); + }); + + it.each([ + ['a plain error chain', wrapWithCauses(new Error('root cause'), 3)], + ['a plain object', { violations }], + ['a string', 'boom'], + ['undefined', undefined], + ])('returns undefined for %s', (_label, value) => { + expect(findPolicyViolations(value)).toBeUndefined(); + }); + }); }); diff --git a/packages/cli/src/policy/policy-violation.error.ts b/packages/cli/src/policy/policy-violation.error.ts index 59df5d96c73..3421e75e218 100644 --- a/packages/cli/src/policy/policy-violation.error.ts +++ b/packages/cli/src/policy/policy-violation.error.ts @@ -63,3 +63,18 @@ export class PolicyViolationError extends UserError { export function isPolicyRefusal(error: unknown): boolean { return error instanceof PolicyViolationError || hasPolicyRefusalMarker(error); } + +/** Bounds the `cause` walk, so a cyclic chain cannot loop. */ +const MAX_CAUSE_DEPTH = 5; + +/** The violations of a refusal, also when a node error wraps it as its `cause`. */ +export function findPolicyViolations(error: unknown): NonEmptyViolations | undefined { + let current = error; + for (let depth = 0; depth < MAX_CAUSE_DEPTH && current instanceof Error; depth++) { + if (current instanceof PolicyViolationError && hasViolations(current.violations)) { + return current.violations; + } + current = current.cause; + } + return undefined; +} diff --git a/packages/frontend/@n8n/design-system/src/components/N8nAiActivityStep/AiActivityStep.test.ts b/packages/frontend/@n8n/design-system/src/components/N8nAiActivityStep/AiActivityStep.test.ts index b130712e476..4243cc009c1 100644 --- a/packages/frontend/@n8n/design-system/src/components/N8nAiActivityStep/AiActivityStep.test.ts +++ b/packages/frontend/@n8n/design-system/src/components/N8nAiActivityStep/AiActivityStep.test.ts @@ -102,4 +102,52 @@ describe('N8nAiActivityStep', () => { expect(container.querySelector('.n8n-icon')).not.toBeInTheDocument(); expect(queryByText('Custom details')).not.toBeInTheDocument(); }); + + it('should default the error icon to triangle-alert', () => { + const { container } = render(AiActivityStep, { + props: { label: 'Search nodes', error: 'Something went wrong' }, + global, + }); + + expect(container.querySelector('.n8n-icon')?.getAttribute('icon')).toBe('triangle-alert'); + }); + + it('should use a custom errorIcon when provided', () => { + const { container } = render(AiActivityStep, { + props: { label: 'Search nodes', error: 'Something went wrong', errorIcon: 'lock' }, + global, + }); + + expect(container.querySelector('.n8n-icon')?.getAttribute('icon')).toBe('lock'); + }); + + it('should render errorCallout slot content instead of the default error text', () => { + const { getByText, queryByText } = render(AiActivityStep, { + props: { label: 'Search nodes', error: 'Something went wrong' }, + slots: { errorCallout: 'Custom callout text' }, + global, + }); + + expect(getByText('Custom callout text')).toBeInTheDocument(); + // The tooltip is hidden by default in this test environment, so the only + // remaining place `error` could render is the callout body it replaced. + expect(queryByText('Something went wrong')).not.toBeInTheDocument(); + }); + + it('should still show the error text in the tooltip when errorCallout slot is used', () => { + const { getByText } = render(AiActivityStep, { + props: { label: 'Search nodes', error: 'Something went wrong' }, + slots: { errorCallout: 'Custom callout text' }, + global: { + ...global, + stubs: { + ...global.stubs, + N8nTooltip: { template: '
' }, + }, + }, + }); + + expect(getByText('Something went wrong')).toBeInTheDocument(); + expect(getByText('Custom callout text')).toBeInTheDocument(); + }); }); diff --git a/packages/frontend/@n8n/design-system/src/components/N8nAiActivityStep/AiActivityStep.vue b/packages/frontend/@n8n/design-system/src/components/N8nAiActivityStep/AiActivityStep.vue index e49624fa256..691ab795715 100644 --- a/packages/frontend/@n8n/design-system/src/components/N8nAiActivityStep/AiActivityStep.vue +++ b/packages/frontend/@n8n/design-system/src/components/N8nAiActivityStep/AiActivityStep.vue @@ -10,6 +10,7 @@ import N8nAiActivityStepResultSection from '../N8nAiActivityStepResultSection'; import N8nAnimatedCollapsibleContent from '../N8nAnimatedCollapsibleContent'; import N8nCallout from '../N8nCallout'; import N8nIcon from '../N8nIcon'; +import type { IconName } from '../N8nIcon/icons'; import N8nTooltip from '../N8nTooltip'; const props = withDefaults( @@ -23,6 +24,7 @@ const props = withDefaults( wrapContent?: boolean; /** Keep error icon/tooltip but skip the inner danger callout. */ hideErrorCallout?: boolean; + errorIcon?: IconName; }>(), { loading: false, @@ -30,9 +32,16 @@ const props = withDefaults( hasContent: true, wrapContent: false, hideErrorCallout: false, + errorIcon: 'triangle-alert', }, ); +defineSlots<{ + default?: () => unknown; + /** Replaces the callout text; the tooltip keeps showing `error`. */ + errorCallout?: () => unknown; +}>(); + const MAX_ERROR_TOOLTIP_LENGTH = 160; const isNested = inject(aiActivityStepGroupContext, false); @@ -57,7 +66,7 @@ const errorTooltip = computed(() => {{ errorTooltip }} theme="danger" :class="$style.errorCallout" > - {{ props.error }} + {{ props.error }} @@ -91,7 +100,7 @@ const errorTooltip = computed(() => {{ errorTooltip }} ({ N8nAiActivityStep: { - props: ['label', 'hasContent', 'loading', 'error', 'hideErrorCallout'], + props: ['label', 'hasContent', 'loading', 'error', 'hideErrorCallout', 'errorIcon'], data: () => ({ isOpen: false }), computed: { labelParts(this: { label: string }): string[] { @@ -25,8 +25,15 @@ vi.mock('@n8n/design-system', () => ({ · {{ part }} - ⚠ -
{{ error }}
+ ⚠ +
+ {{ error }} +
`, @@ -50,6 +57,11 @@ vi.mock('@n8n/design-system', () => ({ template: '', props: ['icon', 'size', 'spin'], }, + N8nLink: { + props: ['underline', 'size'], + emits: ['click'], + template: '', + }, N8nMarkdownEditor: { template: '
{{ modelValue }}
', props: ['modelValue', 'readonly', 'variant', 'showToolbar', 'maxHeight'], @@ -57,6 +69,33 @@ vi.mock('@n8n/design-system', () => ({ N8nTooltip: { template: '
', props: ['content', 'placement'] }, })); +vi.mock('@n8n/frontend-module-type-availability-policies', async () => ({ + getPolicyViolations: ( + await vi.importActual( + '@n8n/frontend-module-type-availability-policies', + ) + ).getPolicyViolations, + ContactInstanceAdminModal: { + props: ['open', 'nodeTypeName', 'kind'], + template: + '
{{ nodeTypeName }}
', + }, +})); + +vi.mock('@/app/stores/nodeTypes.store', () => ({ + useNodeTypesStore: () => ({ + getNodeType: () => undefined, + loadNodeTypesIfNotLoaded: async () => {}, + }), +})); + +vi.mock('@/features/credentials/credentials.store', () => ({ + useCredentialsStore: () => ({ + getCredentialTypeByName: () => undefined, + fetchCredentialTypes: async () => {}, + }), +})); + vi.mock('@n8n/i18n', () => { const i18n = { translations: { @@ -76,6 +115,9 @@ vi.mock('@n8n/i18n', () => { 'agents.chat.writeTodos.hint.difficulty': 'Difficulty', 'agents.chat.writeTodos.hint.subAgent': 'Sub-agent', 'agents.chat.writeTodos.hint.expectedOutput': 'Expected output', + 'agents.chat.toolPolicyRefusal.reason': '{name} is restricted on this instance.', + 'typeAvailabilityPolicies.restrictedNode.scope.instance': 'Restricted on this instance', + 'typeAvailabilityPolicies.restrictedNode.contactAdmin': 'Contact instance admin', } as Record, baseText(key: string, opts?: { interpolate?: { name?: string; count?: string } }) { if (key === 'agents.chat.delegate.label' && opts?.interpolate?.name) { @@ -90,7 +132,11 @@ vi.mock('@n8n/i18n', () => { return `${opts.interpolate.count} tasks`; } if (key === 'agents.chat.writeTodos.summary.done') return 'done'; - return this.translations[key] ?? key; + const template = this.translations[key]; + if (template && opts?.interpolate?.name) { + return template.replace('{name}', opts.interpolate.name); + } + return template ?? key; }, }; @@ -765,4 +811,99 @@ describe('AgentChatToolSteps', () => { expect(wrapper.text()).toContain('Weighing the sources.'); expect(wrapper.text()).not.toContain('Reasoning'); }); + + describe('policy-refused tool calls', () => { + function refusedCall(overrides: Partial = {}): ToolCall { + return { + tool: 'search_nodes', + toolCallId: 'tc-refused', + state: TOOL_CALL_STATE.ERROR, + output: { + isPolicyRefusal: true, + message: 'Blocked by policy', + violations: [ + { + kind: 'node-type-unavailable', + checkId: 'check-1', + message: 'Not allowed', + subject: 'slack', + scope: 'instance', + }, + ], + instruction: 'Ask the user to pick another tool.', + }, + ...overrides, + }; + } + + it('shows a lock with the scope line as tooltip', () => { + const icon = mountSteps([refusedCall()]).get('[data-testid="tool-step-warning"]'); + expect(icon.attributes('data-error-icon')).toBe('lock'); + expect(icon.attributes('title')).toBe('Restricted on this instance'); + }); + + it('keeps the default error icon for a non-refusal error', () => { + const wrapper = mountSteps([ + { + tool: 'search_nodes', + toolCallId: 'tc-err', + state: TOOL_CALL_STATE.ERROR, + output: 'Boom', + }, + ]); + expect( + wrapper.get('[data-testid="tool-step-warning"]').attributes('data-error-icon'), + ).toBeUndefined(); + }); + + it('shows the refusal reason and a contact-admin link in the callout', () => { + const wrapper = mountSteps([refusedCall()]); + const callout = wrapper.get('[data-test-id="tool-step-error"]'); + // Node types are not loaded in this test, so the step label names the subject. + expect(callout.text()).toContain('Search nodes is restricted on this instance.'); + expect( + callout.find('[data-test-id="agent-chat-tool-policy-refusal-contact-admin"]').exists(), + ).toBe(true); + }); + + it('hides the raw Output section even when expanded, unlike a normal error', async () => { + const refusedWrapper = mountSteps([refusedCall()]); + await refusedWrapper.get('button').trigger('click'); + expect(refusedWrapper.find('pre').exists()).toBe(false); + + const normalErrorWrapper = mountSteps([ + { + tool: 'search_nodes', + toolCallId: 'tc-err', + state: TOOL_CALL_STATE.ERROR, + output: { foo: 'bar' }, + }, + ]); + await normalErrorWrapper.get('button').trigger('click'); + expect(normalErrorWrapper.find('pre').exists()).toBe(true); + }); + + it('excludes a refusal from fixable failures while a normal error stays fixable', async () => { + const normalError: ToolCall = { + tool: 'search_nodes', + toolCallId: 'tc-normal', + state: TOOL_CALL_STATE.ERROR, + output: 'Connection failed', + }; + const wrapper = mountSteps([normalError, refusedCall()], { + canFixWithAssistant: true, + executionId: 'exec-1', + }); + + const fixCallout = wrapper.get('[data-test-id="agent-chat-tool-fix-with-assistant-callout"]'); + expect(fixCallout.text()).toContain('Connection failed'); + expect(fixCallout.text()).not.toContain('is restricted on this instance'); + expect(wrapper.find('[data-test-id="agent-chat-tool-policy-refusal"]').exists()).toBe(true); + + await wrapper.get('[data-test-id="agent-chat-tool-fix-with-assistant"]').trigger('click'); + expect(wrapper.emitted('fixWithAssistant')).toEqual([ + [[expect.objectContaining({ toolCallId: 'tc-normal' })]], + ]); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts index 565fc1ccb0e..eac70478752 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts @@ -2292,6 +2292,42 @@ describe('useAgentChatStream — SDK-aligned event handling', () => { expect(hook.messages.value[1].toolCalls?.[0].state).toBe('error'); }); + it('renders a policy-refused tool result as an error step even though the call resolves', async () => { + const events: AgentSseEvent[] = [ + { type: 'start-step' }, + { + type: 'tool-call', + toolCallId: 'tc-refused', + toolName: 'http_request', + input: { url: 'https://example.com' }, + }, + { type: 'finish-step' }, + { + type: 'tool-result', + toolCallId: 'tc-refused', + toolName: 'http_request', + output: { + isPolicyRefusal: true, + message: 'Blocked by policy', + violations: [ + { kind: 'node-type-unavailable', checkId: 'check-1', message: 'Not allowed' }, + ], + instruction: 'Ask the user to pick another tool.', + }, + isError: false, + }, + { type: 'done' }, + ]; + globalThis.fetch = vi.fn(async () => makeSseResponse(events)) as typeof fetch; + + const hook = buildHook(); + await hook.sendMessage('go'); + await flushPromises(); + await nextTick(); + + expect(hook.messages.value[1].toolCalls?.[0].state).toBe('error'); + }); + it('renders a completed delegate_subagent result as a done step', async () => { const events: AgentSseEvent[] = [ { type: 'start-step' }, diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentChatToolPolicyRefusal.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentChatToolPolicyRefusal.vue new file mode 100644 index 00000000000..ea1b7ac3dcf --- /dev/null +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentChatToolPolicyRefusal.vue @@ -0,0 +1,57 @@ + + + diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentChatToolSteps.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentChatToolSteps.vue index d8ff5532999..7c108275611 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentChatToolSteps.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentChatToolSteps.vue @@ -29,6 +29,8 @@ import { writeTodosSummaryLabel, } from '../utils/write-todos-tool'; import { TOOL_CALL_STATE } from '../constants'; +import { parseToolPolicyRefusal } from '@/features/ai/shared/agentsChat/toolPolicyRefusal'; +import AgentChatToolPolicyRefusal from './AgentChatToolPolicyRefusal.vue'; const props = defineProps<{ toolCalls: ToolCall[]; @@ -57,6 +59,8 @@ const fixableFailures = computed(() => { if (dismissed.has(toolCall.toolCallId)) continue; if (toolCall.state !== TOOL_CALL_STATE.ERROR) continue; if (isRecoverablePlanError(toolCall)) continue; + // The assistant cannot lift a policy. + if (toolStepRefusal(toolCall)) continue; const error = toolStepError(toolCall)?.trim(); if (!error) continue; @@ -174,9 +178,17 @@ function toolStepView(tc: ToolCall): ToolStepDisplay { }; } +function toolStepRefusal(tc: ToolCall) { + if (tc.state !== TOOL_CALL_STATE.ERROR) return undefined; + return parseToolPolicyRefusal(tc.output); +} + function toolStepError(tc: ToolCall): string | undefined { if (isRecoverablePlanError(tc)) return i18n.baseText('agents.chat.plan.error.rejected'); if (tc.state !== TOOL_CALL_STATE.ERROR) return undefined; + if (toolStepRefusal(tc)) { + return i18n.baseText('typeAvailabilityPolicies.restrictedNode.scope.instance'); + } if (isEmptyToolErrorPayload(tc.output)) { return i18n.baseText('agents.chat.toolError.generic'); } @@ -184,9 +196,15 @@ function toolStepError(tc: ToolCall): string | undefined { } function hideToolErrorCallout(tc: ToolCall): boolean { + if (toolStepRefusal(tc)) return false; return isRecoverablePlanError(tc) || (showFix.value && tc.state === TOOL_CALL_STATE.ERROR); } +/** The refusal payload is for the model; the callout explains it to the user. */ +function showToolOutput(tc: ToolCall): boolean { + return tc.output !== undefined && !toolStepRefusal(tc); +} + function emitFixWithAssistant() { if (fixableFailures.value.length === 0) return; emit('fixWithAssistant', fixableFailures.value); @@ -225,9 +243,16 @@ function hasActiveToolCall(): boolean { :label="view.label" :loading="isToolStepLoading(tc)" :error="toolStepError(tc)" + :error-icon="toolStepRefusal(tc) ? 'lock' : undefined" :hide-error-callout="hideToolErrorCallout(tc)" :has-content="view.expandable" > +
{{ formatToolData(tc.input) }}
-
+
{{ i18n.baseText('agentSessions.timeline.output') }} @@ -288,9 +313,16 @@ function hasActiveToolCall(): boolean { :label="toolStepView(tc).label" :loading="isToolStepLoading(tc)" :error="toolStepError(tc)" + :error-icon="toolStepRefusal(tc) ? 'lock' : undefined" :hide-error-callout="hideToolErrorCallout(tc)" :has-content="toolStepView(tc).expandable" > +