fix(framework): remove duplicate message from AgentMessageContext fixes NV-7544#11010
fix(framework): remove duplicate message from AgentMessageContext fixes NV-7544#11010djabarovgeorge wants to merge 2 commits intonextfrom
Conversation
…es NV-7544 Remove `message` property from `AgentMessageContext` interface to eliminate the confusing duplication between `payload.message` and `payload.ctx.message` in the `onMessage` handler. The message is now exclusively available via `payload.message` (the destructured first argument). The `ctx` object retains conversation, subscriber, history, platform, and action methods only. - Remove `readonly message` from AgentMessageContext type - Update AgentHandlers JSDoc to reference payload.message - Update README template to show correct usage pattern - Update tests to verify message comes from payload, not ctx Co-authored-by: George Djabarov <djabarovgeorge@users.noreply.github.com>
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
📝 WalkthroughWalkthroughAgent handler typing is clarified: ChangesAgent Handler Payload Restructuring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/framework/src/resources/agent/agent.types.ts (1)
273-276:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftBreaking change without deprecation violates package guidelines.
Per the PR description,
ctx.messageis being removed fromAgentMessageContext, which is a breaking change to the public framework API. As per coding guidelines, package symbols should be deprecated with@deprecatedJSDoc before removal to give consumers a migration window.Consider a two-phase approach:
- Current release: Keep
ctx.messagebut mark it@deprecatedwith a JSDoc comment directing users to destructuremessagefrom the payload- Next major release: Remove the deprecated property
This provides a smoother migration path and follows established semver conventions for packages.
As per coding guidelines: "Deprecate symbols with
@deprecatedJSDoc before removing them from packages" and "Treat all exported symbols in packages as public API; follow semver conventions with breaking changes requiring major bumps."🤖 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/resources/agent/agent.types.ts` around lines 273 - 276, AgentMessageContext currently removes the publicly exported ctx.message property which is a breaking change; restore the message property on AgentMessageContext but mark it with an `@deprecated` JSDoc comment pointing consumers to destructure message from the payload (e.g., "Use payload.message" or similar), keeping the existing type/signature so consumers continue to compile; plan to remove the deprecated message property in the next major release. Ensure the deprecated JSDoc is attached to the message property in the AgentMessageContext interface so linters and IDEs surface the deprecation.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@packages/framework/src/resources/agent/agent.types.ts`:
- Around line 273-276: AgentMessageContext currently removes the publicly
exported ctx.message property which is a breaking change; restore the message
property on AgentMessageContext but mark it with an `@deprecated` JSDoc comment
pointing consumers to destructure message from the payload (e.g., "Use
payload.message" or similar), keeping the existing type/signature so consumers
continue to compile; plan to remove the deprecated message property in the next
major release. Ensure the deprecated JSDoc is attached to the message property
in the AgentMessageContext interface so linters and IDEs surface the
deprecation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b38ea78-0f48-4e5a-80fd-15d0b9fc053b
📒 Files selected for processing (3)
packages/framework/src/resources/agent/agent.test.tspackages/framework/src/resources/agent/agent.types.tspackages/novu/src/commands/init/templates/app-agent/ts/README-template.md
What changed
Removed the
messageproperty from theAgentMessageContextinterface to eliminate the confusing duplication betweenpayload.messageandpayload.ctx.messagein theonMessagehandler.Previously, the same
AgentMessageobject was available at both locations:Now
messageis exclusively available viapayload.message(the destructured first argument). Thectxobject retains only conversation state, subscriber info, history, platform context, and action methods (reply,resolve,trigger,metadata,addReaction).Changes
agent.types.ts— Removedreadonly message: AgentMessagefromAgentMessageContextinterfaceagent.types.ts— UpdatedAgentHandlers.onMessageJSDoc to clarifypayload.messageusageREADME-template.md— Updated agent API docs and code example to usepayload.messagepatternagent.test.ts— Updated test to verify message arrives via payload instead of ctxBreaking changes
ctx.messageis no longer available inonMessagehandlers. Use the destructuredmessagefrom the payload instead:Linear Issue: NV-7544
What changed
The
messageproperty was removed from theAgentMessageContextinterface to eliminate duplication where the message was available both aspayload.messageandpayload.ctx.messageinonMessagehandlers. The message is now accessed only through the destructured payload parameter, whilectxretains conversation state, subscriber information, history, and action methods.Affected areas
framework: Updated
AgentMessageContexttype definition andonMessagehandler JSDoc to reflect thatmessagecomes from the payload parameter, not the context object. Tests were updated to verify the new parameter shape.documentation: README template examples updated to show the new handler signature pattern—
onMessage: async ({ message, ctx }) => { ... }—and updated code snippets to usemessage.textinstead ofctx.message?.text.Key technical decisions
ctx.messageis no longer available inonMessagehandlers. Code must be updated to access the message through the destructuredmessageparameter instead.Testing
Existing test was updated to verify that the message arrives through the payload parameter rather than the context object.