cadence/out-of-your-element
cadence/out-of-your-element
3
7
Fork 2
Code Issues 12 Pull requests 1 Releases 22 Activity

Fix matrix api joinRoom() for remote rooms #60

Open
Elliu wants to merge 6 commits from Elliu/out-of-your-element:fix-remote-join into main
pull from: Elliu/out-of-your-element:fix-remote-join
merge into: cadence:main
Conversation 2 Commits 6 Files changed 6 +19 -7
2 changed files with 19 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 91d62e7b28 - Show all commits

4
src/matrix/api.js
View file

@ -64,9 +64,9 @@ async function createRoom(content) {
/**
* @returns {Promise<string>} room ID
*/
async function joinRoom(roomIDOrAlias, mxid) {
async function joinRoom(roomIDOrAlias, mxid, via) {
/** @type {Ty.R.RoomJoined} */
const root = await mreq.mreq("POST", path(`/client/v3/join/${roomIDOrAlias}`, mxid), {})
const root = await mreq.mreq("POST", path(`/client/v3/join/${roomIDOrAlias}`, mxid, via), {})
return root.room_id
}

22
src/web/routes/link.js
View file

@ -77,7 +77,7 @@ as.router.post("/api/link-space", defineEventHandler(async event => {
// Check space exists and bridge is joined
try {
await api.joinRoom(parsedBody.space_id)
await api.joinRoom(parsedBody.space_id, null, via)
cadence commented 2025-08-22 09:29:28 +00:00
Outdated
Copy link

Where are you going to find via here?

Where are you going to find `via` here?
Elliu commented 2025-08-22 14:40:36 +00:00
Outdated
Copy link

Hmmm, I didn't change that because the bridge needed to be manually invited to the space so that it can be selected.

However indeed, if the bridge is invited to the space, then kicked, the button to the space remains and if clicked, we have the same issue than for joining rooms, didn't notice that.

From the Client-Server API doc there doesn't seem to be "via" or "server_name" information on the m.room.member event, so I'm not sure.
I guess it should be safe to take the HS part of the user from sender in the m.room.member: as this user is able to send an invite, the space must be reachable through the inviter's HS

(unless the invite is old, all users from this HS left the space, and the room was purged from the HS, but nothing we can do about that)

Hmmm, I didn't change that because the bridge needed to be manually invited to the space so that it can be selected. However indeed, if the bridge is invited to the space, then kicked, the button to the space remains and if clicked, we have the same issue than for joining rooms, didn't notice that. From the Client-Server API doc there doesn't seem to be "via" or "server_name" information on the `m.room.member` event, so I'm not sure. I guess it should be safe to take the HS part of the user from `sender` in the `m.room.member`: as this user is able to send an invite, the space must be reachable through the inviter's HS (unless the invite is old, all users from this HS left the space, and the room was purged from the HS, but nothing we can do about that)
Elliu commented 2025-08-22 15:12:50 +00:00
Outdated
Copy link

Last commit uses that, should we add a check to enrich the error message too if for some reasons we cannot detect the via?

Last commit uses that, should we add a check to enrich the error message too if for some reasons we cannot detect the via?
Elliu commented 2025-08-31 11:13:51 +00:00
Outdated
Copy link

Added the error message asking to manually invite in case if /api/link-space joinRoom fails, similarly to /api/link

Added the error message asking to manually invite in case if /api/link-space joinRoom fails, similarly to /api/link
} catch (e) {
throw createError({status: 403, message: e.errcode, data: `${e.errcode} - ${e.message}`})
}
@ -135,19 +135,31 @@ as.router.post("/api/link", defineEventHandler(async event => {
// Check room is part of the guild's space
let found = false
let via = undefined
for await (const room of api.generateFullHierarchy(spaceID)) {
if (room.room_id === parsedBody.matrix && !room.room_type) {
if (via === undefined && room.room_type === "m.space") {
for (state of room.children_state) {
if (state.state_key === parsedBody.matrix){
via = {via: state.content.via}
if (found === true)
break
}
}
}
if (!found && room.room_id === parsedBody.matrix && !room.room_type) {
found = true
break
if (via !== undefined)
break
}
}
if (!found) throw createError({status: 400, message: "Bad Request", data: "Matrix room needs to be part of the bridged space"})
// Check room exists and bridge is joined
try {
await api.joinRoom(parsedBody.matrix)
await api.joinRoom(parsedBody.matrix, null, via ?? {})
} catch (e) {
throw createError({status: 403, message: e.errcode, data: `${e.errcode} - ${e.message}`})
throw createError({status: 403, message: e.errcode, data: `${e.errcode} - ${e.message}${via === null ? " (hint: couln't find a \"via\" in the space children_state for this room in order to help joining this room)" : ""}`})
cadence marked this conversation as resolved Outdated
Elliu commented 2025-08-22 08:21:07 +00:00
Outdated
Copy link

Not sure if adding this hint is a good idea, or if we should rather put it elsewhere (like in a "hint" member), or if we should not put it at all, or if we should add a "please consider manually inviting the bot user <> in the room"

Not sure if adding this hint is a good idea, or if we should rather put it elsewhere (like in a "hint" member), or if we should not put it at all, or if we should add a "please consider manually inviting the bot user <> in the room"
cadence commented 2025-08-22 09:29:05 +00:00
Outdated
Copy link

I added a separate branch for it to say a different error message when the join fails. I felt the ternary was too complicated.

or if we should add a "please consider manually inviting

I decided to do it this way because it's better to see actionable feedback (do this) rather than merely identifying a problem (it tells you that the via data is wrong, but not what correct data would look like, or how to actually fix it) (I don't know any clients that allow you to edit the via data without devtools)

I added a separate branch for it to say a different error message when the join fails. I felt the ternary was too complicated. > or if we should add a "please consider manually inviting I decided to do it this way because it's better to see actionable feedback (do this) rather than merely identifying a problem (it tells you that the via data is wrong, but not what correct data would look like, or how to actually fix it) (I don't know any clients that allow you to edit the via data without devtools)
}
// Check bridge has PL 100