Skip to content

Changed dispatchers to use ctx site and account #1426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rmgpinto wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Changed dispatchers to use ctx site and account #1426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/account/account.entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ export interface Account {
readonly bannerImageUrl: URL | null;
readonly apId: URL;
readonly apFollowers: URL | null;
readonly apFollowing: URL | null;
readonly apInbox: URL | null;
readonly apOutbox: URL | null;
readonly apLiked: URL | null;
readonly isInternal: boolean;
readonly customFields: Record<string, string> | null;
unblock(account: Account): Account;
Expand Down Expand Up @@ -81,7 +84,10 @@ export class AccountEntity implements Account {
public readonly bannerImageUrl: URL | null,
public readonly apId: URL,
public readonly apFollowers: URL | null,
public readonly apFollowing: URL | null,
public readonly apInbox: URL | null,
public readonly apOutbox: URL | null,
public readonly apLiked: URL | null,
public readonly isInternal: boolean,
public readonly customFields: Record<string, string> | null,
private events: AccountEvent[],
Expand All @@ -108,7 +114,10 @@ export class AccountEntity implements Account {
data.bannerImageUrl,
data.apId,
data.apFollowers,
data.apFollowing,
data.apInbox,
data.apOutbox,
data.apLiked,
data.isInternal,
data.customFields,
events,
Expand All @@ -128,7 +137,10 @@ export class AccountEntity implements Account {
draft.bannerImageUrl,
draft.apId,
draft.apFollowers,
draft.apFollowing,
draft.apInbox,
draft.apOutbox,
draft.apLiked,
draft.isInternal,
draft.customFields,
events,
Expand Down
6 changes: 6 additions & 0 deletions src/account/account.repository.knex.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ interface AccountRow {
banner_image_url: string | null;
ap_id: string;
ap_followers_url: string | null;
ap_following_url: string | null;
ap_inbox_url: string | null;
ap_outbox_url: string | null;
ap_liked_url: string | null;
custom_fields: Record<string, string> | null;
site_id: number | null;
}
Expand Down Expand Up @@ -376,7 +379,10 @@ export class KnexAccountRepository {
bannerImageUrl: parseURL(row.banner_image_url),
apId: new URL(row.ap_id),
apFollowers: parseURL(row.ap_followers_url),
apFollowing: parseURL(row.ap_following_url),
apInbox: parseURL(row.ap_inbox_url),
apOutbox: parseURL(row.ap_outbox_url),
apLiked: parseURL(row.ap_liked_url),
isInternal: row.site_id !== null,
customFields: row.custom_fields,
});
Expand Down
37 changes: 23 additions & 14 deletions src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ await configure({
export type ContextData = {
globaldb: KvStore;
logger: Logger;
site?: Site;
account?: Account;
Comment on lines +172 to +173
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Type inconsistency between ContextData and HonoContextVariables.

ContextData declares site and account as optional (site?: Site, account?: Account), while HonoContextVariables declares them as required (site: Site, account: Account). This inconsistency can lead to type safety issues.

Since dispatchers use ctx.data.site and ctx.data.account (which are optional in ContextData), and these can be undefined in queue contexts (per ensureCorrectContext at lines 238-261), the types should be consistent.

Consider making both consistently optional, or add guards in dispatchers to handle undefined cases:

 export type HonoContextVariables = {
     globaldb: KvStore;
     logger: Logger;
     role: GhostRole;
-    site: Site;
-    account: Account;
+    site?: Site;
+    account?: Account;
 };

Also applies to: 513-514

};

// Register all dependencies
Expand Down Expand Up @@ -693,20 +695,6 @@ app.use(async (ctx, next) => {
await next();
});

app.use(async (ctx, next) => {
const globaldb = ctx.get('globaldb');
const logger = ctx.get('logger');

const fedifyContext = globalFedify.createContext(ctx.req.raw as Request, {
globaldb,
logger,
});

const fedifyContextFactory = container.resolve<FedifyContextFactory>(
'fedifyContextFactory',
);
await fedifyContextFactory.registerContext(fedifyContext, next);
});
// This needs to go before the middleware which loads the site
// Because the site doesn't always exist - this is how it's created
app.get(
Expand Down Expand Up @@ -735,6 +723,25 @@ app.use(
),
);

app.use(async (ctx, next) => {
const globaldb = ctx.get('globaldb');
const logger = ctx.get('logger');
const site = ctx.get('site');
const account = ctx.get('account');

const fedifyContext = globalFedify.createContext(ctx.req.raw as Request, {
globaldb,
logger,
site,
account,
});

const fedifyContextFactory = container.resolve<FedifyContextFactory>(
'fedifyContextFactory',
);
await fedifyContextFactory.registerContext(fedifyContext, next);
});

/** Custom API routes */

const routeRegistry = new RouteRegistry();
Expand Down Expand Up @@ -851,6 +858,8 @@ app.use(
return {
globaldb: ctx.get('globaldb'),
logger: ctx.get('logger'),
site: ctx.get('site'),
account: ctx.get('account'),
};
},
),
Expand Down
109 changes: 25 additions & 84 deletions src/dispatchers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
} from '@fedify/fedify';
import * as Sentry from '@sentry/node';

import type { KnexAccountRepository } from '@/account/account.repository.knex';
import type { AccountService } from '@/account/account.service';
import type { FollowersService } from '@/activitypub/followers.service';
import type { ContextData } from '@/app';
Expand All @@ -43,40 +42,34 @@ import type { KnexPostRepository } from '@/post/post.repository.knex';
import type { PostService } from '@/post/post.service';
import type { SiteService } from '@/site/site.service';

export const actorDispatcher = (
siteService: SiteService,
accountService: AccountService,
) =>
export const actorDispatcher = () =>
async function actorDispatcher(
ctx: RequestContext<ContextData>,
identifier: string,
) {
const site = await siteService.getSiteByHost(ctx.host);
if (site === null) return null;

const account = await accountService.getDefaultAccountForSite(site);
const account = ctx.data.account!;
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Non-null assertion on optional field could cause runtime errors.

Line 50 uses ctx.data.account!, but account is optional in ContextData (defined as account?: Account in src/app.ts:173). When this dispatcher is called from queue context via ensureCorrectContext (lines 238-261), the account may not be set, leading to potential runtime errors.

Remove the non-null assertion and add proper handling:

-    const account = ctx.data.account!;
+    const account = ctx.data.account;
+    if (!account) {
+        throw new Error('Account not available in actor dispatcher context');
+    }
🤖 Prompt for AI Agents 
In src/dispatchers.ts around line 50, remove the non-null assertion on
ctx.data.account and instead read const account = ctx.data.account; then add a
runtime guard that handles the missing-account case (e.g. if (!account) throw a
descriptive Error or return an appropriate failure result) so the dispatcher
never dereferences an optional account; keep the error message clear and
consistent with existing error handling.


const person = new Person({
id: new URL(account.ap_id),
id: new URL(account.apId),
name: account.name,
summary: account.bio,
preferredUsername: account.username,
icon: account.avatar_url
icon: account.avatarUrl
? new Image({
url: new URL(account.avatar_url),
url: new URL(account.avatarUrl),
})
: null,
image: account.banner_image_url
image: account.bannerImageUrl
? new Image({
url: new URL(account.banner_image_url),
url: new URL(account.bannerImageUrl),
})
: null,
inbox: new URL(account.ap_inbox_url),
outbox: new URL(account.ap_outbox_url),
following: new URL(account.ap_following_url),
followers: new URL(account.ap_followers_url),
liked: new URL(account.ap_liked_url),
url: new URL(account.url || account.ap_id),
inbox: account.apInbox,
outbox: account.apOutbox,
following: account.apFollowing,
followers: account.apFollowers,
liked: account.apLiked,
url: account.url || account.apId,
publicKeys: (await ctx.getActorKeyPairs(identifier)).map(
(key) => key.cryptographicKey,
),
Expand All @@ -85,16 +78,13 @@ export const actorDispatcher = (
return person;
};

export const keypairDispatcher = (
siteService: SiteService,
accountService: AccountService,
) =>
export const keypairDispatcher = (accountService: AccountService) =>
async function keypairDispatcher(
ctx: Context<ContextData>,
identifier: string,
) {
const site = await siteService.getSiteByHost(ctx.host);
if (site === null) return [];
const site = ctx.data.site!;
if (!site) return [];

const account = await accountService.getDefaultAccountForSite(site);

Expand Down Expand Up @@ -188,7 +178,6 @@ export function createAcceptHandler(accountService: AccountService) {
export async function handleAnnouncedCreate(
ctx: Context<ContextData>,
announce: Announce,
siteService: SiteService,
accountService: AccountService,
postService: PostService,
) {
Expand All @@ -204,11 +193,7 @@ export async function handleAnnouncedCreate(
return;
}

const site = await siteService.getSiteByHost(ctx.host);

if (!site) {
throw new Error(`Site not found for host: ${ctx.host}`);
}
const site = ctx.data.site!;
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Non-null assertion on optional site field.

Similar to other dispatchers, this uses ctx.data.site! where site is optional in ContextData. This could fail when called from queue context.

Add proper guard:

-    const site = ctx.data.site!;
+    const site = ctx.data.site;
+    if (!site) {
+        ctx.data.logger.warn('Site not available in handleAnnouncedCreate context');
+        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.

Suggested change
const site = ctx.data.site!;
const site = ctx.data.site;
if (!site) {
ctx.data.logger.warn('Site not available in handleAnnouncedCreate context');
return;
}
🤖 Prompt for AI Agents 
In src/dispatchers.ts around line 196, the code uses a non-null assertion const
site = ctx.data.site! which can crash when site is absent; replace the assertion
with a runtime guard — check if ctx.data.site is undefined and handle it the
same way other dispatchers do (either throw a descriptive error or early-return
with appropriate handling/logging), then assign const site = ctx.data.site after
the guard so all code downstream safely assumes site is present.


// Validate that the group is followed
if (
Expand Down Expand Up @@ -478,7 +463,6 @@ export const createUndoHandler = (
};

export function createAnnounceHandler(
siteService: SiteService,
accountService: AccountService,
postService: PostService,
postRepository: KnexPostRepository,
Expand Down Expand Up @@ -512,7 +496,6 @@ export function createAnnounceHandler(
return handleAnnouncedCreate(
ctx,
announce,
siteService,
accountService,
postService,
);
Expand Down Expand Up @@ -588,12 +571,6 @@ export function createAnnounceHandler(

ctx.data.globaldb.set([announce.id.href], announceJson);

const site = await siteService.getSiteByHost(ctx.host);

if (!site) {
throw new Error(`Site not found for host: ${ctx.host}`);
}

// This will save the account if it doesn't already exist
const senderAccount = await accountService.getByApId(sender.id);

Expand Down Expand Up @@ -774,21 +751,12 @@ export async function inboxErrorHandler(
});
}

export function createFollowersDispatcher(
siteService: SiteService,
accountRepository: KnexAccountRepository,
followersService: FollowersService,
) {
export function createFollowersDispatcher(followersService: FollowersService) {
return async function dispatchFollowers(
ctx: Context<ContextData>,
_handle: string,
) {
const site = await siteService.getSiteByHost(ctx.host);
if (!site) {
throw new Error(`Site not found for host: ${ctx.host}`);
}

const account = await accountRepository.getBySite(site);
const account = ctx.data.account!;
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Non-null assertion on optional account field.

ctx.data.account! uses non-null assertion on an optional field, which could cause runtime errors when called from queue context.

Add proper guard:

-    const account = ctx.data.account!;
+    const account = ctx.data.account;
+    if (!account) {
+        ctx.data.logger.warn('Account not available in followers dispatcher context');
+        return { items: [] };
+    }
📝 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.

Suggested change
const account = ctx.data.account!;
const account = ctx.data.account;
if (!account) {
ctx.data.logger.warn('Account not available in followers dispatcher context');
return { items: [] };
}
🤖 Prompt for AI Agents 
In src/dispatchers.ts around line 759, the code uses a non-null assertion on
ctx.data.account (ctx.data.account!) which can cause runtime errors when account
is absent (e.g., in queue context); replace the assertion with an explicit
guard: check if ctx.data.account is present and handle the missing case
deterministically (log and return/exit early or throw a descriptive Error)
before proceeding, and then use the guarded account variable for subsequent
logic so callers from queue context won't crash.


const followers = await followersService.getFollowers(account.id);

Expand All @@ -798,10 +766,7 @@ export function createFollowersDispatcher(
};
}

export function createFollowingDispatcher(
siteService: SiteService,
accountService: AccountService,
) {
export function createFollowingDispatcher(accountService: AccountService) {
return async function dispatchFollowing(
ctx: RequestContext<ContextData>,
_handle: string,
Expand All @@ -812,16 +777,8 @@ export function createFollowingDispatcher(
const offset = Number.parseInt(cursor ?? '0', 10);
let nextCursor: string | null = null;

const host = ctx.request.headers.get('host')!;
const site = await siteService.getSiteByHost(host);

if (!site) {
throw new Error(`Site not found for host: ${host}`);
}

// @TODO: Get account by provided handle instead of default account?
const siteDefaultAccount =
await accountService.getDefaultAccountForSite(site);
const siteDefaultAccount = ctx.data.account!;
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Non-null assertion on optional account field.

ctx.data.account! uses non-null assertion on an optional field, which could cause runtime errors when called from queue context.

Add proper guard:

-    const siteDefaultAccount = ctx.data.account!;
+    const siteDefaultAccount = ctx.data.account;
+    if (!siteDefaultAccount) {
+        ctx.data.logger.warn('Account not available in following dispatcher context');
+        return { items: [], nextCursor: null };
+    }
🤖 Prompt for AI Agents 
In src/dispatchers.ts around line 781, remove the non-null assertion
ctx.data.account! and guard against a missing account by checking if
ctx.data.account is present; if it's missing, handle it explicitly (e.g., log an
error and return or throw a descriptive error) and then assign const
siteDefaultAccount = ctx.data.account after the guard so the value is safely
used without risk of runtime null/undefined access.


const results = await accountService.getFollowingAccounts(
siteDefaultAccount,
Expand Down Expand Up @@ -849,43 +806,27 @@ export function createFollowingDispatcher(
};
}

export function createFollowersCounter(
siteService: SiteService,
accountService: AccountService,
) {
export function createFollowersCounter(accountService: AccountService) {
return async function countFollowers(
ctx: RequestContext<ContextData>,
_handle: string,
) {
const site = await siteService.getSiteByHost(ctx.host);
if (!site) {
throw new Error(`Site not found for host: ${ctx.host}`);
}

// @TODO: Get account by provided handle instead of default account?
const siteDefaultAccount = await accountService.getAccountForSite(site);
const siteDefaultAccount = ctx.data.account!;

return await accountService.getFollowerAccountsCount(
siteDefaultAccount.id,
);
};
}
Comment on lines +809 to 821
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Non-null assertion could cause runtime errors.

Line 815 uses ctx.data.account!, but account is optional in ContextData. This will fail when the counter is called from queue context.

Apply this diff:

-    const siteDefaultAccount = ctx.data.account!;
+    const siteDefaultAccount = ctx.data.account;
+    if (!siteDefaultAccount) {
+        ctx.data.logger.warn('Account not available in context');
+        return 0;
+    }
📝 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.

Suggested change
export function createFollowersCounter(accountService: AccountService) {
return async function countFollowers(
ctx: RequestContext<ContextData>,
_handle: string,
) {
const site = await siteService.getSiteByHost(ctx.host);
if (!site) {
throw new Error(`Site not found for host: ${ctx.host}`);
}
// @TODO: Get account by provided handle instead of default account?
const siteDefaultAccount = await accountService.getAccountForSite(site);
const siteDefaultAccount = ctx.data.account!;
return await accountService.getFollowerAccountsCount(
siteDefaultAccount.id,
);
};
}
export function createFollowersCounter(accountService: AccountService) {
return async function countFollowers(
ctx: RequestContext<ContextData>,
_handle: string,
) {
// @TODO: Get account by provided handle instead of default account?
const siteDefaultAccount = ctx.data.account;
if (!siteDefaultAccount) {
ctx.data.logger.warn('Account not available in context');
return 0;
}
return await accountService.getFollowerAccountsCount(
siteDefaultAccount.id,
);
};
}
🤖 Prompt for AI Agents 
In src/dispatchers.ts around lines 809 to 821, the code uses a non-null
assertion on ctx.data.account which can be undefined in queue contexts; update
the function to use the provided handle parameter when present by resolving the
account via accountService (e.g., await accountService.getByHandle(handle)),
otherwise fall back to ctx.data.account, and if no account is available throw a
clear error; then pass the resolved account.id to
accountService.getFollowerAccountsCount (ensure await is used when fetching the
account).


export function createFollowingCounter(
siteService: SiteService,
accountService: AccountService,
) {
export function createFollowingCounter(accountService: AccountService) {
return async function countFollowing(
ctx: RequestContext<ContextData>,
_handle: string,
) {
const site = await siteService.getSiteByHost(ctx.host);
if (!site) {
throw new Error(`Site not found for host: ${ctx.host}`);
}

// @TODO: Get account by provided handle instead of default account?
const siteDefaultAccount = await accountService.getAccountForSite(site);
const siteDefaultAccount = ctx.data.account!;
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Non-null assertion on optional account field.

ctx.data.account! uses non-null assertion on an optional field, which could cause runtime errors when called from queue context.

Add proper guard:

-    const siteDefaultAccount = ctx.data.account!;
+    const siteDefaultAccount = ctx.data.account;
+    if (!siteDefaultAccount) {
+        ctx.data.logger.warn('Account not available in following counter context');
+        return 0;
+    }
🤖 Prompt for AI Agents 
In src/dispatchers.ts around line 829, the code uses a non-null assertion on
ctx.data.account! which can throw at runtime when account is undefined; replace
the assertion with a runtime guard: check if ctx.data.account is present before
using it (if missing, log/contextualize the error and return or throw a
descriptive error), and use the safely-checked value (or optional chaining)
thereafter so callers from queue context won't hit an unexpected exception.


return await accountService.getFollowingAccountsCount(
siteDefaultAccount.id,
Expand Down
6 changes: 6 additions & 0 deletions src/helpers/activitypub/activity.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,10 @@ describe('Build activity', () => {
customFields: null,
apId: new URL('https://example.com/user/testuser'),
apFollowers: new URL('https://example.com/user/testuser/followers'),
apFollowing: new URL('https://example.com/user/testuser/following'),
apInbox: new URL('https://example.com/user/testuser/inbox'),
apOutbox: new URL('https://example.com/user/testuser/outbox'),
apLiked: new URL('https://example.com/user/testuser/liked'),
isInternal: false,
});

Expand All @@ -349,7 +352,10 @@ describe('Build activity', () => {
customFields: null,
apId: new URL('https://example.com/user/author'),
apFollowers: new URL('https://example.com/user/author/followers'),
apFollowing: new URL('https://example.com/user/author/following'),
apInbox: new URL('https://example.com/user/author/inbox'),
apOutbox: new URL('https://example.com/user/author/outbox'),
apLiked: new URL('https://example.com/user/author/liked'),
isInternal: false,
});

Expand Down
2 changes: 2 additions & 0 deletions src/helpers/fedify.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,7 @@ export function createFedifyCtxForHost(
return fedify.createContext(hostUrl, {
globaldb: ctxData.globaldb,
logger: ctxData.logger,
site: ctxData.site,
account: ctxData.account,
});
}
Loading