feat(memory): added memory block and tool#372
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces a comprehensive memory system for workflows, enabling persistent storage and retrieval of both agent and raw data types. The implementation includes API routes, database schema, and memory block tools for managing workflow-specific memories.
- Added new
memorytable with proper indexing and soft delete capability in/apps/sim/db/migrations/0038_shocking_thor.sql - Implemented RESTful memory API endpoints with workflowId validation and type-specific data handling in
/apps/sim/app/api/memory/route.tsand/apps/sim/app/api/memory/[id]/route.ts - Created memory block tools (add, get, getAll, delete) with proper validation and error handling in
/apps/sim/tools/memory/ - Added memory block UI configuration with conditional rendering based on operation type in
/apps/sim/blocks/blocks/memory.ts - Integrated memory tools into the main registry with proper type definitions and interfaces in
/apps/sim/tools/registry.ts
💡 (5/5) You can turn off certain types of comments like style here!
17 file(s) reviewed, 26 comment(s)
Edit PR Review Bot Settings | Greptile
| const workflowId = url.searchParams.get('workflowId') | ||
| const searchQuery = url.searchParams.get('query') | ||
| const type = url.searchParams.get('type') | ||
| const limit = parseInt(url.searchParams.get('limit') || '50') |
There was a problem hiding this comment.
logic: Add validation for negative limit values and set a reasonable maximum limit to prevent excessive data retrieval
| const limit = parseInt(url.searchParams.get('limit') || '50') | |
| const rawLimit = parseInt(url.searchParams.get('limit') || '50') | |
| const limit = Math.min(Math.max(1, rawLimit), 100) // Ensure limit is between 1 and 100 |
| // Soft delete by setting deletedAt timestamp | ||
| await db | ||
| .update(memory) | ||
| .set({ | ||
| deletedAt: new Date(), | ||
| updatedAt: new Date() | ||
| }) | ||
| .where( | ||
| and( | ||
| eq(memory.key, id), | ||
| eq(memory.workflowId, workflowId) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
logic: Missing isNull(memory.deletedAt) in WHERE clause could allow updating already soft-deleted records
| // Soft delete by setting deletedAt timestamp | |
| await db | |
| .update(memory) | |
| .set({ | |
| deletedAt: new Date(), | |
| updatedAt: new Date() | |
| }) | |
| .where( | |
| and( | |
| eq(memory.key, id), | |
| eq(memory.workflowId, workflowId) | |
| ) | |
| ) | |
| // Soft delete by setting deletedAt timestamp | |
| await db | |
| .update(memory) | |
| .set({ | |
| deletedAt: new Date(), | |
| updatedAt: new Date() | |
| }) | |
| .where( | |
| and( | |
| eq(memory.key, id), | |
| eq(memory.workflowId, workflowId), | |
| isNull(memory.deletedAt) | |
| ) | |
| ) |
| const updatedMemories = await db | ||
| .select() | ||
| .from(memory) | ||
| .where( | ||
| and( | ||
| eq(memory.key, id), | ||
| eq(memory.workflowId, workflowId) | ||
| ) | ||
| ) | ||
| .limit(1) |
There was a problem hiding this comment.
logic: Missing isNull(memory.deletedAt) check when fetching updated memory could return soft-deleted records
| const updatedMemories = await db | |
| .select() | |
| .from(memory) | |
| .where( | |
| and( | |
| eq(memory.key, id), | |
| eq(memory.workflowId, workflowId) | |
| ) | |
| ) | |
| .limit(1) | |
| const updatedMemories = await db | |
| .select() | |
| .from(memory) | |
| .where( | |
| and( | |
| eq(memory.key, id), | |
| eq(memory.workflowId, workflowId), | |
| isNull(memory.deletedAt) | |
| ) | |
| ) | |
| .limit(1) |
| // Update the existing memory with appended data | ||
| await db | ||
| .update(memory) | ||
| .set({ | ||
| data: updatedData, | ||
| updatedAt: new Date() | ||
| }) | ||
| .where( | ||
| and( | ||
| eq(memory.key, key), | ||
| eq(memory.workflowId, workflowId) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
logic: Missing deletedAt check in update query could allow updates to soft-deleted records
| // Update the existing memory with appended data | |
| await db | |
| .update(memory) | |
| .set({ | |
| data: updatedData, | |
| updatedAt: new Date() | |
| }) | |
| .where( | |
| and( | |
| eq(memory.key, key), | |
| eq(memory.workflowId, workflowId) | |
| ) | |
| ) | |
| // Update the existing memory with appended data | |
| await db | |
| .update(memory) | |
| .set({ | |
| data: updatedData, | |
| updatedAt: new Date() | |
| }) | |
| .where( | |
| and( | |
| eq(memory.key, key), | |
| eq(memory.workflowId, workflowId), | |
| isNull(memory.deletedAt) | |
| ) | |
| ) |
| operation: { type: 'string', required: true }, | ||
| id: { type: 'string', required: true }, | ||
| type: { type: 'string', required: false }, |
There was a problem hiding this comment.
logic: id is marked as required in inputs but type is optional, which conflicts with validation logic that requires type for 'add' operation
| operation: { type: 'string', required: true }, | |
| id: { type: 'string', required: true }, | |
| type: { type: 'string', required: false }, | |
| operation: { type: 'string', required: true }, | |
| id: { type: 'string', required: true }, | |
| type: { type: 'string', required: true }, |
| import { memoryGetAllTool } from './get_all_memories' | ||
| import { memoryDeleteTool } from './delete_memory' | ||
|
|
||
| export { memoryAddTool, memoryGetTool, memoryGetAllTool, memoryDeleteTool } No newline at end of file |
There was a problem hiding this comment.
style: Missing newline at end of file
| export { memoryAddTool, memoryGetTool, memoryGetAllTool, memoryDeleteTool } | |
| export { memoryAddTool, memoryGetTool, memoryGetAllTool, memoryDeleteTool } |
| import { MemoryResponse } from './types' | ||
|
|
||
| // Get Memory Tool | ||
| export const memoryGetTool: ToolConfig<any, MemoryResponse> = { |
There was a problem hiding this comment.
style: Using any in ToolConfig generic type parameter reduces type safety. Consider creating proper type for the params object.
| export const memoryGetTool: ToolConfig<any, MemoryResponse> = { | |
| export const memoryGetTool: ToolConfig<{ id: string }, MemoryResponse> = { |
| } | ||
| }, | ||
| request: { | ||
| url: (params): any => { |
There was a problem hiding this comment.
style: Return type any on url function could be more specific like string | ErrorResponse
| memory?: any | ||
| memories?: any[] |
There was a problem hiding this comment.
style: Using any type reduces type safety. Consider defining specific types for memory and memories based on MemoryRecord
| } | ||
|
|
||
| export interface RawMemoryData { | ||
| [key: string]: any |
There was a problem hiding this comment.
style: Using index signature with any value type reduces type safety. Consider creating a more specific type or using generics
emir-karabeg
changed the title
waleedlatif1
force-pushed
the
main
branch
4 times, most recently
from
a18a617 to
b29827c
Compare
* feat(memory): added memory block and service * feat(memory): ran migrations * improvement(memory): appending memories; console messages * feat(memory): added agent raw message history input UI * feat(agent-messages): added agent message history * improvement: added tests
1 participant
Description
Created memory block and tool/service. Added agent raw message history and advanced mode.
Type of change
How Has This Been Tested?
Create a memory store and modify it or delete it using the memory block.
Checklist:
bun run test)Security Considerations: