diff --git a/apps/api/src/app/agents/agent-chat/activity-to-events.spec.ts b/apps/api/src/app/agents/agent-chat/activity-to-events.spec.ts index d09ea979698..abf6b693b69 100644 --- a/apps/api/src/app/agents/agent-chat/activity-to-events.spec.ts +++ b/apps/api/src/app/agents/agent-chat/activity-to-events.spec.ts @@ -182,4 +182,104 @@ describe('activity-to-events run lifecycle', () => { ) ).to.deep.equal(['approval-activity-1', 'approval-activity-2']); }); + + it('derives trust action ids at emit time for managed MCP approvals', () => { + const envelopes = mapNewestFirstEventActivities( + [ + activity({ + type: ConversationActivityTypeEnum.TOOL_APPROVAL_REQUEST, + identifier: 'approval-activity-mcp', + sequence: 1, + toolData: { + approvalId: 'call_1', + toolCallId: 'call_1', + toolName: 'create_issue', + approveActionId: 'mcp-approval:approve:call_1', + denyActionId: 'mcp-approval:deny:call_1', + mcpServerName: 'GitHub', + }, + }), + ], + context + ); + + expect(envelopes[0]?.event).to.deep.equal({ + type: 'tool-approval-request', + messageId: 'approval-activity-mcp', + approvalId: 'call_1', + toolUseId: 'call_1', + toolName: 'create_issue', + input: undefined, + approveActionId: 'mcp-approval:approve:call_1', + denyActionId: 'mcp-approval:deny:call_1', + trustToolActionId: 'mcp-approval:approve-tool:call_1:create_issue:GitHub', + trustServerActionId: 'mcp-approval:approve-server:call_1:create_issue:GitHub', + source: { type: 'mcp', serverName: 'GitHub' }, + }); + }); + + it('derives trust action ids at emit time for managed direct approvals', () => { + const envelopes = mapNewestFirstEventActivities( + [ + activity({ + type: ConversationActivityTypeEnum.TOOL_APPROVAL_REQUEST, + identifier: 'approval-activity-direct', + sequence: 1, + toolData: { + approvalId: 'call_2', + toolCallId: 'call_2', + toolName: 'deleteOrder', + approveActionId: 'direct-approval:approve:call_2', + denyActionId: 'direct-approval:deny:call_2', + }, + }), + ], + context + ); + + expect(envelopes[0]?.event).to.deep.equal({ + type: 'tool-approval-request', + messageId: 'approval-activity-direct', + approvalId: 'call_2', + toolUseId: 'call_2', + toolName: 'deleteOrder', + input: undefined, + approveActionId: 'direct-approval:approve:call_2', + denyActionId: 'direct-approval:deny:call_2', + trustToolActionId: 'direct-approval:approve-tool:call_2:deleteOrder', + source: undefined, + }); + }); + + it('omits trust action ids for self-hosted approvals', () => { + const envelopes = mapNewestFirstEventActivities( + [ + activity({ + type: ConversationActivityTypeEnum.TOOL_APPROVAL_REQUEST, + identifier: 'approval-activity-self-hosted', + sequence: 1, + toolData: { + approvalId: 'apr_1', + toolCallId: 'tool-use-1', + toolName: 'runCommand', + approveActionId: 'tool-approval:approve:apr_1', + denyActionId: 'tool-approval:deny:apr_1', + }, + }), + ], + context + ); + + expect(envelopes[0]?.event).to.deep.equal({ + type: 'tool-approval-request', + messageId: 'approval-activity-self-hosted', + approvalId: 'apr_1', + toolUseId: 'tool-use-1', + toolName: 'runCommand', + input: undefined, + approveActionId: 'tool-approval:approve:apr_1', + denyActionId: 'tool-approval:deny:apr_1', + source: undefined, + }); + }); }); diff --git a/apps/api/src/app/agents/agent-chat/activity-to-events.ts b/apps/api/src/app/agents/agent-chat/activity-to-events.ts index 1dd90cc5c34..dc42a702f84 100644 --- a/apps/api/src/app/agents/agent-chat/activity-to-events.ts +++ b/apps/api/src/app/agents/agent-chat/activity-to-events.ts @@ -15,7 +15,8 @@ import { mapRunLifecycleActivityToEvent, runIdFromLifecycleIdentifier, } from '../conversation-runtime/conversation/run-lifecycle-activity'; -import { mintApprovalActionIds } from '../shared/tool-approval/mint-approval-action-ids'; +import { DIRECT_TOOL_APPROVAL_ACTION_PREFIX, MCP_TOOL_APPROVAL_ACTION_PREFIX } from '../shared/tool-approval/action-id'; +import { mintApprovalActionIds, mintManagedApprovalActionIds } from '../shared/tool-approval/mint-approval-action-ids'; type McpConnectionActivityData = { actionId?: string; @@ -40,6 +41,35 @@ function isCardTree(value: unknown): value is Record { return typeof value === 'object' && value !== null && (value as { type?: unknown }).type === 'card'; } +function isManagedToolApprovalRequest(toolData: ConversationActivityEntity['toolData']): boolean { + const approveActionId = toolData?.approveActionId; + if (!approveActionId) { + return false; + } + + return ( + approveActionId.startsWith(`${MCP_TOOL_APPROVAL_ACTION_PREFIX}:`) || + approveActionId.startsWith(`${DIRECT_TOOL_APPROVAL_ACTION_PREFIX}:`) + ); +} + +function mintTrustActionIdsFromStoredToolData(toolData: NonNullable) { + if (!isManagedToolApprovalRequest(toolData) || !toolData.toolCallId || !toolData.toolName) { + return {}; + } + + const managed = mintManagedApprovalActionIds({ + toolUseId: toolData.toolCallId, + toolName: toolData.toolName, + mcpServerName: toolData.mcpServerName, + }); + + return { + trustToolActionId: managed.trustToolActionId, + ...(managed.trustServerActionId ? { trustServerActionId: managed.trustServerActionId } : {}), + }; +} + /** Prefer the stored Card tree. Fall back to markdown when no Card is present. */ export function messageContentFromStored(params: { content?: string; @@ -84,7 +114,10 @@ function mapActivityToEvent(activity: ConversationActivityEntity): AgentEvent | const actionIds = toolData.approveActionId && toolData.denyActionId - ? { approveActionId: toolData.approveActionId, denyActionId: toolData.denyActionId } + ? { + approveActionId: toolData.approveActionId, + denyActionId: toolData.denyActionId, + } : mintApprovalActionIds({ approvalId: toolData.approvalId }); return { @@ -96,6 +129,8 @@ function mapActivityToEvent(activity: ConversationActivityEntity): AgentEvent | input: toolData.input, approveActionId: actionIds.approveActionId, denyActionId: actionIds.denyActionId, + ...mintTrustActionIdsFromStoredToolData(toolData), + source: toolData.mcpServerName ? { type: 'mcp', serverName: toolData.mcpServerName } : undefined, }; } diff --git a/apps/api/src/app/agents/conversation-runtime/conversation/agent-conversation.types.ts b/apps/api/src/app/agents/conversation-runtime/conversation/agent-conversation.types.ts index 81d3353487c..c71fdf5a35c 100644 --- a/apps/api/src/app/agents/conversation-runtime/conversation/agent-conversation.types.ts +++ b/apps/api/src/app/agents/conversation-runtime/conversation/agent-conversation.types.ts @@ -58,6 +58,7 @@ export interface PersistToolApprovalRequestParams extends ConversationActivityCo /** When omitted, self-hosted `tool-approval:*` ids are minted. */ approveActionId?: string; denyActionId?: string; + mcpServerName?: string; } export type MetadataOp = diff --git a/apps/api/src/app/agents/conversation-runtime/conversation/conversation-activity-ledger.ts b/apps/api/src/app/agents/conversation-runtime/conversation/conversation-activity-ledger.ts index e763e7244cb..5ef9b80cebc 100644 --- a/apps/api/src/app/agents/conversation-runtime/conversation/conversation-activity-ledger.ts +++ b/apps/api/src/app/agents/conversation-runtime/conversation/conversation-activity-ledger.ts @@ -276,6 +276,7 @@ export class ConversationActivityLedger { input: params.input, approveActionId: actionIds.approveActionId, denyActionId: actionIds.denyActionId, + mcpServerName: params.mcpServerName, }, sequence, environmentId: params.environmentId, diff --git a/apps/api/src/app/agents/conversation-runtime/reply/handle-agent-reply/handle-agent-reply.usecase.ts b/apps/api/src/app/agents/conversation-runtime/reply/handle-agent-reply/handle-agent-reply.usecase.ts index 2b83ce27d20..66b165cb2ec 100644 --- a/apps/api/src/app/agents/conversation-runtime/reply/handle-agent-reply/handle-agent-reply.usecase.ts +++ b/apps/api/src/app/agents/conversation-runtime/reply/handle-agent-reply/handle-agent-reply.usecase.ts @@ -580,6 +580,7 @@ export class HandleAgentReply { input: request.input, approveActionId: request.approveActionId, denyActionId: request.denyActionId, + mcpServerName: request.mcpServerName, environmentId: command.environmentId, organizationId: command.organizationId, }); diff --git a/apps/api/src/app/agents/managed-runtime/tool-approval/handle-pending-tool-approvals.usecase.ts b/apps/api/src/app/agents/managed-runtime/tool-approval/handle-pending-tool-approvals.usecase.ts index 032d9105976..0cc84e0c65d 100644 --- a/apps/api/src/app/agents/managed-runtime/tool-approval/handle-pending-tool-approvals.usecase.ts +++ b/apps/api/src/app/agents/managed-runtime/tool-approval/handle-pending-tool-approvals.usecase.ts @@ -10,7 +10,7 @@ import { HandlePlanProgressCommand } from '../../conversation-runtime/reply/hand import { HandlePlanProgress } from '../../conversation-runtime/reply/handle-plan-progress/handle-plan-progress.usecase'; import { AgentPlatformEnum, usesProtocolEventApprovals } from '../../shared/enums/agent-platform.enum'; import { captureAgentException, captureAgentWarning } from '../../shared/errors/capture-agent-sentry'; -import { managedApprovalGrammar, mintApprovalActionIds } from '../../shared/tool-approval/mint-approval-action-ids'; +import { mintManagedApprovalActionIds } from '../../shared/tool-approval/mint-approval-action-ids'; import { ManagedAgentService } from '../managed-agent.service'; import { ManagedAgentProviderFactory } from '../managed-agent-provider-factory.service'; import { HandleNovuResolveCommand } from '../novu-resolve/handle-novu-resolve.command'; @@ -412,9 +412,10 @@ export class HandlePendingToolApprovals { platform: command.platform, tool, }); - const actionIds = mintApprovalActionIds({ - approvalId: tool.toolUseId, - grammar: managedApprovalGrammar(tool.mcpServerName), + const actionIds = mintManagedApprovalActionIds({ + toolUseId: tool.toolUseId, + toolName: tool.toolName, + mcpServerName: tool.mcpServerName, }); try { @@ -434,6 +435,7 @@ export class HandlePendingToolApprovals { input: tool.input, approveActionId: actionIds.approveActionId, denyActionId: actionIds.denyActionId, + mcpServerName: tool.mcpServerName, }, }) ); diff --git a/apps/api/src/app/agents/managed-runtime/tool-approval/managed-approval.ts b/apps/api/src/app/agents/managed-runtime/tool-approval/managed-approval.ts index 29296bd8146..67327128f18 100644 --- a/apps/api/src/app/agents/managed-runtime/tool-approval/managed-approval.ts +++ b/apps/api/src/app/agents/managed-runtime/tool-approval/managed-approval.ts @@ -4,6 +4,8 @@ import type { SlackNativeDelivery } from '../../conversation-runtime/egress/slac import type { ReplyContentDto } from '../../shared/dtos/agent-reply-payload.dto'; import { AgentPlatformEnum } from '../../shared/enums/agent-platform.enum'; import { + buildDirectToolApprovalPersistActionId, + buildMcpToolApprovalPersistActionId, buildToolApprovalActionId, DIRECT_TOOL_APPROVAL_ACTION_PREFIX, MCP_TOOL_APPROVAL_ACTION_PREFIX, @@ -56,26 +58,6 @@ function formatToolArgumentsBody(tool: PendingToolApproval): string | undefined return truncatedBody.length <= SLACK_CARD_BODY_MAX ? truncatedBody : truncatedBody.slice(0, SLACK_CARD_BODY_MAX); } -// --------------------------------------------------------------------------- -// Persist ("always allow") action ids — kept in sync with parseToolApprovalActionId -// --------------------------------------------------------------------------- - -function buildMcpToolApprovalPersistActionId( - verdict: 'approve-tool' | 'approve-server', - tool: PendingToolApproval -): string { - const toolName = encodeURIComponent(tool.toolName); - const mcpServerName = encodeURIComponent(tool.mcpServerName ?? ''); - - return `${MCP_TOOL_APPROVAL_ACTION_PREFIX}:${verdict}:${tool.toolUseId}:${toolName}:${mcpServerName}`; -} - -function buildDirectToolApprovalPersistActionId(tool: PendingToolApproval): string { - const toolName = encodeURIComponent(tool.toolName); - - return `${DIRECT_TOOL_APPROVAL_ACTION_PREFIX}:approve-tool:${tool.toolUseId}:${toolName}`; -} - // --------------------------------------------------------------------------- // Thalamus adapter // --------------------------------------------------------------------------- diff --git a/apps/api/src/app/agents/shared/dtos/agent-reply-payload.dto.ts b/apps/api/src/app/agents/shared/dtos/agent-reply-payload.dto.ts index 12c659aec02..b5eede1e877 100644 --- a/apps/api/src/app/agents/shared/dtos/agent-reply-payload.dto.ts +++ b/apps/api/src/app/agents/shared/dtos/agent-reply-payload.dto.ts @@ -400,6 +400,14 @@ export class ToolApprovalRequestPayloadDto { @IsOptional() @IsString() denyActionId?: string; + + @ApiPropertyOptional({ + description: 'MCP server name when the gated tool is from an MCP server (for UI labels).', + example: 'GitHub', + }) + @IsOptional() + @IsString() + mcpServerName?: string; } @ApiExtraModels(MarkdownReplyContentDto, CardReplyContentDto, ToolApprovalCardReplyContentDto, FileRefDto) diff --git a/apps/api/src/app/agents/shared/tool-approval/action-id.ts b/apps/api/src/app/agents/shared/tool-approval/action-id.ts index 22831f2ad7f..27c6062cbae 100644 --- a/apps/api/src/app/agents/shared/tool-approval/action-id.ts +++ b/apps/api/src/app/agents/shared/tool-approval/action-id.ts @@ -23,6 +23,30 @@ export function buildToolApprovalActionId( return `${prefix}:${verdict}:${toolUseId}`; } +export type ToolApprovalPersistTarget = { + toolUseId: string; + toolName: string; + mcpServerName?: string; +}; + +/** Persist-verdict ids for MCP tools — keep in sync with {@link parseToolApprovalActionId}. */ +export function buildMcpToolApprovalPersistActionId( + verdict: 'approve-tool' | 'approve-server', + tool: ToolApprovalPersistTarget +): string { + const toolName = encodeURIComponent(tool.toolName); + const mcpServerName = encodeURIComponent(tool.mcpServerName ?? ''); + + return `${MCP_TOOL_APPROVAL_ACTION_PREFIX}:${verdict}:${tool.toolUseId}:${toolName}:${mcpServerName}`; +} + +/** Persist-verdict id for direct (non-MCP) managed tools. */ +export function buildDirectToolApprovalPersistActionId(tool: ToolApprovalPersistTarget): string { + const toolName = encodeURIComponent(tool.toolName); + + return `${DIRECT_TOOL_APPROVAL_ACTION_PREFIX}:approve-tool:${tool.toolUseId}:${toolName}`; +} + const TOOL_APPROVAL_VERDICTS = ['approve', 'deny', 'approve-tool', 'approve-server'] as const; type ToolApprovalVerdict = (typeof TOOL_APPROVAL_VERDICTS)[number]; diff --git a/apps/api/src/app/agents/shared/tool-approval/mint-approval-action-ids.ts b/apps/api/src/app/agents/shared/tool-approval/mint-approval-action-ids.ts index 626be41d3e9..eb1e4839140 100644 --- a/apps/api/src/app/agents/shared/tool-approval/mint-approval-action-ids.ts +++ b/apps/api/src/app/agents/shared/tool-approval/mint-approval-action-ids.ts @@ -1,9 +1,12 @@ import { buildApprovalActionId } from '@novu/framework/internal'; import { + buildDirectToolApprovalPersistActionId, + buildMcpToolApprovalPersistActionId, buildToolApprovalActionId, DIRECT_TOOL_APPROVAL_ACTION_PREFIX, MCP_TOOL_APPROVAL_ACTION_PREFIX, type ToolApprovalActionPrefix, + type ToolApprovalPersistTarget, } from './action-id'; export type ApprovalActionIdGrammar = { kind: 'self-hosted' } | { kind: 'managed'; prefix: ToolApprovalActionPrefix }; @@ -13,6 +16,11 @@ export type MintedApprovalActionIds = { denyActionId: string; }; +export type MintedManagedApprovalActionIds = MintedApprovalActionIds & { + trustToolActionId: string; + trustServerActionId?: string; +}; + /** * Mint the same action-id grammar Slack/Teams cards already put on buttons. * Self-hosted: `tool-approval:{approve|deny}:{approvalId}` @@ -43,3 +51,23 @@ export function managedApprovalGrammar(mcpServerName: string | undefined): Appro prefix: mcpServerName !== undefined ? MCP_TOOL_APPROVAL_ACTION_PREFIX : DIRECT_TOOL_APPROVAL_ACTION_PREFIX, }; } + +/** + * Mint all four managed approval action ids (approve once, deny, always-allow tool, always-allow server). + * Same ids Slack/Teams cards put on buttons — Agent Chat protocol reuses them. + */ +export function mintManagedApprovalActionIds(tool: ToolApprovalPersistTarget): MintedManagedApprovalActionIds { + const base = mintApprovalActionIds({ + approvalId: tool.toolUseId, + grammar: managedApprovalGrammar(tool.mcpServerName), + }); + const isMcp = tool.mcpServerName !== undefined; + + return { + ...base, + trustToolActionId: isMcp + ? buildMcpToolApprovalPersistActionId('approve-tool', tool) + : buildDirectToolApprovalPersistActionId(tool), + ...(isMcp ? { trustServerActionId: buildMcpToolApprovalPersistActionId('approve-server', tool) } : {}), + }; +} diff --git a/apps/api/src/app/environments-v1/usecases/construct-framework-workflow/construct-framework-workflow.spec.ts b/apps/api/src/app/environments-v1/usecases/construct-framework-workflow/construct-framework-workflow.spec.ts index ef8ef4b7b69..fecc391e9c5 100644 --- a/apps/api/src/app/environments-v1/usecases/construct-framework-workflow/construct-framework-workflow.spec.ts +++ b/apps/api/src/app/environments-v1/usecases/construct-framework-workflow/construct-framework-workflow.spec.ts @@ -1,5 +1,6 @@ import { NotificationStepEntity, NotificationTemplateEntity } from '@novu/dal'; -import { providerSchemas } from '@novu/framework'; +import { Client, providerSchemas } from '@novu/framework'; +import { Event, PostActionEnum, Workflow } from '@novu/framework/internal'; import { CHAT_CONTENT_OVERRIDE_PROVIDER_IDS, ChatProviderIdEnum, @@ -135,3 +136,108 @@ describe('ConstructFrameworkWorkflow content-override channel steps', () => { expect(providerOverrides?.additionalProperties?.additionalProperties).to.equal(true); }); }); + +/** + * Worker-executed steps are hydrated from the job state on the bridge, and the framework validates + * that state against the step's output schema with AJV set to strip additional properties. Without + * an explicit schema the whole HTTP response body was removed, so conditions on its fields never + * matched and the next step was always skipped (NV-8604). + */ +describe('ConstructFrameworkWorkflow worker-executed step hydration', () => { + const HTTP_STEP_ID = 'http-request-step'; + const IN_APP_STEP_ID = 'in-app-step'; + const WORKFLOW_ID = 'http-conditions-workflow'; + + const hydrationDbWorkflow = { + _id: 'workflow-id', + _environmentId: 'env-id', + _organizationId: 'org-id', + name: 'HTTP request conditions', + origin: ResourceOriginEnum.NOVU_CLOUD, + triggers: [{ identifier: WORKFLOW_ID }], + steps: [ + { + stepId: HTTP_STEP_ID, + template: { + type: StepTypeEnum.HTTP_REQUEST, + controls: { + schema: { + type: 'object', + properties: { url: { type: 'string' }, method: { type: 'string' } }, + additionalProperties: false, + }, + }, + }, + }, + { + stepId: IN_APP_STEP_ID, + template: { + type: StepTypeEnum.IN_APP, + controls: { + schema: { + type: 'object', + properties: { body: { type: 'string' }, skip: { type: 'object', additionalProperties: true } }, + additionalProperties: false, + }, + }, + }, + }, + ], + } as unknown as NotificationTemplateEntity; + + function buildHydrationWorkflow(): Workflow { + const hydrationUsecase = Object.create(ConstructFrameworkWorkflow.prototype) as { + logger: { setContext: () => void; warn: () => void; error: () => void }; + inAppOutputRendererUseCase: { execute: () => Promise> }; + constructFrameworkWorkflow: (args: { dbWorkflow: NotificationTemplateEntity }) => Workflow; + }; + + hydrationUsecase.logger = { setContext: () => {}, warn: () => {}, error: () => {} }; + hydrationUsecase.inAppOutputRendererUseCase = { execute: async () => ({ body: 'Enrolled' }) }; + + return hydrationUsecase.constructFrameworkWorkflow({ dbWorkflow: hydrationDbWorkflow }); + } + + function buildEvent(enrolmentCount: number): Event { + return { + action: PostActionEnum.EXECUTE, + workflowId: WORKFLOW_ID, + stepId: IN_APP_STEP_ID, + subscriber: {}, + payload: {}, + controls: { + body: 'Enrolled', + skip: { and: [{ '==': [{ var: `steps.${HTTP_STEP_ID}.enrolmentCount` }, 1] }] }, + }, + state: [ + { + stepId: HTTP_STEP_ID, + outputs: { id: '123', message: { text: 'pawan' }, enrolmentCount }, + state: { status: 'completed', error: false }, + }, + ], + context: {}, + env: { name: 'Test', type: 'dev' }, + } as unknown as Event; + } + + async function executeInAppStep(enrolmentCount: number) { + const client = new Client({ secretKey: 'construct-framework-workflow-secret' }); + await client.addWorkflows([buildHydrationWorkflow()]); + + return client.executeWorkflow(buildEvent(enrolmentCount)); + } + + it('runs the next step when a condition matches a field of the hydrated HTTP response', async () => { + const result = await executeInAppStep(1); + + expect(result.options?.skip).to.equal(false); + expect(result.outputs.body).to.equal('Enrolled'); + }); + + it('skips the next step when the hydrated HTTP response does not satisfy the condition', async () => { + const result = await executeInAppStep(2); + + expect(result.options?.skip).to.equal(true); + }); +}); diff --git a/apps/api/src/app/environments-v1/usecases/construct-framework-workflow/construct-framework-workflow.usecase.ts b/apps/api/src/app/environments-v1/usecases/construct-framework-workflow/construct-framework-workflow.usecase.ts index 09e0c74af9e..dc62e857295 100644 --- a/apps/api/src/app/environments-v1/usecases/construct-framework-workflow/construct-framework-workflow.usecase.ts +++ b/apps/api/src/app/environments-v1/usecases/construct-framework-workflow/construct-framework-workflow.usecase.ts @@ -27,6 +27,7 @@ import { ActionStep, ChannelStep, ChatOutputUnvalidated, + CustomStep, PostActionEnum, Schema, Step, @@ -411,20 +412,13 @@ export class ConstructFrameworkWorkflow { * the workflow graph correctly. The resolve function is a passthrough because execution already happened. */ case StepTypeEnum.HTTP_REQUEST: - return step.custom( - stepId, - async (controlValues) => { - return controlValues; - }, - this.constructActionStepOptions(staticStep, skip) - ); case StepTypeEnum.CUSTOM: return step.custom( stepId, async (controlValues) => { return controlValues; }, - this.constructActionStepOptions(staticStep, skip) + this.constructCustomStepOptions(staticStep, skip) ); default: throw new InternalServerErrorException(`Step type ${stepType} is not supported`); @@ -540,6 +534,24 @@ export class ConstructFrameworkWorkflow { } as Required[2]>; } + /** + * Worker-executed steps (HTTP request, custom) are hydrated from the job state when a later step + * calls the bridge, and the framework validates that state against the step's output schema with + * AJV configured to remove additional properties. Without an explicit schema the framework falls + * back to a closed empty schema, which strips the whole response body and leaves conditions such + * as `steps.http-request-step.enrolmentCount equals 1` evaluating against nothing (NV-8604). + */ + @Instrument() + private constructCustomStepOptions( + staticStep: NotificationStepEntity, + skip: SkipFunction + ): NonNullable[2]> { + return { + ...this.constructActionStepOptions(staticStep, skip), + outputSchema: PERMISSIVE_EMPTY_SCHEMA as unknown as Schema, + }; + } + @Instrument() private constructActionStepOptions( staticStep: NotificationStepEntity, diff --git a/apps/dashboard/src/components/agents/agent-chat-panel/agent-chat-panel.tsx b/apps/dashboard/src/components/agents/agent-chat-panel/agent-chat-panel.tsx index 3a13319e5b6..5756d108f7a 100644 --- a/apps/dashboard/src/components/agents/agent-chat-panel/agent-chat-panel.tsx +++ b/apps/dashboard/src/components/agents/agent-chat-panel/agent-chat-panel.tsx @@ -96,7 +96,7 @@ function AgentChatSurface({ const canSend = !composerDisabled && Boolean(draft.trim()); const isEmpty = messages.length === 0 && !isRunning && !isLoading; const lastMessage = messages[messages.length - 1]; - const showTypingRow = Boolean(typing) || (isRunning && lastMessage?.role !== 'assistant'); + const showTypingRow = Boolean(typing) || isRunning; const lastMessageSignature = lastMessage ? `${lastMessage.id}:${lastMessage.parts.map((part) => (part.type === 'text' ? part.text : part.type)).join('\0')}` : ''; diff --git a/apps/dashboard/src/components/agents/agent-chat-panel/agent-chat-parts.tsx b/apps/dashboard/src/components/agents/agent-chat-panel/agent-chat-parts.tsx index 3f9bcf06b5d..81f97994f70 100644 --- a/apps/dashboard/src/components/agents/agent-chat-panel/agent-chat-parts.tsx +++ b/apps/dashboard/src/components/agents/agent-chat-panel/agent-chat-parts.tsx @@ -28,6 +28,11 @@ type AgentMessagePart = AgentMessage['parts'][number]; type ToolPart = Extract; type TextPart = Extract; type CardPart = Extract; +type ApprovalPart = Extract; +type McpConnectionPart = Extract; + +export type ToolApprovalDecision = 'approved' | 'denied' | 'trust-tool' | 'trust-server'; +export type ToolApprovalRespondHandler = (decision: ToolApprovalDecision) => void; export type CardActionHandler = (args: { actionId: string; sourceMessageId: string; value?: string }) => void; @@ -377,6 +382,207 @@ function ToolChip({ tool }: { tool: ToolPart }) { ); } +type ToolApprovalCardProps = { + id?: string; + toolName: string; + source?: ApprovalPart['source']; + state: ApprovalPart['state']; + trustToolActionId?: string; + trustServerActionId?: string; + disabled?: boolean; + onRespond?: ToolApprovalRespondHandler; + className?: string; +}; + +function ToolApprovalActions({ + disabled, + onRespond, + trustToolActionId, + trustServerActionId, + source, +}: { + disabled?: boolean; + onRespond?: ToolApprovalRespondHandler; + trustToolActionId?: string; + trustServerActionId?: string; + source?: ApprovalPart['source']; +}) { + return ( +
+ + + {trustToolActionId ? ( + + ) : null} + {trustServerActionId && source?.type === 'mcp' ? ( + + ) : null} +
+ ); +} + +export function ChatToolApprovalCard({ + id, + toolName, + source, + state, + trustToolActionId, + trustServerActionId, + disabled, + onRespond, + className, +}: ToolApprovalCardProps) { + const isPending = state === 'pending'; + + return ( +
+
+
+ +
+
+

+ Run {toolName}? +

+

+ {isPending + ? 'The agent is waiting for your approval.' + : state === 'approved' + ? 'You approved this tool call.' + : 'You denied this tool call.'} +

+
+
+ {isPending ? ( + + ) : ( + + {state === 'approved' ? 'Approved' : 'Denied'} + + )} +
+ ); +} + +function ChatMcpConnectionCard({ + id, + mcpId, + displayName, + authorizeUrl, + authorizeUrlWithAutoApprove, + state, + message, +}: { + id?: string; + mcpId: string; + displayName: string; + authorizeUrl: string; + authorizeUrlWithAutoApprove?: string; + state: McpConnectionPart['state']; + message?: string; +}) { + const safeAuthorizeUrl = toSafeExternalUrl(authorizeUrlWithAutoApprove || authorizeUrl); + const isPending = state === 'pending'; + + return ( +
+
+ +
+
+

Connect {displayName}

+

+ {isPending + ? 'The agent needs authorization to continue.' + : state === 'connected' + ? 'Connected successfully.' + : message || 'Connection failed.'} +

+
+ {isPending ? ( + + ) : ( + + {state === 'connected' ? 'Connected' : 'Failed'} + + )} +
+ ); +} + export function ChatTypingRow({ status }: { status?: string }) { // Server statuses often arrive with their own trailing ellipsis or dots. const label = status?.trim().replace(/[.\u2026]+$/, ''); @@ -413,67 +619,29 @@ export function ChatPendingActionCard({ }: { action: AgentPendingAction; disabled: boolean; - onRespond: (decision: 'approved' | 'denied') => void; + onRespond: ToolApprovalRespondHandler; }) { if (action.type === 'mcp-connection') { - // The authorize URL comes from the MCP server's OAuth discovery document, - // so it is external input. Reject anything that is not an absolute http(s) - // URL (e.g. `javascript:`) before it can reach `window.open`. - const authorizeUrl = toSafeExternalUrl(action.authorizeUrlWithAutoApprove || action.authorizeUrl); - return ( -
-
- -
-
-

Connect {action.displayName}

-

The agent needs authorization to continue.

-
- -
+ ); } return ( -
-
- -
-
-

- Run {action.toolName}? -

-

The agent is waiting for your approval.

-
-
- - -
-
+ ); } diff --git a/libs/dal/src/repositories/conversation-activity/conversation-activity.entity.ts b/libs/dal/src/repositories/conversation-activity/conversation-activity.entity.ts index 7db476702d2..070e952e6fc 100644 --- a/libs/dal/src/repositories/conversation-activity/conversation-activity.entity.ts +++ b/libs/dal/src/repositories/conversation-activity/conversation-activity.entity.ts @@ -69,6 +69,8 @@ export interface ConversationActivityToolData { approveActionId?: string; /** Server-minted action id for deny (request). Echoed by headless / card UIs. */ denyActionId?: string; + /** MCP server name when the gated tool is from an MCP server (request). */ + mcpServerName?: string; } export class ConversationActivityEntity { diff --git a/packages/agent-event-protocol/src/agent-event.types.ts b/packages/agent-event-protocol/src/agent-event.types.ts index 43543c61d32..033970451e6 100644 --- a/packages/agent-event-protocol/src/agent-event.types.ts +++ b/packages/agent-event-protocol/src/agent-event.types.ts @@ -29,6 +29,10 @@ export interface AgentApprovalRequest { */ approveActionId?: string; denyActionId?: string; + /** Server-minted always-allow-this-tool action id. Echo via respondToAction / card click. */ + trustToolActionId?: string; + /** Server-minted always-allow-MCP-server action id (MCP tools only). */ + trustServerActionId?: string; } export type AgentSignal = diff --git a/packages/js/src/agent-chat/agent-chat.test.ts b/packages/js/src/agent-chat/agent-chat.test.ts index 85268b0bf8f..f0fbd948368 100644 --- a/packages/js/src/agent-chat/agent-chat.test.ts +++ b/packages/js/src/agent-chat/agent-chat.test.ts @@ -1467,6 +1467,44 @@ describe('AgentChat', () => { expect(derivePendingActions(snapshot?.messages ?? [])[0]?.type).toBe('tool-approval'); }); + it('respondToAction POSTs trust-server action id when decision is trust-server', async () => { + const page = approvalHistoryPage('approval_000001'); + const requestEvent = page.events[2]?.event as { + type: 'tool-approval-request'; + trustToolActionId?: string; + trustServerActionId?: string; + source?: { type: 'mcp'; serverName: string }; + }; + requestEvent.trustToolActionId = 'mcp-approval:approve-tool:approval_000001:deleteOrder:GitHub'; + requestEvent.trustServerActionId = 'mcp-approval:approve-server:approval_000001:deleteOrder:GitHub'; + requestEvent.source = { type: 'mcp', serverName: 'GitHub' }; + getEvents.mockResolvedValue(page); + respondToAction.mockResolvedValue({ + identifier: 'conv_abcdefghijkl', + }); + + await agentChat.loadConversation({ + agentId: 'agent_1', + conversationId: 'conv_abcdefghijkl', + }); + + const result = await agentChat.respondToAction({ + agentId: 'agent_1', + conversationId: 'conv_abcdefghijkl', + actionId: 'approval_000001', + decision: 'trust-server', + }); + + expect(result).toEqual({ + data: { conversationId: 'conv_abcdefghijkl' }, + }); + expect(respondToAction).toHaveBeenCalledWith({ + agentId: 'agent_1', + conversationId: 'conv_abcdefghijkl', + actionId: 'mcp-approval:approve-server:approval_000001:deleteOrder:GitHub', + }); + }); + it('respondToAction resolves pending state only after tool-approval-response envelope', async () => { getEvents.mockResolvedValue(approvalHistoryPage('approval_000001')); respondToAction.mockResolvedValue({ diff --git a/packages/js/src/agent-chat/agent-chat.ts b/packages/js/src/agent-chat/agent-chat.ts index 1cb843fb7d9..698030f83b5 100644 --- a/packages/js/src/agent-chat/agent-chat.ts +++ b/packages/js/src/agent-chat/agent-chat.ts @@ -140,7 +140,12 @@ export class AgentChat extends BaseModule { return { error: new NovuError('Pending action not found', new Error('pending action not found')) }; } - const actionId = args.decision === 'approved' ? pending.approveActionId : pending.denyActionId; + const actionId = { + approved: pending.approveActionId, + denied: pending.denyActionId, + 'trust-tool': pending.trustToolActionId, + 'trust-server': pending.trustServerActionId, + }[args.decision]; if (!actionId) { return { error: new NovuError( @@ -327,6 +332,10 @@ export class AgentChat extends BaseModule { return { error: result.error }; } + // Resume paths (approvals, card clicks) can emit before the HTTP ack; + // catch up like sendMessage so live WS overlap is not dropped. + this.#requestCatchUp(); + return { data: { conversationId: result.identifier } }; } catch (error) { if (error instanceof AgentChatPlanLimitError) { diff --git a/packages/js/src/agent-chat/agent-message.types.ts b/packages/js/src/agent-chat/agent-message.types.ts index 06aabd04155..c5ba85298db 100644 --- a/packages/js/src/agent-chat/agent-message.types.ts +++ b/packages/js/src/agent-chat/agent-message.types.ts @@ -11,6 +11,8 @@ export type AgentToolPartState = 'input-streaming' | 'input-available' | 'output export type AgentApprovalPartState = 'pending' | 'approved' | 'denied'; export type AgentMcpConnectionPartState = 'pending' | 'connected' | 'failed'; +export type AgentToolApprovalDecision = 'approved' | 'denied' | 'trust-tool' | 'trust-server'; + export type AgentTextPart = { type: 'text'; text: string; @@ -46,6 +48,10 @@ export type AgentApprovalPart = { approveActionId?: string; /** Server-minted; echo via respondToAction. Do not invent client-side. */ denyActionId?: string; + /** Server-minted always-allow-this-tool action id. Present for managed tools with trust support. */ + trustToolActionId?: string; + /** Server-minted always-allow-MCP-server action id. Present for MCP tools only. */ + trustServerActionId?: string; }; export type AgentMcpConnectionPart = { diff --git a/packages/js/src/agent-chat/apply-envelope.test.ts b/packages/js/src/agent-chat/apply-envelope.test.ts index e53377852d2..db19b35fe4e 100644 --- a/packages/js/src/agent-chat/apply-envelope.test.ts +++ b/packages/js/src/agent-chat/apply-envelope.test.ts @@ -210,6 +210,27 @@ describe('applyEnvelope', () => { expect(approval).toMatchObject({ type: 'approval', approvalId: 'a1', state: 'approved' }); }); + it('folds trust action ids onto approval parts', () => { + const state = applyEnvelopes(createInitialAgentConversationState(), [ + envelope(1, { + type: 'tool-approval-request', + approvalId: 'a1', + toolUseId: 'tu1', + toolName: 'create_issue', + trustToolActionId: 'mcp-approval:approve-tool:tu1:create_issue:GitHub', + trustServerActionId: 'mcp-approval:approve-server:tu1:create_issue:GitHub', + source: { type: 'mcp', serverName: 'GitHub' }, + }), + ]); + + expect(state.messages[0]?.parts[0]).toMatchObject({ + type: 'approval', + trustToolActionId: 'mcp-approval:approve-tool:tu1:create_issue:GitHub', + trustServerActionId: 'mcp-approval:approve-server:tu1:create_issue:GitHub', + source: { type: 'mcp', serverName: 'GitHub' }, + }); + }); + it('keeps replayed approval requests in their protocol message positions', () => { const state = applyEnvelopes(createInitialAgentConversationState(), [ envelope(1, { diff --git a/packages/js/src/agent-chat/apply-envelope.ts b/packages/js/src/agent-chat/apply-envelope.ts index 6936a8c60ec..c6e1e93dc12 100644 --- a/packages/js/src/agent-chat/apply-envelope.ts +++ b/packages/js/src/agent-chat/apply-envelope.ts @@ -145,6 +145,8 @@ function applyEvent(state: AgentConversationState, envelope: AgentEventEnvelope) source: event.source, approveActionId: event.approveActionId, denyActionId: event.denyActionId, + trustToolActionId: event.trustToolActionId, + trustServerActionId: event.trustServerActionId, state: 'pending', }, ], diff --git a/packages/js/src/agent-chat/index.ts b/packages/js/src/agent-chat/index.ts index 72cac8a0acf..22f5a7fdbd8 100644 --- a/packages/js/src/agent-chat/index.ts +++ b/packages/js/src/agent-chat/index.ts @@ -12,6 +12,7 @@ export type { AgentMessage, AgentPendingAction, AgentToolApprovalAction, + AgentToolApprovalDecision, FetchMoreArgs, FetchMoreResult, LoadConversationArgs, diff --git a/packages/js/src/agent-chat/types.ts b/packages/js/src/agent-chat/types.ts index a720cc0a0c9..00898b10327 100644 --- a/packages/js/src/agent-chat/types.ts +++ b/packages/js/src/agent-chat/types.ts @@ -7,6 +7,7 @@ import type { AgentMessage, AgentPendingAction, AgentToolApprovalAction, + AgentToolApprovalDecision, } from './agent-message.types'; export type { @@ -18,6 +19,7 @@ export type { AgentMessage, AgentPendingAction, AgentToolApprovalAction, + AgentToolApprovalDecision, }; /** @@ -74,7 +76,7 @@ export type FetchMoreResult = { export type RespondToActionArgs = AgentHashFields & { agentId: string; actionId: string; - decision: 'approved' | 'denied'; + decision: AgentToolApprovalDecision; conversationId?: string; key?: string; }; diff --git a/packages/js/src/index.ts b/packages/js/src/index.ts index 85ff672d702..5ef9d0c4f96 100644 --- a/packages/js/src/index.ts +++ b/packages/js/src/index.ts @@ -10,6 +10,7 @@ export type { AgentMessage, AgentPendingAction, AgentToolApprovalAction, + AgentToolApprovalDecision, FetchMoreArgs, FetchMoreResult, LoadConversationArgs, diff --git a/packages/react/src/hooks/useAgentChat.ts b/packages/react/src/hooks/useAgentChat.ts index b000a7a3471..ce4b474f947 100644 --- a/packages/react/src/hooks/useAgentChat.ts +++ b/packages/react/src/hooks/useAgentChat.ts @@ -6,6 +6,7 @@ import type { AgentHashFields, AgentMessage, AgentPendingAction, + AgentToolApprovalDecision, LoadConversationResult, NovuError, RespondToActionResult, @@ -71,7 +72,7 @@ export type UseAgentChatResult = { data?: SendMessageResult; error?: NovuError | AgentChatPlanLimitError; }>; - respondToAction: (args: { actionId: string; decision: 'approved' | 'denied' }) => Promise<{ + respondToAction: (args: { actionId: string; decision: AgentToolApprovalDecision }) => Promise<{ data?: RespondToActionResult; error?: NovuError | AgentChatPlanLimitError; }>; @@ -359,7 +360,7 @@ export const useAgentChat = (props: UseAgentChatProps): UseAgentChatResult => { ); const respondToAction = useCallback( - async (args: { actionId: string; decision: 'approved' | 'denied' }) => { + async (args: { actionId: string; decision: AgentToolApprovalDecision }) => { setError(undefined); const response = await novu.agentChat.respondToAction({ diff --git a/playground/agent-chat/src/components/approval-card.tsx b/playground/agent-chat/src/components/approval-card.tsx index b7008e8ad47..315fdd6bb28 100644 --- a/playground/agent-chat/src/components/approval-card.tsx +++ b/playground/agent-chat/src/components/approval-card.tsx @@ -1,13 +1,13 @@ 'use client'; -import type { AgentMessage, UseAgentChatResult } from '@novu/react'; +import type { AgentMessage, AgentToolApprovalDecision, UseAgentChatResult } from '@novu/react'; import { useState } from 'react'; import { CheckIcon, ChevronIcon, ShieldIcon, XIcon } from './icons'; export type RespondToAction = UseAgentChatResult['respondToAction']; type AgentApprovalPart = Extract; -type Decision = 'approved' | 'denied'; +type Decision = AgentToolApprovalDecision; const STATE_META: Record = { pending: { label: 'Needs review', tone: 'pending' }, @@ -79,9 +79,11 @@ export function ApprovalCard({ part, onRespond }: ApprovalCardProps) { } } - // The SDK resolves the minted action id from the part; without it the call cannot be made. const canApprove = Boolean(part.approveActionId); const canDeny = Boolean(part.denyActionId); + const canTrustTool = Boolean(part.trustToolActionId); + const canTrustServer = Boolean(part.trustServerActionId) && part.source?.type === 'mcp'; + const serverName = part.source?.type === 'mcp' ? part.source.serverName : undefined; return (
{busy === 'approved' ? : } - Approve + Approve once + {canTrustTool ? ( + + ) : null} + {canTrustServer && serverName ? ( + + ) : null} ) : (