Consolidate relay protocol handling - #984
Conversation
Make protocol expectations explicit before consolidating shared relay behavior. Verify that Mastodon immediately accepts approved followers and that LitePub Undo removes pending and accepted followers. Changelog: none Assisted-by: Codex:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-sol
Move shared follow, undo, listener registration, and forwarding behavior into BaseRelay so protocol-specific differences remain explicit in the Mastodon and LitePub implementations. fedify-dev#905 Changelog: none Assisted-by: Codex:gpt-5.6-sol
✅ Deploy Preview for fedify-json-schema canceled.
|
📝 WalkthroughWalkthroughThe relay package centralizes follow, undo, and activity inbox handling in ChangesRelay behavior consolidation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/relay/src/base.ts`:
- Around line 203-210: Update `#relayActivity` to use activity.actorId directly
when constructing excludeBaseUris instead of awaiting activity.getActor(ctx).
Preserve the null/undefined case as an empty exclusion list, and continue
passing the resulting origins to deliverActivity.
In `@packages/relay/src/litepub.ts`:
- Around line 96-97: Update the follower actor lookup in the surrounding relay
handler to pass the existing request context to accept.getActor, matching the
context-aware getObject and BaseRelay.#relayActivity calls. Preserve the
existing actor validation and early return behavior.
- Around line 102-113: Update the follower transition around followerData to
import and call isRelayFollowerData, returning without writing when the stored
value is invalid. Annotate updatedFollowerData as RelayFollowerData so the
accepted state and required actor fields are compiler-validated, then persist it
through the existing kv.set call.
- Around line 28-41: Update shouldSkipFollow to return true whenever an existing
follower record is found, regardless of its state, while retaining the early
return for followers without an id. This prevents accepted or pending followers
from reaching `#handleFollow` and being recreated or followed again.
In `@packages/relay/src/mastodon.test.ts`:
- Around line 693-730: Extend the “handles Announce activity forwarding” test by
registering an accepted follower in the MemoryKvStore under the Announce actor’s
follower key, then intercepting outbound delivery to that follower’s inbox and
asserting it contains the original Announce payload. Update the response status
assertion to include the actual status value in its failure message, while
retaining the existing accepted-status condition.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0137a4fd-e31a-42aa-afef-08d601dbde8c
📒 Files selected for processing (6)
packages/relay/src/base.tspackages/relay/src/litepub.test.tspackages/relay/src/litepub.tspackages/relay/src/mastodon.test.tspackages/relay/src/mastodon.tspackages/relay/src/types.ts
| async #relayActivity( | ||
| ctx: InboxContext<RelayOptions>, | ||
| activity: RelayableActivity, | ||
| ): Promise<void> { | ||
| const sender = await activity.getActor(ctx); | ||
| const excludeBaseUris = sender?.id == null ? [] : [new URL(sender.id)]; | ||
| await this.deliverActivity(ctx, activity, excludeBaseUris); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm `actorId` is a supported accessor on Activity and check existing usage patterns for excludeBaseUris.
set -euo pipefail
# Find the actorId accessor definition on the Activity class.
fd -t f 'vocab.ts$' packages/vocab/src | head -n 5
rg -nP -C4 '\bget\s+actorId\b' packages/vocab/src | head -n 40
# Compare with other excludeBaseUris call sites in the repo.
rg -nP -C6 '\bexcludeBaseUris\b' --type=ts -g '!**/*.test.ts' | head -n 80Repository: fedify-dev/fedify
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -u
echo "Repository top-level files:"
git ls-files | sed -n '1,80p'
echo
echo "Find base.ts and relevant packages:"
fd -t f 'base\.ts$' . | sed -n '1,80p'
echo
echo "Search excludeBaseUris:"
rg -nP -C5 '\bexcludeBaseUris\b' . --glob '*.ts' || true
echo
echo "Search actorId and getActor:"
rg -nP -C3 '\b(actorId|getActor)\b' . --glob '*.ts' | sed -n '1,220p'Repository: fedify-dev/fedify
Length of output: 29175
🏁 Script executed:
#!/bin/bash
set -u
echo "packages/relay/src/base.ts outline:"
ast-grep outline packages/relay/src/base.ts || true
echo
echo "packages/relay/src/base.ts relevant lines:"
sed -n '1,250p' packages/relay/src/base.ts | nl -ba
echo
echo "Find Activity/getActor type declarations:"
rg -nP -C3 '\bclass\s+Activity\b|\bgetActor\b|getActor\(.*ctx' packages -g '*.ts' | sed -n '1,220p'Repository: fedify-dev/fedify
Length of output: 15965
🏁 Script executed:
#!/bin/bash
set -u
echo "Find vocab getActor implementation:"
rg -nP -C12 '\bgetActor\b' packages/vocab packages -g '*.ts' | sed -n '1,240p'
echo
echo "Inspect packages/vocab/src files containing method patterns:"
fd -t f '\.ts$' packages/vocab/src | xargs -r rg -n "actorId|getActor|getObject" | sed -n '1,240p'Repository: fedify-dev/fedify
Length of output: 23500
🏁 Script executed:
#!/bin/bash
set -u
echo "Find files with getActor implementation inside vocab source:"
rg -l '\bclass\s+Activity\b|async\s+getActor\b|getActor\(' packages/vocab/src packages -g '*.ts' | sed -n '1,80p'
echo
echo "Search Activity.getActor implementation in all tracked TS files with line numbers:"
rg -nP -C20 '(^|[^A-Za-z0-9_])getActor[:(]|async\s+getActor\b' packages -g '*.ts' | sed -n '1,280p'
echo
echo "Find declaration files mentioning ActorId/actorId for Activity:"
rg -nP -C8 'actorId\?:|actorId\(|`@property`\(["'\'']actor["'\'']\)|actorId' packages/vocab packages/fedify -g '*.ts' | sed -n '1,220p'Repository: fedify-dev/fedify
Length of output: 34712
🌐 Web query:
Fedify Activity actorId getter getActor context document loader implementation
💡 Result:
In Fedify, the retrieval of an actor from an Activity object or a context is handled via dedicated methods that support document loaders for secure, authenticated, or custom fetching [1][2][3]. Actor Identification and Retrieval 1. Activity.actorId: This is a property of an Activity object that returns the URI (URL) of the actor [4][5]. It does not perform network requests and is used for quick identification [5]. 2. Activity.getActor: This is a dereferencing accessor method [1]. When called, it fetches the remote actor object from the URI if it is not already cached, effectively hydrating the object [5][6]. It accepts an options object to specify a document loader and a context loader, allowing for authenticated or customized network requests [4][1]. Contextual Document Loading The Context object in Fedify manages the document loader, which is essential for fetching remote JSON-LD documents [3][7]. - Default Loader: The Context.documentLoader property holds the default, unauthenticated document loader configured for your federation instance [2][3]. - Authenticated Loader: When you need to access protected resources (such as followers-only content), you should use the Context.getDocumentLoader method [2][3]. This creates an authenticated document loader, which Fedify then uses to sign requests with the appropriate HTTP Signature [2]. - Implementation Pattern: When using methods like getActor, lookupObject, or various collection accessors, you can pass the document loader via the options object [2][3]: const loader = await ctx.getDocumentLoader({ identifier: "alice" }); const actor = await activity.getActor({ documentLoader: loader }); Document Loader vs. Context Loader - Document Loader: Primarily used for fetching remote resources such as actor profiles or object documents [3][7]. - Context Loader: Used for fetching JSON-LD contexts (e.g., ActivityStreams or W3C security contexts) [3][7]. It is typically cached heavily and generally does not require authentication [7]. When using Fedify's vocabulary API, you can often pass the Context object directly as an option, as it is compatible with the expected { documentLoader, contextLoader } structure [3][7].
Citations:
- 1: https://jsr.io/@fedify/vocab/doc/~/Activity.prototype.getActor
- 2: https://fedify.dev/manual/context-advanced
- 3: https://fedify.dev/manual/context
- 4: https://jsr.io/@fedify/vocab/doc/~/Activity
- 5: https://unstable.fedify.dev/
- 6: https://fedify.dev/manual/vocab
- 7: https://unstable.fedify.dev/manual/context
🌐 Web query:
Fedify Activity.getActor implementation "Activity.prototype.getActor"
💡 Result:
In Fedify, there is a distinction between Activity.getActor (a vocabulary/data-level method) and RequestContext.getActor (a framework/routing-level method). Activity.getActor is a method on instances of the Activity class within the @fedify/vocab package [1][2]. It is a dereferencing accessor that fetches and hydrates the actor object associated with an activity [3][4]. When called, it resolves the actor's URI—fetching the remote object over the network if it is not already available—and returns a typed actor object (e.g., Person, Group, or Service) or null [5][4]. For performance, if you only need the actor's URI without fetching the remote resource, you can use the Activity.actorId property [3][4]. In contrast, RequestContext.getActor is a method provided by the Fedify framework's RequestContext [6]. This method is used within the server's request handling logic to dispatch an actor based on an identifier registered via the federation's actor dispatcher [6][7]. It allows the application to retrieve a local actor object from your database or application logic, rather than fetching remote data [6][7]. Key differences: - Activity.getActor: Part of the vocabulary data model; used for dereferencing/fetching remote actor data from an ActivityStreams object [3][4]. - RequestContext.getActor: Part of the server framework; used to look up local actors based on route parameters or identifiers using your defined dispatcher [6][7].
Citations:
- 1: https://jsr.io/@fedify/vocab/doc/~/Activity
- 2: https://jsr.io/@fedify/vocab/doc/~/Activity.prototype.getActor
- 3: https://unstable.fedify.dev/manual/vocab
- 4: https://unstable.fedify.dev/
- 5: https://jsr.io/@fedify/vocab/doc/all_symbols
- 6: https://fedify.dev/manual/context
- 7: https://fedify.dev/manual/actor
Use activity.actorId instead of fetching the sender actor.
activity.getActor(ctx) dereferences the actor URI through Fedify’s document loader, which adds a request per relayed activity. This relay path only needs the activity sender URI to build excludeBaseUris, and ctx.sendActivity matches excludeBaseUris by origin.
If the actor document is unavailable or getActor(ctx) returns null, the relay can re-forward the activity to the originating server. Use activity.actorId for the exclusion origin, preserving the optional null case with an empty exclusion list.
♻️ Proposed refactor
async `#relayActivity`(
ctx: InboxContext<RelayOptions>,
activity: RelayableActivity,
): Promise<void> {
- const sender = await activity.getActor(ctx);
- const excludeBaseUris = sender?.id == null ? [] ; [new URL(sender.id)];
+ const senderId = activity.actorId;
+ const excludeBaseUris = senderId == null ? [] : [senderId];
await this.deliverActivity(ctx, activity, excludeBaseUris);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/relay/src/base.ts` around lines 203 - 210, Update `#relayActivity` to
use activity.actorId directly when constructing excludeBaseUris instead of
awaiting activity.getActor(ctx). Preserve the null/undefined case as an empty
exclusion list, and continue passing the resulting origins to deliverActivity.
| protected readonly initialFollowerState = "pending"; | ||
| protected readonly logger = logger; | ||
|
|
||
| protected override async shouldSkipFollow( | ||
| ctx: InboxContext<RelayOptions>, | ||
| follower: Actor, | ||
| ): Promise<boolean> { | ||
| if (follower.id == null) return true; | ||
| const existingFollow = await ctx.data.kv.get<RelayFollowerData>([ | ||
| "follower", | ||
| follower.id.href, | ||
| ]); | ||
| return existingFollow?.state === "pending"; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the relay followers collection dispatcher for state filtering.
set -euo pipefail
fd -t f -e ts . packages/relay/src
# Find the followers collection dispatcher and any state filtering.
rg -nP -C10 'setFollowersDispatcher|followers' --type=ts -g '!**/*.test.ts' packages/relay/src
# Check every read of the stored follower state.
rg -nP -C4 "state\s*===|\.state\b" --type=ts -g '!**/*.test.ts' packages/relay/srcRepository: fedify-dev/fedify
Length of output: 10719
🏁 Script executed:
#!/bin/bash
# Inspect BaseRelay follow handling and exact followers collection behavior.
set -euo pipefail
wc -l packages/relay/src/base.ts packages/relay/src/litepub.ts packages/relay/src/builder.ts
echo '--- base.ts relevant sections ---'
rg -n 'class BaseRelay|shouldSkipFollow|handleFollow|afterFollowApproved|deliverActivity|followers|announce' packages/relay/src/base.ts packages/relay/src/litepub.ts -C 8
echo '--- builder.ts relevant section ---'
sed -n '60,110p' packages/relay/src/builder.tsRepository: fedify-dev/fedify
Length of output: 13149
Keep repeated Follows idempotent for accepted followers.
shouldSkipFollow only skips followers in "pending" state. For an accepted follower, a repeated Follow reaches #handleFollow, overwrites state with this.initialFollowerState ("pending" for LitePub), and sends another reciprocal Follow. Skip the follow when a follower record already exists.
🐛 Proposed fix
protected override async shouldSkipFollow(
ctx: InboxContext<RelayOptions>,
follower: Actor,
): Promise<boolean> {
if (follower.id == null) return true;
const existingFollow = await ctx.data.kv.get<RelayFollowerData>([
"follower",
follower.id.href,
]);
- return existingFollow?.state === "pending";
+ return existingFollow != null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected readonly initialFollowerState = "pending"; | |
| protected readonly logger = logger; | |
| protected override async shouldSkipFollow( | |
| ctx: InboxContext<RelayOptions>, | |
| follower: Actor, | |
| ): Promise<boolean> { | |
| if (follower.id == null) return true; | |
| const existingFollow = await ctx.data.kv.get<RelayFollowerData>([ | |
| "follower", | |
| follower.id.href, | |
| ]); | |
| return existingFollow?.state === "pending"; | |
| } | |
| protected readonly initialFollowerState = "pending"; | |
| protected readonly logger = logger; | |
| protected override async shouldSkipFollow( | |
| ctx: InboxContext<RelayOptions>, | |
| follower: Actor, | |
| ): Promise<boolean> { | |
| if (follower.id == null) return true; | |
| const existingFollow = await ctx.data.kv.get<RelayFollowerData>([ | |
| "follower", | |
| follower.id.href, | |
| ]); | |
| return existingFollow != null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/relay/src/litepub.ts` around lines 28 - 41, Update shouldSkipFollow
to return true whenever an existing follower record is found, regardless of its
state, while retaining the early return for followers without an id. This
prevents accepted or pending followers from reaching `#handleFollow` and being
recreated or followed again.
| const followerActor = await accept.getActor(); | ||
| if (!isActor(followerActor) || !followerActor.id) return; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pass the context to getActor().
Line 96 calls accept.getActor() with no arguments. The lookup then uses the default document loader. It ignores documentLoaderFactory and authenticatedDocumentLoaderFactory from RelayOptions.
Line 87 passes the context to getObject, and BaseRelay.#relayActivity passes ctx to getActor. Use the same pattern here.
If the actor is embedded in the Accept payload, no fetch occurs and the defect stays hidden. If the actor is a bare URI, the relay dereferences it with the wrong loader. Instances that require signed fetches then reject the request, and the follower never reaches the "accepted" state.
🐛 Proposed fix
// Validate follower actor - accept activity sender
- const followerActor = await accept.getActor();
+ const followerActor = await accept.getActor(ctx);
if (!isActor(followerActor) || !followerActor.id) return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const followerActor = await accept.getActor(); | |
| if (!isActor(followerActor) || !followerActor.id) return; | |
| const followerActor = await accept.getActor(ctx); | |
| if (!isActor(followerActor) || !followerActor.id) return; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/relay/src/litepub.ts` around lines 96 - 97, Update the follower
actor lookup in the surrounding relay handler to pass the existing request
context to accept.getActor, matching the context-aware getObject and
BaseRelay.#relayActivity calls. Preserve the existing actor validation and early
return behavior.
| const followerData = await ctx.data.kv.get([ | ||
| "follower", | ||
| followerActor.id.href, | ||
| ]); | ||
| if (followerData == null) return; | ||
|
|
||
| // Litepub-specific: send reciprocal follow | ||
| const relayActorUri = ctx.getActorUri(RELAY_SERVER_ACTOR); | ||
| await ctx.sendActivity( | ||
| { identifier: RELAY_SERVER_ACTOR }, | ||
| follower, | ||
| new Follow({ | ||
| actor: relayActorUri, | ||
| object: follower.id, | ||
| to: follower.id, | ||
| }), | ||
| ); | ||
| } else { | ||
| await sendFollowResponse(ctx, follow, follower, approved); | ||
| } | ||
| }) | ||
| .on(Accept, async (ctx, accept) => { | ||
| // Validate follow activity from accept activity | ||
| const follow = await accept.getObject({ | ||
| crossOrigin: "trust", | ||
| ...ctx, | ||
| }); | ||
| if (!(follow instanceof Follow)) return; | ||
| const relayActorId = follow.actorId; | ||
| if (relayActorId == null) return; | ||
|
|
||
| // Validate follower actor - accept activity sender | ||
| const followerActor = await accept.getActor(); | ||
| if (!isActor(followerActor) || !followerActor.id) return; | ||
| const parsed = ctx.parseUri(relayActorId); | ||
| if (parsed == null || parsed.type !== "actor") return; | ||
|
|
||
| // Get follower from kv store | ||
| const followerData = await ctx.data.kv.get([ | ||
| "follower", | ||
| followerActor.id.href, | ||
| ]); | ||
| if (followerData == null) return; | ||
|
|
||
| // Update follower state to accepted | ||
| const updatedFollowerData = { ...followerData, state: "accepted" }; | ||
| await ctx.data.kv.set( | ||
| ["follower", followerActor.id.href], | ||
| updatedFollowerData, | ||
| ); | ||
| }) | ||
| .on( | ||
| Undo, | ||
| async (ctx, undo) => await handleUndoFollow(ctx, undo, logger), | ||
| ) | ||
| .on( | ||
| Create, | ||
| async (ctx, create) => await this.#announceToFollowers(ctx, create), | ||
| ) | ||
| .on( | ||
| Update, | ||
| async (ctx, update) => await this.#announceToFollowers(ctx, update), | ||
| ) | ||
| .on( | ||
| Move, | ||
| async (ctx, move) => await this.#announceToFollowers(ctx, move), | ||
| ) | ||
| .on( | ||
| Delete, | ||
| async (ctx, deleteActivity) => | ||
| await this.#announceToFollowers(ctx, deleteActivity), | ||
| ) | ||
| .on( | ||
| Announce, | ||
| async (ctx, announce) => | ||
| await this.#announceToFollowers(ctx, announce), | ||
| ); | ||
| } | ||
| // Update follower state to accepted | ||
| const updatedFollowerData = { ...followerData, state: "accepted" }; | ||
| await ctx.data.kv.set( | ||
| ["follower", followerActor.id.href], | ||
| updatedFollowerData, | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the stored record before the state transition.
Line 102 reads the follower record as unknown. Line 109 spreads it and writes the result back. The code never confirms that the stored value is a RelayFollowerData.
If the stored value lacks an actor field, the write produces { state: "accepted" }. isRelayFollowerData then rejects that record, so parseFollowerData returns null and the follower disappears from listFollowers and getFollower.
The literal "accepted" is also not checked against RelayFollowerState. Annotate the new record so the compiler enforces the contract.
♻️ Proposed refactor
// Get follower from kv store
const followerData = await ctx.data.kv.get([
"follower",
followerActor.id.href,
]);
- if (followerData == null) return;
+ if (!isRelayFollowerData(followerData)) return;
// Update follower state to accepted
- const updatedFollowerData = { ...followerData, state: "accepted" };
+ const updatedFollowerData: RelayFollowerData = {
+ ...followerData,
+ state: "accepted",
+ };
await ctx.data.kv.set(
["follower", followerActor.id.href],
updatedFollowerData,
);Add isRelayFollowerData to the existing import from ./types.ts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const followerData = await ctx.data.kv.get([ | |
| "follower", | |
| followerActor.id.href, | |
| ]); | |
| if (followerData == null) return; | |
| // Litepub-specific: send reciprocal follow | |
| const relayActorUri = ctx.getActorUri(RELAY_SERVER_ACTOR); | |
| await ctx.sendActivity( | |
| { identifier: RELAY_SERVER_ACTOR }, | |
| follower, | |
| new Follow({ | |
| actor: relayActorUri, | |
| object: follower.id, | |
| to: follower.id, | |
| }), | |
| ); | |
| } else { | |
| await sendFollowResponse(ctx, follow, follower, approved); | |
| } | |
| }) | |
| .on(Accept, async (ctx, accept) => { | |
| // Validate follow activity from accept activity | |
| const follow = await accept.getObject({ | |
| crossOrigin: "trust", | |
| ...ctx, | |
| }); | |
| if (!(follow instanceof Follow)) return; | |
| const relayActorId = follow.actorId; | |
| if (relayActorId == null) return; | |
| // Validate follower actor - accept activity sender | |
| const followerActor = await accept.getActor(); | |
| if (!isActor(followerActor) || !followerActor.id) return; | |
| const parsed = ctx.parseUri(relayActorId); | |
| if (parsed == null || parsed.type !== "actor") return; | |
| // Get follower from kv store | |
| const followerData = await ctx.data.kv.get([ | |
| "follower", | |
| followerActor.id.href, | |
| ]); | |
| if (followerData == null) return; | |
| // Update follower state to accepted | |
| const updatedFollowerData = { ...followerData, state: "accepted" }; | |
| await ctx.data.kv.set( | |
| ["follower", followerActor.id.href], | |
| updatedFollowerData, | |
| ); | |
| }) | |
| .on( | |
| Undo, | |
| async (ctx, undo) => await handleUndoFollow(ctx, undo, logger), | |
| ) | |
| .on( | |
| Create, | |
| async (ctx, create) => await this.#announceToFollowers(ctx, create), | |
| ) | |
| .on( | |
| Update, | |
| async (ctx, update) => await this.#announceToFollowers(ctx, update), | |
| ) | |
| .on( | |
| Move, | |
| async (ctx, move) => await this.#announceToFollowers(ctx, move), | |
| ) | |
| .on( | |
| Delete, | |
| async (ctx, deleteActivity) => | |
| await this.#announceToFollowers(ctx, deleteActivity), | |
| ) | |
| .on( | |
| Announce, | |
| async (ctx, announce) => | |
| await this.#announceToFollowers(ctx, announce), | |
| ); | |
| } | |
| // Update follower state to accepted | |
| const updatedFollowerData = { ...followerData, state: "accepted" }; | |
| await ctx.data.kv.set( | |
| ["follower", followerActor.id.href], | |
| updatedFollowerData, | |
| ); | |
| const followerData = await ctx.data.kv.get([ | |
| "follower", | |
| followerActor.id.href, | |
| ]); | |
| if (!isRelayFollowerData(followerData)) return; | |
| // Update follower state to accepted | |
| const updatedFollowerData: RelayFollowerData = { | |
| ...followerData, | |
| state: "accepted", | |
| }; | |
| await ctx.data.kv.set( | |
| ["follower", followerActor.id.href], | |
| updatedFollowerData, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/relay/src/litepub.ts` around lines 102 - 113, Update the follower
transition around followerData to import and call isRelayFollowerData, returning
without writing when the stored value is invalid. Annotate updatedFollowerData
as RelayFollowerData so the accepted state and required actor fields are
compiler-validated, then persist it through the existing kv.set call.
| test("handles Announce activity forwarding", async () => { | ||
| const kv = new MemoryKvStore(); | ||
|
|
||
| const relay = createRelay("mastodon", { | ||
| kv, | ||
| origin: "https://relay.example.com", | ||
| documentLoaderFactory: () => mockDocumentLoader, | ||
| authenticatedDocumentLoaderFactory: () => mockDocumentLoader, | ||
| subscriptionHandler: () => Promise.resolve(true), | ||
| }); | ||
|
|
||
| const announceActivity = new Announce({ | ||
| id: new URL("https://remote.example.com/activities/announce/1"), | ||
| actor: new URL("https://remote.example.com/users/alice"), | ||
| object: new URL("https://remote.example.com/notes/1"), | ||
| }); | ||
|
|
||
| let request = new Request("https://relay.example.com/inbox", { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/activity+json", | ||
| }, | ||
| body: JSON.stringify( | ||
| await announceActivity.toJsonLd({ contextLoader: mockDocumentLoader }), | ||
| ), | ||
| }); | ||
|
|
||
| request = await signRequest( | ||
| request, | ||
| rsaKeyPair.privateKey, | ||
| rsaPublicKey.id, | ||
| ); | ||
|
|
||
| const response = await relay.fetch(request); | ||
|
|
||
| // Verify the request was accepted | ||
| ok(response.status === 200 || response.status === 202); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
The test does not verify forwarding.
The test asserts only that the inbox returned 200 or 202. The relay has no followers in this KV store, so forwardActivity sends nothing. Fedify also returns 202 for an activity type that has no registered listener. The test therefore passes even if the Announce listener is removed from BaseRelay.setupInboxListeners.
The PR objective is Mastodon Announce forwarding coverage. Register a follower before the request, then assert that the relay attempted delivery to that follower's inbox.
Store a follower record under ["follower", "<actor id>"] with state: "accepted", as the approved-follow test at lines 393-399 does. Then intercept the outbound request to the follower inbox and assert it carries the original Announce payload.
The status assertion is also hard to debug. ok(response.status === 200 || response.status === 202) reports no value on failure. Report the actual status in the assertion message.
💚 Proposed change to the status assertion
// Verify the request was accepted
- ok(response.status === 200 || response.status === 202);
+ ok(
+ response.status === 200 || response.status === 202,
+ `Unexpected status: ${response.status}`,
+ );I can write the follower-registration and delivery-assertion version of this test. Tell me if you want it.
Run the following script to find an existing outbound-delivery assertion pattern in this package:
#!/bin/bash
# Description: Locate existing tests that assert outbound delivery from the relay.
set -euo pipefail
fd -t f -e 'test.ts' . packages/relay/src
# Look for fetch interception or queue-based delivery assertions.
rg -nP -C8 'globalThis\.fetch|InProcessMessageQueue|queue:|onOutboxError|sendActivity' --type=ts packages/relay/src | head -n 100
# Check how other tests register followers before exercising delivery.
rg -nP -C6 'kv\.set\(\s*\[\s*"follower"' --type=ts packages/relay/src | head -n 60🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/relay/src/mastodon.test.ts` around lines 693 - 730, Extend the
“handles Announce activity forwarding” test by registering an accepted follower
in the MemoryKvStore under the Announce actor’s follower key, then intercepting
outbound delivery to that follower’s inbox and asserting it contains the
original Announce payload. Update the response status assertion to include the
actual status value in its failure message, while retaining the existing
accepted-status condition.
1 participant
This consolidates the behavior shared by the Mastodon and LitePub relay implementations while keeping their protocol differences explicit. Closes #905.
Background
The relay implementations previously repeated their Follow, Undo, and activity listener chains. Shared behavior was split between the protocol classes and helper functions, which made the actual differences between Mastodon and LitePub harder to identify and test.
Changes
BaseRelay.MastodonRelay.LitePubRelay.This is an internal refactor. It does not change the public
createRelay()API or the delivery semantics of either protocol.Testing
mise run fmtmise run check-each relaymise run test-each relaymise run test:deno packages/relay/src/mastodon.test.tsmise run test:deno packages/relay/src/litepub.test.tssacho check --base upstream/mainAI assistance
Codex (
gpt-5.6-sol) assisted with code analysis, implementation, test planning, validation, and drafting this description. I reviewed the changes and test results.