KvRepository, MemoryRepository, and SqliteRepository share two defects in how they maintain the follower ↔ follow request relationship. PostgresRepository already implements the correct semantics, so the three older backends disagree with it.
1. Undoing one of several follows deletes the follower entirely
removeFollower(identifier, followId, followerId) checks that the given follow request points at the given follower, then deletes both the follow request and the follower record unconditionally.
If the same actor follows a bot twice under different Follow IDs (remote servers can produce this through re-follow flows or duplicate deliveries), the two follow requests share one follower record. When an Undo for just one of them arrives, the follower record is deleted even though the other follow is still in effect: the actor disappears from the followers collection and hasFollower() returns false, while the remote side still considers itself a follower.
2. Reassigning a follow request leaves the old follower orphaned
addFollower(identifier, followId, follower) overwrites an existing follow request entry (INSERT OR REPLACE in SQLite, a plain key overwrite in the KV layout) without cleaning up the follower the request previously pointed at. If a follow request ID is ever re-pointed at a different actor, the previous follower record stays in the followers list forever, since no follow request references it anymore and removeFollower() can no longer reach it.
The reference semantics
PostgresRepository (added in 0.4.0) handles both cases: inside an advisory-lock transaction it deletes the follow request first and removes the follower only when no remaining follow request references it (cleanupFollower), and addFollower() cleans up a previously referenced follower on reassignment. Its test suite covers these scenarios explicitly (e.g. does not leak stale followers during concurrent reassignment cleanup). The fix for this issue is essentially porting those semantics to the other three backends, with regression tests mirroring the PostgreSQL ones.
Notes on the fix
- MemoryRepository and SqliteRepository only need a reference check before deleting the follower (a scan of the in-memory request map; a
SELECT COUNT(*) FROM follow_requests WHERE follower_id = ? in SQLite).
- KvRepository is the hard part: follow requests are keyed by URL and have no index, so they cannot be enumerated through the
KvStore interface. It needs a follower → follow requests reverse index, maintained the same way as the followee index introduced for findFollowedBots(), plus a migration story for existing data.
Impact
Low in practice: in the normal flow an actor has exactly one active follow request, and most servers send an Undo before re-following. The behavior dates back to the pre-PostgreSQL backends, so 0.4.x and earlier are affected as well; whether a fix is worth backporting to 0.4-maintenance can be decided once the fix exists.
Found by a code review during the work on #24; the defects predate that pull request and are orthogonal to it.
KvRepository,MemoryRepository, andSqliteRepositoryshare two defects in how they maintain the follower ↔ follow request relationship.PostgresRepositoryalready implements the correct semantics, so the three older backends disagree with it.1. Undoing one of several follows deletes the follower entirely
removeFollower(identifier, followId, followerId)checks that the given follow request points at the given follower, then deletes both the follow request and the follower record unconditionally.If the same actor follows a bot twice under different
FollowIDs (remote servers can produce this through re-follow flows or duplicate deliveries), the two follow requests share one follower record. When anUndofor just one of them arrives, the follower record is deleted even though the other follow is still in effect: the actor disappears from the followers collection andhasFollower()returnsfalse, while the remote side still considers itself a follower.2. Reassigning a follow request leaves the old follower orphaned
addFollower(identifier, followId, follower)overwrites an existing follow request entry (INSERT OR REPLACEin SQLite, a plain key overwrite in the KV layout) without cleaning up the follower the request previously pointed at. If a follow request ID is ever re-pointed at a different actor, the previous follower record stays in the followers list forever, since no follow request references it anymore andremoveFollower()can no longer reach it.The reference semantics
PostgresRepository(added in 0.4.0) handles both cases: inside an advisory-lock transaction it deletes the follow request first and removes the follower only when no remaining follow request references it (cleanupFollower), andaddFollower()cleans up a previously referenced follower on reassignment. Its test suite covers these scenarios explicitly (e.g. does not leak stale followers during concurrent reassignment cleanup). The fix for this issue is essentially porting those semantics to the other three backends, with regression tests mirroring the PostgreSQL ones.Notes on the fix
SELECT COUNT(*) FROM follow_requests WHERE follower_id = ?in SQLite).KvStoreinterface. It needs a follower → follow requests reverse index, maintained the same way as the followee index introduced forfindFollowedBots(), plus a migration story for existing data.Impact
Low in practice: in the normal flow an actor has exactly one active follow request, and most servers send an
Undobefore re-following. The behavior dates back to the pre-PostgreSQL backends, so 0.4.x and earlier are affected as well; whether a fix is worth backporting to 0.4-maintenance can be decided once the fix exists.Found by a code review during the work on #24; the defects predate that pull request and are orthogonal to it.