feat: add room v11 support - #409
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe change adds Matrix room-version-aware event construction and validation. It supports v11 creator and redaction field locations, resolves room versions through state, updates federation flows, removes generated event-ID helpers, and adds coexistence and authorization coverage. ChangesRoom-version event model
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FederationClient
participant EventService
participant StateService
participant PersistentEventFactory
FederationClient->>EventService: Submit or process event
EventService->>StateService: Resolve room version
EventService->>PersistentEventFactory: Construct or parse versioned event
PersistentEventFactory-->>EventService: Return event ID, creator, or redaction target
EventService-->>FederationClient: Validate, notify, or persist event
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
Room version 11 removed content.creator from m.room.create: the event's sender is the creator. The auth rules and both zod schema sets still required it, so any spec-compliant v11 create event was rejected, and the factory always emitted it. Where the creator lives is now answered by the room version class (resolveCreator / newCreateEventContent) so callers stay version blind, and the factory has a single version-to-class mapping used by both createFromRawEvent and newCreateEvent.
The "only previous event is m.room.create and state_key is the creator" allowance compared against content.creator, which does not exist in v11, so the creator's own first join was rejected and no v11 room could be bootstrapped. Resolve the creator through the room version class instead.
checkSignAndHashes reconstructed the signed bytes with pruneEventDict's
pre-v6 defaults, so verification failed for every event whose redaction
actually differs: for v11 the create content was collapsed to {creator},
invite was dropped from power_levels and origin/membership/prev_state
were kept. Redact through the room version class instead, and require
the room version at the call site.
generateId pruned events with pre-v6 redaction rules, so it produced different IDs than the canonical PersistentEventFactory path for any event whose redaction is version dependent, causing cache misses, duplicate rows and failed lookups. - event-fetcher now matches federated events using the room version's ID - drop EventRepository.upsert, whose only job was minting an ID this way - log lines no longer mint an ID just to name an event - delete generateId along with createEventWithId and the create*Event builders that existed only to attach such an ID (no callers outside their own specs); the specs that assert an event ID now go through PersistentEventFactory and still match their recorded IDs
…cts it v11 moved redacts from the top level into content. Outbound v11 redactions were built with a top level redacts, which v11's allowed keys omit, so the target was lost under redaction and signing; inbound Synapse v11 redactions (content.redacts only) looked like they had no target. The version class now owns both directions: newRedactionEventFields places the target when building, getRedacts reads it when handling one. Applying a redaction also prunes with the room's own rules instead of hardcoded v6+ flags.
validateEvent resolved schemas through a private lookup that threw for any room version other than '10', so every inbound v11 event failed validation with "Unsupported room version: 11" regardless of the rest of the v11 work. Use the shared resolver, which falls back to the canonical schema set as documented.
Only a v10 set existed and every other room version silently resolved to it, so v11 events were validated against v10's shape. Split the sets that differ and register one per supported version: - create requires content.creator below v11 and not from v11 on, which also restores the pre-v11 strictness loosened when v11 creates were first accepted - redaction takes its target from the top level below v11 and from content from v11 on, replacing the "either location" check An unrecognised version resolves to the v11 set, since later versions inherit its event format.
origin is not a PDU field. The make_join and make_leave templates we serve added it, and a join template received from the resident server was signed back with whatever origin it carried, so v11 events went out with a field the version does not define. updateUserPowerLevel also ran its content through the legacy core event builder, which injects origin, only to keep the content and sender it was already given. Build the content directly so no live path reaches that builder.
Several RoomService paths passed PersistentEventFactory.defaultRoomVersion to buildEvent for rooms that already exist, so an event for a room on any other version was built with the wrong redaction rules, giving it the wrong event ID and a signature over the wrong bytes. Room creation keeps using the default, and everything after it uses the version of the create event or the version looked up for the room. This is also a prerequisite for changing the default at all.
The v11 blockers are fixed and the SDK now builds every event with the room's own version, so new rooms default to 11. Existing rooms keep the version recorded in their create event.
joinUser asked for a make_join template with ver=10 only, so a resident server holding a room on any other version answers with M_INCOMPATIBLE_ROOM_VERSION. Omit the argument so the request lists every supported version and take the room's version from the response, which is what the code already builds the join event with.
getRoomInformation returned the create event content verbatim, so callers reading creator got undefined for a v11 room, where the field no longer exists. Report the creator the room version resolves instead. The state service specs read content.creator to find the sender for follow-up events, which made every such test fail against a v11 room. They now use the version blind accessor. These specs only run with RUN_MONGO_TESTS=1, which is why the earlier v11 work did not surface it. Adds a coexistence spec that drives a v10 and a v11 room through create, join, message, redact and ban in the same database, asserting each room keeps its own version, creator resolution and redaction target placement.
8195192 to
36d0a80
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #409 +/- ##
==========================================
+ Coverage 51.81% 52.34% +0.52%
==========================================
Files 114 112 -2
Lines 12704 12615 -89
==========================================
+ Hits 6583 6603 +20
+ Misses 6121 6012 -109 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/core/src/events/m.room.redaction.spec.ts (1)
83-83: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the exact redaction event ID.
The assertion on Line 90 only checks that the factory returned a defined value. It will pass for an incorrect room-version-specific ID. Add a fixed expected ID and assert it with
toBe(expectedEventId).🤖 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 `@packages/core/src/events/m.room.redaction.spec.ts` at line 83, Update the redaction event test around PersistentEventFactory.createFromRawEvent to define the fixed expected redaction event ID and assert redactionEventId with toBe(expectedEventId), replacing the current defined-value-only assertion. Preserve the existing event construction and use the deterministic ID produced for this signed redaction.packages/federation-sdk/src/services/room-version-coexistence.spec.ts (1)
30-41: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClose the MongoDB connection after the test suite.
DatabaseConnectionServiceprovidesdisconnect()and currently no test teardown calls it; addafterAll(async () => { await database.disconnect(); })after the suite setup.🤖 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 `@packages/federation-sdk/src/services/room-version-coexistence.spec.ts` around lines 30 - 41, Add an afterAll teardown alongside the existing beforeEach setup in the test suite, calling database.disconnect() and awaiting its completion to close the DatabaseConnectionService connection after all tests finish.
🤖 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 `@packages/core/src/events/m.reaction.spec.ts`:
- Line 56: Update the assertion following reactionEventId in the reaction event
test so it compares the generated ID against the fixture’s fixed deterministic
expected value, rather than reactionEventId itself. Keep
PersistentEventFactory.createFromRawEvent unchanged and use the exact expected
ID derived from this fixture.
In `@packages/core/src/utils/checkSignAndHashes.spec.ts`:
- Line 101: Update the assertion for the v10 redacted payload in the
verifyJsonSpy call to check its content property with exact equality using
toEqual({}), ensuring retained keys such as m.federate cause the test to fail.
In `@packages/federation-sdk/src/services/state.service.spec.ts`:
- Around line 875-876: Await the promise-rejection assertions in the tests for
getCreateEvent and getRoomVersion so Jest waits for each rejection check to
complete. Update both expect(...).rejects chains while preserving their existing
UnknownRoomError expectations.
---
Nitpick comments:
In `@packages/core/src/events/m.room.redaction.spec.ts`:
- Line 83: Update the redaction event test around
PersistentEventFactory.createFromRawEvent to define the fixed expected redaction
event ID and assert redactionEventId with toBe(expectedEventId), replacing the
current defined-value-only assertion. Preserve the existing event construction
and use the deterministic ID produced for this signed redaction.
In `@packages/federation-sdk/src/services/room-version-coexistence.spec.ts`:
- Around line 30-41: Add an afterAll teardown alongside the existing beforeEach
setup in the test suite, calling database.disconnect() and awaiting its
completion to close the DatabaseConnectionService connection after all tests
finish.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 04c934b4-f3b5-4a2f-a5bc-9f26fd0b2ddf
📒 Files selected for processing (60)
packages/core/src/events/eventBase.tspackages/core/src/events/homeserver-event-signatures.tspackages/core/src/events/m.reaction.spec.tspackages/core/src/events/m.reaction.tspackages/core/src/events/m.room.create.spec.tspackages/core/src/events/m.room.create.tspackages/core/src/events/m.room.guest_access.spec.tspackages/core/src/events/m.room.guest_access.tspackages/core/src/events/m.room.history_visibility.spec.tspackages/core/src/events/m.room.history_visibility.tspackages/core/src/events/m.room.join_rules.spec.tspackages/core/src/events/m.room.join_rules.tspackages/core/src/events/m.room.member-invite.spec.tspackages/core/src/events/m.room.member.spec.tspackages/core/src/events/m.room.member.tspackages/core/src/events/m.room.message.tspackages/core/src/events/m.room.name.spec.tspackages/core/src/events/m.room.name.tspackages/core/src/events/m.room.power_levels.spec.tspackages/core/src/events/m.room.power_levels.tspackages/core/src/events/m.room.redaction.spec.tspackages/core/src/events/m.room.redaction.tspackages/core/src/events/m.room.tombstone.spec.tspackages/core/src/events/m.room.tombstone.tspackages/core/src/events/utils/createEventWithId.spec.tspackages/core/src/events/utils/createSignedEvent.spec.tspackages/core/src/events/utils/createSignedEvent.tspackages/core/src/index.tspackages/core/src/utils/authentication.spec.tspackages/core/src/utils/checkSignAndHashes.spec.tspackages/core/src/utils/checkSignAndHashes.tspackages/core/src/utils/generateId.tspackages/federation-sdk/src/index.tspackages/federation-sdk/src/repositories/event-staging.repository.tspackages/federation-sdk/src/repositories/event.repository.tspackages/federation-sdk/src/sdk.tspackages/federation-sdk/src/services/event-authorization.service.tspackages/federation-sdk/src/services/event-fetcher.service.tspackages/federation-sdk/src/services/event-notifier.service.tspackages/federation-sdk/src/services/event-sender.service.spec.tspackages/federation-sdk/src/services/event-sender.service.tspackages/federation-sdk/src/services/event.service.tspackages/federation-sdk/src/services/message.service.tspackages/federation-sdk/src/services/missing-event.service.tspackages/federation-sdk/src/services/profiles.service.tspackages/federation-sdk/src/services/room-version-coexistence.spec.tspackages/federation-sdk/src/services/room.service.tspackages/federation-sdk/src/services/state.service.spec.tspackages/federation-sdk/src/services/state.service.tspackages/federation-sdk/src/utils/event-schemas.spec.tspackages/federation-sdk/src/utils/event-schemas.tspackages/room/src/authorizartion-rules/rules.spec.tspackages/room/src/authorizartion-rules/rules.tspackages/room/src/manager/event-wrapper.spec.tspackages/room/src/manager/event-wrapper.tspackages/room/src/manager/factory.tspackages/room/src/manager/room-state.tspackages/room/src/manager/v11.tspackages/room/src/manager/v3.tspackages/room/src/types/v3-11.ts
💤 Files with no reviewable changes (19)
- packages/core/src/index.ts
- packages/core/src/utils/generateId.ts
- packages/federation-sdk/src/repositories/event-staging.repository.ts
- packages/core/src/events/utils/createEventWithId.spec.ts
- packages/core/src/events/m.room.history_visibility.ts
- packages/federation-sdk/src/services/event-sender.service.ts
- packages/core/src/events/m.room.power_levels.ts
- packages/core/src/events/utils/createSignedEvent.ts
- packages/federation-sdk/src/repositories/event.repository.ts
- packages/core/src/events/m.room.message.ts
- packages/core/src/events/m.room.member.ts
- packages/core/src/events/m.room.guest_access.ts
- packages/core/src/utils/authentication.spec.ts
- packages/core/src/events/m.room.join_rules.ts
- packages/core/src/events/m.room.name.ts
- packages/core/src/events/m.room.tombstone.ts
- packages/core/src/events/m.reaction.ts
- packages/core/src/events/m.room.create.ts
- packages/core/src/events/m.room.redaction.ts
📜 Review details
🔇 Additional comments (41)
packages/core/src/events/m.reaction.spec.ts (1)
3-3: LGTM!packages/core/src/events/m.room.create.spec.ts (1)
54-54: LGTM!packages/core/src/events/m.room.guest_access.spec.ts (1)
3-3: LGTM!Also applies to: 58-58
packages/core/src/events/m.room.history_visibility.spec.ts (1)
3-3: LGTM!Also applies to: 60-60
packages/core/src/events/m.room.join_rules.spec.ts (1)
3-3: LGTM!Also applies to: 58-58
packages/core/src/events/m.room.member-invite.spec.ts (1)
3-3: LGTM!Also applies to: 121-121
packages/core/src/events/m.room.member.spec.ts (1)
3-12: LGTM!Also applies to: 44-44, 69-69, 87-87, 103-103, 125-125, 159-159, 193-193, 213-213, 233-233, 259-259, 299-299, 333-333, 353-353, 373-373, 399-399
packages/core/src/events/m.room.name.spec.ts (1)
3-14: LGTM!Also applies to: 62-62, 80-80, 99-99
packages/core/src/events/m.room.power_levels.spec.ts (1)
3-3: LGTM!Also applies to: 118-118, 172-172
packages/core/src/events/m.room.redaction.spec.ts (1)
3-3: LGTM!packages/core/src/events/m.room.tombstone.spec.ts (1)
3-6: LGTM!Also applies to: 127-129
packages/core/src/events/utils/createSignedEvent.spec.ts (1)
5-5: LGTM!Also applies to: 9-9, 12-23
packages/room/src/authorizartion-rules/rules.spec.ts (1)
6-6: LGTM!Also applies to: 97-99, 1164-1208
packages/room/src/authorizartion-rules/rules.ts (1)
54-60: LGTM!Also applies to: 155-155
packages/room/src/manager/event-wrapper.spec.ts (1)
5-5: LGTM!Also applies to: 380-415
packages/room/src/manager/event-wrapper.ts (1)
9-18: LGTM!Also applies to: 40-45, 248-269
packages/room/src/manager/factory.ts (1)
1-10: LGTM!Also applies to: 25-35, 52-96, 101-102, 123-127
packages/room/src/manager/room-state.ts (1)
21-21: LGTM!packages/room/src/manager/v11.ts (1)
2-26: LGTM!packages/core/src/events/eventBase.ts (1)
40-41: LGTM!packages/federation-sdk/src/services/room-version-coexistence.spec.ts (1)
96-104: LGTM!Also applies to: 106-261
packages/room/src/types/v3-11.ts (1)
103-108: 🗄️ Data Integrity & IntegrationNo change needed. The pre-v11 and v11+ schemas enforce
content.creator/top-levelredactsandcontent.redactsrespectively, so version-specific invalidm.room.createandm.room.redactionevents are rejected before reaching processing.packages/room/src/manager/v3.ts (1)
4-20: LGTM!Also applies to: 53-59
packages/core/src/events/homeserver-event-signatures.ts (1)
49-50: LGTM!packages/core/src/utils/checkSignAndHashes.spec.ts (1)
17-17: LGTM!Also applies to: 65-65, 78-100, 103-115, 135-135, 155-155
packages/core/src/utils/checkSignAndHashes.ts (1)
2-23: LGTM!packages/federation-sdk/src/utils/event-schemas.ts (1)
1-1: LGTM!Also applies to: 21-22, 31-40, 107-116, 127-153
packages/federation-sdk/src/utils/event-schemas.spec.ts (1)
1-51: LGTM!packages/federation-sdk/src/index.ts (1)
73-73: LGTM!packages/federation-sdk/src/services/event.service.ts (1)
10-10: LGTM!Also applies to: 32-32, 65-77, 159-159, 222-222, 233-233, 260-267, 442-443, 537-558
packages/federation-sdk/src/services/missing-event.service.ts (1)
2-2: LGTM!Also applies to: 11-11
packages/federation-sdk/src/services/state.service.ts (1)
1-1: LGTM!Also applies to: 16-16, 78-107, 244-251, 275-276
packages/federation-sdk/src/services/state.service.spec.ts (1)
12-12: LGTM!Also applies to: 22-22, 760-760, 810-810, 999-999, 1017-1017, 1045-1045, 1219-1219, 1260-1260, 1307-1307, 1389-1389, 1486-1486, 1544-1544, 1609-1609
packages/federation-sdk/src/services/event-fetcher.service.ts (1)
1-7: LGTM!Also applies to: 24-27, 62-65
packages/federation-sdk/src/services/event-notifier.service.ts (1)
3-3: LGTM!Also applies to: 56-61
packages/federation-sdk/src/sdk.ts (1)
134-136: LGTM!packages/federation-sdk/src/services/message.service.ts (1)
2-2: LGTM!Also applies to: 238-253, 263-276, 293-317, 333-351
packages/federation-sdk/src/services/event-sender.service.spec.ts (1)
10-10: LGTM!Also applies to: 82-89
packages/federation-sdk/src/services/profiles.service.ts (1)
1-1: LGTM!Also applies to: 79-86, 107-112
packages/federation-sdk/src/services/event-authorization.service.ts (1)
1-1: LGTM!Also applies to: 35-35
packages/federation-sdk/src/services/room.service.ts (1)
40-40: LGTM!Also applies to: 228-228, 245-245, 277-277, 294-294, 314-314, 476-494, 510-536, 592-615, 631-655, 707-707, 726-746, 917-930, 1132-1132, 1439-1439, 1469-1469, 1486-1486, 1503-1503, 1520-1520, 1557-1557
There was a problem hiding this comment.
All reported issues were addressed across 60 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/core/src/utils/pruneEventDict.spec.ts`:
- Around line 41-43: Update the migration TODO in pruneEventDict.spec.ts to
refer to signJson(..., prune=true) instead of signEvent, while preserving the
existing PersistentEventFactory.createFromRawEvent replacement guidance and
remaining call-site count.
In `@packages/federation-sdk/src/services/profiles.service.spec.ts`:
- Around line 41-47: Update the makeJoin test to stub getLatestRoomState2 and
buildEvent so it exercises the supported-version success path without unrelated
failures. Assert that makeJoin resolves with room version 10 and the expected
membership event, rather than only asserting rejection is not an
IncompatibleRoomVersionError.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e61376c6-7fdd-442c-9ecc-a0b644e97d3f
📒 Files selected for processing (17)
packages/core/src/events/m.reaction.spec.tspackages/core/src/events/m.room.redaction.spec.tspackages/core/src/events/utils/createSignedEvent.spec.tspackages/core/src/events/utils/createSignedEvent.tspackages/core/src/index.tspackages/core/src/utils/checkSignAndHashes.spec.tspackages/core/src/utils/pruneEventDict.spec.tspackages/core/src/utils/pruneEventDict.tspackages/federation-sdk/src/services/event.service.spec.tspackages/federation-sdk/src/services/profiles.service.spec.tspackages/federation-sdk/src/services/profiles.service.tspackages/federation-sdk/src/services/room-version-coexistence.spec.tspackages/federation-sdk/src/services/room.service.tspackages/federation-sdk/src/services/state.service.spec.tspackages/federation-sdk/src/utils/event-schemas.spec.tspackages/federation-sdk/src/utils/event-schemas.tspackages/federation-sdk/src/utils/signJson.spec.ts
💤 Files with no reviewable changes (3)
- packages/core/src/events/utils/createSignedEvent.spec.ts
- packages/core/src/events/utils/createSignedEvent.ts
- packages/core/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/core/src/events/m.room.redaction.spec.ts
- packages/core/src/events/m.reaction.spec.ts
- packages/federation-sdk/src/utils/event-schemas.spec.ts
- packages/federation-sdk/src/services/room-version-coexistence.spec.ts
- packages/core/src/utils/checkSignAndHashes.spec.ts
- packages/federation-sdk/src/services/state.service.spec.ts
- packages/federation-sdk/src/utils/event-schemas.ts
- packages/federation-sdk/src/services/room.service.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Code Quality Checks(lint, test, tsc)
🔇 Additional comments (4)
packages/federation-sdk/src/utils/signJson.spec.ts (1)
3-4: LGTM!Also applies to: 117-152
packages/federation-sdk/src/services/profiles.service.ts (1)
1-1: LGTM!Also applies to: 85-90, 111-116
packages/federation-sdk/src/services/event.service.spec.ts (1)
1-139: LGTM!packages/core/src/utils/pruneEventDict.ts (1)
45-48: LGTM!
There was a problem hiding this comment.
All reported issues were addressed across 17 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
CORE-2527
Summary by CodeRabbit
New Features
Bug Fixes