feat(ts): add persistent_memory and user_id support#124
Conversation
- Add userId and persistentMemory to RunAgentConfig interface - Add user_id and persistent_memory to ExecutionRequest interface - Store fields in RunAgentClient with env var fallback (RUNAGENT_USER_ID) - Add getUserId() and getPersistentMemory() getter methods - Conditionally include user_id and persistent_memory in REST request body - Conditionally include user_id and persistent_memory in WebSocket request payload Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRunAgent client now captures optional ChangesUser identity & persistent-memory flow
Sequence Diagram(s)sequenceDiagram
participant Client as RunAgentClient
participant REST as RestClient
participant Socket as BaseWebSocketClient
participant Server as RunAgent API
Client->>Client: read config / env -> _userId, _persistentMemory
Client->>REST: runAgent(agentId, entrypoint, options, userId, persistentMemory)
REST->>Server: POST /execute {..., user_id?, persistent_memory?}
Server-->>REST: 200 / response
REST-->>Client: ApiResponse
Client->>Socket: runStream(agentId, entrypoint, options, userId, persistentMemory)
Socket->>Server: WS send {..., user_id?, persistent_memory?}
Server-->>Socket: stream events
Socket-->>Client: async events
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@runagent-ts/src/rest/index.ts`:
- Around line 62-75: The REST handler builds requestData and currently uses a
truthy check (if (userId)) which drops explicitly set empty strings; change that
to a nullish check (if (userId != null)) so user_id is included when caller sets
"" but excluded only when null/undefined, and apply the same change pattern to
the analogous logic in websocket/base.ts to keep REST and WS behavior
consistent; update the reference to requestData.user_id and ensure any similar
check for persistentMemory uses the same nullish pattern if intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 312fe65c-865f-450a-882c-04e775de3b53
📒 Files selected for processing (4)
runagent-ts/src/client/index.tsrunagent-ts/src/rest/index.tsrunagent-ts/src/types/index.tsrunagent-ts/src/websocket/base.ts
| const requestData: Record<string, unknown> = { | ||
| entrypoint_tag: entrypointTag, | ||
| input_args: inputArgs, | ||
| input_kwargs: inputKwargs, | ||
| timeout_seconds: timeoutSeconds, | ||
| async_execution: false, | ||
| } as JsonValue; | ||
| }; | ||
|
|
||
| if (userId) { | ||
| requestData.user_id = userId; | ||
| } | ||
| if (persistentMemory) { | ||
| requestData.persistent_memory = persistentMemory; | ||
| } |
There was a problem hiding this comment.
Use a nullish check for userId.
if (userId) drops an explicitly configured empty string, so the field can disappear even when the caller set it. Switching to a null/undefined check keeps the request body aligned with the stored config; please mirror the same behavior in websocket/base.ts so REST/WS stay consistent.
🔧 Proposed fix
- if (userId) {
+ if (userId != null) {
requestData.user_id = userId;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const requestData: Record<string, unknown> = { | |
| entrypoint_tag: entrypointTag, | |
| input_args: inputArgs, | |
| input_kwargs: inputKwargs, | |
| timeout_seconds: timeoutSeconds, | |
| async_execution: false, | |
| } as JsonValue; | |
| }; | |
| if (userId) { | |
| requestData.user_id = userId; | |
| } | |
| if (persistentMemory) { | |
| requestData.persistent_memory = persistentMemory; | |
| } | |
| const requestData: Record<string, unknown> = { | |
| entrypoint_tag: entrypointTag, | |
| input_args: inputArgs, | |
| input_kwargs: inputKwargs, | |
| timeout_seconds: timeoutSeconds, | |
| async_execution: false, | |
| }; | |
| if (userId != null) { | |
| requestData.user_id = userId; | |
| } | |
| if (persistentMemory) { | |
| requestData.persistent_memory = persistentMemory; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@runagent-ts/src/rest/index.ts` around lines 62 - 75, The REST handler builds
requestData and currently uses a truthy check (if (userId)) which drops
explicitly set empty strings; change that to a nullish check (if (userId !=
null)) so user_id is included when caller sets "" but excluded only when
null/undefined, and apply the same change pattern to the analogous logic in
websocket/base.ts to keep REST and WS behavior consistent; update the reference
to requestData.user_id and ensure any similar check for persistentMemory uses
the same nullish pattern if intended.
Code Review — TypeScript SDK persistent_memory / user_id (fix/ts-persistent-memory)The overall structure is correct: fields are added to config/types, stored in the client, threaded through to REST and WebSocket callers, and getters are provided. Two real bugs need fixing before merge. Bug 1 —
|
…upport - Fix: use `persistentMemory !== undefined` instead of `if (persistentMemory)` so `false` can be sent over the wire (REST + WebSocket) - Add RUNAGENT_PERSISTENT_MEMORY env var support with parseEnvBool helper, matching the Go SDK reference implementation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Re-review after 38e9b97 — everything looks correct, approving in spirit (can't self-approve).
Return type Constructor wiring: One pre-existing minor note (not introduced by this PR): All changes in 38e9b97 are correct. Ready to merge. |
Summary
userIdandpersistentMemorytoRunAgentConfiginterfaceuser_idandpersistent_memorytoExecutionRequestinterfaceRunAgentClientwith env var fallback (RUNAGENT_USER_ID)getUserId()andgetPersistentMemory()getter methodsBrings the TypeScript SDK in line with the Rust and C# reference implementations per
sdk_checklist.md.Test plan
getUserId()returns configured user ID (explicit or env var)getPersistentMemory()returns configured flag (defaults tofalse)user_idappears in REST POST body only when setpersistent_memoryappears in REST POST body only whentruenpm run buildto confirm TypeScript compiles🤖 Generated with Claude Code
Summary by CodeRabbit