feat(memory): add read-only event and record commands - #1895
Conversation
github-actions
Bot
added
agentcore-harness-reviewing
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #1895 +/- ##
==========================================
Coverage 96.08% 96.08%
==========================================
Files 224 231 +7
Lines 11282 11569 +287
==========================================
+ Hits 10840 11116 +276
- Misses 442 453 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| @@ -1,3 +1,17 @@ | |||
| import { | |||
There was a problem hiding this comment.
NIT - can this be a TS file since it doesn't contain any JSX
There was a problem hiding this comment.
ah yes, I was just following the convention in the other files. I can get a follow up PR that fixes it for everything
| name: "get", | ||
| description: "get an AgentCore Memory Event", | ||
| flags: [ | ||
| flag("memory", "the ID of the Memory", z.string()), |
There was a problem hiding this comment.
If you check out other primitives right now we use .optional() on everything. The general standard we've set in the CLI is to make all commands optional in Commander and then check for required flags separately and throw InputValidationError. The execution order we are looking for from my understanding is:
- Commander parses and validates required options.
- The router action runs.
- Middleware runs.
- The Runtime handler runs.
This is not meant to be forever, but until we figure out a solution for this. For example check out this PR: fix(runtime): defer invoke required flag validation #1876
There was a problem hiding this comment.
completely forgot about this one! fixing it now
| throw new InputValidationError("'--include-parent-branches' requires '--branch'"); | ||
| } | ||
|
|
||
| const eventMetadata = parseJsonFlag<EventMetadataFilterExpression[]>( |
There was a problem hiding this comment.
parseJsonFlag<EventMetadataFilterExpression[]> only casts the parsed JSON. I tested --metadata-filters '{}', and it reached the SDK as {"filter":{"eventMetadata":{}}} instead of failing locally. The command contract says structured JSON should reject incorrect top-level types and invalid members, so event and record filters should use runtime schemas and test a non-array and malformed expression.
There was a problem hiding this comment.
nice catch, lemme fix it
| agentcore memory get --id <memoryId> | ||
| agentcore memory get --id <memoryId> --view without_decryption | ||
| agentcore memory list --max-results 20 | ||
| agentcore memory event get --memory <memoryId> --actor-id <actorId> --session-id <sessionId> --event-id <eventId> |
There was a problem hiding this comment.
The examples include these commands, but the command tree still shows Memory with only get and list. We should add the event and record groups there too.
There was a problem hiding this comment.
interesting, I thought it did, let me check
There was a problem hiding this comment.
Would be lines 68-70 i believe
aidandaly24
left a comment
aidandaly24
left a comment
There was a problem hiding this comment.
Thank you for making the updates this LGTM
f43ded3 to
ae6a526
Compare
| z.object({ stringValue: z.string() }).strict(), | ||
| z.object({ stringListValue: z.array(z.string()) }).strict(), | ||
| z.object({ numberValue: z.number() }).strict(), | ||
| z.object({ dateTimeValue: z.coerce.date() }).strict(), |
There was a problem hiding this comment.
Can we replace this with something like:
z.object({
dateTimeValue: z.iso.datetime().transform((value) => new Date(value)),
}).strict()
Otherwise, z.coerce.date() will accept values like null and zero and resolve them to dates instead of rejecting the input
5 participants
This PR adds
getandlistcommands for memory events and records and tests for the same.Core + handler implementation only, no-TUI so far.
bun run typecheck, format, lint and test passed (there was one un-related failure, not introduced by this commit. ran
bun test src/handlers/memory/memory.test.tsxto confirm)Smoke tested with my account.