refactor: replace string event IDs with EventID #200
Conversation
WalkthroughType-only migration: most event identifier types changed from plain Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Controller as Homeserver Controller
participant Service as Event/Room Service
participant Repo as Repository / Store
Controller->>Service: call API (params cast to EventID / EventID[])
Service->>Repo: query/persist using EventID or EventID[]
Note over Controller,Repo: All event-id parameters are EventID (type-only)
sequenceDiagram
autonumber
participant RoomSvc as RoomService
participant AuthMap as AuthEvents Map
participant Builder as Tombstone Builder
RoomSvc->>AuthMap: collect auth candidate IDs (may be undefined)
AuthMap-->>RoomSvc: map with optional IDs
RoomSvc->>Builder: filter defined IDs -> auth_events: EventID[]
Builder-->>RoomSvc: tombstone event payload (auth_events typed EventID[])
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
ggazzo
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #200 +/- ##
==========================================
+ Coverage 81.12% 81.14% +0.01%
==========================================
Files 62 63 +1
Lines 4662 4666 +4
==========================================
+ Hits 3782 3786 +4
Misses 880 880 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ated files for improved type safety
ggazzo
force-pushed
the
chore/branded_event_ids
branch
from
54dc4cb to
385b58a
Compare
ggazzo
force-pushed
the
chore/branded_event_ids
branch
from
613a78c to
b50354c
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
packages/federation-sdk/src/repositories/room.repository.ts (2)
54-60: Bug: projection path is wrong;getRoomVersionalways returns null.The version lives under
room.version; projectingversionomits theroomobject.- const room = await this.collection.findOne( - { _id: roomId }, - { projection: { version: 1 } }, - ); + const room = await this.collection.findOne( + { _id: roomId }, + { projection: { 'room.version': 1 } }, + );
62-68: Bug: filter usesroom_idinstead of_id; update will never match.- await this.collection.updateOne( - { room_id: roomId }, - { $set: { name: name } }, - { upsert: false }, - ); + await this.collection.updateOne( + { _id: roomId }, + { $set: { 'room.name': name } }, + { upsert: false }, + );packages/room/src/state_resolution/definitions/algorithm/v2.spec.ts (2)
57-67: Branding mismatch: assigning a plain string to EventID._event_id is branded; constructing it with a string breaks the brand in strict mode. Cast or brand via a helper.
Apply:
- _event_id: EventID; + _event_id: EventID; @@ - this._event_id = `${id}:example.com`; + this._event_id = `${id}:example.com` as EventID;
15-19: MockEventStore signature must match updated EventStore (EventID[]).Interface now requires EventID[]. Update the mock to avoid type mismatch.
Apply:
- async getEvents(eventIds: string[]): Promise<PersistentEventBase[]> { + async getEvents(eventIds: EventID[]): Promise<PersistentEventBase[]> { return this.events.filter((e) => eventIds.includes(e.eventId)); }packages/room/src/types/v3-11.ts (1)
134-137: Migrate remainingevent_idstrings toeventIdSchema.Replace the z.string() event_id fields in packages/room/src/types/v3-11.ts at lines 134–136, 387–390, 397–399, 520–522 with the branded schema.
@@ - event_id: z - .string() - .describe('The event ID of the last known event in the old room.'), + event_id: eventIdSchema.describe( + 'The event ID of the last known event in the old room.', + ), @@ - event_id: z - .string() - .optional() - .describe('The ID of the event that is being related to.'), + event_id: eventIdSchema + .optional() + .describe('The ID of the event that is being related to.'), @@ - event_id: z - .string() - .describe('The ID of the latest event in the thread for fallback'), + event_id: eventIdSchema.describe( + 'The ID of the latest event in the thread for fallback', + ), @@ - event_id: z - .string() - .describe('The ID of the event that is being annotated.'), + event_id: eventIdSchema.describe('The ID of the event that is being annotated.'),packages/federation-sdk/src/repositories/event.repository.ts (2)
296-301: Filter by _id (use EventID[]) — query uses non-existent eventIdThis query filters on a non-existent field (eventId) and will return no results; change to _id and use EventID[]; update callers and add a quick test.
File: packages/federation-sdk/src/repositories/event.repository.ts Lines: 296-301
- findFromNonPublicRooms(eventIds: string[]): FindCursor<EventStore> { - return this.collection.find({ - eventId: { $in: eventIds }, + findFromNonPublicRooms(eventIds: EventID[]): FindCursor<EventStore> { + return this.collection.find({ + _id: { $in: eventIds }, 'event.content.join_rule': { $ne: 'public' }, }); }Update callers to pass EventID[] and add a quick test.
365-385: Wrong field and type for exclusion filter — fix to use _id and EventID[]; add testStored events use _id for the event ID; the current query filters on event_id and accepts string[], so exclusions never match. Update the parameter type and filter.
File: packages/federation-sdk/src/repositories/event.repository.ts (findEventsByRoomAndDepth, ~lines 365–385)
- findEventsByRoomAndDepth( + findEventsByRoomAndDepth( roomId: string, minDepth: number, maxDepth: number, - excludeEventIds: string[], + excludeEventIds: EventID[], limit: number, ): FindCursor<EventStore<Pdu>> { return this.collection .find( { 'event.room_id': roomId, 'event.depth': { $gte: minDepth, $lte: maxDepth }, - event_id: { $nin: excludeEventIds }, + _id: { $nin: excludeEventIds }, }, { projection: { event: 1 }, }, ) .sort({ 'event.depth': 1 }) .limit(limit); }Add a unit/integration test asserting excluded IDs are not returned.
packages/federation-sdk/src/services/event.service.ts (1)
472-498: Fix findEventsByRoomAndDepth to accept EventID[] and filter on _id
findEventsByRoomAndDepth currently types excludeEventIds as string[] and queries event_id: { $nin: ... } (packages/federation-sdk/src/repositories/event.repository.ts, ~line 365). The service passes EventID[] and repository events are stored under _id. Change the method signature to excludeEventIds: EventID[] and update the query to _id: { $nin: excludeEventIds }. After applying the change, re-run missing-events tests.
🧹 Nitpick comments (32)
packages/room/src/state_resolution/definitions/algorithm/v2.ts (2)
107-108: EventID[] migration here is good; tighten the wrapper’s method signature for safety.
eventIdsToFindasEventID[]aligns with the store API. Consider typing the wrapper toEventStoreand the param toreadonly EventID[]to prevent accidental string arrays.- const wrappedStore: typeof store = { - async getEvents(eventIds) { + const wrappedStore: EventStore = { + async getEvents(eventIds: readonly EventID[]) {
175-184: Batch fetch conflicted power events instead of N sequential awaits.Current loop does 1 RPC per ID. Collect and fetch in one call to reduce I/O.
- for (const eventid of fullConflictedSet) { - const [event] = await wrappedStore.getEvents([eventid]); - if (!event) { - console.warn('event not found in eventMap', eventid); - continue; - } - if (isPowerEvent(event)) { - powerEvents.set(event.eventId, event); - } - } + { + const ids = Array.from(fullConflictedSet); + const events = await wrappedStore.getEvents(ids); + for (const event of events) { + if (!event) continue; + if (isPowerEvent(event)) { + powerEvents.set(event.eventId, event); + } + } + }packages/federation-sdk/src/repositories/room.repository.ts (1)
15-16: Align types:tombstone_event_idisEventID; acceptEventIDinmarkRoomAsDeleted.Avoid passing plain
stringto a brandedEventIDfield.- async markRoomAsDeleted( - roomId: string, - tombstoneEventId: string, - ): Promise<void> { + async markRoomAsDeleted( + roomId: string, + tombstoneEventId: EventID, + ): Promise<void> { await this.collection.updateOne( { _id: roomId }, { $set: { 'room.deleted': true, 'room.tombstone_event_id': tombstoneEventId, }, }, ); }Also applies to: 73-86
packages/federation-sdk/src/repositories/state.repository.ts (1)
37-41:getLatestStateMappingreturns the oldest due to ascending sort.Use
-1to fetch the latest.- return this.collection.findOne({ roomId }, { sort: { createdAt: 1 } }); + return this.collection.findOne({ roomId }, { sort: { createdAt: -1 } });packages/core/src/events/m.room.guest_access.ts (1)
1-1: Use package alias and type-only import forEventID; param type changes LGTM.Avoid deep path; keep consistency with other files.
-import { EventID } from 'packages/room/dist'; +import type { EventID } from '@hs/room';Also applies to: 28-30
packages/federation-sdk/src/services/missing-event.service.ts (1)
3-3: Use package alias and type-only import; name the logger after the service.Small hygiene tweaks.
-import { EventID } from 'packages/room/dist'; +import type { EventID } from '@hs/room';-private readonly logger = createLogger('EventService'); +private readonly logger = createLogger('MissingEventService');Also applies to: 17-17
packages/homeserver/src/controllers/internal/room.controller.ts (1)
4-8: MarkEventIDas type-only; parseevent_idinstead of casting.This preserves the brand and avoids leaking values imports.
-import { - EventID, - type PduCreateEventContent, - PersistentEventFactory, -} from '@hs/room'; +import { + type EventID, + type PduCreateEventContent, + PersistentEventFactory, +} from '@hs/room';- const room = await stateService.findStateAtEvent(eventId as EventID); + import { eventIdSchema } from '@hs/room'; + const room = await stateService.findStateAtEvent(eventIdSchema.parse(eventId));Also applies to: 192-193
packages/homeserver/src/controllers/internal/message.controller.ts (1)
119-119: LGTM on EventID casts; make the other call sites consistent.
unsetReactionandredactMessagenow passEventID. Do the same forupdateMessageandsendReactionto avoid mixed typing.Suggested updates elsewhere in this file:
// in updateMessage handler params.messageId as EventID // in sendReaction handler params.messageId as EventIDPlease confirm MessageService method signatures now expect
EventID. If not, align them or narrow here.Also applies to: 158-158
packages/core/src/models/event.model.ts (1)
1-1: Great: persisted _id is EventID. Now align adjacent ID fields.To keep branding consistent and prevent accidental mixing with plain strings, consider updating nearby types:
- missing_dependencies?: EventID[]
- EventStore.nextEventId: EventID
- FetchedEvents.events[].eventId: EventID
- FetchedEvents.missingEventIds: EventID[]
Example (outside selected lines):
interface PersistentEventBase<E = Pdu> { _id: EventID; // ... missing_dependencies?: EventID[]; } export interface EventStore<E = Pdu> extends PersistentEventBase<E> { stateId: string; nextEventId: EventID; } export interface FetchedEvents { events: { eventId: EventID; event: CoreEventBase }[]; missingEventIds: EventID[]; }Also applies to: 7-7
packages/federation-sdk/src/services/send-join.service.ts (1)
3-3: Good: public API now takes EventID; tighten local typing and avoid variable shadowing.
- Type
authChainasEventID[]to preserve branding end‑to‑end.- Avoid reusing the identifier
eventinside thefor...ofloop (shadows the method param).Outside the selected lines:
const authChain: EventID[] = []; for (const ev of state.values()) { const authEvents = await getAuthChain(ev, stateService._getStore(roomVersion)); authChain.push(...authEvents); }Also applies to: 24-24
packages/federation-sdk/src/services/staging-area.service.ts (1)
6-6: Remove redundantas EventIDcasts in emitter payloads.
eventIdis alreadyEventID(via EventStagingStore._id). Casting is unnecessary noise.Apply:
- event_id: eventId as EventID, + event_id: eventId,Repeat for the redaction and membership emitters. The type‑only import on Line 6 is correct.
Also applies to: 166-167, 182-183, 195-196
packages/room/src/manager/v3.ts (1)
19-21: Avoid casts by typing raw_event.prev_events as EventID[] at the source.The cast works, but updating the PDU type would remove the need for
as EventID[].packages/federation-sdk/src/services/room.service.ts (2)
1040-1049: Auth event map now uses actual IDs; consider enforcing required keys.Filtering undefined later is good, but tombstone should require create, power_levels, and member. Fail fast if any are missing.
Apply:
const authEventsMap: TombstoneAuthEvents = { 'm.room.create': authEvents.find( (event) => event.event.type === 'm.room.create', )?._id, 'm.room.power_levels': authEvents.find( (event) => event.event.type === 'm.room.power_levels', )?._id, 'm.room.member': authEvents.find( (event) => event.event.type === 'm.room.member', )?._id, }; + const missing = Object.entries(authEventsMap) + .filter(([, id]) => !id) + .map(([t]) => t); + if (missing.length) { + throw new HttpException( + `Missing auth events for tombstone: ${missing.join(', ')}`, + HttpStatus.FORBIDDEN, + ); + }
1052-1055: Minor: tighten filtering with a type guard.Keeps types precise without an assertion.
Apply:
- const authEventsArray = Object.values(authEventsMap).filter( - (event) => event !== undefined, - ) as EventID[]; + const authEventsArray = Object.values(authEventsMap).filter( + (e): e is EventID => Boolean(e), + );packages/core/src/utils/generateId.ts (2)
2-2: Mark EventID import as type‑only.-import { EventID } from 'packages/room/dist'; +import type { EventID } from 'packages/room/dist';
7-15: Return type change to EventID looks good; casting is acceptable.Optional: centralize branding to reduce repeated
as EventIDcasts (e.g., a tinyasEventID(s: string): EventIDhelper).packages/core/src/events/m.room.tombstone.ts (2)
1-1: Type‑only import for EventID.-import { EventID } from 'packages/room/dist'; +import type { EventID } from 'packages/room/dist';
41-45:isTruthyhelper is duplicated across modules.Consider a shared utility (e.g.,
utils/predicates.ts) to DRY.packages/room/src/types/v3-11.ts (1)
556-561: Use eventIdSchema for top‑levelredactsto finish the migration.- redacts: z - .string() - .describe( - 'The ID of the event that this event redacts. This is an optional field.', - ) - .optional(), + redacts: eventIdSchema + .describe('The ID of the event that this event redacts.') + .optional(),packages/core/src/events/m.room.power_levels.ts (2)
1-1: Type‑only import for EventID.-import { EventID } from 'packages/room/dist'; +import type { EventID } from 'packages/room/dist';
57-60: Make'm.room.member'optional (matches TODO/tests) and avoid cast with a typed predicate.- auth_events: { - 'm.room.create': EventID; - 'm.room.power_levels': EventID; - 'm.room.member': EventID; // TODO: Based on the tests, this is optional, based on the code, this is required check this - }; + auth_events: { + 'm.room.create': EventID; + 'm.room.power_levels': EventID; + 'm.room.member'?: EventID; // optional per tests + }; @@ - auth_events: [ + auth_events: [ auth_events['m.room.create'], auth_events['m.room.power_levels'], auth_events['m.room.member'], - ].filter(Boolean) as EventID[], + ].filter((e): e is EventID => Boolean(e)),If any callers relied on
'm.room.member'being required, CI/type‑check will surface them. Please confirm no regressions.packages/federation-sdk/src/services/message.service.ts (4)
75-76: Fix typos: “white” → “while”.Tiny copy edit in error messages.
-`Room version not found for room ${roomId} white trying to send message`, +`Room version not found for room ${roomId} while trying to send message`,Also applies to: 117-120
379-413: Variable name “redactionEvent” is misleading for an edit event.This block creates an m.room.message replacement, not a redaction. Rename for clarity.
-const redactionEvent = await this.stateService.buildEvent<'m.room.message'>( +const replacementEvent = await this.stateService.buildEvent<'m.room.message'>( ... -await this.stateService.persistTimelineEvent(redactionEvent); -void this.federationService.sendEventToAllServersInRoom(redactionEvent); -return redactionEvent.eventId; +await this.stateService.persistTimelineEvent(replacementEvent); +void this.federationService.sendEventToAllServersInRoom(replacementEvent); +return replacementEvent.eventId;
339-353: Good: unsetReaction now brands the target ID. Also update return type.Return should be EventID for consistency with the new branding.
-): Promise<string> { +): Promise<EventID> {
417-453: Good: redactMessage now brands the target ID. Also update return type.Return should be EventID as well.
-): Promise<string> { +): Promise<EventID> {packages/federation-sdk/src/repositories/event-staging.repository.ts (1)
3-3: Prefer type-only import for Pdu.Avoids emitting a runtime import for a type.
-import { type EventID, Pdu } from '@hs/room'; +import { type EventID, type Pdu } from '@hs/room';packages/federation-sdk/src/utils/event-schemas.ts (1)
13-18: Align prev_events/auth_events with EventID branding.These still accept string/tuple. If you intend to enforce EventID, update schemas to brand event IDs too while preserving tuple support.
-prev_events: z - .array(z.string().or(z.tuple([z.string(), z.string()]))) - .optional(), -auth_events: z - .array(z.string().or(z.tuple([z.string(), z.string()]))) - .optional(), +prev_events: z + .array(eventIdSchema.or(z.tuple([eventIdSchema, z.string()]))) + .optional(), +auth_events: z + .array(eventIdSchema.or(z.tuple([eventIdSchema, z.string()]))) + .optional(),If inbound federation must accept legacy strings, keep the union but brand the event_id member as above.
packages/room/src/types/_common.ts (2)
4-8: Branding scaffold is solid; consider not exporting the brand symbol.Exporting Brand/Branded is fine; exporting the unique symbol is usually unnecessary. Optional.
-export declare const __brand: unique symbol; +declare const __brand: unique symbol; +export { __brand as /* internal */ __brand };Or simply keep it non-exported if not needed by consumers of Brand/Branded.
1-1: Minor consistency nit: prefer named import from zod.Other files import
{ z }. Standardize if you like.-import z from 'zod'; +import { z } from 'zod';packages/core/src/events/m.room.redaction.ts (1)
1-1: Unify import path and make it type-only.Use the public barrel and avoid value import.
-import { EventID } from 'packages/room/dist'; +import type { EventID } from '@hs/room';packages/federation-sdk/src/services/state.service.ts (1)
778-779: Drop redundant cast to EventID[].Pdu.auth_events should already be EventID[]. Remove the cast to keep types honest.
- const authEventsReferencedInMessage = await store.getEvents( - event.event.auth_events as EventID[], - ); + const authEventsReferencedInMessage = await store.getEvents( + event.event.auth_events, + );If this fails TS, there’s a lingering string[] somewhere in Pdu; I can help locate and fix.
packages/federation-sdk/src/repositories/event.repository.ts (1)
205-210: Minor: redundant _id != '' filter.new events are always non-empty ids; this predicate adds noise and no safety.
- .find({ - nextEventId: '', - 'event.room_id': roomId, - _id: { $ne: '' as EventID }, - }) + .find({ + nextEventId: '', + 'event.room_id': roomId, + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
packages/core/src/events/eventBase.ts(4 hunks)packages/core/src/events/m.reaction.ts(5 hunks)packages/core/src/events/m.room.guest_access.ts(2 hunks)packages/core/src/events/m.room.history_visibility.ts(2 hunks)packages/core/src/events/m.room.join_rules.ts(2 hunks)packages/core/src/events/m.room.member.ts(3 hunks)packages/core/src/events/m.room.message.ts(4 hunks)packages/core/src/events/m.room.name.ts(3 hunks)packages/core/src/events/m.room.power_levels.ts(3 hunks)packages/core/src/events/m.room.redaction.ts(4 hunks)packages/core/src/events/m.room.tombstone.ts(3 hunks)packages/core/src/models/event.model.ts(1 hunks)packages/core/src/types.ts(1 hunks)packages/core/src/utils/generateId.ts(1 hunks)packages/federation-sdk/src/index.ts(5 hunks)packages/federation-sdk/src/repositories/event-staging.repository.ts(3 hunks)packages/federation-sdk/src/repositories/event.repository.ts(8 hunks)packages/federation-sdk/src/repositories/room.repository.ts(2 hunks)packages/federation-sdk/src/repositories/state.repository.ts(1 hunks)packages/federation-sdk/src/services/event-fetcher.service.ts(2 hunks)packages/federation-sdk/src/services/event.service.ts(9 hunks)packages/federation-sdk/src/services/message.service.ts(3 hunks)packages/federation-sdk/src/services/missing-event.service.ts(1 hunks)packages/federation-sdk/src/services/profiles.service.ts(2 hunks)packages/federation-sdk/src/services/room.service.ts(3 hunks)packages/federation-sdk/src/services/send-join.service.ts(2 hunks)packages/federation-sdk/src/services/staging-area.service.ts(5 hunks)packages/federation-sdk/src/services/state.service.ts(7 hunks)packages/federation-sdk/src/utils/event-schemas.ts(4 hunks)packages/homeserver/src/controllers/federation/profiles.controller.ts(2 hunks)packages/homeserver/src/controllers/federation/send-join.controller.ts(2 hunks)packages/homeserver/src/controllers/federation/state.controller.ts(3 hunks)packages/homeserver/src/controllers/federation/transactions.controller.ts(2 hunks)packages/homeserver/src/controllers/internal/message.controller.ts(3 hunks)packages/homeserver/src/controllers/internal/room.controller.ts(2 hunks)packages/room/src/manager/event-wrapper.ts(3 hunks)packages/room/src/manager/v3.ts(3 hunks)packages/room/src/state_resolution/definitions/algorithm/v2.spec.ts(2 hunks)packages/room/src/state_resolution/definitions/algorithm/v2.ts(1 hunks)packages/room/src/state_resolution/definitions/definitions.ts(1 hunks)packages/room/src/types/_common.ts(1 hunks)packages/room/src/types/v3-11.ts(4 hunks)
🔇 Additional comments (35)
packages/federation-sdk/src/repositories/state.repository.ts (1)
11-19: EventID branding indelta.eventId: LGTM.Type-level change is consistent with upstream types and assignments.
Also applies to: 70-77
packages/core/src/types.ts (1)
2-10:SignedEvent.event_id→EventID: LGTM.Type-only import keeps runtime clean.
packages/homeserver/src/controllers/federation/send-join.controller.ts (1)
2-2: LGTM: type‑only import and EventID cast are consistent with the migration.If SendJoinService later loosens its input (e.g., branded validators), this cast can be removed.
Also applies to: 23-23
packages/homeserver/src/controllers/federation/state.controller.ts (1)
22-22: Incorrect — event_id is required by DTO; cast is safeGetStateQueryDto and GetStateIdsQueryDto declare event_id as a required t.String (packages/homeserver/src/dtos/federation/state.dto.ts and packages/homeserver/src/dtos/federation/state-ids.dto.ts) and eventService.getState / getStateIds expect a non-optional EventID — no change required.
Likely an incorrect or invalid review comment.
packages/room/src/state_resolution/definitions/algorithm/v2.spec.ts (1)
1-1: Type import alignment looks good.Using a type‑only import for EventID/StateMapKey keeps the test runtime‑clean.
packages/room/src/manager/v3.ts (1)
31-33: EventID computation is correct and branded.Escaped
$and urlSafe base64 match Matrix v3 spec; branding is explicit.packages/room/src/manager/event-wrapper.ts (1)
120-120: Public API change: eventId now branded.Good alignment with EventID across the codebase.
packages/federation-sdk/src/services/profiles.service.ts (1)
109-115: Type migration to EventID[] for missing events is consistent.Signature aligns with EventService; pass‑through preserved.
packages/federation-sdk/src/services/room.service.ts (1)
1064-1064: LGTM on using the filtered auth_events array.Prevents empty/undefined IDs leaking into the tombstone.
packages/core/src/events/m.room.name.ts (1)
20-23: EventID migration here is consistent and correct.Also applies to: 56-56
packages/core/src/events/m.room.tombstone.ts (1)
24-27: Auth/prev_events typed to EventID (optionals filtered) — LGTM.Also applies to: 35-35
packages/room/src/types/v3-11.ts (2)
2-2: Schema shifts to eventIdSchema are correct and consistent with the EventID migration.Also applies to: 200-201, 534-535, 552-553
198-201: Spec alignment: clarify or remove content.redacts in redaction schemaTests in the repo reference content.redacts, so removing it will affect consumers. Locations: packages/room/src/types/v3-11.ts — PduRoomRedactionContentSchema (lines 198–201); usages: packages/room/src/manager/event-wrapper.spec.ts (lines 267, 282, 287). Action: either add an inline comment in the schema documenting that content.redacts is intentionally present for compatibility, or remove content.redacts from the schema and update the referenced tests/consumers to use top‑level redacts.
packages/core/src/events/m.room.member.ts (2)
21-28: AuthEvents/EventID migration looks correct.Keys and the dynamic member mapping now brand to EventID. Nice.
54-54: prev_events → EventID[] is aligned with the new EventBase typing.LGTM.
packages/federation-sdk/src/repositories/event-staging.repository.ts (2)
16-21: create(eventId: EventID, …): upsert logic remains correct.Branding the _id with EventID is aligned with the migration.
43-45: removeByEventId(eventId: EventID): aligned and clear.LGTM.
packages/federation-sdk/src/utils/event-schemas.ts (4)
1-1: Importing eventIdSchema is the right move.Enables branded validation where needed.
10-11: event_id now uses eventIdSchema.Consistent with EventID branding. Good.
66-67: reaction: relates_to.event_id uses eventIdSchema.Matches the spec and the new typing.
101-104: redaction.redacts uses eventIdSchema.Good consistency.
packages/core/src/events/m.room.message.ts (4)
1-1: Type-only EventID import: good.Consistent with the branding approach.
76-86: Relation event_id fields branded to EventID.This tightens types for both m.annotation and m.replace.
94-98: Auth events now require EventID.Correct and consistent with event base changes.
137-144: prev_events: EventID[] is correct.Matches the updated EventBase expectations.
packages/room/src/types/_common.ts (1)
11-14: eventIdSchema and EventID type look good.Runtime validation + branded type are aligned.
packages/federation-sdk/src/services/message.service.ts (1)
17-21: Migrate all event-id params/returns in message.service.ts to EventID — verification requiredRan the ripgrep check; it returned no output, so I could not confirm whether remaining methods still use string-typed event IDs. Re-run this to verify presence of string-typed event IDs:
rg -n --type=ts -C2 $'\b(eventId|eventIdToRedact|eventIdToReplace|eventToReplyTo|threadRootEventId|latestThreadEventId)\s*:\s*string\b' packages/federation-sdk/src/services/message.service.tsIf matches exist, convert these signatures to EventID/Promise (minimal deltas):
- ): Promise<string> { + ): Promise<EventID> { async sendReaction( roomId: string, - eventId: string, + eventId: EventID, emoji: string, senderUserId: string, -): Promise<string> { +): Promise<EventID> { async sendReplyToMessage( roomId: string, rawMessage: string, formattedMessage: string, - eventToReplyTo: string, + eventToReplyTo: EventID, senderUserId: string, ): Promise<PersistentEventBase> { async sendThreadMessage( roomId: string, rawMessage: string, formattedMessage: string, senderUserId: string, - threadRootEventId: string, - latestThreadEventId?: string, + threadRootEventId: EventID, + latestThreadEventId?: EventID, ): Promise<PersistentEventBase> { async sendReplyToInsideThreadMessage( roomId: string, rawMessage: string, formattedMessage: string, senderUserId: string, - threadRootEventId: string, - eventToReplyTo: string, + threadRootEventId: EventID, + eventToReplyTo: EventID, ): Promise<PersistentEventBase> { async updateMessage( roomId: string, rawMessage: string, formattedMessage: string, senderUserId: string, - eventIdToReplace: string, -): Promise<string> { + eventIdToReplace: EventID, +): Promise<EventID> {packages/core/src/events/m.room.redaction.ts (1)
12-15: LGTM: EventID typed redaction fields.Auth event IDs, content.redacts, and prev_events are correctly branded as EventID.
Also applies to: 28-28, 51-51, 54-54
packages/core/src/events/m.reaction.ts (1)
1-1: LGTM: Reaction event surfaces now use EventID.Imports and all event_id fields consistently branded; no logic changes.
Also applies to: 11-11, 23-26, 37-37, 66-66, 71-71
packages/federation-sdk/src/services/state.service.ts (1)
119-121: LGTM: State service API/storage maps migrated to EventID.Signatures and internal maps now carry EventID without behavior changes.
Also applies to: 256-258, 271-273, 348-351, 376-379
packages/federation-sdk/src/index.ts (1)
2-2: LGTM: Public Homeserver event signatures branded with EventID.Consistent, non-breaking type-level changes.
Also applies to: 110-110, 119-119, 121-121, 136-136, 147-147, 154-154, 160-160, 164-164, 170-170
packages/federation-sdk/src/services/event.service.ts (2)
758-759: LGTM: Auth chain tracked as Set.Matches EventID branding all the way to store.getEvents.
69-78: Bug: getEventById passes eventId to findByRoomIdAndType (uses room lookup instead of event ID).Call findById(eventId), return null if not found, then return null if res.event.type !== type; otherwise return res. Add/adjust tests to cover both branches (with and without type). Location: packages/federation-sdk/src/services/event.service.ts lines 69–78
packages/federation-sdk/src/repositories/event.repository.ts (1)
268-274: LGTM: findByIds now typed to EventID[].Consistent with the rest of the repository API.
packages/core/src/events/eventBase.ts (1)
5-6: LGTM: EventID migration on core event base.Types updated correctly for auth_events/prev_events and redacts — confirmed that packages/core/src/events use EventID (e.g., packages/core/src/events/eventBase.ts lines 5–6) and no remaining string-typed auth_events/prev_events/redacts were found. No runtime behavior changes introduced.
| @@ -1,3 +1,4 @@ | |||
| import { EventID } from 'packages/room/dist'; | |||
There was a problem hiding this comment.
Wrong import path for EventID; use package alias.
Using 'packages/room/dist' will break outside the monorepo build. Import from '@hs/room' as a type.
Apply:
-import { EventID } from 'packages/room/dist';
+import type { EventID } from '@hs/room';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { EventID } from 'packages/room/dist'; | |
| import type { EventID } from '@hs/room'; |
🤖 Prompt for AI Agents
In packages/core/src/events/m.room.history_visibility.ts around line 1, the file
currently imports EventID from the local path 'packages/room/dist' which will
break consumers outside the monorepo; change the import to use the package alias
and import EventID as a type from '@hs/room' (e.g. replace the existing import
with a type-only import from '@hs/room') so builds and external consumers
resolve the symbol correctly.
| @@ -1,3 +1,4 @@ | |||
| import { EventID } from 'packages/room/dist'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Fix EventID import path; prefer type‑only import.
Use the same source as the rest of the repo and avoid pulling runtime code for a type.
Apply:
-import { EventID } from 'packages/room/dist';
+import type { EventID } from '@hs/room';The EventID[] parameter changes on Lines 37–38 look correct and align with the project‑wide migration.
Also applies to: 37-38
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== show target file (first 200 lines) ==="
if [ -f packages/core/src/events/m.room.join_rules.ts ]; then
sed -n '1,240p' packages/core/src/events/m.room.join_rules.ts || true
else
echo "MISSING: packages/core/src/events/m.room.join_rules.ts"
fi
echo
echo "=== show lines 1-120 with numbers for quick reference ==="
if [ -f packages/core/src/events/m.room.join_rules.ts ]; then
nl -ba -w3 -s': ' packages/core/src/events/m.room.join_rules.ts | sed -n '1,120p' || true
fi
echo
echo "=== search repo for EventID symbol usages (context) ==="
if command -v rg >/dev/null 2>&1; then
rg -n --hidden -S '\bEventID\b' || true
else
grep -RIn --line-number '\bEventID\b' . || true
fi
echo
echo "=== search for imports that pull from packages/room/dist ==="
if command -v rg >/dev/null 2>&1; then
rg -n --hidden -S "packages/room/dist" || true
else
grep -RIn --line-number "packages/room/dist" . || true
fi
echo
echo "=== search for @hs/room occurrences ==="
if command -v rg >/dev/null 2>&1; then
rg -n --hidden -S "@hs/room" || true
else
grep -RIn --line-number "@hs/room" . || true
fi
echo
echo "=== packages/room package.json (if present) ==="
if [ -f packages/room/package.json ]; then
sed -n '1,240p' packages/room/package.json || true
else
echo "MISSING: packages/room/package.json"
fi
echo
echo "=== list top files under packages/room ==="
if [ -d packages/room ]; then
find packages/room -maxdepth 3 -type f | sed -n '1,200p' || true
else
echo "MISSING: packages/room directory"
fi
echo
echo "=== show root package.json and tsconfig.json (first 200 lines) ==="
if [ -f package.json ]; then
sed -n '1,240p' package.json || true
else
echo "MISSING: package.json"
fi
echo
if [ -f tsconfig.json ]; then
sed -n '1,240p' tsconfig.json || true
else
echo "MISSING: tsconfig.json"
fiLength of output: 29169
Fix EventID import — use a type-only import from @hs/room
Apply:
-import { EventID } from 'packages/room/dist';
+import type { EventID } from '@hs/room';auth_events / prev_events (lines 37–38) are already correctly typed as EventID[].
Also update other occurrences that import from 'packages/room/dist' (e.g. packages/core/src/utils/generateId.ts, packages/core/src/events/*.ts, packages/federation-sdk/src/services/missing-event.service.ts).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { EventID } from 'packages/room/dist'; | |
| import type { EventID } from '@hs/room'; |
| @@ -1,3 +1,4 @@ | |||
| import { EventID } from 'packages/room/dist'; | |||
There was a problem hiding this comment.
Fix import path and make it type‑only.
Use the package alias consistently and avoid a runtime import.
-import { EventID } from 'packages/room/dist';
+import type { EventID } from '@hs/room';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { EventID } from 'packages/room/dist'; | |
| import type { EventID } from '@hs/room'; |
🤖 Prompt for AI Agents
In packages/core/src/events/m.room.member.ts around line 1, replace the runtime
import from 'packages/room/dist' with a type‑only import using the package alias
(e.g. import type { EventID } from 'room';) so the value is not imported at
runtime; ensure EventID is used only as a type or add a type-only import where
referenced.
| @@ -1,3 +1,4 @@ | |||
| import { EventID } from 'packages/room/dist'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
Use a type‑only import for EventID to avoid runtime dependency on 'packages/room/dist'.
Prevents accidental runtime import under verbatimModuleSyntax/ESM.
-import { EventID } from 'packages/room/dist';
+import type { EventID } from 'packages/room/dist';Please confirm tsconfig has either importsNotUsedAsValues: "remove" or that you’re using type‑only imports repo‑wide to avoid runtime loads of types. Run:
🏁 Script executed:
#!/bin/bash
fd -a 'tsconfig*.json' | xargs -I{} jq -r '.compilerOptions.importsNotUsedAsValues?, .compilerOptions.verbatimModuleSyntax?' {} 2>/dev/nullLength of output: 64
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf "PWD: %s\n\n" "$(pwd)"
echo "=== tsconfig files ==="
fd -HI -a 'tsconfig*.json' 2>/dev/null || true
for f in $(fd -HI -a 'tsconfig*.json' 2>/dev/null || true); do
echo -e "\n--- $f ---"
# print the two properties if present, otherwise indicate MISSING
jq -r 'if .compilerOptions then
"importsNotUsedAsValues: " + (.compilerOptions.importsNotUsedAsValues // "MISSING") + "\nverbatimModuleSyntax: " + ( (.compilerOptions.verbatimModuleSyntax // "MISSING") | tostring )
else
"no compilerOptions"
end' "$f" 2>/dev/null || { echo "jq failed; printing first 200 chars:"; sed -n '1,40p' "$f"; }
done
echo -e "\n=== packages/core/src/events/m.room.name.ts ==="
if [ -f packages/core/src/events/m.room.name.ts ]; then
sed -n '1,200p' packages/core/src/events/m.room.name.ts
else
echo "file not found at that exact path; searching for matching files..."
fd -HI 'm.room.name.ts' || true
fi
echo -e "\n=== Search for EventID import patterns ==="
rg -n --hidden --no-ignore --glob '!node_modules' "import\s+type\s*\{[^}]*\bEventID\b[^}]*\}" -S || true
rg -n --hidden --no-ignore --glob '!node_modules' "import\s*\{[^}]*\bEventID\b[^}]*\}" -S || true
echo -e "\n=== Search for imports from packages/room/dist ==="
rg -n --hidden --no-ignore --glob '!node_modules' "from\s+['\"]packages/room/dist['\"]" -S || true
echo -e "\n=== package.json typescript version (if present) ==="
if [ -f package.json ]; then
jq -r '.devDependencies.typescript // .dependencies.typescript // "missing"' package.json 2>/dev/null || true
else
echo "package.json not found at repo root"
fiLength of output: 7223
Use a type-only import for EventID (avoid runtime dependency on 'packages/room/dist')
packages/core/src/events/m.room.name.ts currently uses a value import and no repo tsconfig sets importsNotUsedAsValues or verbatimModuleSyntax, so this will be emitted at runtime. Replace with a type-only import or set importsNotUsedAsValues: "remove" in the relevant tsconfig.
-import { EventID } from 'packages/room/dist';
+import type { EventID } from 'packages/room/dist';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { EventID } from 'packages/room/dist'; | |
| import type { EventID } from 'packages/room/dist'; |
🤖 Prompt for AI Agents
In packages/core/src/events/m.room.name.ts around line 1, the current import
"import { EventID } from 'packages/room/dist';" is a value import causing a
runtime dependency; change it to a type-only import by replacing it with "import
type { EventID } from 'packages/room/dist';" so TypeScript emits no runtime
require, or alternatively set the relevant tsconfig to include
"importsNotUsedAsValues": "remove" (or enable "verbatimModuleSyntax") in the
project that compiles this file.
| import { createLogger } from '@hs/core'; | ||
| import { generateId } from '@hs/core'; | ||
| import { Pdu } from '@hs/room'; | ||
| import { EventID, Pdu } from '@hs/room'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Align EventID branding across the fetcher inputs, outputs, and locals.
You accept EventID[] but still use plain string for IDs internally and in the return type. This weakens type safety and can hide bugs in equality checks.
Suggested updates (outside selected lines):
export interface FetchedEvents {
events: { eventId: EventID; event: Pdu }[];
missingEventIds: EventID[];
}
const localEvents: { eventId: EventID; event: Pdu }[] = [];
private async fetchEventsFromFederation(
eventIds: EventID[],
targetServerName: string,
): Promise<Pdu[]> { /* ... */ }Also confirm EventRepository.findByIds(...) and FederationService.getEvent(...) accept EventID:
Also applies to: 28-28
🏁 Script executed:
#!/bin/bash
rg -nP -C2 --type=ts '\bfindByIds\s*\('
rg -nP -C2 --type=ts '\bgetEvent\s*\('Length of output: 3223
Align EventID branding across the fetcher inputs, outputs, and locals.
Fetcher accepts EventID[] but uses plain string for local IDs and the federation call; convert internals and signatures to EventID to restore type safety.
- Change localEvents and fetcher return types in packages/federation-sdk/src/services/event-fetcher.service.ts (around line 39 and call sites at ~106–109) from string to EventID.
- Change federation service signature in packages/federation-sdk/src/services/federation.service.ts (line ~151) to accept eventId: EventID and update call sites.
- Confirm/ensure EventRepository.findByIds(...) accepts EventID (used in event.service.ts / state.service.ts); update if necessary.
Suggested edits:
export interface FetchedEvents {
events: { eventId: EventID; event: Pdu }[];
missingEventIds: EventID[];
}
const localEvents: { eventId: EventID; event: Pdu }[] = [];
private async fetchEventsFromFederation(
eventIds: EventID[],
targetServerName: string,
): Promise<Pdu[]> { /* ... */ }
async getEvent(domain: string, eventId: EventID): Promise<Pdu> { /* ... */ }🤖 Prompt for AI Agents
In packages/federation-sdk/src/services/event-fetcher.service.ts around line 4
and particularly near line 39 and call sites ~106–109, the code uses plain
string for local event IDs and federation calls while the module imports
EventID; change all internal locals and signatures to use EventID (e.g.
localEvents array item type, FetchedEvents.missingEventIds, and fetcher return
types). Also update the federation service signature in
packages/federation-sdk/src/services/federation.service.ts (around line ~151) to
accept eventId: EventID and update every call site to pass an EventID, and
verify EventRepository.findByIds(...) (used in event.service.ts /
state.service.ts) accepts EventID[] and change its signature/usage if necessary
so types line up end-to-end. Ensure imports remain correct and adjust any
downstream typings/returns (e.g. fetchEventsFromFederation return type and
getEvent parameter/return types) to maintain type safety.
| @@ -1,5 +1,5 @@ | |||
| import { ProfilesService } from '@hs/federation-sdk'; | |||
| import { type RoomVersion } from '@hs/room'; | |||
| import { EventID, type RoomVersion } from '@hs/room'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Avoid unsafe casts; validate/cast to EventID[] at the boundary.
Prefer schema-based parsing over as to keep branding intact.
-import { EventID, type RoomVersion } from '@hs/room';
+import { type EventID, type RoomVersion, eventIdSchema } from '@hs/room';
...
- body.earliest_events as EventID[],
- body.latest_events as EventID[],
+ eventIdSchema.array().parse(body.earliest_events),
+ eventIdSchema.array().parse(body.latest_events),Also applies to: 106-108
🏁 Script executed:
#!/bin/bash
# Find remaining deep imports and unsafe EventID casts.
rg -nP "from ['\"]/?.*packages/room/dist['\"]"
rg -nP "\bas\s+EventID\[\]" -n
rg -nP "\beventId\s*:\s*string\b" --type=ts -n
rg -nP "tombstone_event_id\s*:\s*string\b" --type=ts -nLength of output: 3108
Replace unsafe as EventID[] casts with schema parsing and remove deep 'packages/room/dist' imports
Use a schema (e.g., eventIdSchema.array().parse(...)) at API boundaries and import the schema from '@hs/room' instead of deep 'packages/room/dist'.
Files to fix (precise pointers):
- Deep imports to replace with public export (
@hs/room):- packages/federation-sdk/src/services/missing-event.service.ts:3
- packages/core/src/utils/generateId.ts:2
- packages/core/src/events/m.room.history_visibility.ts:1
- packages/core/src/events/m.room.redaction.ts:1
- packages/core/src/events/m.room.name.ts:1
- packages/core/src/events/m.room.tombstone.ts:1
- packages/core/src/events/m.room.power_levels.ts:1
- packages/core/src/events/m.room.member.ts:1
- packages/core/src/events/m.room.join_rules.ts:1
- packages/core/src/events/m.room.guest_access.ts:1
- Unsafe
as EventID[]casts to replace with parsing:- packages/federation-sdk/src/services/state.service.ts:778
- packages/federation-sdk/src/services/room.service.ts:1054
- packages/homeserver/src/controllers/federation/profiles.controller.ts:106-107
- packages/room/src/manager/v3.ts:20
- packages/room/src/state_resolution/definitions/algorithm/v2.ts:107
- packages/core/src/events/m.room.power_levels.ts:74
Suggested minimal change (example):
-import { EventID, type RoomVersion } from '@hs/room';
+import { type EventID, type RoomVersion, eventIdSchema } from '@hs/room';
...
- body.earliest_events as EventID[],
- body.latest_events as EventID[],
+ eventIdSchema.array().parse(body.earliest_events),
+ eventIdSchema.array().parse(body.latest_events),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { EventID, type RoomVersion } from '@hs/room'; | |
| import { type EventID, type RoomVersion, eventIdSchema } from '@hs/room'; | |
| ... | |
| eventIdSchema.array().parse(body.earliest_events), | |
| eventIdSchema.array().parse(body.latest_events), |
🤖 Prompt for AI Agents
In packages/homeserver/src/controllers/federation/profiles.controller.ts (fix
around the import at top and the usage at lines ~106-107), remove any deep
imports from 'packages/room/dist' and import the public schema/export from
'@hs/room' instead; replace unsafe "as EventID[]" casts by validating/parsing
incoming values with eventIdSchema.array().parse(...) (import eventIdSchema from
'@hs/room'), and use the parsed EventID[] result for downstream logic; ensure
types still reference EventID/RoomVersion from '@hs/room' only and propagate
parse errors appropriately (throw/return a 4xx) at the API boundary.
| @@ -1,4 +1,5 @@ | |||
| import { ConfigService, EventService } from '@hs/federation-sdk'; | |||
| import { EventID } from '@hs/room'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Import eventIdSchema (not just EventID) for boundary validation.
Apply:
-import { EventID } from '@hs/room';
+import { EventID, eventIdSchema } from '@hs/room';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { EventID } from '@hs/room'; | |
| import { EventID, eventIdSchema } from '@hs/room'; |
🤖 Prompt for AI Agents
In packages/homeserver/src/controllers/federation/transactions.controller.ts
around line 2, the current import only pulls EventID from '@hs/room' but the
file needs eventIdSchema for boundary validation; update the import to include
eventIdSchema (e.g., import { EventID, eventIdSchema } from '@hs/room') and then
use eventIdSchema where incoming event IDs are validated so boundary validation
uses the canonical schema.
| const eventData = await eventService.getEventById( | ||
| params.eventId as EventID, | ||
| ); |
There was a problem hiding this comment.
Do not cast params.eventId; validate and brand it.
Unsafe cast can pass invalid IDs into the service. Validate with eventIdSchema and return 400 on failure.
Apply:
- const eventData = await eventService.getEventById(
- params.eventId as EventID,
- );
+ const parsed = eventIdSchema.safeParse(params.eventId);
+ if (!parsed.success) {
+ set.status = 400;
+ return { errcode: 'M_INVALID_PARAM', error: 'Invalid event ID' };
+ }
+ const eventData = await eventService.getEventById(parsed.data);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const eventData = await eventService.getEventById( | |
| params.eventId as EventID, | |
| ); | |
| const parsed = eventIdSchema.safeParse(params.eventId); | |
| if (!parsed.success) { | |
| set.status = 400; | |
| return { errcode: 'M_INVALID_PARAM', error: 'Invalid event ID' }; | |
| } | |
| const eventData = await eventService.getEventById(parsed.data); |
🤖 Prompt for AI Agents
In packages/homeserver/src/controllers/federation/transactions.controller.ts
around lines 47 to 49, do not cast params.eventId; instead validate it using
eventIdSchema and, on success, use the branded value as EventID when calling
eventService.getEventById; if validation fails return a 400 response (with an
appropriate error message) and do not call the service. Ensure you replace the
unsafe cast with the validated/branched value and keep error handling consistent
with other controller validations.
| export interface EventStore { | ||
| getEvents(eventIds: string[]): Promise<PersistentEventBase[]>; | ||
| getEvents(eventIds: EventID[]): Promise<PersistentEventBase[]>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
API shift acknowledged: EventStore.getEvents now takes EventID[].
This aligns the state resolution APIs with branded IDs.
Run to locate remaining string[] signatures:
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -C2 '\bgetEvents\s*\(\s*eventIds\s*:\s*string\[\]'Length of output: 1076
Update remaining test mocks: change getEvents(eventIds: string[]) → EventID[]
Two test mocks still declare getEvents with string[] — update their signatures and any related Maps/types to EventID to match the EventStore API.
- packages/room/src/authorizartion-rules/rules.spec.ts — async getEvents(eventIds: string[]): Promise<PersistentEventBase[]> (line ~16)
- packages/room/src/state_resolution/definitions/algorithm/v2.spec.ts — async getEvents(eventIds: string[]): Promise<PersistentEventBase[]> (line ~17)
Example change: async getEvents(eventIds: EventID[]): Promise<PersistentEventBase[]> { ... } (also update Map<string, ...> → Map<EventID, ...> where applicable).
🤖 Prompt for AI Agents
packages/room/src/state_resolution/definitions/definitions.ts lines 83-85:
Update test mocks in the two referenced spec files to match the EventStore
interface by changing getEvents parameter types from string[] to EventID[] and
any related Maps/keys to use EventID instead of string; specifically edit
packages/room/src/authorizartion-rules/rules.spec.ts (around line ~16) and
packages/room/src/state_resolution/definitions/algorithm/v2.spec.ts (around line
~17) to declare async getEvents(eventIds: EventID[]):
Promise<PersistentEventBase[]> { ... } and change Map<string, ...> →
Map<EventID, ...> or other types/looksups that use string keys so all signatures
and usages align with EventID.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/federation-sdk/src/services/state.service.ts (1)
376-379: Tighten internal cache/key types to EventID.Minor type hygiene: cache and local arrays can use EventID/PersistentEventBase to avoid any[] widening.
- const cache = new Map<string, PersistentEventBase>(); + const cache = new Map<EventID, PersistentEventBase>(); return { - getEvents: async (eventIds: EventID[]): Promise<PersistentEventBase[]> => { - const events = []; - const toFind = []; + getEvents: async (eventIds: EventID[]): Promise<PersistentEventBase[]> => { + const events: PersistentEventBase[] = []; + const toFind: EventID[] = [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/federation-sdk/src/services/staging-area.service.ts(3 hunks)packages/federation-sdk/src/services/state.service.ts(7 hunks)packages/room/src/manager/event-wrapper.ts(2 hunks)packages/room/src/manager/v3.ts(4 hunks)packages/room/src/types/_common.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/room/src/manager/event-wrapper.ts
- packages/room/src/manager/v3.ts
- packages/room/src/types/_common.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/federation-sdk/src/services/state.service.ts (4)
packages/room/src/manager/v3.ts (1)
eventId(22-34)packages/room/src/types/_common.ts (2)
EventID(8-8)StateMapKey(10-10)packages/room/src/manager/room-state.ts (1)
RoomState(15-154)packages/room/src/manager/event-wrapper.ts (1)
event(102-112)
packages/federation-sdk/src/services/staging-area.service.ts (1)
packages/room/src/types/_common.ts (1)
EventID(8-8)
🔇 Additional comments (9)
packages/federation-sdk/src/services/staging-area.service.ts (3)
6-6: Type-only import is correct.Importing EventID as a type avoids runtime impact and matches the PR goal.
171-175: Reaction 'm.relates_to.event_id' changed to EventID — verify consumers expecting stringFile: packages/federation-sdk/src/services/staging-area.service.ts (lines 171–175)
Repo search returned no matches for consumers typed as string. Absence of matches is not proof — manually verify external/third-party callers, serialization/deserialization paths, and any runtime code casting or treating event_id as string.
151-154: Annotate m.relates_to.event_id as EventID — verify downstream consumers.Location: packages/federation-sdk/src/services/staging-area.service.ts (lines 151–154). Typing event_id as EventID tightens the emitter payload; automated search produced no matches for consumers declaring event_id: string, so manual verification is required. Confirm all downstream consumers/handlers of homeserver.matrix.message and any processors of m.relates_to accept EventID or update their types.
packages/federation-sdk/src/services/state.service.ts (6)
3-3: EventID import at source — aligned with branding.Consistent with room’s branded ID schema.
271-273: Param now Map<StateMapKey, EventID> — consistent with the previous change.
348-351: getFullRoomStateBeforeEvent2(eventId: EventID) — API consistency looks good.
776-779: Passing EventID[] to store.getEvents — correct.Matches the new store signature; no runtime impact.
118-121: findStateAtEvent now takes EventID — verify callers pass an EventID
- Call sites found:
- packages/homeserver/src/controllers/internal/room.controller.ts:192 — passes
eventId as EventID(OK).- packages/federation-sdk/src/services/state.service.ts:349 — internal call with
eventId: EventID(OK).- packages/federation-sdk/src/services/state.service.ts:631 — calls
this.findStateAtEvent(lastEvent._id, 'always')— CONFIRMlastEvent._idis an EventID (not a DB _id); if not, use the event's EventID field or cast.- packages/federation-sdk/src/services/event.service.ts:696 and 755 — pass
eventId— VERIFY the originating variable is typed as EventID.- Action: confirm types at the listed locations and update any caller that passes a DB id or wrong type to an EventID (or add an explicit cast).
254-258: Return type is correct — no caller changes required.
buildFullRoomStateFromEvents is only invoked inside packages/federation-sdk/src/services/state.service.ts and its callers (buildFullRoomStateFromStateMapping, buildFullRoomStateFromStateMappings, buildFullRoomStateStoredEvents) already use Map<StateMapKey, EventID>.
Summary by CodeRabbit