Skip to content

Improve types of homeserver.matrix.message #207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ggazzo merged 4 commits into from
Sep 22, 2025
Merged

Improve types of homeserver.matrix.message #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
17fc361
Improve typing of homeserver.matrix.message
rodrigok Sep 21, 2025
cda4ca2
Improve types for media handling
rodrigok Sep 21, 2025
7bf7795
Improve type of processInvite
rodrigok Sep 21, 2025
ba7a37b
fix: processInvite event type definition
ggazzo Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions packages/core/src/events/m.room.message.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,14 @@ declare module './eventBase' {
export type MessageRelation = {
rel_type: RelationType;
event_id: EventID;
} & (RelationTypeReplace | Record<string, never>);
} & (
| RelationTypeReplace
| RelationTypeAnnotation
| RelationTypeThread
| Record<string, never>
);

export type RelationType = 'm.replace' | 'm.annotation';
export type RelationType = 'm.replace' | 'm.annotation' | 'm.thread';
Comment on lines 76 to +86
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Support reply‑only relations in m.relates_to; don’t force rel_type.

Matrix replies can be expressed as { "m.relates_to": { "m.in_reply_to": { event_id } } } without a rel_type. The current MessageRelation shape requires rel_type, rejecting valid reply‑only events.

Apply this diff to relax the type and stop mandating rel_type:

-export type MessageRelation = {
-	rel_type: RelationType;
-	event_id: EventID;
-} & (
-	| RelationTypeReplace
-	| RelationTypeAnnotation
-	| RelationTypeThread
-	| Record<string, never>
-);
+export type MessageRelation =
+	| RelationTypeReplace
+	| RelationTypeAnnotation
+	| RelationTypeThread;

Outside this range, add a reply‑only variant and widen the reference on BaseMessageContent:

// add near the relation types
export type ReplyRelation = {
  'm.in_reply_to': {
    event_id: EventID;
    room_id?: string;
    sender?: string;
    origin_server_ts?: number;
  };
};

// update BaseMessageContent (line 19)
'm.relates_to'?: MessageRelation | ReplyRelation;
🤖 Prompt for AI Agents 
In packages/core/src/events/m.room.message.ts around lines 76 to 86 and update
BaseMessageContent at line 19: the current MessageRelation forces a rel_type
which rejects reply‑only forms; add a new ReplyRelation type (with
'm.in_reply_to' containing event_id and optional room_id, sender,
origin_server_ts) near the relation types, make rel_type optional on the
existing MessageRelation (or add a variant that omits rel_type), and update
BaseMessageContent's 'm.relates_to' at line 19 to accept MessageRelation |
ReplyRelation so reply‑only relations are allowed.


export type RelationTypeReplace = {
rel_type: 'm.replace';
Expand All @@ -91,6 +96,24 @@ export type RelationTypeReplace = {
};
};

export type RelationTypeAnnotation = {
rel_type: 'm.annotation';
event_id: EventID;
key: string;
};

export type RelationTypeThread = {
rel_type: 'm.thread';
event_id: EventID;
'm.in_reply_to'?: {
event_id: EventID;
room_id: string;
sender: string;
origin_server_ts: number;
};
is_falling_back?: boolean;
};

export type MessageAuthEvents = {
'm.room.create': EventID;
'm.room.power_levels': EventID;
Expand Down
65 changes: 52 additions & 13 deletions packages/federation-sdk/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Membership } from '@hs/core';
import type { Membership, MessageType } from '@hs/core';
import type { EventID } from '@hs/room';
import { container } from 'tsyringe';
import { ConfigService } from './services/config.service';
Expand Down Expand Up @@ -115,23 +115,62 @@ export type HomeserverEventSignatures = {
origin_server_ts: number;
content: {
body: string;
msgtype: string;
'm.relates_to'?: {
rel_type: 'm.replace' | 'm.annotation' | 'm.thread';
event_id: EventID;
'm.in_reply_to'?: {
event_id: EventID;
room_id: string;
sender: string;
origin_server_ts: number;
};
};
msgtype: MessageType;
url?: string;
'm.relates_to'?:
| {
rel_type: 'm.replace';
event_id: EventID;
}
| {
rel_type: 'm.annotation';
event_id: EventID;
key: string;
}
| {
rel_type: 'm.thread';
event_id: EventID;
'm.in_reply_to'?: {
event_id: EventID;
room_id: string;
sender: string;
origin_server_ts: number;
};
is_falling_back?: boolean;
};
'm.new_content'?: {
Comment on lines +118 to 141
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add reply‑only variant to m.relates_to union.

Replies without a rel_type (only m.in_reply_to) are valid; excluding them can break consumers.

Apply this diff:

 			'm.relates_to'?:
 				| {
 						rel_type: 'm.replace';
 						event_id: EventID;
 				  }
 				| {
 						rel_type: 'm.annotation';
 						event_id: EventID;
 						key: string;
 				  }
 				| {
 						rel_type: 'm.thread';
 						event_id: EventID;
-						'm.in_reply_to'?: {
-							event_id: EventID;
-							room_id: string;
-							sender: string;
-							origin_server_ts: number;
-						};
+						'm.in_reply_to'?: {
+							event_id: EventID;
+							room_id?: string;
+							sender?: string;
+							origin_server_ts?: number;
+						};
 						is_falling_back?: boolean;
-				  };
+				  }
+				| {
+						'm.in_reply_to': {
+							event_id: EventID;
+						};
+				  };
📝 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.

Suggested change
msgtype: MessageType;
url?: string;
'm.relates_to'?:
| {
rel_type: 'm.replace';
event_id: EventID;
}
| {
rel_type: 'm.annotation';
event_id: EventID;
key: string;
}
| {
rel_type: 'm.thread';
event_id: EventID;
'm.in_reply_to'?: {
event_id: EventID;
room_id: string;
sender: string;
origin_server_ts: number;
};
is_falling_back?: boolean;
};
'm.new_content'?: {
msgtype: MessageType;
url?: string;
'm.relates_to'?:
| {
rel_type: 'm.replace';
event_id: EventID;
}
| {
rel_type: 'm.annotation';
event_id: EventID;
key: string;
}
| {
rel_type: 'm.thread';
event_id: EventID;
'm.in_reply_to'?: {
event_id: EventID;
room_id?: string;
sender?: string;
origin_server_ts?: number;
};
is_falling_back?: boolean;
}
| {
'm.in_reply_to': {
event_id: EventID;
};
};
'm.new_content'?: {
🤖 Prompt for AI Agents 
In packages/federation-sdk/src/index.ts around lines 118 to 141, the
m.relates_to union currently requires a rel_type for all variants but Matrix
allows a reply-only form that contains only m.in_reply_to; add a union variant
that models an object with an 'm.in_reply_to' property (matching the existing
shape: event_id: EventID, room_id: string, sender: string, origin_server_ts:
number) and any necessary optional flags (e.g., is_falling_back?: boolean)
without a rel_type so reply-only relations are accepted by the type.

body: string;
msgtype: string;
msgtype: MessageType;
'm.mentions'?: Record<string, string>;
};
formatted_body?: string;
info?: {
mimetype?: string;
w?: number;
h?: number;
size?: number;
thumbnail_file?: {
hashes: {
sha256: string;
};
iv: string;
key: {
alg: string;
ext: boolean;
k: string;
key_ops: ['encrypt' | 'decrypt'];
kty: string;
};
url: string;
v: 'v2';
};
thumbnail_info?: {
w?: number;
h?: number;
size?: number;
mimetype?: string;
};
};
Comment on lines +147 to +173
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix key.key_ops typing; current form is a one‑element tuple.

key_ops: ['encrypt' | 'decrypt'] means “array with exactly one element”. Use an array of allowed ops (or a fixed tuple if you require both).

           thumbnail_file?: {
             hashes: {
               sha256: string;
             };
             iv: string;
             key: {
               alg: string;
               ext: boolean;
               k: string;
-              key_ops: ['encrypt' | 'decrypt'];
+              key_ops: Array<'encrypt' | 'decrypt'>;
               kty: string;
             };
             url: string;
             v: 'v2';
           };
📝 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.

Suggested change
info?: {
mimetype?: string;
w?: number;
h?: number;
size?: number;
thumbnail_file?: {
hashes: {
sha256: string;
};
iv: string;
key: {
alg: string;
ext: boolean;
k: string;
key_ops: ['encrypt' | 'decrypt'];
kty: string;
};
url: string;
v: 'v2';
};
thumbnail_info?: {
w?: number;
h?: number;
size?: number;
mimetype?: string;
};
};
info?: {
mimetype?: string;
w?: number;
h?: number;
size?: number;
thumbnail_file?: {
hashes: {
sha256: string;
};
iv: string;
key: {
alg: string;
ext: boolean;
k: string;
key_ops: Array<'encrypt' | 'decrypt'>;
kty: string;
};
url: string;
v: 'v2';
};
thumbnail_info?: {
w?: number;
h?: number;
size?: number;
mimetype?: string;
};
};
🤖 Prompt for AI Agents 
In packages/federation-sdk/src/index.ts around lines 147 to 173, the key.key_ops
type is currently declared as a single‑element tuple (`key_ops: ['encrypt' |
'decrypt']`) which enforces exactly one entry; change it to an array of allowed
ops by replacing that type with a string array type such as `Array<'encrypt' |
'decrypt'>` or `('encrypt' | 'decrypt')[]` (or use a fixed tuple
`['encrypt','decrypt']` only if you require both values always present).

};
};
'homeserver.matrix.accept-invite': {
Expand Down
31 changes: 17 additions & 14 deletions packages/federation-sdk/src/services/invite.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { EventBase, HttpException, HttpStatus } from '@hs/core';
import { PersistentEventFactory, RoomVersion } from '@hs/room';
import {
PduForType,
PersistentEventBase,
PersistentEventFactory,
RoomVersion,
} from '@hs/room';
import { singleton } from 'tsyringe';
import { createLogger } from '../utils/logger';
import { ConfigService } from './config.service';
Expand Down Expand Up @@ -121,26 +126,24 @@ export class InviteService {
};
}

async processInvite<
T extends Omit<EventBase, 'origin'> & {
origin?: string | undefined;
room_id: string;
state_key: string;
},
>(event: T, roomId: string, eventId: string, roomVersion: RoomVersion) {
async processInvite(
event: PduForType<'m.room.member'>,
roomId: string,
eventId: string,
roomVersion: RoomVersion,
) {
// SPEC: when a user invites another user on a different homeserver, a request to that homeserver to have the event signed and verified must be made

const residentServer = roomId.split(':').pop();
if (!residentServer) {
throw new Error(`Invalid roomId ${roomId}`);
}

const inviteEvent = PersistentEventFactory.createFromRawEvent(
event as unknown as Parameters<
typeof PersistentEventFactory.createFromRawEvent
>[0],
roomVersion,
);
const inviteEvent =
PersistentEventFactory.createFromRawEvent<'m.room.member'>(
event,
roomVersion,
);

if (inviteEvent.eventId !== eventId) {
throw new Error(`Invalid eventId ${eventId}`);
Expand Down
26 changes: 18 additions & 8 deletions packages/federation-sdk/src/services/staging-area.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { EventBase, EventStagingStore, Membership } from '@hs/core';
import { singleton } from 'tsyringe';

import { createLogger, isRedactedEvent } from '@hs/core';
import { Pdu, PduPowerLevelsEventContent } from '@hs/room';
import { MessageType, createLogger, isRedactedEvent } from '@hs/core';
import { PduPowerLevelsEventContent } from '@hs/room';
import type { EventID } from '@hs/room';
import { EventAuthorizationService } from './event-authorization.service';
import { EventEmitterService } from './event-emitter.service';
Expand Down Expand Up @@ -148,14 +148,24 @@ export class StagingAreaService {
content: {
...event.event.content,
body: event.event.content?.body as string,
msgtype: event.event.content?.msgtype as string,
'm.relates_to': event.event.content?.['m.relates_to'] as {
rel_type: 'm.replace' | 'm.annotation' | 'm.thread';
event_id: EventID;
},
msgtype: event.event.content?.msgtype as MessageType,
'm.relates_to': event.event.content?.['m.relates_to'] as
| {
rel_type: 'm.replace';
event_id: EventID;
}
| {
rel_type: 'm.annotation';
event_id: EventID;
key: string;
}
| {
rel_type: 'm.thread';
event_id: EventID;
},
Comment on lines +151 to +165
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Only include optional m.relates_to when present; align shape with index (incl. replies).

Asserting a non‑optional union while passing possibly undefined is unsound. Also add the reply‑only variant to match your signature in index.ts.

Apply this diff:

-						'm.relates_to': event.event.content?.['m.relates_to'] as
-							| {
-									rel_type: 'm.replace';
-									event_id: EventID;
-							  }
-							| {
-									rel_type: 'm.annotation';
-									event_id: EventID;
-									key: string;
-							  }
-							| {
-									rel_type: 'm.thread';
-									event_id: EventID;
-							  },
+						...(event.event.content?.['m.relates_to']
+							? {
+									'm.relates_to': event.event.content['m.relates_to'] as
+										| {
+												rel_type: 'm.replace';
+												event_id: EventID;
+										  }
+										| {
+												rel_type: 'm.annotation';
+												event_id: EventID;
+												key: string;
+										  }
+										| {
+												rel_type: 'm.thread';
+												event_id: EventID;
+												'm.in_reply_to'?: {
+													event_id: EventID;
+													room_id: string;
+													sender: string;
+													origin_server_ts: number;
+												};
+												is_falling_back?: boolean;
+										  }
+										| {
+												'm.in_reply_to': {
+													event_id: EventID;
+												};
+										  };
+							  }
+							: {}),
📝 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.

Suggested change
msgtype: event.event.content?.msgtype as MessageType,
'm.relates_to': event.event.content?.['m.relates_to'] as
| {
rel_type: 'm.replace';
event_id: EventID;
}
| {
rel_type: 'm.annotation';
event_id: EventID;
key: string;
}
| {
rel_type: 'm.thread';
event_id: EventID;
},
msgtype: event.event.content?.msgtype as MessageType,
...(event.event.content?.['m.relates_to']
? {
'm.relates_to': event.event.content['m.relates_to'] as
| {
rel_type: 'm.replace';
event_id: EventID;
}
| {
rel_type: 'm.annotation';
event_id: EventID;
key: string;
}
| {
rel_type: 'm.thread';
event_id: EventID;
'm.in_reply_to'?: {
event_id: EventID;
room_id: string;
sender: string;
origin_server_ts: number;
};
is_falling_back?: boolean;
}
| {
'm.in_reply_to': {
event_id: EventID;
};
};
}
: {}),

'm.new_content': event.event.content?.['m.new_content'] as {
body: string;
msgtype: string;
msgtype: MessageType;
},
Comment on lines 166 to 169
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Don’t emit m.new_content when absent.

Current code emits the key with an undefined value; keep it optional.

Apply this diff:

-						'm.new_content': event.event.content?.['m.new_content'] as {
-							body: string;
-							msgtype: MessageType;
-						},
+						...(event.event.content?.['m.new_content']
+							? {
+									'm.new_content': event.event.content['m.new_content'] as {
+										body: string;
+										msgtype: MessageType;
+									};
+							  }
+							: {}),

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In packages/federation-sdk/src/services/staging-area.service.ts around lines 166
to 169, the code always emits the 'm.new_content' key with a possibly undefined
value; change it to only include that property when
event.event.content?.['m.new_content'] is present (non-null/undefined).
Implement this by conditionally adding the property (e.g., check if value !=
null and then assign or spread the key/value) so the key is omitted entirely
when absent.

formatted_body: (event.event.content?.formatted_body ||
'') as string,
Expand Down