From c26fb4d326868df1d13c53286f71453783ac8724 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 16:54:29 +0900 Subject: [PATCH 01/21] Fix follower cleanup semantics Preserve follower records while any accepted follow request still points at the actor, and only report an unfollow when the last active request is removed. This keeps repository state and Bot.onUnfollow delivery aligned across the memory, KV, SQLite, and PostgreSQL backends. Also clean up stale followers when a follow request is reassigned, add regression tests for duplicate and reassigned follow requests, and record the change in the changelog. Fixes https://github.com/fedify-dev/botkit/issues/25 Assisted-by: Codex:gpt-5.5 --- CHANGES.md | 23 ++++++ deno.lock | 30 ++++++++ packages/botkit-postgres/src/mod.test.ts | 4 +- packages/botkit-postgres/src/mod.ts | 12 ++-- packages/botkit-sqlite/src/mod.test.ts | 89 ++++++++++++++++++++++++ packages/botkit-sqlite/src/mod.ts | 50 ++++++++++--- packages/botkit/src/bot-impl.test.ts | 47 +++++++++++++ packages/botkit/src/repository.test.ts | 79 +++++++++++++++++++++ packages/botkit/src/repository.ts | 84 ++++++++++++++++------ 9 files changed, 380 insertions(+), 38 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c0a64fe..9b39a69 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,6 +17,29 @@ Released on June 4, 2026. [CVE-2026-50131]: https://github.com/fedify-dev/fedify/security/advisories/GHSA-xw9q-2mv6-9fr8 +### @fedify/botkit + + - Fixed `MemoryRepository`, `KvRepository`, and `MemoryCachedRepository` so + removing one of multiple active follow requests for the same actor no + longer deletes the follower too early or fires a premature unfollow event, + and reassigning a follow request no longer leaves stale followers behind. + [[#25], [#26]] + +[#25]: https://github.com/fedify-dev/botkit/issues/25 +[#26]: https://github.com/fedify-dev/botkit/pull/26 + +### @fedify/botkit-sqlite + + - Fixed `SqliteRepository` so removing one of multiple active follow requests + for the same actor no longer fails with a foreign key error, and reassigning + a follow request no longer leaves stale followers behind. [[#25]] + +### @fedify/botkit-postgres + + - Fixed `PostgresRepository` so removing one of multiple active follow + requests for the same actor no longer reports a follower removal until the + last active follow request is removed. [[#25]] + Version 0.4.2 ------------- diff --git a/deno.lock b/deno.lock index d92b3c8..fbcd638 100644 --- a/deno.lock +++ b/deno.lock @@ -7,11 +7,15 @@ "jsr:@fedify/markdown-it-hashtag@0.3": "0.3.0", "jsr:@fedify/markdown-it-mention@0.3": "0.3.0", "jsr:@fedify/vocab-runtime@^2.1.15": "2.2.4", + "jsr:@fedify/vocab-runtime@^2.1.17": "2.2.4", "jsr:@fedify/vocab-runtime@^2.1.2": "2.2.4", "jsr:@fedify/vocab-runtime@^2.2.4": "2.2.4", + "jsr:@fedify/vocab-runtime@~2.1.15": "2.1.17", "jsr:@fedify/vocab@^2.1.15": "2.2.4", "jsr:@fedify/vocab@^2.1.2": "2.2.4", + "jsr:@fedify/vocab@~2.1.15": "2.1.17", "jsr:@fedify/webfinger@^2.1.15": "2.2.4", + "jsr:@fedify/webfinger@^2.1.17": "2.2.4", "jsr:@fedify/webfinger@^2.2.4": "2.2.4", "jsr:@hongminhee/x-forwarded-fetch@0.2": "0.2.0", "jsr:@hono/hono@^4.8.2": "4.12.23", @@ -32,6 +36,8 @@ "npm:@logtape/logtape@^1.3.5": "1.3.7", "npm:@multiformats/base-x@^4.0.1": "4.0.1", "npm:@opentelemetry/api@^1.9.0": "1.9.1", + "npm:@opentelemetry/core@^2.5.0": "2.6.1_@opentelemetry+api@1.9.1", + "npm:@opentelemetry/sdk-trace-base@^2.5.0": "2.6.1_@opentelemetry+api@1.9.1", "npm:@opentelemetry/semantic-conventions@^1.39.0": "1.40.0", "npm:@types/markdown-it@^14.1.1": "14.1.2", "npm:asn1js@^3.0.6": "3.0.7", @@ -81,6 +87,8 @@ "jsr:@fedify/webfinger@^2.1.15", "jsr:@logtape/logtape@^2.0.5", "npm:@opentelemetry/api", + "npm:@opentelemetry/core", + "npm:@opentelemetry/sdk-trace-base", "npm:@opentelemetry/semantic-conventions", "npm:byte-encodings", "npm:es-toolkit@^1.43.0", @@ -104,6 +112,16 @@ "npm:@types/markdown-it" ] }, + "@fedify/vocab@2.1.17": { + "integrity": "4202c6aa2f76679ee6da5ad97db436ab7692f15a2ef42f6e1ff02f2eba2d983a", + "dependencies": [ + "jsr:@fedify/vocab-runtime@^2.1.17", + "jsr:@fedify/webfinger@^2.1.17", + "jsr:@logtape/logtape@^2.0.5", + "npm:@opentelemetry/api", + "npm:es-toolkit@^1.43.0" + ] + }, "@fedify/vocab@2.2.4": { "integrity": "de2fa46b141e2cc94c6ddd066826fcdbfe1b0831c8535e994601813d6d00d9cc", "dependencies": [ @@ -114,6 +132,18 @@ "npm:es-toolkit@^1.43.0" ] }, + "@fedify/vocab-runtime@2.1.17": { + "integrity": "6fc564fe42746a7cae6e985b7fc1d0b8743abda4d7e22f18b9895ddb2335e0f2", + "dependencies": [ + "jsr:@logtape/logtape@^2.0.5", + "npm:@multiformats/base-x", + "npm:@opentelemetry/api", + "npm:asn1js", + "npm:byte-encodings", + "npm:jsonld", + "npm:pkijs" + ] + }, "@fedify/vocab-runtime@2.2.4": { "integrity": "0bc68243d7cc455459435743f73f4d9876cb006a5690f1b9542b74cb503f2226", "dependencies": [ diff --git a/packages/botkit-postgres/src/mod.test.ts b/packages/botkit-postgres/src/mod.test.ts index 3922974..291060e 100644 --- a/packages/botkit-postgres/src/mod.test.ts +++ b/packages/botkit-postgres/src/mod.test.ts @@ -872,8 +872,8 @@ if (postgresUrl == null) { assert.deepStrictEqual(await repo.countFollowers(), 2); assert.ok(await repo.hasFollower(followerA.id!)); assert.deepStrictEqual( - await (await repo.removeFollower(followA, followerA.id!))?.toJsonLd(), - await followerA.toJsonLd(), + await repo.removeFollower(followA, followerA.id!), + undefined, ); assert.ok(await repo.hasFollower(followerA.id!)); assert.deepStrictEqual(await repo.countFollowers(), 2); diff --git a/packages/botkit-postgres/src/mod.ts b/packages/botkit-postgres/src/mod.ts index 7006ecc..c7b4fa5 100644 --- a/packages/botkit-postgres/src/mod.ts +++ b/packages/botkit-postgres/src/mod.ts @@ -539,8 +539,8 @@ export class PostgresRepository implements Repository, AsyncDisposable { WHERE follow_request_id = $1`, [followId.href], ); - await this.cleanupFollower(sql, followerId.href); - return await parseActor(row.actor_json); + const removed = await this.cleanupFollower(sql, followerId.href); + return removed ? await parseActor(row.actor_json) : undefined; }); } @@ -758,9 +758,9 @@ export class PostgresRepository implements Repository, AsyncDisposable { private async cleanupFollower( sql: Queryable, followerId: string, - ): Promise { + ): Promise { await this.lockFollower(sql, followerId); - await this.query( + const rows = await this.query<{ readonly follower_id: string }>( sql, `DELETE FROM ${this.table("followers")} WHERE follower_id = $1 @@ -768,9 +768,11 @@ export class PostgresRepository implements Repository, AsyncDisposable { SELECT 1 FROM ${this.table("follow_requests")} WHERE follower_id = $1 - )`, + ) + RETURNING follower_id`, [followerId], ); + return rows.length > 0; } private async ensureReady(): Promise { diff --git a/packages/botkit-sqlite/src/mod.test.ts b/packages/botkit-sqlite/src/mod.test.ts index c18b5e6..98cf1cd 100644 --- a/packages/botkit-sqlite/src/mod.test.ts +++ b/packages/botkit-sqlite/src/mod.test.ts @@ -167,6 +167,95 @@ describe("SqliteRepository", () => { } }); + test("followers with multiple follow requests", async () => { + const repo = createSqliteRepository(); + try { + const follower = new Person({ + id: new URL("https://example.com/ap/actor/alice"), + preferredUsername: "alice", + }); + const followA = new URL( + "https://example.com/ap/follow/f2fb7255-d3ad-4fef-8f9a-1d0f2c2ec0b4", + ); + const followB = new URL( + "https://example.com/ap/follow/a3d4cc4f-af93-4a9f-a7b3-0b7c0fe4901d", + ); + + await repo.addFollower(followA, follower); + await repo.addFollower(followB, follower); + assert.deepStrictEqual(await repo.countFollowers(), 1); + assert.ok(await repo.hasFollower(follower.id!)); + + assert.deepStrictEqual( + await repo.removeFollower(followA, follower.id!), + undefined, + ); + assert.deepStrictEqual(await repo.countFollowers(), 1); + assert.ok(await repo.hasFollower(follower.id!)); + + assert.deepStrictEqual( + await (await repo.removeFollower(followB, follower.id!))?.toJsonLd(), + await follower.toJsonLd(), + ); + assert.deepStrictEqual(await repo.countFollowers(), 0); + assert.deepStrictEqual(await repo.hasFollower(follower.id!), false); + } finally { + repo.close(); + } + }); + + test("followers with reassigned follow requests", async () => { + const repo = createSqliteRepository(); + try { + const oldFollower = new Person({ + id: new URL("https://example.com/ap/actor/alice"), + preferredUsername: "alice", + }); + const newFollower = new Person({ + id: new URL("https://example.com/ap/actor/bob"), + preferredUsername: "bob", + }); + const followA = new URL( + "https://example.com/ap/follow/f2fb7255-d3ad-4fef-8f9a-1d0f2c2ec0b4", + ); + const followB = new URL( + "https://example.com/ap/follow/a3d4cc4f-af93-4a9f-a7b3-0b7c0fe4901d", + ); + + await repo.addFollower(followA, oldFollower); + await repo.addFollower(followB, oldFollower); + await repo.addFollower(followA, newFollower); + + assert.deepStrictEqual(await repo.countFollowers(), 2); + assert.ok(await repo.hasFollower(oldFollower.id!)); + assert.ok(await repo.hasFollower(newFollower.id!)); + + assert.deepStrictEqual( + await (await repo.removeFollower(followB, oldFollower.id!)) + ?.toJsonLd(), + await oldFollower.toJsonLd(), + ); + assert.deepStrictEqual(await repo.countFollowers(), 1); + assert.deepStrictEqual(await repo.hasFollower(oldFollower.id!), false); + assert.ok(await repo.hasFollower(newFollower.id!)); + + await repo.addFollower(followA, oldFollower); + assert.deepStrictEqual(await repo.countFollowers(), 1); + assert.ok(await repo.hasFollower(oldFollower.id!)); + assert.deepStrictEqual(await repo.hasFollower(newFollower.id!), false); + assert.deepStrictEqual( + await Promise.all( + (await Array.fromAsync(repo.getFollowers())).map((follower) => + follower.toJsonLd() + ), + ), + [await oldFollower.toJsonLd()], + ); + } finally { + repo.close(); + } + }); + test("poll voting", async () => { const repo = createSqliteRepository(); try { diff --git a/packages/botkit-sqlite/src/mod.ts b/packages/botkit-sqlite/src/mod.ts index 8423c18..6e5fb13 100644 --- a/packages/botkit-sqlite/src/mod.ts +++ b/packages/botkit-sqlite/src/mod.ts @@ -130,6 +130,11 @@ export class SqliteRepository implements Repository, Disposable { ) `); + this.db.exec(` + CREATE INDEX IF NOT EXISTS idx_follow_requests_follower + ON follow_requests(follower_id) + `); + // Sent follows table this.db.exec(` CREATE TABLE IF NOT EXISTS sent_follows ( @@ -374,19 +379,35 @@ export class SqliteRepository implements Repository, Disposable { ); const insertFollowerStmt = this.db.prepare(` - INSERT OR REPLACE INTO followers (follower_id, actor_json) + INSERT INTO followers (follower_id, actor_json) VALUES (?, ?) + ON CONFLICT(follower_id) + DO UPDATE SET actor_json = excluded.actor_json `); const insertRequestStmt = this.db.prepare(` - INSERT OR REPLACE INTO follow_requests (follow_request_id, follower_id) + INSERT INTO follow_requests (follow_request_id, follower_id) VALUES (?, ?) + ON CONFLICT(follow_request_id) + DO UPDATE SET follower_id = excluded.follower_id + `); + + const previousFollowerStmt = this.db.prepare(` + SELECT follower_id FROM follow_requests WHERE follow_request_id = ? `); this.db.exec("BEGIN TRANSACTION"); try { + const previousRow = previousFollowerStmt.get(followRequestId.href) as + | { follower_id: string } + | undefined; insertFollowerStmt.run(follower.id.href, followerJson); insertRequestStmt.run(followRequestId.href, follower.id.href); + if ( + previousRow != null && previousRow.follower_id !== follower.id.href + ) { + this.cleanupFollower(previousRow.follower_id); + } this.db.exec("COMMIT"); } catch (error) { this.db.exec("ROLLBACK"); @@ -413,25 +434,23 @@ export class SqliteRepository implements Repository, Disposable { if (!row) return undefined; - // Remove the follower and follow request const deleteRequestStmt = this.db.prepare(` DELETE FROM follow_requests WHERE follow_request_id = ? `); - const deleteFollowerStmt = this.db.prepare(` - DELETE FROM followers WHERE follower_id = ? - `); - + let removed = false; this.db.exec("BEGIN TRANSACTION"); try { deleteRequestStmt.run(followRequestId.href); - deleteFollowerStmt.run(actorId.href); + removed = this.cleanupFollower(actorId.href); this.db.exec("COMMIT"); } catch (error) { this.db.exec("ROLLBACK"); throw error; } + if (!removed) return undefined; + try { const actorData = JSON.parse(row.actor_json); const actor = await Object.fromJsonLd(actorData); @@ -446,6 +465,21 @@ export class SqliteRepository implements Repository, Disposable { return undefined; } + private cleanupFollower(followerId: string): boolean { + const deleteFollowerStmt = this.db.prepare(` + DELETE FROM followers + WHERE follower_id = ? + AND NOT EXISTS ( + SELECT 1 FROM follow_requests WHERE follower_id = ? + ) + `); + + const result = deleteFollowerStmt.run(followerId, followerId) as { + readonly changes: number; + }; + return result.changes > 0; + } + hasFollower(followerId: URL): Promise { const stmt = this.db.prepare(` SELECT 1 FROM followers WHERE follower_id = ? diff --git a/packages/botkit/src/bot-impl.test.ts b/packages/botkit/src/bot-impl.test.ts index 752abb3..cc4aef1 100644 --- a/packages/botkit/src/bot-impl.test.ts +++ b/packages/botkit/src/bot-impl.test.ts @@ -1482,6 +1482,53 @@ test("BotImpl.onUnfollowed()", async (t) => { }); }); +test("BotImpl.onUnfollowed() with multiple follow requests", async () => { + const repository = new MemoryRepository(); + const bot = new BotImpl({ + kv: new MemoryKvStore(), + repository, + username: "bot", + }); + const unfollowed: [Session, Actor][] = []; + bot.onUnfollow = (session, actor) => void (unfollowed.push([session, actor])); + const ctx = createMockInboxContext(bot, "https://example.com", "bot"); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/john"), + preferredUsername: "john", + }); + const followA = new URL("https://example.com/ap/actor/john/follows/bot"); + const followB = new URL( + "https://example.com/ap/actor/john/follows/bot-again", + ); + + await repository.addFollower(followA, follower); + await repository.addFollower(followB, follower); + + await bot.onUnfollowed( + ctx, + new Undo({ + actor: follower.id, + object: new Follow({ id: followA }), + }), + ); + assert.deepStrictEqual(await repository.countFollowers(), 1); + assert.ok(await repository.hasFollower(follower.id!)); + assert.deepStrictEqual(unfollowed, []); + + await bot.onUnfollowed( + ctx, + new Undo({ + actor: follower.id, + object: new Follow({ id: followB }), + }), + ); + assert.deepStrictEqual(await repository.countFollowers(), 0); + assert.deepStrictEqual(unfollowed.length, 1); + const [session, actor] = unfollowed[0] as [Session, Actor]; + assert.deepStrictEqual(session.bot, bot); + assert.deepStrictEqual(actor.id, follower.id); +}); + test("BotImpl.onFollowAccepted()", async (t) => { const repository = new MemoryRepository(); const bot = new BotImpl({ diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 0376526..8d04622 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -634,6 +634,85 @@ for (const name in factories) { assert.deepStrictEqual(await repo.hasFollower(followerB.id!), false); }); + test("followers with multiple follow requests", async () => { + const follower = new Person({ + id: new URL("https://example.com/ap/actor/alice"), + preferredUsername: "alice", + }); + const followA = new URL( + "https://example.com/ap/follow/f2fb7255-d3ad-4fef-8f9a-1d0f2c2ec0b4", + ); + const followB = new URL( + "https://example.com/ap/follow/a3d4cc4f-af93-4a9f-a7b3-0b7c0fe4901d", + ); + + await repo.addFollower(followA, follower); + await repo.addFollower(followB, follower); + assert.deepStrictEqual(await repo.countFollowers(), 1); + assert.ok(await repo.hasFollower(follower.id!)); + + assert.deepStrictEqual( + await repo.removeFollower(followA, follower.id!), + undefined, + ); + assert.deepStrictEqual(await repo.countFollowers(), 1); + assert.ok(await repo.hasFollower(follower.id!)); + + assert.deepStrictEqual( + await (await repo.removeFollower(followB, follower.id!))?.toJsonLd(), + await follower.toJsonLd(), + ); + assert.deepStrictEqual(await repo.countFollowers(), 0); + assert.deepStrictEqual(await repo.hasFollower(follower.id!), false); + }); + + test("followers with reassigned follow requests", async () => { + const oldFollower = new Person({ + id: new URL("https://example.com/ap/actor/alice"), + preferredUsername: "alice", + }); + const newFollower = new Person({ + id: new URL("https://example.com/ap/actor/bob"), + preferredUsername: "bob", + }); + const followA = new URL( + "https://example.com/ap/follow/f2fb7255-d3ad-4fef-8f9a-1d0f2c2ec0b4", + ); + const followB = new URL( + "https://example.com/ap/follow/a3d4cc4f-af93-4a9f-a7b3-0b7c0fe4901d", + ); + + await repo.addFollower(followA, oldFollower); + await repo.addFollower(followB, oldFollower); + await repo.addFollower(followA, newFollower); + + assert.deepStrictEqual(await repo.countFollowers(), 2); + assert.ok(await repo.hasFollower(oldFollower.id!)); + assert.ok(await repo.hasFollower(newFollower.id!)); + + assert.deepStrictEqual( + await (await repo.removeFollower(followB, oldFollower.id!)) + ?.toJsonLd(), + await oldFollower.toJsonLd(), + ); + assert.deepStrictEqual(await repo.countFollowers(), 1); + assert.deepStrictEqual(await repo.hasFollower(oldFollower.id!), false); + assert.ok(await repo.hasFollower(newFollower.id!)); + + await repo.addFollower(followA, oldFollower); + assert.deepStrictEqual(await repo.countFollowers(), 1); + assert.ok(await repo.hasFollower(oldFollower.id!)); + assert.deepStrictEqual(await repo.hasFollower(newFollower.id!), false); + assert.deepStrictEqual( + await Promise.all( + (await Array.fromAsync(repo.getFollowers())).map((follower) => + follower.toJsonLd() + ), + ), + [await oldFollower.toJsonLd()], + ); + }); + test("sent follows", async () => { const follow = new Follow({ id: new URL( diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 2b9a050..b8774a6 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -123,8 +123,9 @@ export interface Repository { * Removes a follower from the repository. * @param followId The URL of the follow request. * @param followerId The ID of the actor to remove. - * @returns The removed actor. If the follower does not exist or the follow - * request is not about the follower, `undefined` will be returned. + * @returns The removed actor. If the follower does not exist, the follow + * request is not about the follower, or the actor still has another + * active follow request, `undefined` will be returned. */ removeFollower(followId: URL, followerId: URL): Promise; @@ -511,24 +512,22 @@ export class KvRepository implements Repository { if (follower.id == null) { throw new TypeError("The follower ID is missing."); } + const followerId = follower.id.href; + const followRequestKey: KvKey = [ + ...this.prefixes.followRequests, + followRequestId.href, + ]; + const previousFollowerId = await this.kv.get(followRequestKey); const followerKey: KvKey = [...this.prefixes.followers, follower.id.href]; await this.kv.set( followerKey, await follower.toJsonLd({ format: "compact" }), ); - const lockKey: KvKey = [...this.prefixes.followers, "lock"]; - const listKey: KvKey = this.prefixes.followers; - do { - await this.kv.set(lockKey, follower.id.href); - const list = await this.kv.get(listKey) ?? []; - if (!list.includes(follower.id.href)) list.push(follower.id.href); - await this.kv.set(listKey, list); - } while (await this.kv.get(lockKey) !== follower.id.href); - const followRequestKey: KvKey = [ - ...this.prefixes.followRequests, - followRequestId.href, - ]; - await this.kv.set(followRequestKey, follower.id.href); + await this.addFollowerId(followerId); + await this.kv.set(followRequestKey, followerId); + if (previousFollowerId != null && previousFollowerId !== followerId) { + await this.cleanupFollower(previousFollowerId); + } } async removeFollower( @@ -552,6 +551,23 @@ export class KvRepository implements Repository { return undefined; } if (!isActor(follower)) return undefined; + await this.kv.delete(followRequestKey); + return await this.cleanupFollower(followerId) ? follower : undefined; + } + + private async addFollowerId(followerId: string): Promise { + const lockKey: KvKey = [...this.prefixes.followers, "lock"]; + const listKey: KvKey = this.prefixes.followers; + do { + await this.kv.set(lockKey, followerId); + const list = await this.kv.get(listKey) ?? []; + if (!list.includes(followerId)) list.push(followerId); + await this.kv.set(listKey, list); + } while (await this.kv.get(lockKey) !== followerId); + } + + private async cleanupFollower(followerId: string): Promise { + if (await this.hasFollowRequestForFollower(followerId)) return false; const lockKey: KvKey = [...this.prefixes.followers, "lock"]; const listKey: KvKey = this.prefixes.followers; do { @@ -560,9 +576,19 @@ export class KvRepository implements Repository { list = list.filter((id) => id !== followerId); await this.kv.set(listKey, list); } while (await this.kv.get(lockKey) !== followerId); - await this.kv.delete(followerKey); - await this.kv.delete(followRequestKey); - return follower; + await this.kv.delete([...this.prefixes.followers, followerId]); + return true; + } + + private async hasFollowRequestForFollower( + followerId: string, + ): Promise { + const followRequestKeyLength = this.prefixes.followRequests.length + 1; + for await (const entry of this.kv.list(this.prefixes.followRequests)) { + if (entry.key.length !== followRequestKeyLength) continue; + if (entry.value === followerId) return true; + } + return false; } async hasFollower(followerId: URL): Promise { @@ -851,8 +877,14 @@ export class MemoryRepository implements Repository { if (follower.id == null) { throw new TypeError("The follower ID is missing."); } + const previousFollowerId = this.followRequests[followId.href]; this.followers.set(follower.id.href, follower); this.followRequests[followId.href] = follower.id.href; + if ( + previousFollowerId != null && previousFollowerId !== follower.id.href + ) { + this.cleanupFollower(previousFollowerId); + } return Promise.resolve(); } @@ -863,8 +895,16 @@ export class MemoryRepository implements Repository { } delete this.followRequests[followId.href]; const follower = this.followers.get(followerId.href); - this.followers.delete(followerId.href); - return Promise.resolve(follower); + const removed = this.cleanupFollower(followerId.href); + return Promise.resolve(removed ? follower : undefined); + } + + private cleanupFollower(followerId: string): boolean { + if (globalThis.Object.values(this.followRequests).includes(followerId)) { + return false; + } + this.followers.delete(followerId); + return true; } hasFollower(followerId: URL): Promise { @@ -1060,9 +1100,7 @@ export class MemoryCachedRepository implements Repository { followId, followerId, ); - if (removedFollower !== undefined) { - await this.cache.removeFollower(followId, followerId); - } + await this.cache.removeFollower(followId, followerId); return removedFollower; } From 7ac3462076b96faf562f5f9ae65cc462c284986b Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 20:44:18 +0900 Subject: [PATCH 02/21] Close follower cleanup races Move follower removal checks into the protected update path so stale follow request state cannot delete or report the wrong follower. SQLite now validates and deletes by both follow_request_id and follower_id in one transaction, while the KV repository serializes updates per follow request and revalidates follower state before cleanup. https://github.com/fedify-dev/botkit/pull/26#pullrequestreview-4631025727 Assisted-by: Codex:gpt-5.5 --- packages/botkit-sqlite/src/mod.ts | 34 +++-- packages/botkit/src/repository.ts | 244 +++++++++++++++++++++++------- 2 files changed, 212 insertions(+), 66 deletions(-) diff --git a/packages/botkit-sqlite/src/mod.ts b/packages/botkit-sqlite/src/mod.ts index 6e5fb13..c239209 100644 --- a/packages/botkit-sqlite/src/mod.ts +++ b/packages/botkit-sqlite/src/mod.ts @@ -419,29 +419,37 @@ export class SqliteRepository implements Repository, Disposable { followRequestId: URL, actorId: URL, ): Promise { - // Check if the follow request exists and matches the actor const checkStmt = this.db.prepare(` - SELECT fr.follower_id, f.actor_json - FROM follow_requests fr - JOIN followers f ON fr.follower_id = f.follower_id + SELECT fr.follower_id, f.actor_json + FROM follow_requests fr + JOIN followers f ON fr.follower_id = f.follower_id WHERE fr.follow_request_id = ? AND fr.follower_id = ? `); - const row = checkStmt.get(followRequestId.href, actorId.href) as { - follower_id: string; - actor_json: string; - } | undefined; - - if (!row) return undefined; - const deleteRequestStmt = this.db.prepare(` - DELETE FROM follow_requests WHERE follow_request_id = ? + DELETE FROM follow_requests + WHERE follow_request_id = ? AND follower_id = ? `); + let row: { follower_id: string; actor_json: string } | undefined; let removed = false; this.db.exec("BEGIN TRANSACTION"); try { - deleteRequestStmt.run(followRequestId.href); + row = checkStmt.get(followRequestId.href, actorId.href) as + | { follower_id: string; actor_json: string } + | undefined; + if (row == null) { + this.db.exec("COMMIT"); + return undefined; + } + const deleteResult = deleteRequestStmt.run( + followRequestId.href, + actorId.href, + ) as { readonly changes: number }; + if (deleteResult.changes < 1) { + this.db.exec("COMMIT"); + return undefined; + } removed = this.cleanupFollower(actorId.href); this.db.exec("COMMIT"); } catch (error) { diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index b8774a6..ce4e0dc 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -512,83 +512,221 @@ export class KvRepository implements Repository { if (follower.id == null) { throw new TypeError("The follower ID is missing."); } + const followRequestIdString = followRequestId.href; const followerId = follower.id.href; - const followRequestKey: KvKey = [ - ...this.prefixes.followRequests, - followRequestId.href, - ]; - const previousFollowerId = await this.kv.get(followRequestKey); - const followerKey: KvKey = [...this.prefixes.followers, follower.id.href]; - await this.kv.set( - followerKey, - await follower.toJsonLd({ format: "compact" }), + await this.withKvLock( + this.getFollowRequestLockKey(followRequestIdString), + async () => { + const followRequestKey = this.getFollowRequestKey( + followRequestIdString, + ); + const previousFollowerId = await this.kv.get(followRequestKey); + const followerJson = await follower.toJsonLd({ format: "compact" }); + await this.withKvLock(this.getFollowersLockKey(), async () => { + const followerKey: KvKey = [...this.prefixes.followers, followerId]; + await this.kv.set(followerKey, followerJson); + await this.addFollowerIdLocked(followerId); + await this.kv.set(followRequestKey, followerId); + await this.addFollowRequestForFollowerLocked( + followerId, + followRequestIdString, + ); + }); + if ( + previousFollowerId != null && previousFollowerId !== followerId + ) { + await this.withKvLock(this.getFollowersLockKey(), async () => { + await this.removeFollowRequestForFollowerLocked( + previousFollowerId, + followRequestIdString, + ); + await this.cleanupFollowerLocked(previousFollowerId); + }); + } + }, ); - await this.addFollowerId(followerId); - await this.kv.set(followRequestKey, followerId); - if (previousFollowerId != null && previousFollowerId !== followerId) { - await this.cleanupFollower(previousFollowerId); - } } async removeFollower( followRequestId: URL, actorId: URL, ): Promise { - const followRequestKey: KvKey = [ - ...this.prefixes.followRequests, - followRequestId.href, - ]; - const followerId = await this.kv.get(followRequestKey); - if (followerId == null) return undefined; - const followerKey: KvKey = [...this.prefixes.followers, followerId]; - if (followerId !== actorId.href) return undefined; - const followerJson = await this.kv.get(followerKey); - if (followerJson == null) return undefined; - let follower: Object; - try { - follower = await Object.fromJsonLd(followerJson); - } catch { - return undefined; - } - if (!isActor(follower)) return undefined; - await this.kv.delete(followRequestKey); - return await this.cleanupFollower(followerId) ? follower : undefined; + const followRequestIdString = followRequestId.href; + return await this.withKvLock( + this.getFollowRequestLockKey(followRequestIdString), + async () => { + const followRequestKey = this.getFollowRequestKey( + followRequestIdString, + ); + const followerId = await this.kv.get(followRequestKey); + if (followerId == null || followerId !== actorId.href) { + return undefined; + } + return await this.withKvLock(this.getFollowersLockKey(), async () => { + const currentFollowerId = await this.kv.get(followRequestKey); + if (currentFollowerId !== followerId) return undefined; + const followerKey: KvKey = [...this.prefixes.followers, followerId]; + const followerJson = await this.kv.get(followerKey); + if (followerJson == null) return undefined; + let follower: Object; + try { + follower = await Object.fromJsonLd(followerJson); + } catch { + return undefined; + } + if (!isActor(follower)) return undefined; + await this.kv.delete(followRequestKey); + await this.removeFollowRequestForFollowerLocked( + followerId, + followRequestIdString, + ); + return await this.cleanupFollowerLocked(followerId) + ? follower + : undefined; + }); + }, + ); } - private async addFollowerId(followerId: string): Promise { - const lockKey: KvKey = [...this.prefixes.followers, "lock"]; + private async addFollowerIdLocked(followerId: string): Promise { const listKey: KvKey = this.prefixes.followers; - do { - await this.kv.set(lockKey, followerId); - const list = await this.kv.get(listKey) ?? []; - if (!list.includes(followerId)) list.push(followerId); - await this.kv.set(listKey, list); - } while (await this.kv.get(lockKey) !== followerId); + const list = await this.kv.get(listKey) ?? []; + if (!list.includes(followerId)) { + await this.kv.set(listKey, [...list, followerId]); + } } - private async cleanupFollower(followerId: string): Promise { - if (await this.hasFollowRequestForFollower(followerId)) return false; - const lockKey: KvKey = [...this.prefixes.followers, "lock"]; + private async cleanupFollowerLocked(followerId: string): Promise { + if (await this.hasFollowRequestForFollowerLocked(followerId)) { + return false; + } const listKey: KvKey = this.prefixes.followers; - do { - await this.kv.set(lockKey, followerId); - let list = await this.kv.get(listKey) ?? []; - list = list.filter((id) => id !== followerId); - await this.kv.set(listKey, list); - } while (await this.kv.get(lockKey) !== followerId); + const list = await this.kv.get(listKey) ?? []; + await this.kv.set( + listKey, + list.filter((id) => id !== followerId), + ); await this.kv.delete([...this.prefixes.followers, followerId]); return true; } - private async hasFollowRequestForFollower( + private getFollowRequestKey(followRequestId: string): KvKey { + return [...this.prefixes.followRequests, followRequestId]; + } + + private getFollowRequestLockKey(followRequestId: string): KvKey { + return [...this.prefixes.followRequests, "lock", followRequestId]; + } + + private getFollowerFollowRequestsKey(followerId: string): KvKey { + return [...this.prefixes.followRequests, "followers", followerId]; + } + + private getFollowersLockKey(): KvKey { + return [...this.prefixes.followers, "lock"]; + } + + private async addFollowRequestForFollowerLocked( + followerId: string, + followRequestId: string, + ): Promise { + const followRequestIds = await this.getFollowRequestsForFollowerLocked( + followerId, + ); + if (!followRequestIds.includes(followRequestId)) { + await this.kv.set(this.getFollowerFollowRequestsKey(followerId), [ + ...followRequestIds, + followRequestId, + ]); + } + } + + private async removeFollowRequestForFollowerLocked( + followerId: string, + followRequestId: string, + ): Promise { + const followRequestIds = await this.getFollowRequestsForFollowerLocked( + followerId, + ); + await this.kv.set( + this.getFollowerFollowRequestsKey(followerId), + followRequestIds.filter((id) => id !== followRequestId), + ); + } + + private async hasFollowRequestForFollowerLocked( followerId: string, ): Promise { + const followRequestIds = await this.getFollowRequestsForFollowerLocked( + followerId, + ); + const currentFollowRequestIds: string[] = []; + for (const followRequestId of followRequestIds) { + if ( + await this.kv.get(this.getFollowRequestKey(followRequestId)) === + followerId + ) { + currentFollowRequestIds.push(followRequestId); + } + } + if (currentFollowRequestIds.length !== followRequestIds.length) { + await this.kv.set( + this.getFollowerFollowRequestsKey(followerId), + currentFollowRequestIds, + ); + } + return currentFollowRequestIds.length > 0; + } + + private async getFollowRequestsForFollowerLocked( + followerId: string, + ): Promise { + const indexKey = this.getFollowerFollowRequestsKey(followerId); + const followRequestIds = await this.kv.get(indexKey); + if (followRequestIds != null) return followRequestIds; + + const indexedFollowRequestIds: string[] = []; const followRequestKeyLength = this.prefixes.followRequests.length + 1; for await (const entry of this.kv.list(this.prefixes.followRequests)) { if (entry.key.length !== followRequestKeyLength) continue; - if (entry.value === followerId) return true; + const followRequestId = entry.key[this.prefixes.followRequests.length]; + if (typeof followRequestId !== "string") continue; + if (entry.value === followerId) { + indexedFollowRequestIds.push(followRequestId); + } + } + await this.kv.set(indexKey, indexedFollowRequestIds); + return indexedFollowRequestIds; + } + + private async withKvLock( + lockKey: KvKey, + operation: () => Promise, + ): Promise { + const lockId = crypto.randomUUID(); + if (this.kv.cas == null) { + while (true) { + await this.kv.set(lockKey, lockId); + if (await this.kv.get(lockKey) !== lockId) continue; + try { + return await operation(); + } finally { + if (await this.kv.get(lockKey) === lockId) { + await this.kv.delete(lockKey); + } + } + } + } + while (!await this.kv.cas(lockKey, undefined, lockId)) { + await new Promise((resolve) => setTimeout(resolve, 0)); + } + try { + return await operation(); + } finally { + if (await this.kv.get(lockKey) === lockId) { + await this.kv.delete(lockKey); + } } - return false; } async hasFollower(followerId: URL): Promise { From f333e9bf13fc51130cd772f1e7c8adcabbd53435 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 21:09:09 +0900 Subject: [PATCH 03/21] Use immediate SQLite transactions Use immediate transactions for follower add and remove paths so SQLite reserves the write lock before reading rows that the same transaction will update. This avoids deferred transaction upgrades failing with SQLITE_BUSY under concurrent writers. https://github.com/fedify-dev/botkit/pull/26#discussion_r3524575794 Assisted-by: Codex:gpt-5.5 --- packages/botkit-sqlite/src/mod.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/botkit-sqlite/src/mod.ts b/packages/botkit-sqlite/src/mod.ts index c239209..144f996 100644 --- a/packages/botkit-sqlite/src/mod.ts +++ b/packages/botkit-sqlite/src/mod.ts @@ -396,7 +396,7 @@ export class SqliteRepository implements Repository, Disposable { SELECT follower_id FROM follow_requests WHERE follow_request_id = ? `); - this.db.exec("BEGIN TRANSACTION"); + this.db.exec("BEGIN IMMEDIATE TRANSACTION"); try { const previousRow = previousFollowerStmt.get(followRequestId.href) as | { follower_id: string } @@ -433,7 +433,7 @@ export class SqliteRepository implements Repository, Disposable { let row: { follower_id: string; actor_json: string } | undefined; let removed = false; - this.db.exec("BEGIN TRANSACTION"); + this.db.exec("BEGIN IMMEDIATE TRANSACTION"); try { row = checkStmt.get(followRequestId.href, actorId.href) as | { follower_id: string; actor_json: string } From 9f816eace0e49563f53c1539175813397e1717ea Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 21:09:15 +0900 Subject: [PATCH 04/21] Move cleanup notes to 0.4.4 Place the follower cleanup changelog entries under the unreleased 0.4.4 section instead of the already released 0.4.3 section. This keeps the release history intact and leaves the upcoming release notes populated. https://github.com/fedify-dev/botkit/pull/26#discussion_r3524580405 Assisted-by: Codex:gpt-5.5 --- CHANGES.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 9b39a69..8589bd4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,17 +6,6 @@ Version 0.4.4 To be released. - -Version 0.4.3 -------------- - -Released on June 4, 2026. - - - Upgraded Fedify to 2.1.15, which fixes an SSRF protection bypass - vulnerability. [[CVE-2026-50131]] - -[CVE-2026-50131]: https://github.com/fedify-dev/fedify/security/advisories/GHSA-xw9q-2mv6-9fr8 - ### @fedify/botkit - Fixed `MemoryRepository`, `KvRepository`, and `MemoryCachedRepository` so @@ -41,6 +30,17 @@ Released on June 4, 2026. last active follow request is removed. [[#25]] +Version 0.4.3 +------------- + +Released on June 4, 2026. + + - Upgraded Fedify to 2.1.15, which fixes an SSRF protection bypass + vulnerability. [[CVE-2026-50131]] + +[CVE-2026-50131]: https://github.com/fedify-dev/fedify/security/advisories/GHSA-xw9q-2mv6-9fr8 + + Version 0.4.2 ------------- From 6549650b385af10a8b69f2a2c2b45e821e48b572 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 21:26:57 +0900 Subject: [PATCH 05/21] Expire KV follower locks Store follower lock keys with a short TTL so a process crash after lock acquisition cannot leave persistent CAS-backed stores blocked forever. This keeps the existing lock protocol but lets abandoned lock entries age out without manual cleanup. https://github.com/fedify-dev/botkit/pull/26#discussion_r3524877420 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 46 +++++++++++++++++++++++++- packages/botkit/src/repository.ts | 5 +-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 8d04622..520e859 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -13,7 +13,11 @@ // // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -import { MemoryKvStore } from "@fedify/fedify/federation"; +import { + type KvKey, + type KvStoreSetOptions, + MemoryKvStore, +} from "@fedify/fedify/federation"; import { importJwk } from "@fedify/fedify/sig"; import { Create, Follow, Note, Person, PUBLIC_COLLECTION } from "@fedify/vocab"; import assert from "node:assert"; @@ -43,6 +47,29 @@ const factories: Record Repository> = { MemoryCachedRepository: createMemoryCachedRepository, }; +class RecordingMemoryKvStore extends MemoryKvStore { + readonly lockOptions: KvStoreSetOptions[] = []; + + override set( + key: KvKey, + value: unknown, + options?: KvStoreSetOptions, + ): Promise { + if (key.includes("lock")) this.lockOptions.push(options ?? {}); + return super.set(key, value, options); + } + + override cas( + key: KvKey, + expectedValue: unknown, + newValue: unknown, + options?: KvStoreSetOptions, + ): Promise { + if (key.includes("lock")) this.lockOptions.push(options ?? {}); + return super.cas(key, expectedValue, newValue, options); + } +} + const keyPairs: CryptoKeyPair[] = [ { publicKey: await importJwk({ @@ -97,6 +124,23 @@ const keyPairs: CryptoKeyPair[] = [ }, ]; +test("KvRepository uses expiring follower locks", async () => { + const kv = new RecordingMemoryKvStore(); + const repo = new KvRepository(kv); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/lock-test"), + preferredUsername: "lock-test", + }); + + await repo.addFollower( + new URL("https://example.com/ap/follow/lock-test"), + follower, + ); + + assert.ok(kv.lockOptions.length > 0); + assert.ok(kv.lockOptions.every((options) => options.ttl != null)); +}); + for (const name in factories) { const factory = factories[name]; diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index ce4e0dc..d2ae98c 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -29,6 +29,7 @@ export type { KvKey, KvStore } from "@fedify/fedify/federation"; export { Announce, Create } from "@fedify/vocab"; const logger = getLogger(["botkit", "repository"]); +const kvLockTtl = Temporal.Duration.from({ minutes: 5 }); /** * A UUID (universally unique identifier). @@ -706,7 +707,7 @@ export class KvRepository implements Repository { const lockId = crypto.randomUUID(); if (this.kv.cas == null) { while (true) { - await this.kv.set(lockKey, lockId); + await this.kv.set(lockKey, lockId, { ttl: kvLockTtl }); if (await this.kv.get(lockKey) !== lockId) continue; try { return await operation(); @@ -717,7 +718,7 @@ export class KvRepository implements Repository { } } } - while (!await this.kv.cas(lockKey, undefined, lockId)) { + while (!await this.kv.cas(lockKey, undefined, lockId, { ttl: kvLockTtl })) { await new Promise((resolve) => setTimeout(resolve, 0)); } try { From 6f6d38ae5b934d03786bb564980326fb4c1fd53f Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 22:23:28 +0900 Subject: [PATCH 06/21] Yield while waiting for KV locks Let the non-CAS lock acquisition path yield to the event loop before retrying when it loses the lock race. This avoids a tight retry loop under contention while keeping the existing fallback locking behavior. https://github.com/fedify-dev/botkit/pull/26#discussion_r3524903548 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index d2ae98c..4ff764b 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -708,7 +708,10 @@ export class KvRepository implements Repository { if (this.kv.cas == null) { while (true) { await this.kv.set(lockKey, lockId, { ttl: kvLockTtl }); - if (await this.kv.get(lockKey) !== lockId) continue; + if (await this.kv.get(lockKey) !== lockId) { + await new Promise((resolve) => setTimeout(resolve, 0)); + continue; + } try { return await operation(); } finally { From 38182f6ac26b7387464c23f3736f072702dd1f6e Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 22:42:28 +0900 Subject: [PATCH 07/21] Clean up KV follower follow-ups Delete the follower-to-follow-request index when follower cleanup removes the actor, and mark the package as side-effectful because the Node build intentionally injects the Temporal polyfill into the module scope. https://github.com/fedify-dev/botkit/pull/26#discussion_r3524972177 https://github.com/fedify-dev/botkit/pull/26#discussion_r3524972179 Assisted-by: Codex:gpt-5.5 --- packages/botkit/package.json | 2 +- packages/botkit/src/repository.test.ts | 22 ++++++++++++++++++++++ packages/botkit/src/repository.ts | 1 + 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/botkit/package.json b/packages/botkit/package.json index 0e5f2da..8a0daa4 100644 --- a/packages/botkit/package.json +++ b/packages/botkit/package.json @@ -76,7 +76,7 @@ }, "./package.json": "./package.json" }, - "sideEffects": false, + "sideEffects": true, "files": [ "dist", "LICENSE", diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 520e859..20f0eeb 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -141,6 +141,28 @@ test("KvRepository uses expiring follower locks", async () => { assert.ok(kv.lockOptions.every((options) => options.ttl != null)); }); +test("KvRepository deletes stale follower indexes", async () => { + const kv = new MemoryKvStore(); + const repo = new KvRepository(kv); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/index-test"), + preferredUsername: "index-test", + }); + const follow = new URL("https://example.com/ap/follow/index-test"); + const indexKey: KvKey = [ + "_botkit", + "followRequests", + "followers", + follower.id!.href, + ]; + + await repo.addFollower(follow, follower); + assert.deepStrictEqual(await kv.get(indexKey), [follow.href]); + + await repo.removeFollower(follow, follower.id!); + assert.deepStrictEqual(await kv.get(indexKey), undefined); +}); + for (const name in factories) { const factory = factories[name]; diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 4ff764b..f0fa96e 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -608,6 +608,7 @@ export class KvRepository implements Repository { list.filter((id) => id !== followerId), ); await this.kv.delete([...this.prefixes.followers, followerId]); + await this.kv.delete(this.getFollowerFollowRequestsKey(followerId)); return true; } From e8b64a5e855980d4b66df0e63612a41f1765c0be Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 22:55:10 +0900 Subject: [PATCH 08/21] Recover legacy KV follower locks Allow CAS-backed repositories to replace follower lock values written by earlier versions that did not attach a TTL. New lock values are structured so legacy string values can be detected and converted to expiring locks. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525007890 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 19 ++++++++++++ packages/botkit/src/repository.ts | 40 ++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 20f0eeb..afa9b5f 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -163,6 +163,25 @@ test("KvRepository deletes stale follower indexes", async () => { assert.deepStrictEqual(await kv.get(indexKey), undefined); }); +test("KvRepository recovers legacy follower locks", async () => { + const kv = new MemoryKvStore(); + const repo = new KvRepository(kv); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/legacy-lock"), + preferredUsername: "legacy-lock", + }); + const lockKey: KvKey = ["_botkit", "followers", "lock"]; + + await kv.set(lockKey, follower.id!.href); + await repo.addFollower( + new URL("https://example.com/ap/follow/legacy-lock"), + follower, + ); + + assert.ok(await repo.hasFollower(follower.id!)); + assert.deepStrictEqual(await kv.get(lockKey), undefined); +}); + for (const name in factories) { const factory = factories[name]; diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index f0fa96e..c0213f1 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -30,6 +30,21 @@ export { Announce, Create } from "@fedify/vocab"; const logger = getLogger(["botkit", "repository"]); const kvLockTtl = Temporal.Duration.from({ minutes: 5 }); +const uuidPattern = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +interface KvLock { + readonly id: string; +} + +function isKvLock(value: unknown): value is KvLock { + return typeof value === "object" && value != null && "id" in value && + typeof value.id === "string"; +} + +function isLegacyKvLock(value: unknown): value is string { + return typeof value === "string" && !uuidPattern.test(value); +} /** * A UUID (universally unique identifier). @@ -705,30 +720,43 @@ export class KvRepository implements Repository { lockKey: KvKey, operation: () => Promise, ): Promise { - const lockId = crypto.randomUUID(); + const lock: KvLock = { id: crypto.randomUUID() }; if (this.kv.cas == null) { while (true) { - await this.kv.set(lockKey, lockId, { ttl: kvLockTtl }); - if (await this.kv.get(lockKey) !== lockId) { + await this.kv.set(lockKey, lock, { ttl: kvLockTtl }); + const currentLock = await this.kv.get(lockKey); + if (!isKvLock(currentLock) || currentLock.id !== lock.id) { await new Promise((resolve) => setTimeout(resolve, 0)); continue; } try { return await operation(); } finally { - if (await this.kv.get(lockKey) === lockId) { + const currentLock = await this.kv.get(lockKey); + if (isKvLock(currentLock) && currentLock.id === lock.id) { await this.kv.delete(lockKey); } } } } - while (!await this.kv.cas(lockKey, undefined, lockId, { ttl: kvLockTtl })) { + while (true) { + if (await this.kv.cas(lockKey, undefined, lock, { ttl: kvLockTtl })) { + break; + } + const currentLock = await this.kv.get(lockKey); + if ( + isLegacyKvLock(currentLock) && + await this.kv.cas(lockKey, currentLock, lock, { ttl: kvLockTtl }) + ) { + break; + } await new Promise((resolve) => setTimeout(resolve, 0)); } try { return await operation(); } finally { - if (await this.kv.get(lockKey) === lockId) { + const currentLock = await this.kv.get(lockKey); + if (isKvLock(currentLock) && currentLock.id === lock.id) { await this.kv.delete(lockKey); } } From 825eb246d7f708ebde23f3c168d744b5652212ad Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 23:06:00 +0900 Subject: [PATCH 09/21] Tighten KV follower locks Keep reassigned follower cleanup under the same followers lock as the new follower write, release CAS-backed locks with CAS, and reject follower mutations for KV stores that cannot provide CAS semantics. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525021898 https://github.com/fedify-dev/botkit/pull/26#discussion_r3525021902 https://github.com/fedify-dev/botkit/pull/26#discussion_r3525022874 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 53 +++++++++++++++++++++++++- packages/botkit/src/repository.ts | 42 +++++++------------- 2 files changed, 66 insertions(+), 29 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index afa9b5f..87e02c1 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -15,6 +15,8 @@ // along with this program. If not, see . import { type KvKey, + type KvStore, + type KvStoreListEntry, type KvStoreSetOptions, MemoryKvStore, } from "@fedify/fedify/federation"; @@ -49,6 +51,7 @@ const factories: Record Repository> = { class RecordingMemoryKvStore extends MemoryKvStore { readonly lockOptions: KvStoreSetOptions[] = []; + readonly lockReleases: KvKey[] = []; override set( key: KvKey, @@ -65,11 +68,41 @@ class RecordingMemoryKvStore extends MemoryKvStore { newValue: unknown, options?: KvStoreSetOptions, ): Promise { - if (key.includes("lock")) this.lockOptions.push(options ?? {}); + if (key.includes("lock")) { + if (newValue === undefined) { + this.lockReleases.push(key); + } else { + this.lockOptions.push(options ?? {}); + } + } return super.cas(key, expectedValue, newValue, options); } } +class NonCasMemoryKvStore implements KvStore { + readonly #kv = new MemoryKvStore(); + + get(key: KvKey): Promise { + return this.#kv.get(key); + } + + set( + key: KvKey, + value: unknown, + options?: KvStoreSetOptions, + ): Promise { + return this.#kv.set(key, value, options); + } + + delete(key: KvKey): Promise { + return this.#kv.delete(key); + } + + list(prefix?: KvKey): AsyncIterable { + return this.#kv.list(prefix); + } +} + const keyPairs: CryptoKeyPair[] = [ { publicKey: await importJwk({ @@ -139,6 +172,7 @@ test("KvRepository uses expiring follower locks", async () => { assert.ok(kv.lockOptions.length > 0); assert.ok(kv.lockOptions.every((options) => options.ttl != null)); + assert.ok(kv.lockReleases.length > 0); }); test("KvRepository deletes stale follower indexes", async () => { @@ -182,6 +216,23 @@ test("KvRepository recovers legacy follower locks", async () => { assert.deepStrictEqual(await kv.get(lockKey), undefined); }); +test("KvRepository requires CAS for follower mutations", async () => { + const repo = new KvRepository(new NonCasMemoryKvStore()); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/no-cas"), + preferredUsername: "no-cas", + }); + const follow = new URL("https://example.com/ap/follow/no-cas"); + const error = { + name: "TypeError", + message: + "KvRepository follower mutations require a KvStore with CAS support.", + }; + + await assert.rejects(() => repo.addFollower(follow, follower), error); + await assert.rejects(() => repo.removeFollower(follow, follower.id!), error); +}); + for (const name in factories) { const factory = factories[name]; diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index c0213f1..8f5a616 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -547,18 +547,16 @@ export class KvRepository implements Repository { followerId, followRequestIdString, ); - }); - if ( - previousFollowerId != null && previousFollowerId !== followerId - ) { - await this.withKvLock(this.getFollowersLockKey(), async () => { + if ( + previousFollowerId != null && previousFollowerId !== followerId + ) { await this.removeFollowRequestForFollowerLocked( previousFollowerId, followRequestIdString, ); await this.cleanupFollowerLocked(previousFollowerId); - }); - } + } + }); }, ); } @@ -720,33 +718,21 @@ export class KvRepository implements Repository { lockKey: KvKey, operation: () => Promise, ): Promise { - const lock: KvLock = { id: crypto.randomUUID() }; - if (this.kv.cas == null) { - while (true) { - await this.kv.set(lockKey, lock, { ttl: kvLockTtl }); - const currentLock = await this.kv.get(lockKey); - if (!isKvLock(currentLock) || currentLock.id !== lock.id) { - await new Promise((resolve) => setTimeout(resolve, 0)); - continue; - } - try { - return await operation(); - } finally { - const currentLock = await this.kv.get(lockKey); - if (isKvLock(currentLock) && currentLock.id === lock.id) { - await this.kv.delete(lockKey); - } - } - } + const cas = this.kv.cas?.bind(this.kv); + if (cas == null) { + throw new TypeError( + "KvRepository follower mutations require a KvStore with CAS support.", + ); } + const lock: KvLock = { id: crypto.randomUUID() }; while (true) { - if (await this.kv.cas(lockKey, undefined, lock, { ttl: kvLockTtl })) { + if (await cas(lockKey, undefined, lock, { ttl: kvLockTtl })) { break; } const currentLock = await this.kv.get(lockKey); if ( isLegacyKvLock(currentLock) && - await this.kv.cas(lockKey, currentLock, lock, { ttl: kvLockTtl }) + await cas(lockKey, currentLock, lock, { ttl: kvLockTtl }) ) { break; } @@ -757,7 +743,7 @@ export class KvRepository implements Repository { } finally { const currentLock = await this.kv.get(lockKey); if (isKvLock(currentLock) && currentLock.id === lock.id) { - await this.kv.delete(lockKey); + await cas(lockKey, currentLock, undefined); } } } From bc8d5e0e1f896f98a627f62547b1a5faed7d1a51 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 23:14:08 +0900 Subject: [PATCH 10/21] Rebuild KV follower indexes Treat the per-follower reverse index as a cache and rebuild it from the authoritative follow request mappings. This keeps stale or incomplete indexes from causing follower cleanup while another request still points at the actor. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525036670 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 32 ++++++++++++++++++++++++++ packages/botkit/src/repository.ts | 3 --- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 87e02c1..74d22e2 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -197,6 +197,38 @@ test("KvRepository deletes stale follower indexes", async () => { assert.deepStrictEqual(await kv.get(indexKey), undefined); }); +test("KvRepository rebuilds incomplete follower indexes", async () => { + const kv = new MemoryKvStore(); + const repo = new KvRepository(kv); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/rebuild-index"), + preferredUsername: "rebuild-index", + }); + const firstFollow = new URL( + "https://example.com/ap/follow/rebuild-index/1", + ); + const secondFollow = new URL( + "https://example.com/ap/follow/rebuild-index/2", + ); + const indexKey: KvKey = [ + "_botkit", + "followRequests", + "followers", + follower.id!.href, + ]; + + await repo.addFollower(firstFollow, follower); + await repo.addFollower(secondFollow, follower); + await kv.set(indexKey, [firstFollow.href]); + + assert.deepStrictEqual( + await repo.removeFollower(firstFollow, follower.id!), + undefined, + ); + assert.ok(await repo.hasFollower(follower.id!)); + assert.deepStrictEqual(await kv.get(indexKey), [secondFollow.href]); +}); + test("KvRepository recovers legacy follower locks", async () => { const kv = new MemoryKvStore(); const repo = new KvRepository(kv); diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 8f5a616..71d9ddd 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -697,9 +697,6 @@ export class KvRepository implements Repository { followerId: string, ): Promise { const indexKey = this.getFollowerFollowRequestsKey(followerId); - const followRequestIds = await this.kv.get(indexKey); - if (followRequestIds != null) return followRequestIds; - const indexedFollowRequestIds: string[] = []; const followRequestKeyLength = this.prefixes.followRequests.length + 1; for await (const entry of this.kv.list(this.prefixes.followRequests)) { From 10e6f2c42d67d446445f4dbfe97ce6df883a8015 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 23:25:33 +0900 Subject: [PATCH 11/21] Trim KV follower lock overhead Use the reverse index for add/remove bookkeeping while keeping cleanup checks authoritative, wait longer between lock acquisition attempts, and release locks with expiring tombstones instead of permanent undefined entries. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525043314 https://github.com/fedify-dev/botkit/pull/26#discussion_r3525043316 https://github.com/fedify-dev/botkit/pull/26#discussion_r3525050419 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 52 ++++++++++++++++++++++++-- packages/botkit/src/repository.ts | 37 +++++++++++++----- 2 files changed, 75 insertions(+), 14 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 74d22e2..800af7d 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -51,7 +51,8 @@ const factories: Record Repository> = { class RecordingMemoryKvStore extends MemoryKvStore { readonly lockOptions: KvStoreSetOptions[] = []; - readonly lockReleases: KvKey[] = []; + readonly lockReleaseOptions: KvStoreSetOptions[] = []; + readonly undefinedLockReleases: KvKey[] = []; override set( key: KvKey, @@ -70,7 +71,12 @@ class RecordingMemoryKvStore extends MemoryKvStore { ): Promise { if (key.includes("lock")) { if (newValue === undefined) { - this.lockReleases.push(key); + this.undefinedLockReleases.push(key); + } else if ( + typeof newValue === "object" && newValue != null && + "released" in newValue + ) { + this.lockReleaseOptions.push(options ?? {}); } else { this.lockOptions.push(options ?? {}); } @@ -79,6 +85,15 @@ class RecordingMemoryKvStore extends MemoryKvStore { } } +class RecordingListMemoryKvStore extends MemoryKvStore { + listCalls = 0; + + override list(prefix?: KvKey): AsyncIterable { + this.listCalls++; + return super.list(prefix); + } +} + class NonCasMemoryKvStore implements KvStore { readonly #kv = new MemoryKvStore(); @@ -172,7 +187,9 @@ test("KvRepository uses expiring follower locks", async () => { assert.ok(kv.lockOptions.length > 0); assert.ok(kv.lockOptions.every((options) => options.ttl != null)); - assert.ok(kv.lockReleases.length > 0); + assert.ok(kv.lockReleaseOptions.length > 0); + assert.ok(kv.lockReleaseOptions.every((options) => options.ttl != null)); + assert.deepStrictEqual(kv.undefinedLockReleases, []); }); test("KvRepository deletes stale follower indexes", async () => { @@ -229,6 +246,33 @@ test("KvRepository rebuilds incomplete follower indexes", async () => { assert.deepStrictEqual(await kv.get(indexKey), [secondFollow.href]); }); +test("KvRepository uses follower indexes when adding requests", async () => { + const kv = new RecordingListMemoryKvStore(); + const repo = new KvRepository(kv); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/index-add"), + preferredUsername: "index-add", + }); + const firstFollow = new URL("https://example.com/ap/follow/index-add/1"); + const secondFollow = new URL("https://example.com/ap/follow/index-add/2"); + const indexKey: KvKey = [ + "_botkit", + "followRequests", + "followers", + follower.id!.href, + ]; + + await repo.addFollower(firstFollow, follower); + kv.listCalls = 0; + await repo.addFollower(secondFollow, follower); + + assert.deepStrictEqual(await kv.get(indexKey), [ + firstFollow.href, + secondFollow.href, + ]); + assert.deepStrictEqual(kv.listCalls, 0); +}); + test("KvRepository recovers legacy follower locks", async () => { const kv = new MemoryKvStore(); const repo = new KvRepository(kv); @@ -245,7 +289,7 @@ test("KvRepository recovers legacy follower locks", async () => { ); assert.ok(await repo.hasFollower(follower.id!)); - assert.deepStrictEqual(await kv.get(lockKey), undefined); + assert.notDeepStrictEqual(await kv.get(lockKey), follower.id!.href); }); test("KvRepository requires CAS for follower mutations", async () => { diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 71d9ddd..75adbe1 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -30,11 +30,16 @@ export { Announce, Create } from "@fedify/vocab"; const logger = getLogger(["botkit", "repository"]); const kvLockTtl = Temporal.Duration.from({ minutes: 5 }); +const kvLockPollIntervalMs = 100; +const kvLockReleaseTtl = Temporal.Duration.from({ + milliseconds: kvLockPollIntervalMs, +}); const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; interface KvLock { readonly id: string; + readonly released?: boolean; } function isKvLock(value: unknown): value is KvLock { @@ -645,9 +650,10 @@ export class KvRepository implements Repository { followerId: string, followRequestId: string, ): Promise { - const followRequestIds = await this.getFollowRequestsForFollowerLocked( - followerId, - ); + const followRequestIds = await this + .getIndexedFollowRequestsForFollowerLocked( + followerId, + ); if (!followRequestIds.includes(followRequestId)) { await this.kv.set(this.getFollowerFollowRequestsKey(followerId), [ ...followRequestIds, @@ -660,9 +666,10 @@ export class KvRepository implements Repository { followerId: string, followRequestId: string, ): Promise { - const followRequestIds = await this.getFollowRequestsForFollowerLocked( - followerId, - ); + const followRequestIds = await this + .getIndexedFollowRequestsForFollowerLocked( + followerId, + ); await this.kv.set( this.getFollowerFollowRequestsKey(followerId), followRequestIds.filter((id) => id !== followRequestId), @@ -672,7 +679,7 @@ export class KvRepository implements Repository { private async hasFollowRequestForFollowerLocked( followerId: string, ): Promise { - const followRequestIds = await this.getFollowRequestsForFollowerLocked( + const followRequestIds = await this.rebuildFollowRequestsForFollowerLocked( followerId, ); const currentFollowRequestIds: string[] = []; @@ -693,7 +700,15 @@ export class KvRepository implements Repository { return currentFollowRequestIds.length > 0; } - private async getFollowRequestsForFollowerLocked( + private async getIndexedFollowRequestsForFollowerLocked( + followerId: string, + ): Promise { + return await this.kv.get( + this.getFollowerFollowRequestsKey(followerId), + ) ?? []; + } + + private async rebuildFollowRequestsForFollowerLocked( followerId: string, ): Promise { const indexKey = this.getFollowerFollowRequestsKey(followerId); @@ -733,14 +748,16 @@ export class KvRepository implements Repository { ) { break; } - await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, kvLockPollIntervalMs)); } try { return await operation(); } finally { const currentLock = await this.kv.get(lockKey); if (isKvLock(currentLock) && currentLock.id === lock.id) { - await cas(lockKey, currentLock, undefined); + await cas(lockKey, currentLock, { ...currentLock, released: true }, { + ttl: kvLockReleaseTtl, + }); } } } From 21d7cb7e01f0b8a29b29d17eea8f89cc3b6f18b8 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 23:36:49 +0900 Subject: [PATCH 12/21] Preserve KV follower compatibility Use indexed follower follow requests for the common cleanup path, while keeping an authoritative rebuild before deleting a follower. Keep get/set/delete/list-only KV stores working by serializing follower mutations inside the repository process when CAS is unavailable. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525060006 https://github.com/fedify-dev/botkit/pull/26#discussion_r3525065443 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 26 ++++++---- packages/botkit/src/repository.ts | 68 ++++++++++++++++++++++---- 2 files changed, 75 insertions(+), 19 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 800af7d..d680f48 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -271,6 +271,14 @@ test("KvRepository uses follower indexes when adding requests", async () => { secondFollow.href, ]); assert.deepStrictEqual(kv.listCalls, 0); + + kv.listCalls = 0; + assert.deepStrictEqual( + await repo.removeFollower(firstFollow, follower.id!), + undefined, + ); + assert.deepStrictEqual(kv.listCalls, 0); + assert.deepStrictEqual(await kv.get(indexKey), [secondFollow.href]); }); test("KvRepository recovers legacy follower locks", async () => { @@ -292,21 +300,21 @@ test("KvRepository recovers legacy follower locks", async () => { assert.notDeepStrictEqual(await kv.get(lockKey), follower.id!.href); }); -test("KvRepository requires CAS for follower mutations", async () => { +test("KvRepository supports non-CAS follower mutations", async () => { const repo = new KvRepository(new NonCasMemoryKvStore()); const follower = new Person({ id: new URL("https://example.com/ap/actor/no-cas"), preferredUsername: "no-cas", }); const follow = new URL("https://example.com/ap/follow/no-cas"); - const error = { - name: "TypeError", - message: - "KvRepository follower mutations require a KvStore with CAS support.", - }; - - await assert.rejects(() => repo.addFollower(follow, follower), error); - await assert.rejects(() => repo.removeFollower(follow, follower.id!), error); + + await repo.addFollower(follow, follower); + assert.ok(await repo.hasFollower(follower.id!)); + assert.deepStrictEqual( + await repo.removeFollower(follow, follower.id!), + follower, + ); + assert.ok(!await repo.hasFollower(follower.id!)); }); for (const name in factories) { diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 75adbe1..2b9b233 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -356,6 +356,7 @@ export interface KvStoreRepositoryPrefixes { export class KvRepository implements Repository { readonly kv: KvStore; readonly prefixes: KvStoreRepositoryPrefixes; + private readonly nonCasLocks = new Map>(); /** * Creates a new key-value store repository. @@ -547,11 +548,11 @@ export class KvRepository implements Repository { const followerKey: KvKey = [...this.prefixes.followers, followerId]; await this.kv.set(followerKey, followerJson); await this.addFollowerIdLocked(followerId); - await this.kv.set(followRequestKey, followerId); await this.addFollowRequestForFollowerLocked( followerId, followRequestIdString, ); + await this.kv.set(followRequestKey, followerId); if ( previousFollowerId != null && previousFollowerId !== followerId ) { @@ -679,9 +680,10 @@ export class KvRepository implements Repository { private async hasFollowRequestForFollowerLocked( followerId: string, ): Promise { - const followRequestIds = await this.rebuildFollowRequestsForFollowerLocked( - followerId, - ); + const followRequestIds = await this + .getIndexedFollowRequestsForFollowerLocked( + followerId, + ); const currentFollowRequestIds: string[] = []; for (const followRequestId of followRequestIds) { if ( @@ -691,13 +693,37 @@ export class KvRepository implements Repository { currentFollowRequestIds.push(followRequestId); } } - if (currentFollowRequestIds.length !== followRequestIds.length) { + if (currentFollowRequestIds.length > 0) { + if (currentFollowRequestIds.length !== followRequestIds.length) { + await this.kv.set( + this.getFollowerFollowRequestsKey(followerId), + currentFollowRequestIds, + ); + } + return true; + } + const rebuiltFollowRequestIds = await this + .rebuildFollowRequestsForFollowerLocked( + followerId, + ); + const rebuiltCurrentFollowRequestIds: string[] = []; + for (const followRequestId of rebuiltFollowRequestIds) { + if ( + await this.kv.get(this.getFollowRequestKey(followRequestId)) === + followerId + ) { + rebuiltCurrentFollowRequestIds.push(followRequestId); + } + } + if ( + rebuiltCurrentFollowRequestIds.length !== rebuiltFollowRequestIds.length + ) { await this.kv.set( this.getFollowerFollowRequestsKey(followerId), - currentFollowRequestIds, + rebuiltCurrentFollowRequestIds, ); } - return currentFollowRequestIds.length > 0; + return rebuiltCurrentFollowRequestIds.length > 0; } private async getIndexedFollowRequestsForFollowerLocked( @@ -732,9 +758,7 @@ export class KvRepository implements Repository { ): Promise { const cas = this.kv.cas?.bind(this.kv); if (cas == null) { - throw new TypeError( - "KvRepository follower mutations require a KvStore with CAS support.", - ); + return await this.withNonCasKvLock(lockKey, operation); } const lock: KvLock = { id: crypto.randomUUID() }; while (true) { @@ -762,6 +786,30 @@ export class KvRepository implements Repository { } } + private async withNonCasKvLock( + lockKey: KvKey, + operation: () => Promise, + ): Promise { + const encodedLockKey = JSON.stringify(lockKey); + const previousLock = this.nonCasLocks.get(encodedLockKey) ?? + Promise.resolve(); + let releaseLock: () => void; + const lock = new Promise((resolve) => { + releaseLock = resolve; + }); + const tail = previousLock.then(() => lock, () => lock); + this.nonCasLocks.set(encodedLockKey, tail); + await previousLock.catch(() => {}); + try { + return await operation(); + } finally { + releaseLock!(); + if (this.nonCasLocks.get(encodedLockKey) === tail) { + this.nonCasLocks.delete(encodedLockKey); + } + } + } + async hasFollower(followerId: URL): Promise { return await this.kv.get([ ...this.prefixes.followers, From 9bd030cde66c6f2a051a9c943c2a31050226afa7 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 23:45:36 +0900 Subject: [PATCH 13/21] Reacquire released KV locks Treat released KV lock markers as immediately acquirable so sequential follower mutations do not wait for the release TTL before taking the same lock again. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525079495 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 26 ++++++++++++++++++++++++++ packages/botkit/src/repository.ts | 6 +++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index d680f48..7ff3ccc 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -53,6 +53,7 @@ class RecordingMemoryKvStore extends MemoryKvStore { readonly lockOptions: KvStoreSetOptions[] = []; readonly lockReleaseOptions: KvStoreSetOptions[] = []; readonly undefinedLockReleases: KvKey[] = []; + releasedLockAcquisitions = 0; override set( key: KvKey, @@ -77,6 +78,11 @@ class RecordingMemoryKvStore extends MemoryKvStore { "released" in newValue ) { this.lockReleaseOptions.push(options ?? {}); + } else if ( + typeof expectedValue === "object" && expectedValue != null && + "released" in expectedValue + ) { + this.releasedLockAcquisitions++; } else { this.lockOptions.push(options ?? {}); } @@ -192,6 +198,26 @@ test("KvRepository uses expiring follower locks", async () => { assert.deepStrictEqual(kv.undefinedLockReleases, []); }); +test("KvRepository reacquires released follower locks", async () => { + const kv = new RecordingMemoryKvStore(); + const repo = new KvRepository(kv); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/released-lock"), + preferredUsername: "released-lock", + }); + + await repo.addFollower( + new URL("https://example.com/ap/follow/released-lock/1"), + follower, + ); + await repo.addFollower( + new URL("https://example.com/ap/follow/released-lock/2"), + follower, + ); + + assert.ok(kv.releasedLockAcquisitions > 0); +}); + test("KvRepository deletes stale follower indexes", async () => { const kv = new MemoryKvStore(); const repo = new KvRepository(kv); diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 2b9b233..0258c69 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -47,6 +47,10 @@ function isKvLock(value: unknown): value is KvLock { typeof value.id === "string"; } +function isReleasedKvLock(value: unknown): value is KvLock { + return isKvLock(value) && value.released === true; +} + function isLegacyKvLock(value: unknown): value is string { return typeof value === "string" && !uuidPattern.test(value); } @@ -767,7 +771,7 @@ export class KvRepository implements Repository { } const currentLock = await this.kv.get(lockKey); if ( - isLegacyKvLock(currentLock) && + (isLegacyKvLock(currentLock) || isReleasedKvLock(currentLock)) && await cas(lockKey, currentLock, lock, { ttl: kvLockTtl }) ) { break; From 26c0269b3d9eb7814bdd950db3b0612e5af19766 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 23:48:18 +0900 Subject: [PATCH 14/21] Trust existing follower indexes Avoid rebuilding follower follow-request indexes when the index key already exists. Complete unfollows now trust the empty index produced by the locked remove path, while legacy data with no index key still rebuilds once. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525076437 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 32 ++++++++++++--- packages/botkit/src/repository.ts | 55 +++++++++++++++----------- 2 files changed, 58 insertions(+), 29 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 7ff3ccc..29af59e 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -240,18 +240,18 @@ test("KvRepository deletes stale follower indexes", async () => { assert.deepStrictEqual(await kv.get(indexKey), undefined); }); -test("KvRepository rebuilds incomplete follower indexes", async () => { +test("KvRepository rebuilds missing follower indexes", async () => { const kv = new MemoryKvStore(); const repo = new KvRepository(kv); const follower = new Person({ - id: new URL("https://example.com/ap/actor/rebuild-index"), - preferredUsername: "rebuild-index", + id: new URL("https://example.com/ap/actor/missing-index"), + preferredUsername: "missing-index", }); const firstFollow = new URL( - "https://example.com/ap/follow/rebuild-index/1", + "https://example.com/ap/follow/missing-index/1", ); const secondFollow = new URL( - "https://example.com/ap/follow/rebuild-index/2", + "https://example.com/ap/follow/missing-index/2", ); const indexKey: KvKey = [ "_botkit", @@ -262,7 +262,7 @@ test("KvRepository rebuilds incomplete follower indexes", async () => { await repo.addFollower(firstFollow, follower); await repo.addFollower(secondFollow, follower); - await kv.set(indexKey, [firstFollow.href]); + await kv.delete(indexKey); assert.deepStrictEqual( await repo.removeFollower(firstFollow, follower.id!), @@ -272,6 +272,26 @@ test("KvRepository rebuilds incomplete follower indexes", async () => { assert.deepStrictEqual(await kv.get(indexKey), [secondFollow.href]); }); +test("KvRepository trusts empty follower indexes", async () => { + const kv = new RecordingListMemoryKvStore(); + const repo = new KvRepository(kv); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/empty-index"), + preferredUsername: "empty-index", + }); + const follow = new URL("https://example.com/ap/follow/empty-index"); + + await repo.addFollower(follow, follower); + kv.listCalls = 0; + + assert.deepStrictEqual( + await repo.removeFollower(follow, follower.id!), + follower, + ); + assert.deepStrictEqual(kv.listCalls, 0); + assert.ok(!await repo.hasFollower(follower.id!)); +}); + test("KvRepository uses follower indexes when adding requests", async () => { const kv = new RecordingListMemoryKvStore(); const repo = new KvRepository(kv); diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 0258c69..1e2672d 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -658,7 +658,7 @@ export class KvRepository implements Repository { const followRequestIds = await this .getIndexedFollowRequestsForFollowerLocked( followerId, - ); + ) ?? await this.rebuildFollowRequestsForFollowerLocked(followerId); if (!followRequestIds.includes(followRequestId)) { await this.kv.set(this.getFollowerFollowRequestsKey(followerId), [ ...followRequestIds, @@ -674,7 +674,7 @@ export class KvRepository implements Repository { const followRequestIds = await this .getIndexedFollowRequestsForFollowerLocked( followerId, - ); + ) ?? await this.rebuildFollowRequestsForFollowerLocked(followerId); await this.kv.set( this.getFollowerFollowRequestsKey(followerId), followRequestIds.filter((id) => id !== followRequestId), @@ -684,10 +684,34 @@ export class KvRepository implements Repository { private async hasFollowRequestForFollowerLocked( followerId: string, ): Promise { + const indexKey = this.getFollowerFollowRequestsKey(followerId); const followRequestIds = await this .getIndexedFollowRequestsForFollowerLocked( followerId, ); + if (followRequestIds != null) { + return await this.hasCurrentFollowRequestForFollowerLocked( + followerId, + followRequestIds, + indexKey, + ); + } + const rebuiltFollowRequestIds = await this + .rebuildFollowRequestsForFollowerLocked( + followerId, + ); + return await this.hasCurrentFollowRequestForFollowerLocked( + followerId, + rebuiltFollowRequestIds, + indexKey, + ); + } + + private async hasCurrentFollowRequestForFollowerLocked( + followerId: string, + followRequestIds: readonly string[], + indexKey: KvKey, + ): Promise { const currentFollowRequestIds: string[] = []; for (const followRequestId of followRequestIds) { if ( @@ -706,36 +730,21 @@ export class KvRepository implements Repository { } return true; } - const rebuiltFollowRequestIds = await this - .rebuildFollowRequestsForFollowerLocked( - followerId, - ); - const rebuiltCurrentFollowRequestIds: string[] = []; - for (const followRequestId of rebuiltFollowRequestIds) { - if ( - await this.kv.get(this.getFollowRequestKey(followRequestId)) === - followerId - ) { - rebuiltCurrentFollowRequestIds.push(followRequestId); - } - } - if ( - rebuiltCurrentFollowRequestIds.length !== rebuiltFollowRequestIds.length - ) { + if (currentFollowRequestIds.length !== followRequestIds.length) { await this.kv.set( - this.getFollowerFollowRequestsKey(followerId), - rebuiltCurrentFollowRequestIds, + indexKey, + currentFollowRequestIds, ); } - return rebuiltCurrentFollowRequestIds.length > 0; + return false; } private async getIndexedFollowRequestsForFollowerLocked( followerId: string, - ): Promise { + ): Promise { return await this.kv.get( this.getFollowerFollowRequestsKey(followerId), - ) ?? []; + ); } private async rebuildFollowRequestsForFollowerLocked( From c5736d2b639eff1c818b29ca6de50fe04bbace0e Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 23:50:36 +0900 Subject: [PATCH 15/21] Clean corrupt KV follow requests Delete the follow-request mapping and update follower indexes before parsing stored actor data during unfollow cleanup. Corrupt or missing follower rows no longer leave stale follow requests behind. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525088114 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 32 ++++++++++++++++++++++++++ packages/botkit/src/repository.ts | 15 ++++++------ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 29af59e..8c25f9d 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -292,6 +292,38 @@ test("KvRepository trusts empty follower indexes", async () => { assert.ok(!await repo.hasFollower(follower.id!)); }); +test("KvRepository removes requests with missing followers", async () => { + const kv = new MemoryKvStore(); + const repo = new KvRepository(kv); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/missing-follower"), + preferredUsername: "missing-follower", + }); + const follow = new URL("https://example.com/ap/follow/missing-follower"); + const followRequestKey: KvKey = [ + "_botkit", + "followRequests", + follow.href, + ]; + const indexKey: KvKey = [ + "_botkit", + "followRequests", + "followers", + follower.id!.href, + ]; + + await repo.addFollower(follow, follower); + await kv.delete(["_botkit", "followers", follower.id!.href]); + + assert.deepStrictEqual( + await repo.removeFollower(follow, follower.id!), + undefined, + ); + assert.deepStrictEqual(await kv.get(followRequestKey), undefined); + assert.deepStrictEqual(await kv.get(indexKey), undefined); + assert.ok(!await repo.hasFollower(follower.id!)); +}); + test("KvRepository uses follower indexes when adding requests", async () => { const kv = new RecordingListMemoryKvStore(); const repo = new KvRepository(kv); diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 1e2672d..75e6e99 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -591,6 +591,12 @@ export class KvRepository implements Repository { if (currentFollowerId !== followerId) return undefined; const followerKey: KvKey = [...this.prefixes.followers, followerId]; const followerJson = await this.kv.get(followerKey); + await this.kv.delete(followRequestKey); + await this.removeFollowRequestForFollowerLocked( + followerId, + followRequestIdString, + ); + const removed = await this.cleanupFollowerLocked(followerId); if (followerJson == null) return undefined; let follower: Object; try { @@ -599,14 +605,7 @@ export class KvRepository implements Repository { return undefined; } if (!isActor(follower)) return undefined; - await this.kv.delete(followRequestKey); - await this.removeFollowRequestForFollowerLocked( - followerId, - followRequestIdString, - ); - return await this.cleanupFollowerLocked(followerId) - ? follower - : undefined; + return removed ? follower : undefined; }); }, ); From ac3a37e0d686820c7d1107d6d54d6d38c9ab558e Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 23:52:55 +0900 Subject: [PATCH 16/21] Preserve KV lock operation errors Log failures while releasing CAS-backed KV locks instead of letting release errors mask the original follower mutation failure. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525092399 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 45 ++++++++++++++++++++++++++ packages/botkit/src/repository.ts | 14 +++++--- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 8c25f9d..8e113da 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -124,6 +124,34 @@ class NonCasMemoryKvStore implements KvStore { } } +class FailingReleaseMemoryKvStore extends MemoryKvStore { + override set( + key: KvKey, + value: unknown, + options?: KvStoreSetOptions, + ): Promise { + if (!key.includes("lock")) { + throw new TypeError("Write failed."); + } + return super.set(key, value, options); + } + + override cas( + key: KvKey, + expectedValue: unknown, + newValue: unknown, + options?: KvStoreSetOptions, + ): Promise { + if ( + key.includes("lock") && typeof newValue === "object" && + newValue != null && "released" in newValue + ) { + throw new TypeError("Release failed."); + } + return super.cas(key, expectedValue, newValue, options); + } +} + const keyPairs: CryptoKeyPair[] = [ { publicKey: await importJwk({ @@ -395,6 +423,23 @@ test("KvRepository supports non-CAS follower mutations", async () => { assert.ok(!await repo.hasFollower(follower.id!)); }); +test("KvRepository preserves errors when lock release fails", async () => { + const repo = new KvRepository(new FailingReleaseMemoryKvStore()); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/release-failure"), + preferredUsername: "release-failure", + }); + + await assert.rejects( + () => + repo.addFollower( + new URL("https://example.com/ap/follow/release-failure"), + follower, + ), + { name: "TypeError", message: "Write failed." }, + ); +}); + for (const name in factories) { const factory = factories[name]; diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 75e6e99..43b629a 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -789,11 +789,15 @@ export class KvRepository implements Repository { try { return await operation(); } finally { - const currentLock = await this.kv.get(lockKey); - if (isKvLock(currentLock) && currentLock.id === lock.id) { - await cas(lockKey, currentLock, { ...currentLock, released: true }, { - ttl: kvLockReleaseTtl, - }); + try { + const currentLock = await this.kv.get(lockKey); + if (isKvLock(currentLock) && currentLock.id === lock.id) { + await cas(lockKey, currentLock, { ...currentLock, released: true }, { + ttl: kvLockReleaseTtl, + }); + } + } catch (error) { + logger.warn("Failed to release KV lock: {error}", { error }); } } } From f4de91e81920a933596f51e0976b06732e0b25b3 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 5 Jul 2026 23:54:16 +0900 Subject: [PATCH 17/21] Simplify follower index pruning Collapse duplicated follower-index pruning branches into one update path while preserving the same cleanup result. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525094882 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 43b629a..d89efd6 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -720,22 +720,13 @@ export class KvRepository implements Repository { currentFollowRequestIds.push(followRequestId); } } - if (currentFollowRequestIds.length > 0) { - if (currentFollowRequestIds.length !== followRequestIds.length) { - await this.kv.set( - this.getFollowerFollowRequestsKey(followerId), - currentFollowRequestIds, - ); - } - return true; - } if (currentFollowRequestIds.length !== followRequestIds.length) { await this.kv.set( indexKey, currentFollowRequestIds, ); } - return false; + return currentFollowRequestIds.length > 0; } private async getIndexedFollowRequestsForFollowerLocked( From a6fb5872210c37113adc3ed9ad8563f4640fbe80 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 6 Jul 2026 00:02:05 +0900 Subject: [PATCH 18/21] Skip new follower index rebuilds Avoid scanning the follow-request keyspace when adding the first request for a brand new follower. New follower rows can start from an empty reverse index, while existing followers still rebuild missing legacy indexes as needed. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525098444 https://github.com/fedify-dev/botkit/pull/26#discussion_r3525098450 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 1 + packages/botkit/src/repository.ts | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 8e113da..60cc49c 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -369,6 +369,7 @@ test("KvRepository uses follower indexes when adding requests", async () => { ]; await repo.addFollower(firstFollow, follower); + assert.deepStrictEqual(kv.listCalls, 0); kv.listCalls = 0; await repo.addFollower(secondFollow, follower); diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index d89efd6..5a19016 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -550,11 +550,13 @@ export class KvRepository implements Repository { const followerJson = await follower.toJsonLd({ format: "compact" }); await this.withKvLock(this.getFollowersLockKey(), async () => { const followerKey: KvKey = [...this.prefixes.followers, followerId]; + const isNewFollower = await this.kv.get(followerKey) == null; await this.kv.set(followerKey, followerJson); await this.addFollowerIdLocked(followerId); await this.addFollowRequestForFollowerLocked( followerId, followRequestIdString, + isNewFollower, ); await this.kv.set(followRequestKey, followerId); if ( @@ -653,11 +655,15 @@ export class KvRepository implements Repository { private async addFollowRequestForFollowerLocked( followerId: string, followRequestId: string, + isNewFollower = false, ): Promise { const followRequestIds = await this .getIndexedFollowRequestsForFollowerLocked( followerId, - ) ?? await this.rebuildFollowRequestsForFollowerLocked(followerId); + ) ?? + (isNewFollower + ? [] + : await this.rebuildFollowRequestsForFollowerLocked(followerId)); if (!followRequestIds.includes(followRequestId)) { await this.kv.set(this.getFollowerFollowRequestsKey(followerId), [ ...followRequestIds, From 5523b67a327e1f1f199a8a124a4e3ccd88c57dfe Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 6 Jul 2026 00:11:58 +0900 Subject: [PATCH 19/21] Preserve legacy KV follows Rebuild a missing follower reverse index even when the follower row is being created. Upgraded stores can still have legacy follow request mappings for that actor, so treating every missing follower row as a clean slate can recreate the premature-unfollow case. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525117352 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository.test.ts | 35 +++++++++++++++++++++++++- packages/botkit/src/repository.ts | 8 +----- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/packages/botkit/src/repository.test.ts b/packages/botkit/src/repository.test.ts index 60cc49c..3008a2b 100644 --- a/packages/botkit/src/repository.test.ts +++ b/packages/botkit/src/repository.test.ts @@ -369,7 +369,6 @@ test("KvRepository uses follower indexes when adding requests", async () => { ]; await repo.addFollower(firstFollow, follower); - assert.deepStrictEqual(kv.listCalls, 0); kv.listCalls = 0; await repo.addFollower(secondFollow, follower); @@ -388,6 +387,40 @@ test("KvRepository uses follower indexes when adding requests", async () => { assert.deepStrictEqual(await kv.get(indexKey), [secondFollow.href]); }); +test("KvRepository rebuilds legacy requests for new follower rows", async () => { + const kv = new MemoryKvStore(); + const repo = new KvRepository(kv); + const follower = new Person({ + id: new URL("https://example.com/ap/actor/legacy-new-row"), + preferredUsername: "legacy-new-row", + }); + const legacyFollow = new URL( + "https://example.com/ap/follow/legacy-new-row/1", + ); + const newFollow = new URL( + "https://example.com/ap/follow/legacy-new-row/2", + ); + const indexKey: KvKey = [ + "_botkit", + "followRequests", + "followers", + follower.id!.href, + ]; + + await kv.set( + ["_botkit", "followRequests", legacyFollow.href], + follower.id!.href, + ); + await repo.addFollower(newFollow, follower); + + assert.deepStrictEqual( + await repo.removeFollower(newFollow, follower.id!), + undefined, + ); + assert.ok(await repo.hasFollower(follower.id!)); + assert.deepStrictEqual(await kv.get(indexKey), [legacyFollow.href]); +}); + test("KvRepository recovers legacy follower locks", async () => { const kv = new MemoryKvStore(); const repo = new KvRepository(kv); diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index 5a19016..d89efd6 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -550,13 +550,11 @@ export class KvRepository implements Repository { const followerJson = await follower.toJsonLd({ format: "compact" }); await this.withKvLock(this.getFollowersLockKey(), async () => { const followerKey: KvKey = [...this.prefixes.followers, followerId]; - const isNewFollower = await this.kv.get(followerKey) == null; await this.kv.set(followerKey, followerJson); await this.addFollowerIdLocked(followerId); await this.addFollowRequestForFollowerLocked( followerId, followRequestIdString, - isNewFollower, ); await this.kv.set(followRequestKey, followerId); if ( @@ -655,15 +653,11 @@ export class KvRepository implements Repository { private async addFollowRequestForFollowerLocked( followerId: string, followRequestId: string, - isNewFollower = false, ): Promise { const followRequestIds = await this .getIndexedFollowRequestsForFollowerLocked( followerId, - ) ?? - (isNewFollower - ? [] - : await this.rebuildFollowRequestsForFollowerLocked(followerId)); + ) ?? await this.rebuildFollowRequestsForFollowerLocked(followerId); if (!followRequestIds.includes(followRequestId)) { await this.kv.set(this.getFollowerFollowRequestsKey(followerId), [ ...followRequestIds, From b614c2352a2aafe116b36e24a1b3c251bb05b9ce Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 6 Jul 2026 00:40:52 +0900 Subject: [PATCH 20/21] Delay KV lock durations Avoid evaluating Temporal.Duration at repository module load time. Some source-loading environments register the Temporal polyfill after import ordering is already underway, so the lock durations now initialize only when the CAS lock path runs. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525126550 https://github.com/fedify-dev/botkit/pull/26#discussion_r3525126551 Assisted-by: Codex:gpt-5.5 --- packages/botkit/src/repository-load.test.ts | 50 +++++++++++++++++++++ packages/botkit/src/repository.ts | 14 +++--- 2 files changed, 57 insertions(+), 7 deletions(-) create mode 100644 packages/botkit/src/repository-load.test.ts diff --git a/packages/botkit/src/repository-load.test.ts b/packages/botkit/src/repository-load.test.ts new file mode 100644 index 0000000..2f764f5 --- /dev/null +++ b/packages/botkit/src/repository-load.test.ts @@ -0,0 +1,50 @@ +// BotKit by Fedify: A framework for creating ActivityPub bots +// Copyright (C) 2025–2026 Hong Minhee +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . +import assert from "node:assert"; +import { test } from "node:test"; + +test("repository module loads before Temporal is registered", async () => { + if (typeof Deno === "undefined") return; + const repositoryUrl = new URL( + `./repository.ts?no-temporal=${crypto.randomUUID()}`, + import.meta.url, + ).href; + const workerCode = ` + delete (globalThis as Record).Temporal; + try { + await import(${JSON.stringify(repositoryUrl)}); + self.postMessage({ ok: true }); + } catch (error) { + self.postMessage({ + ok: false, + message: error instanceof Error ? error.message : String(error), + }); + } + `; + const worker = new Worker( + `data:application/typescript,${encodeURIComponent(workerCode)}`, + { type: "module" }, + ); + const result = await new Promise((resolve, reject) => { + worker.addEventListener("message", (event: MessageEvent) => { + resolve(event.data); + }, { once: true }); + worker.addEventListener("error", reject, { once: true }); + }); + worker.terminate(); + + assert.deepStrictEqual(result, { ok: true }); +}); diff --git a/packages/botkit/src/repository.ts b/packages/botkit/src/repository.ts index d89efd6..befab6d 100644 --- a/packages/botkit/src/repository.ts +++ b/packages/botkit/src/repository.ts @@ -29,11 +29,7 @@ export type { KvKey, KvStore } from "@fedify/fedify/federation"; export { Announce, Create } from "@fedify/vocab"; const logger = getLogger(["botkit", "repository"]); -const kvLockTtl = Temporal.Duration.from({ minutes: 5 }); const kvLockPollIntervalMs = 100; -const kvLockReleaseTtl = Temporal.Duration.from({ - milliseconds: kvLockPollIntervalMs, -}); const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; @@ -764,14 +760,18 @@ export class KvRepository implements Repository { return await this.withNonCasKvLock(lockKey, operation); } const lock: KvLock = { id: crypto.randomUUID() }; + const lockTtl = Temporal.Duration.from({ minutes: 5 }); + const lockReleaseTtl = Temporal.Duration.from({ + milliseconds: kvLockPollIntervalMs, + }); while (true) { - if (await cas(lockKey, undefined, lock, { ttl: kvLockTtl })) { + if (await cas(lockKey, undefined, lock, { ttl: lockTtl })) { break; } const currentLock = await this.kv.get(lockKey); if ( (isLegacyKvLock(currentLock) || isReleasedKvLock(currentLock)) && - await cas(lockKey, currentLock, lock, { ttl: kvLockTtl }) + await cas(lockKey, currentLock, lock, { ttl: lockTtl }) ) { break; } @@ -784,7 +784,7 @@ export class KvRepository implements Repository { const currentLock = await this.kv.get(lockKey); if (isKvLock(currentLock) && currentLock.id === lock.id) { await cas(lockKey, currentLock, { ...currentLock, released: true }, { - ttl: kvLockReleaseTtl, + ttl: lockReleaseTtl, }); } } catch (error) { From cc0c83b52655bbb3f872f55a892cd0637b73cabe Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 6 Jul 2026 00:50:10 +0900 Subject: [PATCH 21/21] Clarify SQLite follower narrowing Guard the removed follower row explicitly before reading actor_json. This keeps the post-transaction parse path easy for strict null checking to follow without changing the removal behavior. https://github.com/fedify-dev/botkit/pull/26#discussion_r3525165423 Assisted-by: Codex:gpt-5.5 --- packages/botkit-sqlite/src/mod.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/botkit-sqlite/src/mod.ts b/packages/botkit-sqlite/src/mod.ts index 144f996..6804968 100644 --- a/packages/botkit-sqlite/src/mod.ts +++ b/packages/botkit-sqlite/src/mod.ts @@ -457,7 +457,7 @@ export class SqliteRepository implements Repository, Disposable { throw error; } - if (!removed) return undefined; + if (!removed || row == null) return undefined; try { const actorData = JSON.parse(row.actor_json);