fix(room-worker): fix org-member duplication and empty sys-message content#185
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds sys-message formatting and name-validation helpers; introduces typed member-left/removed message constants; persists DM participant ChangesRoom-worker membership fixes & DM participant persistence
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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)
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}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
room-worker/handler.go (1)
479-481: ⚡ Quick winUse message-type constants for
MemberRemoveEvent.Type.On Line 480 and Line 542,
MemberRemoveEvent.Typeis set frommodel.OutboxMemberRemoved. Even if literals currently match, this couples member-event payload semantics to outbox constants. Prefermodel.MessageTypeMemberRemovedfor payload contract clarity.💡 Suggested fix
- Type: model.OutboxMemberRemoved, + Type: model.MessageTypeMemberRemoved, ... - Type: model.OutboxMemberRemoved, + Type: model.MessageTypeMemberRemoved,Also applies to: 541-543
🤖 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 `@room-worker/handler.go` around lines 479 - 481, The MemberRemoveEvent is using the outbox constant model.OutboxMemberRemoved for its Type, which couples payload semantics to outbox constants; change the assignment in the memberEvt creation (and the other occurrence where MemberRemoveEvent.Type is set) to use the payload-specific constant model.MessageTypeMemberRemoved instead (update the Type field in the MemberRemoveEvent initialization and any subsequent assignments that set Type from model.OutboxMemberRemoved).
🤖 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 `@docs/client-api.md`:
- Line 950: The system-message doc row uses non-schema names `content` and
`userAccount`; update that text to use the schema-consistent field names `msg`
and `sender` (e.g., "For all four, `msg` is populated with a server-rendered
human-readable body and `sender` is the responsible actor..."), keeping the
`type` values and examples intact so clients can map directly to the Message
fields `type`, `msg`, and `sender`.
In `@docs/superpowers/plans/2026-05-14-room-worker-membership-fixes.md`:
- Around line 1207-1211: The verification step wrongly expects no diffs after
running `make generate SERVICE=room-worker` because later changes expand the
`SubscriptionStore` interface and legitimately regenerate `mock_store_test.go`;
update the Step 3 checkpoint (and the duplicate at 1383-1387) to instead assert
that generated changes are intentional or to run `git diff --stat` and compare
against an approved golden (or skip this check when `SubscriptionStore` was
modified), and mention `SubscriptionStore` and `mock_store_test.go` by name so
reviewers know to accept the regenerated mock output rather than treating it as
a failure.
- Around line 7-8: The plan header incorrectly states "no store/model changes"
while Tasks 11–12 introduce model fields and a store API; update the plan text
and affected task summaries to reflect the true scope by noting the
Room.UIDs/Accounts model additions, the new BuildDMParticipants helper, and the
store interface change UpdateDMParticipants, and adjust execution/review
checkpoints accordingly (also ensure internal files referenced like
room-worker/sysmsg.go and handlers processAddMembers, processCreateRoomChannel,
processRemoveIndividual, processRemoveOrg, publishChannelSysMessages mention the
dependency on these model/store changes where relevant).
In `@docs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.md`:
- Around line 94-95: The documentation contradicts itself by stating "No store
interface changes" while the diff adds UpdateDMParticipants to the
SubscriptionStore; update the spec so the summary and any top-level bullets
reflect that SubscriptionStore has been extended, or remove the added method
from the diff. Specifically, ensure the document text around the header that
currently claims no store changes is edited to mention the new SubscriptionStore
method UpdateDMParticipants (or revert the addition of UpdateDMParticipants) so
implementers see a consistent contract referencing the SubscriptionStore
interface and the new UpdateDMParticipants operation.
In `@room-worker/handler.go`:
- Around line 683-688: The code currently treats an error from
h.store.HasOrgRoomMembers(ctx, req.RoomID) as hadOrgsBefore=false and continues;
instead, fail closed by returning the error (or wrapping it) so the handler
retries rather than mutating state. Update the block around HasOrgRoomMembers in
handler.go to check err and return it (or a wrapped context-aware error)
immediately instead of logging and continuing; ensure downstream logic that sets
writeIndividuals (which depends on hadOrgsBefore and req.Orgs) only runs when
HasOrgRoomMembers successfully returned.
In `@room-worker/store_mongo.go`:
- Around line 138-147: UpdateDMParticipants currently treats UpdateOne miss as
success; after calling s.rooms.UpdateOne(ctx, ...), inspect the returned
UpdateResult (the first return value) and if result.MatchedCount == 0 return an
error indicating no room matched for the given roomID (e.g., "no room found" or
similar). Keep the existing error wrapping for actual UpdateOne errors from
s.rooms.UpdateOne and only return the new "no match" error when there is no
match; reference the function UpdateDMParticipants and the s.rooms.UpdateOne
call to locate where to add the check.
---
Nitpick comments:
In `@room-worker/handler.go`:
- Around line 479-481: The MemberRemoveEvent is using the outbox constant
model.OutboxMemberRemoved for its Type, which couples payload semantics to
outbox constants; change the assignment in the memberEvt creation (and the other
occurrence where MemberRemoveEvent.Type is set) to use the payload-specific
constant model.MessageTypeMemberRemoved instead (update the Type field in the
MemberRemoveEvent initialization and any subsequent assignments that set Type
from model.OutboxMemberRemoved).
🪄 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: 10590e1e-fee2-461f-8cc4-ba88a645ad2f
📒 Files selected for processing (14)
docs/client-api.mddocs/superpowers/plans/2026-05-14-room-worker-membership-fixes.mddocs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.mdpkg/model/event.gopkg/model/model_test.gopkg/model/room.gopkg/model/room_test.goroom-worker/handler.goroom-worker/handler_test.goroom-worker/mock_store_test.goroom-worker/store.goroom-worker/store_mongo.goroom-worker/sysmsg.goroom-worker/sysmsg_test.go
vjauhari-work
force-pushed
the
claude/fix-org-membership-duplication-l5LfI
branch
from
11c3f44 to
0bd38b0
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
docs/client-api.md (1)
948-948:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the history-service field names in this schema note.
This table is documenting the projected
Messagepayload, so the note should referencemsgandsender, notcontentanduserAccount. As written, clients are sent looking for fields that are absent from this response shape.🤖 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 `@docs/client-api.md` at line 948, Update the schema note for the Message payload to use the history-service field names: refer to `msg` instead of `content` and `sender` instead of `userAccount`; keep the `type` values (`"room_created"`, `"members_added"`, `"member_removed"`, `"member_left"`) and clarify that for those system types `msg` contains the server-rendered human-readable body and `sender` is the responsible actor (e.g., the requester for adds/removes-by-other and room-creates, and the leaving user for self-leave).room-worker/handler.go (1)
683-686:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed when
HasOrgRoomMemberscannot be read.If this lookup fails and
hadOrgsBeforesilently falls back tofalse, the handler can either re-run first-org backfill or skip required individualroom_memberswrites. That reopens the duplication/missing-doc cases this patch is trying to eliminate.Suggested fix
hadOrgsBefore, err := h.store.HasOrgRoomMembers(ctx, req.RoomID) if err != nil { - slog.Warn("check existing org room members failed", "error", err, "roomID", req.RoomID) + return fmt.Errorf("check existing org room members: %w", err) } writeIndividuals := len(req.Orgs) > 0 || hadOrgsBeforeAs per coding guidelines "Always wrap errors with context using
fmt.Errorf("short description: %w", err)".🤖 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 `@room-worker/handler.go` around lines 683 - 686, The call to h.store.HasOrgRoomMembers currently swallows errors and leaves hadOrgsBefore false; change this to "fail closed" by returning the error instead of just logging: if h.store.HasOrgRoomMembers(ctx, req.RoomID) returns an error, return fmt.Errorf("check existing org room members: %w", err) (or otherwise propagate the wrapped error) so the handler aborts rather than proceeding with a potentially incorrect hadOrgsBefore; update the code around the hadOrgsBefore assignment in handler.go to replace the slog.Warn branch with error propagation using fmt.Errorf.
🤖 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 `@docs/superpowers/plans/2026-05-14-room-worker-membership-fixes.md`:
- Around line 21-22: The markdown links to code files in this plan use
repository-relative paths like room-worker/handler.go which won't resolve from
docs/superpowers/plans; update those links to proper repo-root-relative paths by
prefixing with the correct number of ../ (e.g., ../../.. or ../../ depending on
directory depth) so they point to the actual files; specifically fix links that
reference room-worker/handler.go and room-worker/handler_test.go (and their
anchors for functions like processAddMembers, processCreateRoomChannel,
processRemoveIndividual, processRemoveOrg, publishChannelSysMessages) and apply
the same normalization to the other occurrences noted around lines 26 and 30–33.
In `@docs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.md`:
- Around line 21-23: The doc uses relative source links like
room-worker/handler.go#L649-L661 which are not resolvable from
docs/superpowers/specs/..., so update all broken links (e.g., the references for
processAddMembers and processCreateRoomChannel) to correct relative paths or
absolute repository paths that point to the real file and line anchors; search
for occurrences of room-worker/handler.go#... (also at the other noted lines
26,30,48,51-52,54,62,72,74) and replace them with the proper path from the docs
directory (or a full GitHub/Repo URL with `#L`... anchors) so reviewers can
navigate directly to the handler.go locations for functions like
processAddMembers and processCreateRoomChannel.
In `@room-worker/handler_test.go`:
- Around line 3412-3433: TestHandler_ProcessAddMembers_AddedUserEmptyName
currently asserts processAddMembers returns errPermanent but does not verify the
async-job error was published; update the test to capture/inspect the
Handler.publish call (or replace publish with a test stub) and assert that an
async-job error event was published for RequestID "test-req-task5-d3" with
status indicating failure and a sanitized error message when processAddMembers
returns errPermanent; place this assertion after the require.Error and
assert.ErrorIs checks and reference the Handler.publish stub and
processAddMembers function when implementing the verification.
- Around line 3362-3384: The test
TestHandler_ProcessAddMembers_RequesterNotFound fails to assert that
publishAsyncJobResult was called for permanent errors; update the test to verify
the async-job error publication by instrumenting the Handler.publish callback
(or mocking publishAsyncJobResult) and asserting that when processAddMembers
returns errPermanent it also published an async-job result to the requester's
response subject with status "error" and a sanitized message that contains
"missing-requester" — use the same pattern as the other permanent-error tests
(they assert publish was invoked with the request ID from
natsutil.WithRequestID, status "error", and the sanitized error string) and
refer to Handler.processAddMembers, publish (or publishAsyncJobResult) and
errPermanent to locate the relevant test and assertions.
- Around line 3387-3409: The test
TestHandler_ProcessAddMembers_RequesterEmptyName fails to assert that the
handler publishes an async-job error for permanent errors; update the test to
wrap the Handler.publish with a mock/assertion that captures the published
subject/payload when processAddMembers returns errPermanent and verify the
published event contains the request ID from natsutil.WithRequestID(ctx), a
status indicating a permanent error, and a sanitized error message; reference
the Handler.publish function and the processAddMembers call and assert the
published payload fields (status and message) match the permanent-error contract
for async-job events.
In `@room-worker/handler.go`:
- Around line 572-575: The code currently accepts any non-empty request ID from
RequestIDFromContext and returns newPermanent("missing X-Request-ID") only when
empty; update these handlers to validate the format using idgen.IsValidUUID and
reject non-UUIDs the same way handleSyncCreateDM does: after obtaining requestID
:= natsutil.RequestIDFromContext(ctx), call idgen.IsValidUUID(requestID)
(case-insensitive, hyphenated v4/v7) and return newPermanent("invalid
X-Request-ID") when it returns false; apply this change to the occurrences
around RequestIDFromContext in the current block and the other location
referenced (lines ~945-948) so reply subjects and deterministic IDs only receive
validated UUIDs.
---
Duplicate comments:
In `@docs/client-api.md`:
- Line 948: Update the schema note for the Message payload to use the
history-service field names: refer to `msg` instead of `content` and `sender`
instead of `userAccount`; keep the `type` values (`"room_created"`,
`"members_added"`, `"member_removed"`, `"member_left"`) and clarify that for
those system types `msg` contains the server-rendered human-readable body and
`sender` is the responsible actor (e.g., the requester for adds/removes-by-other
and room-creates, and the leaving user for self-leave).
In `@room-worker/handler.go`:
- Around line 683-686: The call to h.store.HasOrgRoomMembers currently swallows
errors and leaves hadOrgsBefore false; change this to "fail closed" by returning
the error instead of just logging: if h.store.HasOrgRoomMembers(ctx, req.RoomID)
returns an error, return fmt.Errorf("check existing org room members: %w", err)
(or otherwise propagate the wrapped error) so the handler aborts rather than
proceeding with a potentially incorrect hadOrgsBefore; update the code around
the hadOrgsBefore assignment in handler.go to replace the slog.Warn branch with
error propagation using fmt.Errorf.
🪄 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: 528d2659-3257-477a-8229-d75fefb242e8
📒 Files selected for processing (11)
docs/client-api.mddocs/superpowers/plans/2026-05-14-room-worker-membership-fixes.mddocs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.mdpkg/model/event.gopkg/model/model_test.gopkg/model/room.gopkg/model/room_test.goroom-worker/handler.goroom-worker/handler_test.goroom-worker/sysmsg.goroom-worker/sysmsg_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/model/room.go
🚧 Files skipped from review as they are similar to previous changes (5)
- room-worker/sysmsg.go
- pkg/model/event.go
- pkg/model/model_test.go
- room-worker/sysmsg_test.go
- pkg/model/room_test.go
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)
room-worker/handler.go (1)
731-760:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed on first-org backfill read failures.
This path is one-shot. If
GetSubscriptionAccountsorFindUsersByAccountsfails and you continue, backfill is skipped permanently after org membership is written.As per coding guidelines "Never ignore errors silently — comment if intentionally discarded (blank import or explicit comment required)".💡 Suggested fix
if len(req.Orgs) > 0 && !hadOrgsBefore { existingAccounts, err := h.store.GetSubscriptionAccounts(ctx, req.RoomID) if err != nil { - slog.Warn("get subscription accounts for backfill failed", "error", err) - } else { - var backfillAccounts []string - for _, account := range existingAccounts { - if _, isNew := resolvedAccountSet[account]; !isNew { - backfillAccounts = append(backfillAccounts, account) - } - } - if len(backfillAccounts) > 0 { - backfillUsers, err := h.store.FindUsersByAccounts(ctx, backfillAccounts) - if err != nil { - slog.Warn("find users for backfill failed", "error", err) - } else { - for i := range backfillUsers { - roomMembers = append(roomMembers, &model.RoomMember{ - ID: idgen.GenerateUUIDv7(), - RoomID: req.RoomID, - Ts: acceptedAt, - Member: model.RoomMemberEntry{ - ID: backfillUsers[i].ID, - Type: model.RoomMemberIndividual, - Account: backfillUsers[i].Account, - }, - }) - } - } - } + return fmt.Errorf("get subscription accounts for backfill: %w", err) + } + var backfillAccounts []string + for _, account := range existingAccounts { + if _, isNew := resolvedAccountSet[account]; !isNew { + backfillAccounts = append(backfillAccounts, account) + } + } + if len(backfillAccounts) > 0 { + backfillUsers, err := h.store.FindUsersByAccounts(ctx, backfillAccounts) + if err != nil { + return fmt.Errorf("find users for backfill: %w", err) + } + for i := range backfillUsers { + roomMembers = append(roomMembers, &model.RoomMember{ + ID: idgen.GenerateUUIDv7(), + RoomID: req.RoomID, + Ts: acceptedAt, + Member: model.RoomMemberEntry{ + ID: backfillUsers[i].ID, + Type: model.RoomMemberIndividual, + Account: backfillUsers[i].Account, + }, + }) + } } }🤖 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 `@room-worker/handler.go` around lines 731 - 760, The code currently logs and continues when h.store.GetSubscriptionAccounts or h.store.FindUsersByAccounts fails during the org backfill, which causes the one-shot backfill to be skipped permanently; change these warning-only branches to return the error (or wrap it with context) so the handler fails fast instead of proceeding and silently dropping backfill work. Locate the GetSubscriptionAccounts call and its associated slog.Warn and replace the warn-path to return an error (e.g., wrap with fmt.Errorf or errors.Join) and do the same for the FindUsersByAccounts error branch; this ensures the handler for req.RoomID, hadOrgsBefore, resolvedAccountSet and roomMembers does not continue when those store reads fail.
♻️ Duplicate comments (1)
room-worker/store_mongo.go (1)
138-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
UpdateOneno-match as an error.
UpdateDMParticipantscurrently returns success even whenroomIDmatches no room. That silently drops DM participant persistence.As per coding guidelines "Check `mongo.ErrNoDocuments` explicitly when a missing record is expected in MongoDB queries".💡 Suggested fix
func (s *MongoStore) UpdateDMParticipants(ctx context.Context, roomID string, uids, accounts []string) error { - _, err := s.rooms.UpdateOne(ctx, + res, err := s.rooms.UpdateOne(ctx, bson.M{"_id": roomID}, bson.M{"$set": bson.M{"uids": uids, "accounts": accounts}}, ) if err != nil { return fmt.Errorf("update dm participants (room %s): %w", roomID, err) } + if res.MatchedCount == 0 { + return fmt.Errorf("update dm participants (room %s): room not found", roomID) + } return nil }🤖 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 `@room-worker/store_mongo.go` around lines 138 - 147, UpdateDMParticipants currently treats a zero-match UpdateOne as success; change it to treat no-match as an error by capturing the UpdateResult (res) from s.rooms.UpdateOne and checking res.MatchedCount (or res.ModifiedCount) after the call, and if it is 0 return an explicit error (wrap or return mongo.ErrNoDocuments or a descriptive fmt.Errorf like "room not found: %s"). Ensure this logic is added inside UpdateDMParticipants immediately after s.rooms.UpdateOne and before returning nil so missing roomIDs are surfaced instead of silently succeeding.
🤖 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 `@docs/superpowers/plans/2026-05-14-room-worker-membership-fixes.md`:
- Around line 957-958: The test snippets marshal req into a local variable named
data that is never used (data, _ := json.Marshal(req)), causing an unused
variable compile error; remove the unused json.Marshal call or change it to
discard the result (e.g., use only json.Marshal(req) with no assignment or
assign to _), and keep the call to require.NoError(t,
h.processRemoveIndividual(context.Background(), &req)); update both occurrences
(the one referencing data and the second instance noted) so no unused variable
remains.
- Around line 239-243: The current code calls h.store.HasOrgRoomMembers(ctx,
req.RoomID) and only logs the error, then proceeds to set writeIndividuals based
on hadOrgsBefore; change this to fail-closed: if HasOrgRoomMembers returns an
error, return that error (or wrap and return) instead of continuing, so the
handler (the function containing this call) does not proceed with
writeIndividuals computation or further processing when hadOrgsBefore is
unknown; adjust the error handling around HasOrgRoomMembers and remove the
permissive fall-through that sets writeIndividuals using an uninitialized
hadOrgsBefore.
---
Outside diff comments:
In `@room-worker/handler.go`:
- Around line 731-760: The code currently logs and continues when
h.store.GetSubscriptionAccounts or h.store.FindUsersByAccounts fails during the
org backfill, which causes the one-shot backfill to be skipped permanently;
change these warning-only branches to return the error (or wrap it with context)
so the handler fails fast instead of proceeding and silently dropping backfill
work. Locate the GetSubscriptionAccounts call and its associated slog.Warn and
replace the warn-path to return an error (e.g., wrap with fmt.Errorf or
errors.Join) and do the same for the FindUsersByAccounts error branch; this
ensures the handler for req.RoomID, hadOrgsBefore, resolvedAccountSet and
roomMembers does not continue when those store reads fail.
---
Duplicate comments:
In `@room-worker/store_mongo.go`:
- Around line 138-147: UpdateDMParticipants currently treats a zero-match
UpdateOne as success; change it to treat no-match as an error by capturing the
UpdateResult (res) from s.rooms.UpdateOne and checking res.MatchedCount (or
res.ModifiedCount) after the call, and if it is 0 return an explicit error (wrap
or return mongo.ErrNoDocuments or a descriptive fmt.Errorf like "room not found:
%s"). Ensure this logic is added inside UpdateDMParticipants immediately after
s.rooms.UpdateOne and before returning nil so missing roomIDs are surfaced
instead of silently succeeding.
🪄 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: 9c510884-ebfd-4447-98e3-b82a49c422b0
📒 Files selected for processing (8)
docs/client-api.mddocs/superpowers/plans/2026-05-14-room-worker-membership-fixes.mddocs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.mdroom-worker/handler.goroom-worker/handler_test.goroom-worker/mock_store_test.goroom-worker/store.goroom-worker/store_mongo.go
✅ Files skipped from review due to trivial changes (2)
- room-worker/mock_store_test.go
- docs/client-api.md
vjauhari-work
force-pushed
the
claude/fix-org-membership-duplication-l5LfI
branch
from
b7467d7 to
ca4f612
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.md (1)
96-96:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMisleading "No store interface changes" statement.
Line 96 states "No store interface changes" at the conclusion of §2.6 (Helpers), but §3.4 (lines 142-155) introduces
UpdateDMParticipantsto theSubscriptionStoreinterface for DM participant persistence. While the statement is technically accurate for the formatters section in isolation, it misleads readers about the overall scope of the specification.This issue was flagged in a previous review and marked as addressed, but the text remains unchanged. Either remove the sentence or qualify it to clarify that it applies only to §2 (membership fixes), not §3 (DM fields).
Suggested fix
-Unit tests in `room-worker/sysmsg_test.go`. No store interface changes. +Unit tests in `room-worker/sysmsg_test.go`. §2 (membership fixes) requires no store interface changes; §3 (DM participant fields) adds `UpdateDMParticipants`.🤖 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 `@docs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.md` at line 96, The concluding sentence "No store interface changes" in §2.6 is misleading because §3.4 adds UpdateDMParticipants to the SubscriptionStore interface; update the text to either remove that sentence or reword it to clarify scope (e.g., state that no store interface changes apply to the formatters/membership fixes in §2, while §3.4 introduces UpdateDMParticipants on SubscriptionStore for DM participant persistence), and ensure you reference the SubscriptionStore and UpdateDMParticipants identifiers so readers can find the related change.
🤖 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 `@docs/superpowers/plans/2026-05-14-room-worker-membership-fixes.md`:
- Around line 7-8: Update the scope summary and related statements in the
document to acknowledge the model and store interface changes introduced by the
DM participant persistence work: explicitly mention that Task 11 adds Room.UIDs
and Room.Accounts to pkg/model/room.go and the BuildDMParticipants helper, and
that Task 12 adds UpdateDMParticipants(...) to the SubscriptionStore interface
(room-worker/store.go); replace any claims like "no store interface, model, or
wire-protocol changes" and "No `store.go` changes" with text that reflects these
specific additions so the scope is accurate and no longer contradicts the tasks.
---
Duplicate comments:
In `@docs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.md`:
- Line 96: The concluding sentence "No store interface changes" in §2.6 is
misleading because §3.4 adds UpdateDMParticipants to the SubscriptionStore
interface; update the text to either remove that sentence or reword it to
clarify scope (e.g., state that no store interface changes apply to the
formatters/membership fixes in §2, while §3.4 introduces UpdateDMParticipants on
SubscriptionStore for DM participant persistence), and ensure you reference the
SubscriptionStore and UpdateDMParticipants identifiers so readers can find the
related change.
🪄 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: 0c2a55cd-9251-4865-8112-0de940a43676
📒 Files selected for processing (14)
docs/client-api.mddocs/superpowers/plans/2026-05-14-room-worker-membership-fixes.mddocs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.mdpkg/model/event.gopkg/model/model_test.gopkg/model/room.gopkg/model/room_test.goroom-worker/handler.goroom-worker/handler_test.goroom-worker/mock_store_test.goroom-worker/store.goroom-worker/store_mongo.goroom-worker/sysmsg.goroom-worker/sysmsg_test.go
✅ Files skipped from review due to trivial changes (2)
- room-worker/mock_store_test.go
- docs/client-api.md
🚧 Files skipped from review as they are similar to previous changes (10)
- pkg/model/model_test.go
- pkg/model/event.go
- room-worker/store_mongo.go
- room-worker/sysmsg.go
- room-worker/store.go
- pkg/model/room_test.go
- pkg/model/room.go
- room-worker/sysmsg_test.go
- room-worker/handler.go
- room-worker/handler_test.go
vjauhari-work
force-pushed
the
claude/fix-org-membership-duplication-l5LfI
branch
from
ca4f612 to
2c0e500
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
docs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.md (1)
96-96:⚠️ Potential issue | 🟠 Major | ⚡ Quick win"No store interface changes" still contradicts section 3.4.
Line 96 states "No store interface changes," but section 3.4 (lines 142-151) explicitly adds
UpdateDMParticipantsto theSubscriptionStoreinterface inroom-worker/store.go. This contradiction was flagged in a previous review and marked as addressed in commits 9ccf2c1–0bd38b0, but the text has not been corrected.Please update line 96 to reflect the store interface extension.
Suggested fix
-Unit tests in `room-worker/sysmsg_test.go`. No store interface changes. +Unit tests in `room-worker/sysmsg_test.go`. Store interface extended in §3.4 to add `UpdateDMParticipants`.🤖 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 `@docs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.md` at line 96, Update the sentence at line 96 to remove the assertion that there are "No store interface changes" and instead state that the SubscriptionStore interface was extended; specifically mention that section 3.4 adds the UpdateDMParticipants method to the SubscriptionStore (referencing SubscriptionStore and UpdateDMParticipants) so the doc no longer contradicts the interface change described later.docs/superpowers/plans/2026-05-14-room-worker-membership-fixes.md (1)
7-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope claims still contradict Tasks 11-12 despite being marked as addressed.
Lines 7-8 state "no store interface, model, or wire-protocol changes" and lines 24-25 claim "No
store.gochanges", but:
- Task 11 (lines 1231-1338) adds
Room.UIDs/Room.Accountsfields topkg/model/room.goplus theBuildDMParticipantshelper — these are model additions.- Task 12 (lines 1341-1401) adds
UpdateDMParticipants(...)to theSubscriptionStoreinterface inroom-worker/store.go— this is a store interface extension.This contradiction was flagged in a previous review and marked as addressed in commits a741f34–44eab49, but the summary text has not been updated. Please revise the scope statements to acknowledge the model and store interface changes introduced by the DM participant persistence work.
Suggested fix
-**Architecture:** Introduce a `room-worker/sysmsg.go` helper file with five formatter functions. Apply localized changes inside `processAddMembers`, `processCreateRoomChannel`, `processRemoveIndividual`, `processRemoveOrg`, and `publishChannelSysMessages` in `room-worker/handler.go`. All changes are internal to `room-worker`; no store interface, model, or wire-protocol changes. +**Architecture:** Introduce `room-worker/sysmsg.go` formatter helpers and update `room-worker/handler.go` membership flows. Also add DM participant persistence spanning `pkg/model` (`Room.UIDs/Accounts`, `BuildDMParticipants`) and `room-worker` store contract (`UpdateDMParticipants`). No wire-protocol changes.-No `store.go` changes — every method the plan needs is already on the `SubscriptionStore` interface. +`store.go` is extended in Task 12 to add `UpdateDMParticipants` to the `SubscriptionStore` interface for DM participant persistence.🤖 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 `@docs/superpowers/plans/2026-05-14-room-worker-membership-fixes.md` around lines 7 - 8, Update the document's scope/summary statements to acknowledge that DM participant persistence introduced model and store-interface changes: explicitly note the addition of Room.UIDs and Room.Accounts fields and the BuildDMParticipants helper to the Room model, and the addition of UpdateDMParticipants(...) to the SubscriptionStore interface; replace the lines claiming "no store interface, model, or wire-protocol changes" and "No `store.go` changes" with text that accurately reflects these model and store interface changes.
🤖 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.
Duplicate comments:
In `@docs/superpowers/plans/2026-05-14-room-worker-membership-fixes.md`:
- Around line 7-8: Update the document's scope/summary statements to acknowledge
that DM participant persistence introduced model and store-interface changes:
explicitly note the addition of Room.UIDs and Room.Accounts fields and the
BuildDMParticipants helper to the Room model, and the addition of
UpdateDMParticipants(...) to the SubscriptionStore interface; replace the lines
claiming "no store interface, model, or wire-protocol changes" and "No
`store.go` changes" with text that accurately reflects these model and store
interface changes.
In `@docs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.md`:
- Line 96: Update the sentence at line 96 to remove the assertion that there are
"No store interface changes" and instead state that the SubscriptionStore
interface was extended; specifically mention that section 3.4 adds the
UpdateDMParticipants method to the SubscriptionStore (referencing
SubscriptionStore and UpdateDMParticipants) so the doc no longer contradicts the
interface change described later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b8760d2-1d7c-40b1-8df5-72f57b8cc408
📒 Files selected for processing (14)
docs/client-api.mddocs/superpowers/plans/2026-05-14-room-worker-membership-fixes.mddocs/superpowers/specs/2026-05-13-room-worker-membership-fixes-design.mdpkg/model/event.gopkg/model/model_test.gopkg/model/room.gopkg/model/room_test.goroom-worker/handler.goroom-worker/handler_test.goroom-worker/mock_store_test.goroom-worker/store.goroom-worker/store_mongo.goroom-worker/sysmsg.goroom-worker/sysmsg_test.go
✅ Files skipped from review due to trivial changes (2)
- room-worker/mock_store_test.go
- docs/client-api.md
🚧 Files skipped from review as they are similar to previous changes (10)
- room-worker/store_mongo.go
- pkg/model/room_test.go
- room-worker/store.go
- pkg/model/model_test.go
- pkg/model/event.go
- pkg/model/room.go
- room-worker/sysmsg.go
- room-worker/sysmsg_test.go
- room-worker/handler.go
- room-worker/handler_test.go
vjauhari-work
force-pushed
the
claude/fix-org-membership-duplication-l5LfI
branch
2 times, most recently
from
f986c3d to
f813cf2
Compare
Spec for the room-worker membership filtering, system-message sender/content population, request-ID validation, fail-closed HasOrgRoomMembers handling, and DM participant fields work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task-by-task plan covering the membership-fix work: filter individual room docs, populate sender/content on system messages, fail-closed HasOrgRoomMembers, X-Request-ID UUID validation, and DM participant fields (Tasks 11-17). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Align the system-message type row with the new msg/sender population and document the new Room.uids and Room.accounts DM-participant fields. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lper DM rooms now carry both the deterministic UID list and the matching account list as siblings, so clients can render DM members without an extra round-trip. BuildDMParticipants returns the canonically sorted pair for any two users. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
\$set's uids/accounts on a dm/botDM room after the counterpart account has been resolved. Idempotent under JetStream redelivery; returns an error when the target room is missing (the caller treats this as a permanent failure rather than silently skipping). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eate/add
The MongoDB org-membership backfill was over-broad: it created individual
sub-room docs for every member of the source org rooms, not just the users
being added. processAddMembers now filters indiv docs to req.Users only,
createRoom filters to ResolvedUsers ∪ {requester}, and the backfill gate
fires only on the first-org transition so subsequent orgs don't replay it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously member_added / member_removed / member_left system messages went out with bare member lists and no sender, leaving clients nothing to render the actor or the affected users with. The room-worker now fetches the requester (validating EngName / SectName / SectID along the way), harvests SectName from the unfiltered org members so sect details survive the indiv-doc filter, and renders Content via new sysmsg formatters. Single- and multi-form Content variants cover the individual path and the org-bearing path; a 1-member org expansion correctly stays on multi form so future org members are accounted for. Adds pkg/model.MessageTypeMemberRemoved and MessageTypeMemberLeft to name the two new sys-msg variants the room-worker now emits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…osed org backfill Two defensive fixes in the same path: - processAddMembers and processCreateRoom now validate that the X-Request-ID header carries a well-formed UUID (v4 or v7) via idgen.IsValidUUID; non-UUIDs return a permanent error so log correlation stays usable. - HasOrgRoomMembers failures no longer fall through silently with the zero-value bool — defaulting hadOrgsBefore=false on error would spuriously trigger first-org backfill on a room that already has org docs, and the duplicate inserts would only be masked by the Mongo unique index. Surface the error so JetStream redelivery retries with fresh state. Comments on these branches trimmed to single-line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DM and botDM rooms now persist matching uids/accounts pairs at room creation across all three create paths: - Async DM create (processCreateRoom, dm type) — calls UpdateDMParticipants after counterpart resolution. - Async botDM create (processCreateRoom, botDM type) — same path, bot account resolved like a regular user. - Sync DM create (handleSyncCreateDM) — sets UIDs/Accounts on the CreateRoom literal so the participants land in the same write as the room itself; on a dup-key (the DM existed already), the path is forward-only and does not backfill — the participants are immutable for a DM room, so no recovery work is needed. UpdateDMParticipants returns an error when no room matches the ID (room-not-found is permanent, not silently skipped). The channel create path is left untouched; a guard test pins that channels must omit UIDs/Accounts so omitempty drops them from BSON. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After rebasing onto main (which brought in #171 room encryption keys), two classes of test adaptations were needed: 1. #171 tests of processAddMembers / processRemoveIndividual now hit the PR's stricter name-field validation. Added EngName/ChineseName to user fixtures and added the GetUser expectation for the requester account. 2. PR tests of processAddMembers that build the Handler via struct literal now reach the unconditional buildAndFanOutRoomKey call added in #171. Added keyStore: testKeyStore, keySender: testKeySender to all such Handler inits so the no-op stubs satisfy the fan-out call. Also retired one non-UUID request ID literal ("req-add-members-ordering") that the PR's UUID-format validation now rejects. No production-code changes — purely test-side reconciliation between the two feature branches.
…d integration tests Documentation: the plan's architecture summary and file-map preamble, and the spec's §2.6 closing line, all claimed there were no model or store interface changes — contradicting Tasks 11-12 (Room.UIDs/ Accounts + BuildDMParticipants, UpdateDMParticipants on SubscriptionStore). Updated the three lines so the scope statements match what the plan and spec actually deliver. Tests: the existing room-worker feature integration tests exercised the code paths for the four bug fixes but did not assert the user-facing artifacts those fixes produce. Extended: - TestSyncCreateDM_DM_PersistsRoomAndSubs + TestProcessCreateRoomDM- PersistsTwoSubsAndZeroMembers: assert room.UIDs and room.Accounts are persisted (sorted by uid, paired by index) — covers DM participant fields persistence. - TestProcessAddMembers_PublishesLocalInbox_Integration: assert the canonical members_added sys-message carries UserAccount=requester and formatter-rendered Content — covers empty-Content + missing- sender bugs on add. - TestProcessRemoveIndividual_PublishesLocalInbox_Integration: same assertion for the member_removed sys-message — covers empty-Content + missing-sender bugs on forced removal. Org-member duplication remains covered by handler_test.go unit tests; the existing AddMembers integration test uses only direct users so it doesn't exercise the duplication scenario.
mliu33
reviewed
May 18, 2026
mliu33
reviewed
May 18, 2026
mliu33
reviewed
May 18, 2026
…ounts; skip bots fanOutMutationEvent previously called ListSubscriptions on every DM edit/delete to recover recipient accounts — but Room.Accounts (denormalized onto the room doc by room-worker for DM/botDM rooms) already carries the same data. Drop the Mongo round-trip and iterate room.Accounts directly. Merge the DM and BotDM cases (BotDM previously fell into the unknown-room-type default branch and logged a misleading warning) and skip bot accounts so the bot in a botDM does not receive its own edit/delete echo on a subject nothing listens on. Add a local isBot helper mirroring the predicate already present in message-gatekeeper and room-service; promotion to a shared pkg/botid is the right cleanup once a fourth duplicate is no longer acceptable. Tests: - existing DM edit/delete fan-out tests switched fixtures from a ListSubscriptions mock to Accounts on the Room struct (same coverage) - new TestHandleUpdated_BotDMRoom_SkipsBotAccount asserts the bot is filtered in a botDM room Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses two PR #185 review comments from mliu: 1. processCreateRoom previously inserted the room and then ran a follow-up UpdateDMParticipants UpdateOne to backfill UIDs/Accounts after the counterpart user was resolved. That second Mongo round-trip is avoidable for new rooms: fetch the counterpart upfront, populate Room.UIDs/Accounts on the literal, and let CreateRoom persist the complete document in one write — matching the sync DM path's pattern. UpdateDMParticipants is removed from SubscriptionStore (interface + Mongo impl + mock); no callers remain. The duplicate-key replay branch keeps the existing identity-equality semantics (Type/SiteID/Name/CreatedBy must match); it no longer rewrites UIDs/Accounts onto the existing doc, since rooms created by this code path already have those fields and legacy rooms are out of scope for the hot path. The newly-similar processCreateRoomDM/BotDM helpers collapsed into the switch, mirroring the inline if-RoomTypeBotDM pattern handleSyncCreateDM already uses. 2. displayName(u) used to return TrimSpace(EngName + " " + ChineseName). For account-only users where EngName == ChineseName, that rendered as "Bob Bob". Short-circuit to a single rendering when the two fields are equal. Channels remain the only consumer of system-message content; DM/botDM paths don't render via this helper. Tests: - F1/F2 (DM/BotDM SetsParticipantFields) now capture the Room passed to CreateRoom and assert UIDs/Accounts on it, replacing the previous UpdateDMParticipants arg-pin - CollisionMismatchType test threads the upfront counterpart GetUser - TestFormatLeft_EngEqualsChineseRendersOnce pins the new dedupe contract - mocks regenerated; integration test compile-clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vjauhari-work
force-pushed
the
claude/fix-org-membership-duplication-l5LfI
branch
from
f813cf2 to
3c6f817
Compare
mliu33
reviewed
May 18, 2026
Three follow-ups from PR #185 review: 1. Drop validateUserNames + its five call sites (mliu PR comment). EngName and ChineseName are now treated as optional. displayName falls through to a single non-empty side, and finally to the Account string when both are blank — the rendered body never collapses to a bare quote pair. 2. Quote display names in every channel sys-message: `"Alice Anderson 王愛麗" added "Frank Fischer 費法蘭" to the channel` (and analogous shapes for added-multi / removed-user / removed-org / left). formatRemovedOrg quotes the SectName too. 3. Populate the sender envelope (Message.UserID + UserAccount) on the member_left / member_removed sys-messages emitted by processRemoveIndividual and processRemoveOrg. message-worker looks up the user by UserID to fill the Cassandra `sender` Participant column; previously these rows landed with sender = null. Self-leave reuses the already-fetched leaving user; forced-removal and org-removal fetch the requester via the existing store.GetUser (no new store method needed). Older Cassandra rows are intentionally not backfilled — only newly emitted messages get the populated sender. Also capitalise "A new room has been created" on the room_created sys-message body. Tests: - sysmsg_test: drop TestValidateUserNames; add TestDisplayName covering every fallback branch and TestFormatLeft_FallsBackToAccount / TestFormatAddedSingle_SingleNameSide; update existing formatter assertions for the new quoted shape. - handler_test: drop the four empty-name permanent-error tests (TestHandler_ProcessAddMembers_RequesterEmptyName, _AddedUserEmptyName, TestHandler_ProcessRemoveIndividual_EmptyName, TestProcessCreateRoom_RequesterEmptyName_ReturnsPermanent); update existing content assertions to the quoted format; mock GetUser on the requester for forced-remove / org-remove paths and assert sysMsg.UserID is set to the requester ID. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…msg shape Two integration assertions still expected the unquoted sys-message bodies; mirror the change from the prior commit so the live MongoDB+Cassandra container test agrees with the in-memory handler tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
vjauhari-work
added a commit
that referenced
this pull request
May 18, 2026
…ectly (#193) SystemMessage previously decoded `sysMsgData` and synthesised the visible text client-side (`Alice added bob, dave`, etc). The room-worker now publishes the rendered string in Message.Content (PR #185 for members_added; room_created still ships a placeholder), so the frontend can just render `message.content` verbatim and the formatting lives in one place — the server. Drop decodeSysData, senderLabel, describeMembersAdded, describeRoomCreated, and the type switch. Drop the unused data-system-type attribute (no CSS or JS consumer). Test suite collapses to a parametrised "renders Content verbatim" case plus a timestamp assertion. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 20, 2026
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
3 participants
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.
Summary
Fixes three room-worker bugs affecting membership management and system messages:
room_membersdocs, appearing twice in listingsmembers_addedcontent: System messages lackedContentfield, rendering blank in UImember_removed/member_left: System messages had noUserAccountorContent, showing "Unknown sent the message" with blank bodyAll fixes are internal to
room-workerwith no wire-schema or data-migration changes.Key Changes
New Files
room-worker/sysmsg.go: Five formatter functions (formatAddedSingle,formatAddedMulti,formatRemovedUser,formatRemovedOrg,formatLeft) and adisplayNamehelper that renders user names fromEngNameandChineseNamefields. IncludesvalidateUserNamesto enforce non-empty name fields required for system-message rendering.room-worker/sysmsg_test.go: Unit tests for all formatter functions, including edge cases for empty name fields.pkg/model/room_test.go: Tests forBuildDMParticipantshelper (sorting and account permutation).Modified Files
room-worker/handler.goHasOrgRoomMemberscall to always execute first, then tighten backfill condition to fire only on first-org transition (len(req.Orgs) > 0 && !hadOrgsBefore), preventing re-introduction of duplication on subsequent org adds.room_membersfilter inprocessAddMembers(§2.1): Added whitelist check so individual docs are written only for accounts inreq.Users, not for all org-expanded members.room_membersfilter inprocessCreateRoomChannel(§2.1): Added whitelist check so individual docs are written only for accounts inreq.ResolvedUsers ∪ {requester.Account}.processAddMembers(§2.3): Addedstore.GetUsercall to fetch requester and validate both requester and added users have non-emptyEngNameandChineseNamefields; permanent error on miss or empty names.processRemoveIndividual(§2.4):UserAccountto requester (sender envelope)ContentusingformatLeftorformatRemovedUserbased on whether removal is self-leave or forcedMessageTypeMemberLeft,MessageTypeMemberRemoved,OutboxMemberRemoved)processRemoveOrg(§2.4):UserAccountto requesterContentusingformatRemovedOrgwith org'sSectNamepublishChannelSysMessages(§2.5):members_added: setUserAccountto requester andContentusingformatAddedSingle(single user) orformatAddedMulti(multiple users)members_addedwith single user: useformatAddedSingle(requester, addedUser)members_addedwith multiple users: useformatAddedMulti(requester)room-worker/handler_test.goprocessAddMemberstests to includeRequesterAccountin request and mockGetUserexpectation with valid name fieldsprocessRemoveIndividualandprocessRemoveOrgtests to includeEngNameandChineseNamein user fixturesTestHandler_ProcessRemoveMember_OwnerRemovesOrghttps://claude.ai/code/session_01WDo6nRMn4Fm2jwMR7bV6Pe
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests