From 04aa2fddfedd182c11fac37831b1d3145512f0bf Mon Sep 17 00:00:00 2001 From: Jiwon Kwon Date: Mon, 29 Dec 2025 17:13:30 +0900 Subject: [PATCH 1/4] Fix race conditions in @fedify/relay follower management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced array-based follower storage with individual key-value pairs to eliminate race conditions during concurrent follower operations. Previously, follower list used read-modify-write pattern on a single ["followers"] array, causing data loss when concurrent processes added or removed followers simultaneously. Changes: - Use KvStore.list() for reading followers in builder.ts - Use atomic set() operations for adding followers - Use atomic delete() operations for removing followers - Remove all read-modify-write operations on ["followers"] array - Update tests to match new storage structure Fixes #505 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- packages/relay/src/builder.ts | 11 ++-- packages/relay/src/follow.ts | 8 +-- packages/relay/src/litepub.test.ts | 42 +++++--------- packages/relay/src/litepub.ts | 8 +-- packages/relay/src/mastodon.test.ts | 89 ++++++++++++----------------- packages/relay/src/mastodon.ts | 7 +-- 6 files changed, 58 insertions(+), 107 deletions(-) diff --git a/packages/relay/src/builder.ts b/packages/relay/src/builder.ts index 9ab634ac0..4aa4bed5e 100644 --- a/packages/relay/src/builder.ts +++ b/packages/relay/src/builder.ts @@ -76,19 +76,16 @@ relayBuilder.setActorDispatcher( async function getFollowerActors( ctx: Context, ): Promise { - const followers = await ctx.data.kv.get(["followers"]) ?? []; - const actors: Actor[] = []; - for (const followerId of followers) { - const follower = await ctx.data.kv.get([ - "follower", - followerId, - ]); + + for await (const { value } of ctx.data.kv.list(["follower"])) { + const follower = value as RelayFollower; if (!follower) continue; const actor = await Object.fromJsonLd(follower.actor); if (!isActor(actor)) continue; actors.push(actor); } + return actors; } diff --git a/packages/relay/src/follow.ts b/packages/relay/src/follow.ts index 1f0475278..5094df656 100644 --- a/packages/relay/src/follow.ts +++ b/packages/relay/src/follow.ts @@ -89,13 +89,7 @@ export async function handleUndoFollow( if (activity instanceof Follow) { if (activity.id == null || activity.actorId == null) return; - const followers = await ctx.data.kv.get(["followers"]) ?? []; - const updatedFollowers = followers.filter((id) => - id !== activity.actorId?.href - ); - - await ctx.data.kv.set(["followers"], updatedFollowers); - await ctx.data.kv.delete(["follower", activity.actorId?.href]); + await ctx.data.kv.delete(["follower", activity.actorId.href]); } else { logger.warn( "Unsupported object type ({type}) for Undo activity: {object}", diff --git a/packages/relay/src/litepub.test.ts b/packages/relay/src/litepub.test.ts index 34389d8c1..a5ab56589 100644 --- a/packages/relay/src/litepub.test.ts +++ b/packages/relay/src/litepub.test.ts @@ -191,7 +191,6 @@ describe("LitePubRelay", () => { const follower1Id = "https://remote1.example.com/users/alice"; const follower2Id = "https://remote2.example.com/users/bob"; - await kv.set(["followers"], [follower1Id, follower2Id]); await kv.set( ["follower", follower1Id], { actor: await follower1.toJsonLd(), state: "accepted" }, @@ -364,10 +363,6 @@ describe("LitePubRelay", () => { "https://remote.example.com/users/alice", ]); strictEqual(followerData, undefined); - - // Verify followers list is empty - const followers = await kv.get(["followers"]); - ok(!followers || followers.length === 0); }); test("handles public Follow activity", async () => { @@ -578,12 +573,6 @@ describe("LitePubRelay", () => { ]); ok(followerData); strictEqual((followerData as any).state, "accepted"); - - // Verify follower was added to followers list - const followers = await kv.get(["followers"]); - ok(followers); - strictEqual(followers.length, 1); - strictEqual(followers[0], "https://remote.example.com/users/alice"); }); test("handles Undo Follow activity", async () => { @@ -597,7 +586,6 @@ describe("LitePubRelay", () => { inbox: new URL("https://remote.example.com/users/alice/inbox"), }); - await kv.set(["followers"], [followerId]); await kv.set( ["follower", followerId], { actor: await follower.toJsonLd(), state: "accepted" }, @@ -641,10 +629,6 @@ describe("LitePubRelay", () => { await relay.fetch(request); // Verify follower was removed - const followers = await kv.get(["followers"]); - ok(followers); - strictEqual(followers.length, 0); - const followerData = await kv.get(["follower", followerId]); strictEqual(followerData, undefined); }); @@ -660,7 +644,6 @@ describe("LitePubRelay", () => { inbox: new URL("https://remote.example.com/users/alice/inbox"), }); - await kv.set(["followers"], [followerId]); await kv.set( ["follower", followerId], { actor: await follower.toJsonLd(), state: "accepted" }, @@ -868,29 +851,30 @@ describe("LitePubRelay", () => { const kv = new MemoryKvStore(); // Simulate multiple accepted followers - const followIds = [ + const followerIds = [ "https://remote1.example.com/users/user1", "https://remote2.example.com/users/user2", "https://remote3.example.com/users/user3", ]; - const followers: string[] = []; - for (const followId of followIds) { - followers.push(followId); + for (let i = 0; i < followerIds.length; i++) { + const followerId = followerIds[i]; const actor = new Person({ - id: new URL(followId), - preferredUsername: `user${followers.length}`, - inbox: new URL(`${followId}/inbox`), + id: new URL(followerId), + preferredUsername: `user${i + 1}`, + inbox: new URL(`${followerId}/inbox`), }); await kv.set( - ["follower", followId], + ["follower", followerId], { actor: await actor.toJsonLd(), state: "accepted" }, ); } - await kv.set(["followers"], followers); - const storedFollowers = await kv.get(["followers"]); - ok(storedFollowers); - strictEqual(storedFollowers.length, 3); + // Verify all followers are stored + let count = 0; + for await (const _ of kv.list(["follower"])) { + count++; + } + strictEqual(count, 3); }); }); diff --git a/packages/relay/src/litepub.ts b/packages/relay/src/litepub.ts index a51a77d4f..ef44d7e74 100644 --- a/packages/relay/src/litepub.ts +++ b/packages/relay/src/litepub.ts @@ -126,18 +126,12 @@ export class LitePubRelay extends BaseRelay { ]); if (followerData == null) return; - // Update follower state + // Update follower state to accepted const updatedFollowerData = { ...followerData, state: "accepted" }; await ctx.data.kv.set( ["follower", followerActor.id.href], updatedFollowerData, ); - - // Update followers list - const followers = await ctx.data.kv.get(["followers"]) ?? - []; - followers.push(followerActor.id.href); - await ctx.data.kv.set(["followers"], followers); }) .on( Undo, diff --git a/packages/relay/src/mastodon.test.ts b/packages/relay/src/mastodon.test.ts index 13544a204..bf2fa3bc2 100644 --- a/packages/relay/src/mastodon.test.ts +++ b/packages/relay/src/mastodon.test.ts @@ -190,7 +190,6 @@ describe("MastodonRelay", () => { const follower1Id = "https://remote1.example.com/users/alice"; const follower2Id = "https://remote2.example.com/users/bob"; - await kv.set(["followers"], [follower1Id, follower2Id]); await kv.set( ["follower", follower1Id], { actor: await follower1.toJsonLd(), state: "accepted" }, @@ -228,29 +227,21 @@ describe("MastodonRelay", () => { const kv = new MemoryKvStore(); // Manually simulate what happens when a Follow is approved - const followActivityId = "https://remote.example.com/activities/follow/1"; + const followerId = "https://remote.example.com/users/alice"; const follower = new Person({ - id: new URL("https://remote.example.com/users/alice"), + id: new URL(followerId), preferredUsername: "alice", inbox: new URL("https://remote.example.com/users/alice/inbox"), }); // Simulate the relay's internal logic - const followers = (await kv.get(["followers"])) ?? []; - followers.push(followActivityId); - await kv.set(["followers"], followers); await kv.set( - ["follower", followActivityId], + ["follower", followerId], { actor: await follower.toJsonLd(), state: "accepted" }, ); // Verify storage - const storedFollowers = await kv.get(["followers"]); - ok(storedFollowers); - strictEqual(storedFollowers?.length, 1); - strictEqual(storedFollowers[0], followActivityId); - - const storedActor = await kv.get(["follower", followActivityId]); + const storedActor = await kv.get(["follower", followerId]); ok(storedActor); }); @@ -265,23 +256,15 @@ describe("MastodonRelay", () => { inbox: new URL("https://remote.example.com/users/alice/inbox"), }); - await kv.set(["followers"], [followerId]); await kv.set( ["follower", followerId], { actor: await follower.toJsonLd(), state: "accepted" }, ); // Simulate the Undo Follow logic - const followers = (await kv.get(["followers"])) ?? []; - const updatedFollowers = followers.filter((id) => id !== followerId); - await kv.set(["followers"], updatedFollowers); await kv.delete(["follower", followerId]); // Verify removal - const storedFollowers = await kv.get(["followers"]); - ok(storedFollowers); - strictEqual(storedFollowers.length, 0); - const storedActor = await kv.get(["follower", followerId]); strictEqual(storedActor, undefined); }); @@ -321,30 +304,31 @@ describe("MastodonRelay", () => { const kv = new MemoryKvStore(); // Simulate multiple Follow activities - const followIds = [ + const followerIds = [ "https://remote1.example.com/users/user1", "https://remote2.example.com/users/user2", "https://remote3.example.com/users/user3", ]; - const followers: string[] = []; - for (const followId of followIds) { - followers.push(followId); + for (let i = 0; i < followerIds.length; i++) { + const followerId = followerIds[i]; const actor = new Person({ - id: new URL(followId), - preferredUsername: `user${followers.length}`, - inbox: new URL(`${followId}/inbox`), + id: new URL(followerId), + preferredUsername: `user${i + 1}`, + inbox: new URL(`${followerId}/inbox`), }); await kv.set( - ["follower", followId], + ["follower", followerId], { actor: await actor.toJsonLd(), state: "accepted" }, ); } - await kv.set(["followers"], followers); - const storedFollowers = await kv.get(["followers"]); - ok(storedFollowers); - strictEqual(storedFollowers.length, 3); + // Verify all followers are stored + let count = 0; + for await (const _ of kv.list(["follower"])) { + count++; + } + strictEqual(count, 3); }); test("handles Follow activity with subscription approval", async () => { @@ -399,13 +383,11 @@ describe("MastodonRelay", () => { ok(handlerActor); // Verify follower was stored - const followers = await kv.get(["followers"]); - ok(followers); - strictEqual(followers.length, 1); - strictEqual( - followers[0], + const followerData = await kv.get([ + "follower", "https://remote.example.com/users/alice", - ); + ]); + ok(followerData); }); test("handles Follow activity with subscription rejection", async () => { @@ -452,8 +434,11 @@ describe("MastodonRelay", () => { await relay.fetch(request); // Verify follower was NOT stored - const followers = await kv.get(["followers"]); - ok(!followers || followers.length === 0); + const followerData = await kv.get([ + "follower", + "https://remote.example.com/users/alice", + ]); + strictEqual(followerData, undefined); }); test("handles Undo Follow activity", async () => { @@ -468,7 +453,6 @@ describe("MastodonRelay", () => { }); const followActivityId = "https://remote.example.com/activities/follow/1"; - await kv.set(["followers"], [followerId]); await kv.set( ["follower", followerId], { actor: await follower.toJsonLd(), state: "accepted" }, @@ -512,9 +496,8 @@ describe("MastodonRelay", () => { await relay.fetch(request); // Verify follower was removed - const followers = await kv.get(["followers"]); - ok(followers); - strictEqual(followers.length, 0); + const followerData = await kv.get(["follower", followerId]); + strictEqual(followerData, undefined); }); test("handles Create activity forwarding", async () => { @@ -528,7 +511,6 @@ describe("MastodonRelay", () => { inbox: new URL("https://remote.example.com/users/alice/inbox"), }); - await kv.set(["followers"], [followerId]); await kv.set( ["follower", followerId], { actor: await follower.toJsonLd(), state: "accepted" }, @@ -730,8 +712,11 @@ describe("MastodonRelay", () => { await relay.fetch(request); // Verify follower was NOT stored - const followers = await kv.get(["followers"]); - ok(!followers || followers.length === 0); + const followerData = await kv.get([ + "follower", + "https://remote.example.com/users/alice", + ]); + strictEqual(followerData, undefined); }); test("handles public Follow activity", async () => { @@ -777,8 +762,10 @@ describe("MastodonRelay", () => { await relay.fetch(request); // Verify follower was stored - const followers = await kv.get(["followers"]); - ok(followers); - strictEqual(followers.length, 1); + const followerData = await kv.get([ + "follower", + "https://remote.example.com/users/alice", + ]); + ok(followerData); }); }); diff --git a/packages/relay/src/mastodon.ts b/packages/relay/src/mastodon.ts index a27a8060f..c4d19d35e 100644 --- a/packages/relay/src/mastodon.ts +++ b/packages/relay/src/mastodon.ts @@ -58,12 +58,7 @@ export class MastodonRelay extends BaseRelay { } if (approved) { - // Mastodon-specific: immediately add to followers list - const followers = await ctx.data.kv.get(["followers"]) ?? - []; - followers.push(follower.id.href); - await ctx.data.kv.set(["followers"], followers); - + // Mastodon-specific: immediately add to followers list with accepted state await ctx.data.kv.set( ["follower", follower.id.href], { actor: await follower.toJsonLd(), state: "accepted" }, From 7dbe704ece112c8e833dc42cee4f0a7f60b5d01d Mon Sep 17 00:00:00 2001 From: Jiwon Kwon Date: Mon, 29 Dec 2025 17:26:57 +0900 Subject: [PATCH 2/4] Add comprehensive test coverage for KvStore.list() in relay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added test cases to verify list() behavior across all scenarios: - Empty list on initialization - Multiple follower additions and retrieval - Correct exclusion after deletions - Complete actor data preservation - State filtering (pending vs accepted for LitePub) These tests ensure the race condition fix using individual key-value pairs works correctly and list() properly replaces the array-based approach. Related to #505 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- packages/relay/src/litepub.test.ts | 162 ++++++++++++++++++++++++++++ packages/relay/src/mastodon.test.ts | 112 +++++++++++++++++++ 2 files changed, 274 insertions(+) diff --git a/packages/relay/src/litepub.test.ts b/packages/relay/src/litepub.test.ts index a5ab56589..a971f8333 100644 --- a/packages/relay/src/litepub.test.ts +++ b/packages/relay/src/litepub.test.ts @@ -877,4 +877,166 @@ describe("LitePubRelay", () => { } strictEqual(count, 3); }); + + test("list() returns empty when no followers exist", async () => { + const kv = new MemoryKvStore(); + + // Verify list is initially empty + let count = 0; + for await (const _ of kv.list(["follower"])) { + count++; + } + strictEqual(count, 0); + }); + + test("list() returns all followers after additions", async () => { + const kv = new MemoryKvStore(); + + // Add multiple followers + const followerIds = [ + "https://server1.example.com/users/alice", + "https://server2.example.com/users/bob", + "https://server3.example.com/users/carol", + ]; + + for (const followerId of followerIds) { + const actor = new Person({ + id: new URL(followerId), + preferredUsername: followerId.split("/").pop(), + inbox: new URL(`${followerId}/inbox`), + }); + await kv.set( + ["follower", followerId], + { actor: await actor.toJsonLd(), state: "accepted" }, + ); + } + + // Verify list returns all followers + const retrievedIds: string[] = []; + for await (const { key, value } of kv.list(["follower"])) { + strictEqual(key.length, 2); + strictEqual(key[0], "follower"); + retrievedIds.push(key[1] as string); + ok(value); + strictEqual((value as any).state, "accepted"); + } + + strictEqual(retrievedIds.length, 3); + for (const id of followerIds) { + ok(retrievedIds.includes(id)); + } + }); + + test("list() excludes followers after deletion", async () => { + const kv = new MemoryKvStore(); + + // Add three followers + const follower1Id = "https://server1.example.com/users/alice"; + const follower2Id = "https://server2.example.com/users/bob"; + const follower3Id = "https://server3.example.com/users/carol"; + + for (const followerId of [follower1Id, follower2Id, follower3Id]) { + const actor = new Person({ + id: new URL(followerId), + preferredUsername: followerId.split("/").pop(), + inbox: new URL(`${followerId}/inbox`), + }); + await kv.set( + ["follower", followerId], + { actor: await actor.toJsonLd(), state: "accepted" }, + ); + } + + // Delete one follower + await kv.delete(["follower", follower2Id]); + + // Verify list only returns remaining followers + const retrievedIds: string[] = []; + for await (const { key } of kv.list(["follower"])) { + retrievedIds.push(key[1] as string); + } + + strictEqual(retrievedIds.length, 2); + ok(retrievedIds.includes(follower1Id)); + ok(!retrievedIds.includes(follower2Id)); // Deleted follower not in list + ok(retrievedIds.includes(follower3Id)); + }); + + test("list() distinguishes between pending and accepted followers", async () => { + const kv = new MemoryKvStore(); + + // Add followers with different states + const pendingFollowerId = "https://server1.example.com/users/alice"; + const acceptedFollowerId = "https://server2.example.com/users/bob"; + + const pendingFollower = new Person({ + id: new URL(pendingFollowerId), + preferredUsername: "alice", + inbox: new URL("https://server1.example.com/users/alice/inbox"), + }); + + const acceptedFollower = new Person({ + id: new URL(acceptedFollowerId), + preferredUsername: "bob", + inbox: new URL("https://server2.example.com/users/bob/inbox"), + }); + + await kv.set( + ["follower", pendingFollowerId], + { actor: await pendingFollower.toJsonLd(), state: "pending" }, + ); + + await kv.set( + ["follower", acceptedFollowerId], + { actor: await acceptedFollower.toJsonLd(), state: "accepted" }, + ); + + // Verify list returns both with correct states + const followers: { id: string; state: string }[] = []; + for await (const { key, value } of kv.list(["follower"])) { + const followerData = value as any; + followers.push({ + id: key[1] as string, + state: followerData.state, + }); + } + + strictEqual(followers.length, 2); + + const pendingEntry = followers.find((f) => f.id === pendingFollowerId); + ok(pendingEntry); + strictEqual(pendingEntry.state, "pending"); + + const acceptedEntry = followers.find((f) => f.id === acceptedFollowerId); + ok(acceptedEntry); + strictEqual(acceptedEntry.state, "accepted"); + }); + + test("list() returns correct actor data", async () => { + const kv = new MemoryKvStore(); + + const followerId = "https://remote.example.com/users/alice"; + const follower = new Person({ + id: new URL(followerId), + preferredUsername: "alice", + name: "Alice Wonderland", + inbox: new URL("https://remote.example.com/users/alice/inbox"), + }); + + await kv.set( + ["follower", followerId], + { actor: await follower.toJsonLd(), state: "accepted" }, + ); + + // Verify list returns complete actor data + for await (const { key, value } of kv.list(["follower"])) { + strictEqual(key[1], followerId); + ok(value); + const followerData = value as any; + strictEqual(followerData.state, "accepted"); + ok(followerData.actor); + strictEqual(followerData.actor.preferredUsername, "alice"); + strictEqual(followerData.actor.name, "Alice Wonderland"); + } + }); }); diff --git a/packages/relay/src/mastodon.test.ts b/packages/relay/src/mastodon.test.ts index bf2fa3bc2..b5328d49a 100644 --- a/packages/relay/src/mastodon.test.ts +++ b/packages/relay/src/mastodon.test.ts @@ -768,4 +768,116 @@ describe("MastodonRelay", () => { ]); ok(followerData); }); + + test("list() returns empty when no followers exist", async () => { + const kv = new MemoryKvStore(); + + // Verify list is initially empty + let count = 0; + for await (const _ of kv.list(["follower"])) { + count++; + } + strictEqual(count, 0); + }); + + test("list() returns all followers after additions", async () => { + const kv = new MemoryKvStore(); + + // Add multiple followers + const followerIds = [ + "https://server1.example.com/users/alice", + "https://server2.example.com/users/bob", + "https://server3.example.com/users/carol", + ]; + + for (const followerId of followerIds) { + const actor = new Person({ + id: new URL(followerId), + preferredUsername: followerId.split("/").pop(), + inbox: new URL(`${followerId}/inbox`), + }); + await kv.set( + ["follower", followerId], + { actor: await actor.toJsonLd(), state: "accepted" }, + ); + } + + // Verify list returns all followers + const retrievedIds: string[] = []; + for await (const { key, value } of kv.list(["follower"])) { + strictEqual(key.length, 2); + strictEqual(key[0], "follower"); + retrievedIds.push(key[1] as string); + ok(value); + strictEqual((value as any).state, "accepted"); + } + + strictEqual(retrievedIds.length, 3); + for (const id of followerIds) { + ok(retrievedIds.includes(id)); + } + }); + + test("list() excludes followers after deletion", async () => { + const kv = new MemoryKvStore(); + + // Add three followers + const follower1Id = "https://server1.example.com/users/alice"; + const follower2Id = "https://server2.example.com/users/bob"; + const follower3Id = "https://server3.example.com/users/carol"; + + for (const followerId of [follower1Id, follower2Id, follower3Id]) { + const actor = new Person({ + id: new URL(followerId), + preferredUsername: followerId.split("/").pop(), + inbox: new URL(`${followerId}/inbox`), + }); + await kv.set( + ["follower", followerId], + { actor: await actor.toJsonLd(), state: "accepted" }, + ); + } + + // Delete one follower + await kv.delete(["follower", follower2Id]); + + // Verify list only returns remaining followers + const retrievedIds: string[] = []; + for await (const { key } of kv.list(["follower"])) { + retrievedIds.push(key[1] as string); + } + + strictEqual(retrievedIds.length, 2); + ok(retrievedIds.includes(follower1Id)); + ok(!retrievedIds.includes(follower2Id)); // Deleted follower not in list + ok(retrievedIds.includes(follower3Id)); + }); + + test("list() returns correct actor data", async () => { + const kv = new MemoryKvStore(); + + const followerId = "https://remote.example.com/users/alice"; + const follower = new Person({ + id: new URL(followerId), + preferredUsername: "alice", + name: "Alice Wonderland", + inbox: new URL("https://remote.example.com/users/alice/inbox"), + }); + + await kv.set( + ["follower", followerId], + { actor: await follower.toJsonLd(), state: "accepted" }, + ); + + // Verify list returns complete actor data + for await (const { key, value } of kv.list(["follower"])) { + strictEqual(key[1], followerId); + ok(value); + const followerData = value as any; + strictEqual(followerData.state, "accepted"); + ok(followerData.actor); + strictEqual(followerData.actor.preferredUsername, "alice"); + strictEqual(followerData.actor.name, "Alice Wonderland"); + } + }); }); From 7e97abc79014b1a05c1934242bb15af2f7cf2c4f Mon Sep 17 00:00:00 2001 From: Jiwon Kwon Date: Mon, 29 Dec 2025 17:52:55 +0900 Subject: [PATCH 3/4] Replace type casting with type predicate in relay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces unsafe type casting with a type predicate function following Fedify's established patterns (similar to isActor()). Changes: - Add isRelayFollower() type predicate in types.ts - Replace "value as RelayFollower" cast with isRelayFollower(value) in builder.ts - Remove unused RelayFollower type import Benefits: - Compile-time type narrowing after guard check - Runtime validation of KV store data - Consistent with Fedify's type-safe design philosophy - No unsafe type assertions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- packages/relay/src/builder.ts | 7 +++---- packages/relay/src/types.ts | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/relay/src/builder.ts b/packages/relay/src/builder.ts index 4aa4bed5e..67676281c 100644 --- a/packages/relay/src/builder.ts +++ b/packages/relay/src/builder.ts @@ -9,8 +9,8 @@ import { import { Application, isActor, Object } from "@fedify/fedify/vocab"; import type { Actor } from "@fedify/fedify/vocab"; import { + isRelayFollower, RELAY_SERVER_ACTOR, - type RelayFollower, type RelayOptions, } from "./types.ts"; @@ -79,9 +79,8 @@ async function getFollowerActors( const actors: Actor[] = []; for await (const { value } of ctx.data.kv.list(["follower"])) { - const follower = value as RelayFollower; - if (!follower) continue; - const actor = await Object.fromJsonLd(follower.actor); + if (!isRelayFollower(value)) continue; + const actor = await Object.fromJsonLd(value.actor); if (!isActor(actor)) continue; actors.push(actor); } diff --git a/packages/relay/src/types.ts b/packages/relay/src/types.ts index aba752155..0d411ede6 100644 --- a/packages/relay/src/types.ts +++ b/packages/relay/src/types.ts @@ -37,3 +37,21 @@ export interface RelayFollower { readonly actor: unknown; readonly state: "pending" | "accepted"; } + +/** + * Type predicate to check if a value is a valid RelayFollower. + * Provides both runtime validation and compile-time type narrowing. + * + * @param value The value to check + * @returns true if the value is a RelayFollower + */ +export function isRelayFollower(value: unknown): value is RelayFollower { + if (!value || typeof value !== "object") return false; + const obj = value as Record; + return ( + "actor" in obj && + "state" in obj && + typeof obj.state === "string" && + (obj.state === "pending" || obj.state === "accepted") + ); +} From 875edd8288115453acf89eea6521f5ee747897db Mon Sep 17 00:00:00 2001 From: Jiwon Kwon Date: Mon, 29 Dec 2025 18:17:28 +0900 Subject: [PATCH 4/4] Export isRelayFollower and replace type casts in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves type safety by making isRelayFollower part of the public API and replacing unsafe type assertions with proper type guards in tests. Changes: - Export isRelayFollower from mod.ts for public use - Add state filtering in builder.ts (only return accepted followers) - Replace RelayFollower "as any" casts with isRelayFollower guards in tests - Use Record for actor properties instead of "as any" Benefits: - Public API now includes type predicate for RelayFollower validation - Test code has compile-time type safety - Consistent with Fedify's pattern of exporting type predicates - Runtime validation ensures data integrity from KV store 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- packages/relay/src/builder.ts | 1 + packages/relay/src/litepub.test.ts | 34 ++++++++++++++--------------- packages/relay/src/mastodon.test.ts | 18 +++++++-------- packages/relay/src/mod.ts | 1 + 4 files changed, 28 insertions(+), 26 deletions(-) diff --git a/packages/relay/src/builder.ts b/packages/relay/src/builder.ts index 67676281c..0731db064 100644 --- a/packages/relay/src/builder.ts +++ b/packages/relay/src/builder.ts @@ -80,6 +80,7 @@ async function getFollowerActors( for await (const { value } of ctx.data.kv.list(["follower"])) { if (!isRelayFollower(value)) continue; + if (value.state !== "accepted") continue; const actor = await Object.fromJsonLd(value.actor); if (!isActor(actor)) continue; actors.push(actor); diff --git a/packages/relay/src/litepub.test.ts b/packages/relay/src/litepub.test.ts index a971f8333..91adb220e 100644 --- a/packages/relay/src/litepub.test.ts +++ b/packages/relay/src/litepub.test.ts @@ -19,7 +19,7 @@ import { } from "@fedify/vocab-runtime"; import { ok, strictEqual } from "node:assert"; import test, { describe } from "node:test"; -import { createRelay, type RelayOptions } from "@fedify/relay"; +import { createRelay, isRelayFollower, type RelayOptions } from "@fedify/relay"; // Simple mock document loader that returns a minimal context const mockDocumentLoader = async (url: string): Promise => { @@ -310,8 +310,8 @@ describe("LitePubRelay", () => { "follower", "https://remote.example.com/users/alice", ]); - ok(followerData); - strictEqual((followerData as any).state, "pending"); + ok(isRelayFollower(followerData)); + strictEqual(followerData.state, "pending"); }); test("handles Follow activity with subscription rejection", async () => { @@ -412,8 +412,8 @@ describe("LitePubRelay", () => { "follower", "https://remote.example.com/users/alice", ]); - ok(followerData); - strictEqual((followerData as any).state, "pending"); + ok(isRelayFollower(followerData)); + strictEqual(followerData.state, "pending"); }); test("ignores Follow activity without required fields", async () => { @@ -571,8 +571,8 @@ describe("LitePubRelay", () => { "follower", "https://remote.example.com/users/alice", ]); - ok(followerData); - strictEqual((followerData as any).state, "accepted"); + ok(isRelayFollower(followerData)); + strictEqual(followerData.state, "accepted"); }); test("handles Undo Follow activity", async () => { @@ -917,8 +917,8 @@ describe("LitePubRelay", () => { strictEqual(key.length, 2); strictEqual(key[0], "follower"); retrievedIds.push(key[1] as string); - ok(value); - strictEqual((value as any).state, "accepted"); + ok(isRelayFollower(value)); + strictEqual(value.state, "accepted"); } strictEqual(retrievedIds.length, 3); @@ -994,10 +994,10 @@ describe("LitePubRelay", () => { // Verify list returns both with correct states const followers: { id: string; state: string }[] = []; for await (const { key, value } of kv.list(["follower"])) { - const followerData = value as any; + if (!isRelayFollower(value)) continue; followers.push({ id: key[1] as string, - state: followerData.state, + state: value.state, }); } @@ -1031,12 +1031,12 @@ describe("LitePubRelay", () => { // Verify list returns complete actor data for await (const { key, value } of kv.list(["follower"])) { strictEqual(key[1], followerId); - ok(value); - const followerData = value as any; - strictEqual(followerData.state, "accepted"); - ok(followerData.actor); - strictEqual(followerData.actor.preferredUsername, "alice"); - strictEqual(followerData.actor.name, "Alice Wonderland"); + ok(isRelayFollower(value)); + strictEqual(value.state, "accepted"); + ok(value.actor && typeof value.actor === "object"); + const actor = value.actor as Record; + strictEqual(actor.preferredUsername, "alice"); + strictEqual(actor.name, "Alice Wonderland"); } }); }); diff --git a/packages/relay/src/mastodon.test.ts b/packages/relay/src/mastodon.test.ts index b5328d49a..1f138807d 100644 --- a/packages/relay/src/mastodon.test.ts +++ b/packages/relay/src/mastodon.test.ts @@ -17,7 +17,7 @@ import { } from "@fedify/vocab-runtime"; import { ok, strictEqual } from "node:assert"; import test, { describe } from "node:test"; -import { createRelay, type RelayOptions } from "@fedify/relay"; +import { createRelay, isRelayFollower, type RelayOptions } from "@fedify/relay"; // Simple mock document loader that returns a minimal context const mockDocumentLoader = async (url: string): Promise => { @@ -808,8 +808,8 @@ describe("MastodonRelay", () => { strictEqual(key.length, 2); strictEqual(key[0], "follower"); retrievedIds.push(key[1] as string); - ok(value); - strictEqual((value as any).state, "accepted"); + ok(isRelayFollower(value)); + strictEqual(value.state, "accepted"); } strictEqual(retrievedIds.length, 3); @@ -872,12 +872,12 @@ describe("MastodonRelay", () => { // Verify list returns complete actor data for await (const { key, value } of kv.list(["follower"])) { strictEqual(key[1], followerId); - ok(value); - const followerData = value as any; - strictEqual(followerData.state, "accepted"); - ok(followerData.actor); - strictEqual(followerData.actor.preferredUsername, "alice"); - strictEqual(followerData.actor.name, "Alice Wonderland"); + ok(isRelayFollower(value)); + strictEqual(value.state, "accepted"); + ok(value.actor && typeof value.actor === "object"); + const actor = value.actor as Record; + strictEqual(actor.preferredUsername, "alice"); + strictEqual(actor.name, "Alice Wonderland"); } }); }); diff --git a/packages/relay/src/mod.ts b/packages/relay/src/mod.ts index dcdf5b9da..aa9b45dff 100644 --- a/packages/relay/src/mod.ts +++ b/packages/relay/src/mod.ts @@ -12,6 +12,7 @@ export { createRelay } from "./factory.ts"; export { LitePubRelay } from "./litepub.ts"; export { MastodonRelay } from "./mastodon.ts"; export { + isRelayFollower, RELAY_SERVER_ACTOR, type RelayFollower, type RelayOptions,