feat: store sent emails as memories when room_id provided#97
feat: store sent emails as memories when room_id provided#97sweetmantech wants to merge 4 commits intotestfrom
Conversation
- Call insertMemories to store assistant email content - Call insertMemoryEmail to link email with memory - Only insert memory when room_id is provided - Add tests for memory insertion behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 |
| await insertMemories({ | ||
| id: memoryId, | ||
| room_id, | ||
| content: { | ||
| role: "assistant", | ||
| content: emailContent, | ||
| }, | ||
| }); | ||
|
|
||
| await insertMemoryEmail({ | ||
| email_id: result.id, | ||
| memory: memoryId, | ||
| message_id: result.id, | ||
| }); |
There was a problem hiding this comment.
The calls to insertMemories and insertMemoryEmail are not wrapped in error handling, but both functions throw errors on failure. This means if memory insertion fails, the tool will crash even though the email was successfully sent, returning a failure to the user when the operation actually succeeded.
View Details
📝 Patch Details
diff --git a/lib/mcp/tools/__tests__/registerSendEmailTool.test.ts b/lib/mcp/tools/__tests__/registerSendEmailTool.test.ts
index d093934..1098dcb 100644
--- a/lib/mcp/tools/__tests__/registerSendEmailTool.test.ts
+++ b/lib/mcp/tools/__tests__/registerSendEmailTool.test.ts
@@ -152,4 +152,57 @@ describe("registerSendEmailTool", () => {
expect(mockInsertMemories).not.toHaveBeenCalled();
expect(mockInsertMemoryEmail).not.toHaveBeenCalled();
});
+
+ it("returns success even if insertMemories fails when room_id is provided", async () => {
+ mockSendEmailWithResend.mockResolvedValue({ id: "email-456" });
+ mockInsertMemories.mockRejectedValue(new Error("Database connection failed"));
+
+ const result = await registeredHandler({
+ to: ["test@example.com"],
+ subject: "Test Subject",
+ text: "Email content for memory",
+ room_id: "room-789",
+ });
+
+ // Should still return success because the email was sent
+ expect(result).toEqual({
+ content: [
+ {
+ type: "text",
+ text: expect.stringContaining("Email sent successfully"),
+ },
+ ],
+ });
+
+ // insertMemories was called but error was handled gracefully
+ expect(mockInsertMemories).toHaveBeenCalled();
+ expect(mockInsertMemoryEmail).not.toHaveBeenCalled();
+ });
+
+ it("returns success even if insertMemoryEmail fails when room_id is provided", async () => {
+ mockSendEmailWithResend.mockResolvedValue({ id: "email-456" });
+ mockInsertMemories.mockResolvedValue({ id: "mock-uuid-123" });
+ mockInsertMemoryEmail.mockRejectedValue(new Error("Permission denied"));
+
+ const result = await registeredHandler({
+ to: ["test@example.com"],
+ subject: "Test Subject",
+ text: "Email content for memory",
+ room_id: "room-789",
+ });
+
+ // Should still return success because the email was sent
+ expect(result).toEqual({
+ content: [
+ {
+ type: "text",
+ text: expect.stringContaining("Email sent successfully"),
+ },
+ ],
+ });
+
+ // Both were called but error was handled gracefully
+ expect(mockInsertMemories).toHaveBeenCalled();
+ expect(mockInsertMemoryEmail).toHaveBeenCalled();
+ });
});
diff --git a/lib/mcp/tools/registerSendEmailTool.ts b/lib/mcp/tools/registerSendEmailTool.ts
index 9216858..a6bccfa 100644
--- a/lib/mcp/tools/registerSendEmailTool.ts
+++ b/lib/mcp/tools/registerSendEmailTool.ts
@@ -48,23 +48,29 @@ export function registerSendEmailTool(server: McpServer): void {
// If room_id is provided, store the sent email as a memory
if (room_id && result.id) {
- const memoryId = generateUUID();
- const emailContent = text || html || "";
+ try {
+ const memoryId = generateUUID();
+ const emailContent = text || html || "";
- await insertMemories({
- id: memoryId,
- room_id,
- content: {
- role: "assistant",
- content: emailContent,
- },
- });
+ await insertMemories({
+ id: memoryId,
+ room_id,
+ content: {
+ role: "assistant",
+ content: emailContent,
+ },
+ });
- await insertMemoryEmail({
- email_id: result.id,
- memory: memoryId,
- message_id: result.id,
- });
+ await insertMemoryEmail({
+ email_id: result.id,
+ memory: memoryId,
+ message_id: result.id,
+ });
+ } catch (error) {
+ // Log memory insertion error but don't fail the email send operation
+ // The email was successfully sent, so we report success even if memory storage failed
+ console.error("Error storing email in memory:", error);
+ }
}
return getToolResultSuccess({
Analysis
Missing error handling for memory storage in sendEmailTool allows email success to be misreported as failure
What fails: registerSendEmailTool calls insertMemories() and insertMemoryEmail() without try-catch blocks (lines 54-67 in registerSendEmailTool.ts). Both functions throw errors on failure, causing unhandled exceptions that bubble up and fail the entire tool handler even when the email was successfully sent.
How to reproduce:
// Call sendEmailTool with valid email parameters and room_id
// Configure insertMemories to fail with database error (e.g., connection timeout, permission denied)
const handler = registerSendEmailTool(server);
await handler({
to: ["test@example.com"],
subject: "Test",
text: "Test body",
room_id: "room-123" // This triggers memory storage
});
// insertMemories throws: Error("Database connection failed")
// Handler throws unhandled error instead of returning tool success responseResult: MCP tool handler throws unhandled error. Per MCP SDK specification, tool execution errors should be reported within the result object using isError: true, not as unhandled exceptions that bubble up as protocol-level errors.
Expected: Tool returns success (email was sent) with error logged for memory storage failure. User sees successful result because the primary operation (email send) succeeded; memory storage is a side effect.
Root cause: Memory insertion code (lines 54-67) lacks try-catch block. Similar tools in the codebase (registerUpdateArtistSocialsTool, registerGenerateTxtFileTool) follow the pattern of wrapping async operations in try-catch.
Fix: Wrapped memory insertion in try-catch block that logs errors but allows tool to return success since email was already sent.
- Merge test branch changes (marked, zod email validation) - Use result.id from Resend as memoryId for consistency with inbound emails - Remove generateUUID dependency 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ory content Consistent with validateNewEmailMemory pattern for storing email memories 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| await insertMemories({ | ||
| id: result.id, | ||
| room_id, | ||
| content: filterMessageContentForMemories(assistantMessage), | ||
| }); | ||
|
|
||
| await insertMemoryEmail({ | ||
| email_id: result.id, | ||
| memory: result.id, | ||
| message_id: result.id, | ||
| }); |
There was a problem hiding this comment.
| await insertMemories({ | |
| id: result.id, | |
| room_id, | |
| content: filterMessageContentForMemories(assistantMessage), | |
| }); | |
| await insertMemoryEmail({ | |
| email_id: result.id, | |
| memory: result.id, | |
| message_id: result.id, | |
| }); | |
| // Only insert memory if content is not empty | |
| if (assistantMessage) { | |
| await insertMemories({ | |
| id: result.id, | |
| room_id, | |
| content: filterMessageContentForMemories(assistantMessage), | |
| }); | |
| await insertMemoryEmail({ | |
| email_id: result.id, | |
| memory: result.id, | |
| message_id: result.id, | |
| }); | |
| } |
The code will crash when attempting to store an email as memory if both text and html are empty or not provided, because getMessages() returns an empty array and accessing [0] gives undefined, which then causes filterMessageContentForMemories() to fail when trying to read properties.
View Details
Analysis
Email memory insertion crash with empty content
What fails: registerSendEmailTool() crashes when attempting to store an email as memory with empty text and html fields because getMessages() returns an empty array, and accessing [0] returns undefined, which then causes filterMessageContentForMemories() to throw TypeError: Cannot read properties of undefined (reading 'role')
How to reproduce:
# Call the send_email MCP tool with empty email content and room_id:
# - text: undefined or ""
# - html: "" (default)
# - room_id: "some-room-id" (triggers memory insertion)await registeredHandler({
to: ["test@example.com"],
subject: "Test Subject",
text: undefined, // empty
html: "", // empty
room_id: "room-789", // triggers memory insertion code path
});Result: Unhandled exception: TypeError: Cannot read properties of undefined (reading 'role')
Expected: Email should be sent successfully without attempting memory insertion when content is empty
Fix: Added null check for assistantMessage at line 57 in lib/mcp/tools/registerSendEmailTool.ts. Memory insertion only occurs if the message has content.
Memory linking should be handled in Chat repo where message_id is available after the assistant message is saved to the memories table. The MCP tool now just: - Sends the email via Resend - Returns email_id in result for Chat to use with insertMemoryEmail 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Closing - memory linking will be implemented in the Chat repo where message_id is available |
Summary
insertMemoriesto store assistant email content as a memoryinsertMemoryEmailto link the sent email with the memoryroom_idis provided (to associate with a conversation)Test plan
🤖 Generated with Claude Code