feat(team): per-request userId + TEAM_MODE=private enforcement#689
Open
chdlc wants to merge 5 commits into
Open
feat(team): per-request userId + TEAM_MODE=private enforcement#689chdlc wants to merge 5 commits into
chdlc wants to merge 5 commits into
Conversation
Add resolveUserId() helper that accepts optional per-request override, falling back to AGENTMEMORY_USER_ID env var. Trimmed + length-capped to match agentId conventions. resolveTeamId() reads TEAM_ID from server env (no per-request override since team is server-level config). Updated team-share to accept optional userId in its payload, used for sharedBy field and audit trail. team-feed and team-profile now use resolveTeamId() for consistent team ID resolution but do not accept userId since they are team-level read operations. MCP server memory_team_share and memory_team_feed read AGENTMEMORY_USER_ID from their process env and pass it through automatically. This enables single-server multi-user deployments where each integration (MCP client, plugin, etc.) defines AGENTMEMORY_USER_ID in its own env, and the identity flows through to agentmemory without modifying requests. Signed-off-by: Christian de la Cruz <chrisdlc119@outlook.com>
team-feed and team-profile now respect TEAM_MODE: - shared (default): shows all team items, no filtering - private: filters by configured userId, ignores body override team-share in private mode forces sharedBy to config.userId, preventing impersonation — users can only share as themselves. Body userId override is ignored in private mode for all three operations (team-share, team-feed, team-profile), ensuring true read+write privacy. Signed-off-by: Christian de la Cruz <chrisdlc119@outlook.com>
Add userId to memory_team_share and memory_team_feed inputSchema so MCP clients (Cursor, Claude Desktop, Codex, Kilo, OpenCode, etc.) can pass per-request userId without relying on env vars. This enables multi-user deployments behind a single MCP server where each client identifies itself via the userId parameter. Signed-off-by: Christian de la Cruz <chrisdlc119@outlook.com>
…ER_ID Add section explaining TEAM_MODE=private/shared behavior, per-request userId override via MCP tool calls, and combined multi-agent + multi-user setup. Update env vars section to show USER_ID as server-level default with AGENTMEMORY_USER_ID as per-integration override. Signed-off-by: Christian de la Cruz <chrisdlc119@outlook.com>
|
@chdlc is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
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 (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds multi-user team memory support with per-request user identity overrides and private-mode isolation. It introduces config resolution helpers ( ChangesMulti-user team memory with private mode enforcement
Sequence DiagramsequenceDiagram
participant MCP as MCP Tool Call
participant Server as server.ts
participant Resolve as resolveUserId
participant TeamFn as Team Function
participant Mode as Mode Check
participant KV as KV/Audit
MCP->>Server: memory_team_share<br/>(userId optional)
Server->>Resolve: resolveUserId(args.userId)
Resolve-->>Server: canonical userId
Server->>TeamFn: mem::team-share payload
TeamFn->>Mode: private or shared?
Mode-->>TeamFn: private: use config.userId only
Mode-->>TeamFn: shared: use resolved userId
TeamFn->>KV: write shared item,<br/>audit with resolved IDs
KV-->>TeamFn: success
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/functions/team.ts (1)
113-181:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
mem::team-profiledrops per-request identity and audits the wrong actor
data.userIdis accepted but never resolved/used, andrecordAudit(..., config.userId)hardcodes identity. In shared mode this can misattribute audit entries for profile operations.Suggested fix
sdk.registerFunction("mem::team-profile", async (data?: { userId?: string }) => { const teamId = resolveTeamId() ?? config.teamId; + const actorUserId = + config.mode === "private" + ? config.userId + : (resolveUserId(data?.userId) ?? config.userId); let items = await kv.list<TeamSharedItem>(KV.teamShared(teamId)); @@ await recordAudit( @@ - config.userId, + actorUserId, ); return profile; });🤖 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 `@src/functions/team.ts` around lines 113 - 181, The team profile handler ("mem::team-profile") currently ignores the per-request identity (data.userId) and always audits using config.userId; change it to respect data.userId in non-private mode and use that same resolved actor when calling recordAudit. Specifically: keep the existing private-mode behavior that forces the filter to config.userId, but otherwise resolve an actorId = data?.userId ?? config.userId (or use a resolveUserId helper if available), use that actorId for any per-request scoping and for the members list/audit metadata, and replace the hardcoded config.userId in the recordAudit(...) call with this resolved actorId so the audit entry reflects the actual requester.
🧹 Nitpick comments (1)
src/config.ts (1)
272-281: ⚡ Quick winRemove WHAT-style comments in helper section
These comments describe behavior that should be expressed by naming/tests instead of inline prose in
src/**/*.ts.As per coding guidelines, "
src/**/*.ts: Avoid code comments explaining WHAT — use clear naming instead".🤖 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 `@src/config.ts` around lines 272 - 281, Remove the WHAT-style inline comments above the helper functions: delete the prose blocks preceding resolveTeamId and resolveUserId in src/config.ts and instead rely on clear naming and tests; if any behavior needs clarification, encode it via more descriptive function names or add unit tests for resolveTeamId() and resolveUserId(override?: string) that assert env-override precedence and trimming/length rules rather than keeping the explanatory comments.
🤖 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.
Inline comments:
In `@src/mcp/server.ts`:
- Line 533: Update the log/error string that currently reads "Team memory not
enabled. Set TEAM_ID and AGENTMEMORY_USER_ID" to accurately reflect that
AGENTMEMORY_USER_ID is preferred but USER_ID is accepted as a fallback; mention
both keys (e.g., "Set TEAM_ID and AGENTMEMORY_USER_ID (or USER_ID as a
fallback)") and replace the two occurrences in src/mcp/server.ts (the two
message literals at the locations around the current text and the other one at
the second occurrence).
In `@test/config-resolve.test.ts`:
- Around line 12-16: The afterEach restoration loop for process.env (in the
afterEach block referencing originalEnv and process.env) only overwrites
existing keys and leaves any new keys added by tests, leaking state; update the
afterEach to fully restore the environment by removing keys not present in
originalEnv and then restoring originalEnv entries (or replace process.env with
a shallow copy of originalEnv) so that tests cannot leak environment variables
across runs; apply the same fix to the other afterEach copy referenced around
lines 36-40.
---
Outside diff comments:
In `@src/functions/team.ts`:
- Around line 113-181: The team profile handler ("mem::team-profile") currently
ignores the per-request identity (data.userId) and always audits using
config.userId; change it to respect data.userId in non-private mode and use that
same resolved actor when calling recordAudit. Specifically: keep the existing
private-mode behavior that forces the filter to config.userId, but otherwise
resolve an actorId = data?.userId ?? config.userId (or use a resolveUserId
helper if available), use that actorId for any per-request scoping and for the
members list/audit metadata, and replace the hardcoded config.userId in the
recordAudit(...) call with this resolved actorId so the audit entry reflects the
actual requester.
---
Nitpick comments:
In `@src/config.ts`:
- Around line 272-281: Remove the WHAT-style inline comments above the helper
functions: delete the prose blocks preceding resolveTeamId and resolveUserId in
src/config.ts and instead rely on clear naming and tests; if any behavior needs
clarification, encode it via more descriptive function names or add unit tests
for resolveTeamId() and resolveUserId(override?: string) that assert
env-override precedence and trimming/length rules rather than keeping the
explanatory comments.
🪄 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: 6fb07e12-92e2-4c3c-a573-0d6cf2dd2f9c
📒 Files selected for processing (7)
README.mdsrc/config.tssrc/functions/team.tssrc/mcp/server.tssrc/mcp/tools-registry.tstest/config-resolve.test.tstest/team.test.ts
- Update team memory error messages to mention USER_ID as fallback - Fix test env leak in config-resolve.test.ts afterEach blocks - Resolve per-request userId in team-profile audit entry - Remove WHAT-style inline comments from resolve helpers Signed-off-by: Christian <chrisdlc119@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
TEAM_MODEwas read but never used for filtering —privateandsharedbehaved identically. There was no way to identify which user made a team operation from the MCP server or integrations.team-shareaccepted anysharedByvalue in the request body, enabling impersonation.Why
Without per-request userId, a single MCP server cannot enforce privacy boundaries between different users' team memories. All team operations appear as unscoped, and any caller can impersonate any other user by setting
sharedByin the request body.How
src/config.ts:resolveUserId()readsAGENTMEMORY_USER_IDfrom process env, falling back toUSER_ID(server-level default). This lets each integration define its own user identity.src/functions/team.ts:team-share: accepts optionaluserId?in payload. InTEAM_MODE=private, forcessharedByto the configured userId (prevents impersonation). Insharedmode, body override works.team-feed: accepts optionaluserId?. InTEAM_MODE=private, filters items to configured user only. Inshared, shows all.team-profile: same filtering logic as team-feed.src/mcp/server.ts: All three team handlers (team-share,team-feed,team-profile) readAGENTMEMORY_USER_IDfrom env viaresolveUserId()and inject it automatically.src/mcp/tools-registry.ts:memory_team_shareandmemory_team_feedtools exposeuserIdas an optional parameter, so MCP clients can identify themselves without relying on env vars.README.md: Documents the server-level vs integration-level env distinction (USER_ID vs AGENTMEMORY_USER_ID), examples with alice/bob, and impersonation protection.Verification
AGENTMEMORY_USER_IDenv override vsUSER_IDfallbackuserIdper-request override from MCP tool argumentsTEAM_MODE=privatefiltering in team-feed, team-profilesharedByignored in private modeRelated
agentIdper-call isolation)agentIdscopes regular memory operations (remember, smart-search, observe)userIdscopes team operations (team-share, team-feed, team-profile)Summary by CodeRabbit
New Features
Documentation
Tests