Fix group invite discovery after direct add#373
Merged
Conversation
📝 WalkthroughWalkthrough
ChangesGroup invite materialization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant cmd_group_list
participant ingest_group_invites
participant Inbox
participant GroupState
User->>cmd_group_list: wire group list
cmd_group_list->>ingest_group_invites: ingest when initialized
ingest_group_invites->>Inbox: read group_invite events
ingest_group_invites->>GroupState: persist matching verified roster
GroupState-->>cmd_group_list: return materialized groups
cmd_group_list-->>User: display group list
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/group.rs (1)
153-175: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse canonical full DIDs consistently for direct-invite recipients. The ingester compares against a reconstructed bare DID, and the e2e fixtures encode the same format; canonical signed events addressed to fingerprinted DIDs are rejected, while the roster-rejection fixture masks the membership gate.
src/cli/group.rs#L153-L175: compareevent["to"]withself_didreturned bygroup_self(), notdid:wire:<handle>.tests/e2e_group.rs#L425-L426: sign the wrong-recipient invite with Bob’s full DID and the excluded-roster invite witheve_did, so each rejection path is exercised independently.🤖 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/cli/group.rs` around lines 153 - 175, Use the canonical self_did returned by group_self() when validating direct-invite recipients in the group invite ingestion logic; remove the reconstructed did:wire:<handle> comparison. In tests/e2e_group.rs lines 425-426, sign the wrong-recipient invite with Bob’s full DID and the excluded-roster invite with eve_did so the two rejection paths are tested independently.
🤖 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 `@SESSION_LOG_2026_07_19.md`:
- Line 24: Update the impact record for ingest_group_invites to explicitly
document the added recipient-DID and roster-membership validation gates,
replacing the claim that its validation logic was unchanged; retain the existing
risk and caller information.
In `@tests/e2e_group.rs`:
- Around line 425-426: Update the test fixtures in the relevant E2E test to use
full recipient DIDs: construct the wrong-recipient event with Bob’s full DID and
construct the excluded-roster event with the existing eve_did value. Preserve
the distinct scenarios so recipient filtering does not mask the roster-exclusion
check.
---
Outside diff comments:
In `@src/cli/group.rs`:
- Around line 153-175: Use the canonical self_did returned by group_self() when
validating direct-invite recipients in the group invite ingestion logic; remove
the reconstructed did:wire:<handle> comparison. In tests/e2e_group.rs lines
425-426, sign the wrong-recipient invite with Bob’s full DID and the
excluded-roster invite with eve_did so the two rejection paths are tested
independently.
🪄 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 Plus
Run ID: 1d20e4d1-3a0d-4cb0-9b31-853f43997e10
📒 Files selected for processing (5)
SESSION_LOG_2026_07_19.mddocs/superpowers/specs/2026-07-19-group-invite-materialization-design.mdsrc/cli/group.rstests/cli.rstests/e2e_group.rs
| - Regression reproduced: `cargo test --test e2e_group group_bidirectional_room_with_introduce_pin -- --nocapture` failed before the code fix because the recipient listed zero groups after pull. | ||
| - Focused verification passed: `cargo fmt --check`, `cargo test --test e2e_group -- --nocapture`, and `cargo test --test cli group_list_empty_reports_no_groups -- --nocapture`. | ||
| - Full verification passed: `cargo test -q`. | ||
| - GitNexus impact: `cmd_group_list` had one direct caller / LOW risk; shared `ingest_group_invites` was HIGH risk due to `send` and `tail` callers, so its validation logic was unchanged. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the impact record.
Line 24 says ingest_group_invites validation was unchanged, but this PR adds recipient-DID and roster-membership gates there. Record those validation changes explicitly.
🤖 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 `@SESSION_LOG_2026_07_19.md` at line 24, Update the impact record for
ingest_group_invites to explicitly document the added recipient-DID and
roster-membership validation gates, replacing the claim that its validation
logic was unchanged; retain the existing risk and caller information.
Comment on lines
+425
to
+426
| let wrong_recipient = sign_invite(format!("did:wire:{bob_h}"), &roster_including_eve); | ||
| let excluded_recipient = sign_invite(format!("did:wire:{eve_h}"), &group); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use full DIDs so the roster gate is tested independently.
Lines 425-426 use bare DIDs. After recipient matching is corrected, the Eve-targeted fixture fails recipient filtering first, so this test no longer proves that a signed roster excluding Eve is rejected. Use Bob’s full DID for the wrong-recipient event and eve_did for the excluded-roster event.
🤖 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 `@tests/e2e_group.rs` around lines 425 - 426, Update the test fixtures in the
relevant E2E test to use full recipient DIDs: construct the wrong-recipient
event with Bob’s full DID and construct the excluded-roster event with the
existing eve_did value. Preserve the distinct scenarios so recipient filtering
does not mask the roster-exclusion check.
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
Directly added verified members now materialize a signed group invite when listing groups, so they do not need to request a separate wire-group join code.
Safety
Invite intake requires both the local recipient and local roster membership before persisting the shared room credential.
Verification
Summary by CodeRabbit
New Features
wire group list, without entering a separate join code.Bug Fixes
Documentation