Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 7 additions & 10 deletions apps/api/src/app/agents/e2e/mock-agent-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ if (!NOVU_SECRET_KEY) {
}

const echoBot = agent('novu-agent', {
onMessage: async (ctx) => {
onMessage: async ({ message, ctx }) => {
console.log('\n─────────────────────────────────────────');
console.log(`[${ctx.event}] from ${ctx.subscriber?.firstName ?? 'unknown'} on ${ctx.platform}`);
console.log(`Message: ${ctx.message?.text ?? '(none)'}`);
console.log(`[onMessage] from ${ctx.subscriber?.firstName ?? 'unknown'} on ${ctx.platform}`);
console.log(`Message: ${message.text ?? '(none)'}`);
console.log(`Conversation: ${ctx.conversation.identifier} (${ctx.conversation.status})`);
console.log(`History: ${ctx.history.length} entries`);
console.log('─────────────────────────────────────────');

const userText = ctx.message?.text ?? '';
const userText = message.text ?? '';
const turnCount = (ctx.conversation.metadata?.turnCount as number) ?? 0;

ctx.metadata.set('turnCount', turnCount + 1);
Expand Down Expand Up @@ -128,14 +128,11 @@ const echoBot = agent('novu-agent', {
await ctx.reply(`Echo: ${userText}`);
},

onAction: async (ctx) => {
onAction: async ({ actionId, value, ctx }) => {
console.log('\n─────────────────────────────────────────');
console.log(`[${ctx.event}] action: ${ctx.action?.actionId} = ${ctx.action?.value ?? '(no value)'}`);
console.log(`[onAction] action: ${actionId} = ${value ?? '(no value)'}`);
console.log('─────────────────────────────────────────');

const actionId = ctx.action?.actionId ?? 'unknown';
const value = ctx.action?.value;

if (actionId === 'ack') {
await ctx.reply(
Card({
Expand All @@ -162,7 +159,7 @@ const echoBot = agent('novu-agent', {
}
},

onResolve: async (ctx) => {
onResolve: async ({ ctx }) => {
console.log(`\n[onResolve] Conversation ${ctx.conversation.identifier} closed.`);
ctx.metadata.set('resolvedAt', new Date().toISOString());
},
Expand Down
68 changes: 53 additions & 15 deletions packages/framework/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ import {
SigningKeyNotFoundError,
} from './errors';
import { isPlatformError } from './errors/guard.errors';
import type { Agent, AgentBridgeRequest, MessageContent } from './resources/agent';
import type {
Agent,
AgentActionContext,
AgentBridgeRequest,
AgentMessageContext,
AgentReactionContext,
AgentResolveContext,
MessageContent,
} from './resources/agent';
import { AgentContextImpl, AgentDeliveryError, AgentEventEnum } from './resources/agent';
import type { Awaitable, EventTriggerParams, Workflow } from './types';
import { createHmacSubtle, initApiClient } from './utils';
Expand Down Expand Up @@ -309,21 +317,51 @@ export class NovuRequestHandler<Input extends any[] = any[], Output = any> {
}

private async runAgentHandler(registeredAgent: Agent, event: string, ctx: AgentContextImpl): Promise<void> {
const handlerMap = {
[AgentEventEnum.ON_MESSAGE]: registeredAgent.handlers.onMessage,
[AgentEventEnum.ON_REACTION]: registeredAgent.handlers.onReaction,
[AgentEventEnum.ON_ACTION]: registeredAgent.handlers.onAction,
[AgentEventEnum.ON_RESOLVE]: registeredAgent.handlers.onResolve,
} as Partial<Record<AgentEventEnum, (ctx: AgentContextImpl) => Awaitable<MessageContent | void>>>;

if (!Object.prototype.hasOwnProperty.call(handlerMap, event)) {
throw new InvalidActionError(event, AgentEventEnum);
}

const handler = handlerMap[event as AgentEventEnum];
if (handler) {
const result = await handler(ctx);
const replyIfPresent = async (result: MessageContent | void) => {
if (result != null) await ctx.reply(result);
};

switch (event) {
case AgentEventEnum.ON_MESSAGE:
await replyIfPresent(
await registeredAgent.handlers.onMessage({
message: ctx.message!,
ctx: ctx as unknown as AgentMessageContext,
})
);
break;
case AgentEventEnum.ON_ACTION:
if (registeredAgent.handlers.onAction) {
await replyIfPresent(
await registeredAgent.handlers.onAction({
actionId: ctx.action!.actionId,
value: ctx.action!.value,
ctx: ctx as unknown as AgentActionContext,
})
);
}
break;
case AgentEventEnum.ON_REACTION:
if (registeredAgent.handlers.onReaction) {
await replyIfPresent(
await registeredAgent.handlers.onReaction({
reaction: ctx.reaction!,
ctx: ctx as unknown as AgentReactionContext,
})
);
}
break;
case AgentEventEnum.ON_RESOLVE:
if (registeredAgent.handlers.onResolve) {
await replyIfPresent(
await registeredAgent.handlers.onResolve({
ctx: ctx as unknown as AgentResolveContext,
})
);
Comment on lines +326 to +360
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Non-null assertions on ctx.message / ctx.action / ctx.reaction may forward undefined to handlers on malformed payloads.

For each event branch, the corresponding field is forced with ! (Lines 328, 337-338, 348) and then handed to user code typed as required. If a request reaches AGENT_EVENT with event=onMessage but the body lacks message (or action.actionId / reaction for the other branches), the handler will be invoked with undefined despite the type promising otherwise, leading to confusing Cannot read properties of undefined errors inside user code rather than a clear framework-level rejection.

Consider validating presence before dispatch and throwing a descriptive error (e.g. via InvalidActionError or a dedicated agent error) so the contract between event name and payload shape is enforced at the boundary.

🛡️ Sketch of a guard at dispatch time
       case AgentEventEnum.ON_MESSAGE:
+        if (!ctx.message) {
+          throw new InvalidActionError(event, AgentEventEnum);
+        }
         await replyIfPresent(
           await registeredAgent.handlers.onMessage({
-            message: ctx.message!,
+            message: ctx.message,
             ctx: ctx as unknown as AgentMessageContext,
           })
         );
         break;

Apply the analogous guard to ON_ACTION (ctx.action), ON_REACTION (ctx.reaction), and ON_RESOLVE if it has any required fields.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/framework/src/handler.ts` around lines 326 - 360, The handlers
dispatch currently uses non-null assertions (ctx.message!, ctx.action!,
ctx.reaction!) and may call
registeredAgent.handlers.onMessage/onAction/onReaction/onResolve with undefined
on malformed payloads; update the dispatch in the switch branches to validate
the required payload fields (e.g., ensure ctx.message exists for onMessage,
ctx.action and ctx.action.actionId for onAction, ctx.reaction for onReaction,
etc.) before calling replyIfPresent/handler, and if validation fails throw a
clear framework error (e.g., InvalidAgentEventPayload or InvalidActionError)
with a descriptive message so user handlers are never invoked with undefined.

}
break;
default:
throw new InvalidActionError(event, AgentEventEnum);
}

await ctx.flush();
Expand Down
Loading
Loading