[iFixRobots]

Some users still have rooms that the bridge bot is in but that have no portal behind them, left over from an old reset that recreated the whole portal set next to them. Sends there fail with room is not a portal, and clients can open them for a contact instead of the real portal.

This adds an opt-in bridge.cleanup_unknown_rooms that runs once per bridge database. At the end of StartConnectors, before any logins are started, it lists the bridge bot's joined rooms and picks every room that isn't a portal, a user's management room or a login's space. Doing it before logins start matters because portals and spaces get their room created and joined before the room ID is saved. The deletes then run in the background, re-checking the portal table per room. A KV flag is set at the end so it doesn't run again. If the bridge stops mid-way, the next start picks it up again.

Tested on a Beeper staging account with the Dummy bridge: of 3 portals and a login space, only the portal whose database row had been removed was deleted, and a later restart didn't run the cleanup again.

Tag @indent to continue the conversation here.

[indent]

Issues

2 potential issues found:

• The KV flag is set even when some deletes or portal lookups failed, so those rooms are never retried without editing the KV table by hand.
  _{Found by Indent Review Agent}
• The pre-delete re-check covers portals but not a room that just became a user's management room via set-management-room while the cleanup goroutine runs. Such a room is still deleted.
  _{Found by Indent Review Agent}

↑ Select any checkbox above to have Indent auto-fix the issue

2 issues already resolved

• Cleanup deletes any room the bot joined that isn't a portal, management room or space, including user-owned group rooms and extra DMs where users invited the bot. With room yeeting support the room is deleted for all members. (fixed by commit f094cbd)
  _{Found by Indent Review Agent}
• The cleanup runs while logins are still starting. A portal or space room created but not yet saved to the DB can be deleted, because the re-check queries the DB and skips the portal cache. Space rooms get no re-check. (fixed by commit f094cbd)
  _{Found by Indent Review Agent}

CI Checks

All required CI checks passed on f094cbd.

Review agents

Select any unchecked box below to run or rerun that agent.

Found issues (1)

• Indent Review Agent · Only nits left: retry flag, mgmt room re-check

Full results

Indent Review Agent

• Summary: Only nits left: retry flag, mgmt room re-check
• Last ran on commit: f094cbdf
• Latest result

  {
    "summary": "Only nits left: retry flag, mgmt room re-check",
    "findings": [
      {
        "fix": "Only set the flag when `failedCount == 0`. Otherwise leave it unset so the next start retries; rooms that were already deleted won't show up in `joined_rooms` again.",
        "line": 77,
        "path": "bridgev2/unknownrooms.go",
        "issue": "`deleteUnknownRooms` returns early without setting `KeyUnknownRoomsCleanedUp` when the context is cancelled. When the loop finishes, though, it sets the flag to \"true\" even if `failedCount > 0`. A transient homeserver error on `DeleteRoom`, or a DB error on `GetByMXID`, during the single run leaves those rooms in place permanently, and the only way to retry is to edit the KV table by hand.",
        "title": "Cleanup flag is set even when some deletes or portal lookups failed, so failures are never retried",
        "severity": "nit"
      },
      {
        "fix": "In the loop, also skip rooms that are now any user's `ManagementRoom`, for example with a `GetByManagementRoom` query or by rebuilding the known-room set, alongside the existing `Portal.GetByMXID` check.",
        "line": 57,
        "path": "bridgev2/unknownrooms.go",
        "issue": "The unknown-room list is captured in `startUnknownRoomCleanup` after `Matrix.Start`, so Matrix events are already being processed while the deletion goroutine works through the list. If a user runs `set-management-room` in one of the listed rooms during that time (for example an old DM with the bot), the room becomes their management room and is then deleted anyway, because the loop only re-checks `Portal.GetByMXID`. On servers with room yeeting, the room is deleted for everyone.",
        "title": "Re-check before deleting covers portals but not a room that just became a management room",
        "severity": "nit"
      }
    ]
  }

[indent]

⚪️ Nit · Cleanup flag set despite failures

The flag is set even when failedCount > 0, so a transient homeserver or DB error leaves those rooms in place with no retry. Only set it when everything succeeded.

_{Found by Indent Review Agent}

[indent]

⚪️ Nit · Re-check before deleting misses newly set management rooms

The room list is captured after Matrix.Start, so events are processed while the deletion goroutine runs. If a user runs set-management-room in a listed room (e.g. an old DM with the bot), it becomes their management room but is still deleted, since only Portal.GetByMXID is re-checked. With room yeeting it's deleted for everyone.

_{Found by Indent Review Agent}