Changed dispatchers to use ctx site and account
#1426
Conversation
WalkthroughThree new ActivityPub URL fields—apFollowing, apOutbox, and apLiked—are added to the Account entity and propagated through repositories, data loaders, and query mappings. The database schema is extended with corresponding columns in AccountRow. Simultaneously, application context handling is refactored to inject site and account data through middleware and pass them via ctx.data to downstream handlers. Multiple dispatcher functions are simplified by removing SiteService and AccountService parameters, now retrieving these values from context instead. Test fixtures are updated to include the new fields. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (9)
🧰 Additional context used📓 Path-based instructions (2)src/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/dispatchers.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-08-21T13:32:58.582ZApplied to files:
📚 Learning: 2025-09-22T13:57:08.390ZApplied to files:
📚 Learning: 2025-09-22T13:57:08.390ZApplied to files:
📚 Learning: 2025-04-24T10:41:15.124ZApplied to files:
📚 Learning: 2025-07-31T16:35:35.813ZApplied to files:
🧬 Code graph analysis (3)src/account/account.repository.knex.ts (1)
src/app.ts (4)
src/dispatchers.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/dispatchers.ts (1)
842-908: Inconsistent with PR objective: outbox dispatchers still fetch site from database.The
createOutboxDispatcherandcreateOutboxCounterfunctions continue to acceptsiteServiceand fetch site viasiteService.getSiteByHost(), which is inconsistent with the PR's stated objective to use site from context instead of re-fetching from the database.Consider updating these dispatchers to use
ctx.data.sitelike the other dispatchers in this PR:For
createOutboxDispatcher(lines 842-891), apply:export function createOutboxDispatcher( accountService: AccountService, postService: PostService, - siteService: SiteService, ) { return async function outboxDispatcher( ctx: RequestContext<ContextData>, _handle: string, cursor: string | null, ) { ctx.data.logger.info('Outbox Dispatcher'); - const host = ctx.request.headers.get('host')!; - const site = await siteService.getSiteByHost(host); - if (!site) { - throw new Error(`Site not found for host: ${host}`); - } + const site = ctx.data.site!; const siteDefaultAccount = await accountService.getAccountForSite(site);For
createOutboxCounter(lines 893-908), apply:export function createOutboxCounter( - siteService: SiteService, accountService: AccountService, postService: PostService, ) { return async function countOutboxItems(ctx: RequestContext<ContextData>) { - const site = await siteService.getSiteByHost(ctx.host); - if (!site) { - throw new Error(`Site not found for host: ${ctx.host}`); - } + const site = ctx.data.site!; const siteDefaultAccount = await accountService.getAccountForSite(site);Note: Similar to other dispatchers, verify that
ctx.data.siteis guaranteed to be set for outbox handlers before using the non-null assertion.
🧹 Nitpick comments (1)
src/account/account.repository.knex.ts (1)
62-64: Consider consistent field ordering for readability.The insertion correctly maps the new AP URL fields from the draft. However, note that the field order here (
ap_outbox_url,ap_following_url,ap_liked_url) differs from the interface order inAccountRow(ap_following_url,ap_outbox_url,ap_liked_url). While functionally correct, consistent ordering improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/account/account.entity.ts(4 hunks)src/account/account.repository.knex.ts(2 hunks)src/app.ts(3 hunks)src/dispatchers.ts(7 hunks)src/helpers/activitypub/activity.unit.test.ts(2 hunks)src/helpers/fedify.ts(1 hunks)src/http/host-data-context-loader.ts(2 hunks)src/post/content.unit.test.ts(3 hunks)src/post/post.repository.knex.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Never use direct string comparisons for ActivityPub IDs; use SHA256 hash-based lookups instead
Use Result helper functions (isError, getError, getValue) and avoid destructuring Result values directly
Awilix uses CLASSIC injection: constructor parameter names must match their registration names
Define routes using decorators rather than direct registration
Prefer immutable entities that emit domain events over mutable entities with dirty flags
Use structured error objects with context in Result types instead of string literals
Prefer class-based handlers with dependency injection over function factories
Files:
src/app.tssrc/account/account.repository.knex.tssrc/helpers/fedify.tssrc/helpers/activitypub/activity.unit.test.tssrc/http/host-data-context-loader.tssrc/post/content.unit.test.tssrc/account/account.entity.tssrc/post/post.repository.knex.tssrc/dispatchers.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use the .unit.test.ts file extension for unit tests
Files:
src/helpers/activitypub/activity.unit.test.tssrc/post/content.unit.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
All unit and integration test files should use the .test.ts suffix
Files:
src/helpers/activitypub/activity.unit.test.tssrc/post/content.unit.test.ts
src/dispatchers.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not add new legacy factory handlers to dispatchers.ts (legacy file)
Files:
src/dispatchers.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1223
File: src/http/api/bluesky.controller.ts:0-0
Timestamp: 2025-08-21T13:32:58.582Z
Learning: In the Ghost ActivityPub codebase, `ctx.get('account')` in controller methods should always return an account because the middleware guarantees this. Guards for missing account are unnecessary in this system.
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 1010
File: src/app.ts:744-744
Timestamp: 2025-07-09T06:47:50.064Z
Learning: In the TryGhost/ActivityPub repository, allouis prefers to keep refactoring PRs focused on code restructuring only, without adding new functionality or changing behavior. When moving existing code that has issues, the issues should be preserved in the refactoring and addressed separately.
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 602
File: src/http/api/follow.ts:80-88
Timestamp: 2025-05-01T07:06:59.816Z
Learning: In the ActivityPub codebase, Account entities use the `apId` property when referencing ActivityPub IDs, not `ap_id`.
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 737
File: migrate/migrations/000054_populate-outboxes-table.up.sql:56-63
Timestamp: 2025-05-27T09:09:22.110Z
Learning: In the outboxes table migration for Ghost ActivityPub, the author_id field always refers to the author of the original post, not the person performing the outbox action. For reposts (outbox_type = 1), author_id should be posts.author_id (original author), while user_id captures who made the repost via the joins through accounts and users tables.
📚 Learning: 2025-09-25T12:39:45.755Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1329
File: .github/renovate.json5:31-34
Timestamp: 2025-09-25T12:39:45.755Z
Learning: The jobs/update-external-accounts directory was removed from the TryGhost/ActivityPub repository and no longer exists, so Renovate configuration exclusions referencing this path are no longer needed.
Applied to files:
src/account/account.repository.knex.tssrc/account/account.entity.ts
📚 Learning: 2025-05-01T07:06:59.816Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 602
File: src/http/api/follow.ts:80-88
Timestamp: 2025-05-01T07:06:59.816Z
Learning: In the ActivityPub codebase, Account entities use the `apId` property when referencing ActivityPub IDs, not `ap_id`.
Applied to files:
src/helpers/activitypub/activity.unit.test.tssrc/account/account.entity.ts
📚 Learning: 2025-05-01T13:08:36.516Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 606
File: src/activitypub/fediverse-bridge.unit.test.ts:158-167
Timestamp: 2025-05-01T13:08:36.516Z
Learning: In the ActivityPub FediverseBridge test suite, expect assertions on sendActivity calls should verify the `username` property, not `handle`, as username is the correct parameter name.
Applied to files:
src/helpers/activitypub/activity.unit.test.ts
📚 Learning: 2025-04-24T10:41:15.124Z
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 566
File: src/account/account.entity.ts:110-123
Timestamp: 2025-04-24T10:41:15.124Z
Learning: In the Account entity's `updateProfile` method, the current implementation using a helper function that checks for undefined values is type-safe at runtime, even though TypeScript might not perfectly infer this due to the function's return type declaration.
Applied to files:
src/post/content.unit.test.ts
📚 Learning: 2025-09-22T13:57:08.390Z
Learnt from: CR
Repo: TryGhost/ActivityPub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T13:57:08.390Z
Learning: Applies to src/dispatchers.ts : Do not add new legacy factory handlers to dispatchers.ts (legacy file)
Applied to files:
src/dispatchers.ts
📚 Learning: 2025-09-22T13:57:08.390Z
Learnt from: CR
Repo: TryGhost/ActivityPub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T13:57:08.390Z
Learning: Applies to src/**/*.ts : Prefer class-based handlers with dependency injection over function factories
Applied to files:
src/dispatchers.ts
🧬 Code graph analysis (4)
src/app.ts (2)
src/site/site.service.ts (1)
Site(9-13)src/account/account.entity.ts (1)
Account(16-48)
src/account/account.repository.knex.ts (1)
src/core/url.ts (1)
parseURL(1-7)
src/post/post.repository.knex.ts (1)
src/core/url.ts (1)
parseURL(1-7)
src/dispatchers.ts (2)
src/app.ts (1)
ContextData(169-174)src/activitypub/followers.service.ts (1)
FollowersService(7-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build / Build Docker Images (linux/amd64, amd64)
🔇 Additional comments (24)
src/helpers/fedify.ts (1)
21-22: LGTM!The changes correctly propagate
siteandaccountfrom the context data into the Fedify context, aligning with the PR objective of making these available from context rather than fetching them separately.src/post/content.unit.test.ts (3)
258-262: LGTM!Test fixtures correctly updated to include the new ActivityPub URL fields (
apFollowing,apOutbox,apLiked) consistent with the expandedAccountEntitystructure.
555-559: LGTM!Test fixtures correctly updated to include the new ActivityPub URL fields consistent with the expanded
AccountEntitystructure.
574-578: LGTM!Test fixtures correctly updated to include the new ActivityPub URL fields consistent with the expanded
AccountEntitystructure.src/http/host-data-context-loader.ts (2)
42-45: LGTM!SQL select correctly extended to fetch the new ActivityPub URL fields (
ap_following_url,ap_outbox_url,ap_liked_url) with proper aliasing for account-scoped data.
81-84: LGTM!The new AP URL fields are correctly mapped from the SQL result into the account entity via
createFromRow, maintaining consistency with the database schema.src/helpers/activitypub/activity.unit.test.ts (2)
336-340: LGTM!Test fixture correctly includes the new ActivityPub URL fields with appropriate URL values for testing Announce activity construction.
355-359: LGTM!Test fixture correctly includes the new ActivityPub URL fields with appropriate URL values for the author entity.
src/post/post.repository.knex.ts (1)
67-70: LGTM!
PostRowinterface correctly extended with the three new author ActivityPub URL fields to support the expandedAccountEntitystructure.src/account/account.repository.knex.ts (2)
34-37: LGTM!
AccountRowinterface correctly extended with the three new ActivityPub URL fields for following, outbox, and liked collections.
382-385: LGTM!The mapping correctly propagates the new AP URL fields from database rows to
AccountEntityusing theparseURLhelper for safe URL parsing.src/app.ts (2)
172-173: LGTM!
ContextDatacorrectly extended with optionalsiteandaccountfields. The optional nature is appropriate since this type is used in contexts beyond HTTP requests where these values may not always be available.
860-861: LGTM!The federation middleware correctly passes
siteandaccountfrom Hono context to Fedify'sContextData, aligning with the PR objective of using context data instead of re-fetching from services.src/account/account.entity.ts (5)
27-30: LGTM!
Accountinterface correctly extended with the three new ActivityPub URL fields (apFollowing,apOutbox,apLiked) as nullable URLs, consistent with other ActivityPub endpoint fields.
87-90: LGTM!Constructor correctly extended with the new ActivityPub URL fields, maintaining consistency with the
Accountinterface.
117-120: LGTM!
AccountEntity.createcorrectly propagates the new AP URL fields from the input data to the entity instance.
140-143: LGTM!
AccountEntity.fromDraftcorrectly propagates the new AP URL fields from the draft to the entity instance.
165-170: LGTM!The
draftmethod correctly constructs AP URLs for internal accounts using the standard paths (/.ghost/activitypub/following/index, etc.) and preserves provided values for external accounts.src/dispatchers.ts (6)
46-80: LGTM! Context-based account retrieval simplifies dispatcher.The refactor to use
ctx.data.accountdirectly eliminates the need for service dependencies and aligns with the PR objective. The non-null assertion is safe here as middleware guarantees account is set. Based on learnings.
755-768: LGTM! Simplified dispatcher signature.The refactor correctly uses
ctx.data.accountfrom context instead of requiring service dependencies. The non-null assertion is safe as middleware guarantees account is set. Based on learnings.
770-808: LGTM! Context-driven account access.The refactor eliminates the siteService dependency by using
ctx.data.accountdirectly. The non-null assertion is safe as middleware guarantees account is set. Based on learnings.
810-822: LGTM! Consistent refactor pattern.Correctly uses
ctx.data.accountfrom context. The non-null assertion is safe as middleware guarantees account is set. Based on learnings.
824-836: LGTM! Consistent refactor pattern.Correctly uses
ctx.data.accountfrom context. The non-null assertion is safe as middleware guarantees account is set. Based on learnings.
197-197: The non-null assertion is correct and justified.The
ctx.data.site!assertion at line 197 is safe becausecreateHostDataContextMiddlewareis registered globally and guarantees site is set before any inbox handler executes. The middleware returns early with an error response if site cannot be loaded, preventing handler execution entirely. This pattern aligns with how the codebase handles similar middleware-guaranteed context values (as confirmed in the learnings forctx.get('account')).Likely an incorrect or invalid review comment.
rmgpinto
force-pushed
the
dispatchers-n+1
branch
3 times, most recently
from
343ab8c to
ba1bdfb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/app.ts (1)
238-261: ensureCorrectContext doesn't set site and account.The
ensureCorrectContextwrapper is used for inbox listeners (lines 293-374) and other handlers that run outside the HTTP request context. However, it only initializesglobaldbandlogger, leavingsiteandaccountundefined. This will cause runtime errors in dispatchers that usectx.data.site!orctx.data.account!(e.g.,handleAnnouncedCreateat line 196 of src/dispatchers.ts).Apply this diff to set site and account when missing, or update dispatchers to handle undefined values:
function ensureCorrectContext<B, R>( fn: (ctx: Context<ContextData>, b: B) => Promise<R>, ) { return async (ctx: Context<ContextData>, b: B) => { if (!ctx.data) { // TODO: Clean up the any type // biome-ignore lint/suspicious/noExplicitAny: Legacy code needs proper typing (ctx as any).data = {}; } if (!ctx.data.globaldb) { ctx.data.globaldb = globalKv; } if (!ctx.data.logger) { ctx.data.logger = globalLogging; } + // TODO: Set site and account when running in queue context + // if (!ctx.data.site) { + // ctx.data.site = ...; + // } + // if (!ctx.data.account) { + // ctx.data.account = ...; + // } const fedifyContextFactory = container.resolve<FedifyContextFactory>( 'fedifyContextFactory', ); return fedifyContextFactory.registerContext(ctx, () => { return fn(ctx, b); }); }; }src/dispatchers.ts (6)
45-79: Non-null assertion could cause runtime errors.Line 50 uses
ctx.data.account!which assumesaccountis always set. However,ContextDatadefinesaccountas optional (src/app.ts line 173), and theensureCorrectContextfunction doesn't set it for queue-processed activities. This will cause runtime errors when the actor dispatcher is called from inbox queue processing.Apply this diff to safely handle undefined account:
- const account = ctx.data.account!; + const account = ctx.data.account; + if (!account) { + ctx.data.logger.warn('Account not available in context'); + return null; + }
178-360: Non-null assertion could cause runtime errors in queue context.Line 196 uses
ctx.data.site!which assumessiteis always set. However, whenhandleAnnouncedCreateis called from the announce inbox handler viaensureCorrectContext(src/app.ts line 336),sitewon't be set becauseensureCorrectContextonly initializesglobaldbandlogger.Apply this diff to handle undefined site:
- const site = ctx.data.site!; + const site = ctx.data.site; + if (!site) { + ctx.data.logger.info('Site not available in context, exit early'); + return; + }
754-767: Non-null assertion could cause runtime errors.Line 759 uses
ctx.data.account!, butaccountis optional inContextData. When called from queue context viaensureCorrectContext, this will cause a runtime error.Apply this diff:
- const account = ctx.data.account!; + const account = ctx.data.account; + if (!account) { + ctx.data.logger.warn('Account not available in context'); + return { items: [] }; + }
769-807: Non-null assertion could cause runtime errors.Line 781 uses
ctx.data.account!, butaccountis optional inContextData. This will fail when the dispatcher runs in 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 { items: [], nextCursor: null }; + }
823-835: Non-null assertion could cause runtime errors.Line 829 uses
ctx.data.account!, butaccountis optional inContextData. 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; + }
841-907: Incomplete refactoring: outbox dispatchers still fetch from database.The
createOutboxDispatcher(lines 841-890) andcreateOutboxCounter(lines 892-907) still callsiteService.getSiteByHost()andaccountService.getAccountForSite()instead of usingctx.data.siteandctx.data.account. This is inconsistent with the PR objective to use context values instead of re-fetching from the database.Consider refactoring these functions to use
ctx.data.siteandctx.data.accountlike the other dispatchers:export function createOutboxDispatcher( accountService: AccountService, postService: PostService, - siteService: SiteService, ) { return async function outboxDispatcher( ctx: RequestContext<ContextData>, _handle: string, cursor: string | null, ) { ctx.data.logger.info('Outbox Dispatcher'); - const host = ctx.request.headers.get('host')!; - const site = await siteService.getSiteByHost(host); - if (!site) { - throw new Error(`Site not found for host: ${host}`); + const site = ctx.data.site; + if (!site) { + ctx.data.logger.warn('Site not available in context'); + return { items: [], nextCursor: null }; } - const siteDefaultAccount = await accountService.getAccountForSite(site); + const siteDefaultAccount = ctx.data.account; + if (!siteDefaultAccount) { + ctx.data.logger.warn('Account not available in context'); + return { items: [], nextCursor: null }; + }Similarly for
createOutboxCounter.
♻️ Duplicate comments (1)
src/dispatchers.ts (1)
86-87: Remove contradictory non-null assertion.The non-null assertion operator (
!) on line 86 contradicts the null check on line 87. Sincesiteis optional inContextData, the assertion is incorrect and masks the actual optionality.Apply this diff:
- const site = ctx.data.site!; - if (!site) return []; + const site = ctx.data.site; + if (!site) return [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/test.yml(1 hunks)src/account/account.entity.ts(4 hunks)src/account/account.repository.knex.ts(2 hunks)src/app.ts(3 hunks)src/dispatchers.ts(7 hunks)src/helpers/activitypub/activity.unit.test.ts(2 hunks)src/helpers/fedify.ts(1 hunks)src/http/host-data-context-loader.ts(2 hunks)src/post/content.unit.test.ts(3 hunks)src/post/post.repository.knex.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/http/host-data-context-loader.ts
- src/account/account.repository.knex.ts
- src/helpers/activitypub/activity.unit.test.ts
- src/post/post.repository.knex.ts
- src/account/account.entity.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.unit.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use the .unit.test.ts file extension for unit tests
Files:
src/post/content.unit.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
All unit and integration test files should use the .test.ts suffix
Files:
src/post/content.unit.test.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Never use direct string comparisons for ActivityPub IDs; use SHA256 hash-based lookups instead
Use Result helper functions (isError, getError, getValue) and avoid destructuring Result values directly
Awilix uses CLASSIC injection: constructor parameter names must match their registration names
Define routes using decorators rather than direct registration
Prefer immutable entities that emit domain events over mutable entities with dirty flags
Use structured error objects with context in Result types instead of string literals
Prefer class-based handlers with dependency injection over function factories
Files:
src/post/content.unit.test.tssrc/helpers/fedify.tssrc/dispatchers.tssrc/app.ts
src/dispatchers.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not add new legacy factory handlers to dispatchers.ts (legacy file)
Files:
src/dispatchers.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1223
File: src/http/api/bluesky.controller.ts:0-0
Timestamp: 2025-08-21T13:32:58.582Z
Learning: In the Ghost ActivityPub codebase, `ctx.get('account')` in controller methods should always return an account because the middleware guarantees this. Guards for missing account are unnecessary in this system.
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 1010
File: src/app.ts:744-744
Timestamp: 2025-07-09T06:47:50.064Z
Learning: In the TryGhost/ActivityPub repository, allouis prefers to keep refactoring PRs focused on code restructuring only, without adding new functionality or changing behavior. When moving existing code that has issues, the issues should be preserved in the refactoring and addressed separately.
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 602
File: src/http/api/follow.ts:80-88
Timestamp: 2025-05-01T07:06:59.816Z
Learning: In the ActivityPub codebase, Account entities use the `apId` property when referencing ActivityPub IDs, not `ap_id`.
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 737
File: migrate/migrations/000054_populate-outboxes-table.up.sql:56-63
Timestamp: 2025-05-27T09:09:22.110Z
Learning: In the outboxes table migration for Ghost ActivityPub, the author_id field always refers to the author of the original post, not the person performing the outbox action. For reposts (outbox_type = 1), author_id should be posts.author_id (original author), while user_id captures who made the repost via the joins through accounts and users tables.
📚 Learning: 2025-04-24T10:41:15.124Z
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 566
File: src/account/account.entity.ts:110-123
Timestamp: 2025-04-24T10:41:15.124Z
Learning: In the Account entity's `updateProfile` method, the current implementation using a helper function that checks for undefined values is type-safe at runtime, even though TypeScript might not perfectly infer this due to the function's return type declaration.
Applied to files:
src/post/content.unit.test.ts
📚 Learning: 2025-09-22T13:57:08.390Z
Learnt from: CR
Repo: TryGhost/ActivityPub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T13:57:08.390Z
Learning: Applies to src/dispatchers.ts : Do not add new legacy factory handlers to dispatchers.ts (legacy file)
Applied to files:
src/dispatchers.ts
📚 Learning: 2025-07-31T16:35:35.813Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1136
File: src/instrumentation.ts:36-48
Timestamp: 2025-07-31T16:35:35.813Z
Learning: In the TryGhost/ActivityPub repository, the SQL queries captured by extractQueryInfoFromError from Knex's query-error event are already normalized with placeholders, so additional SQL normalization is not needed for Sentry fingerprinting.
Applied to files:
src/dispatchers.ts
📚 Learning: 2025-08-21T13:32:58.582Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1223
File: src/http/api/bluesky.controller.ts:0-0
Timestamp: 2025-08-21T13:32:58.582Z
Learning: In the Ghost ActivityPub codebase, `ctx.get('account')` in controller methods should always return an account because the middleware guarantees this. Guards for missing account are unnecessary in this system.
Applied to files:
src/app.ts
📚 Learning: 2025-09-22T13:57:08.390Z
Learnt from: CR
Repo: TryGhost/ActivityPub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T13:57:08.390Z
Learning: Applies to src/**/*.ts : Define routes using decorators rather than direct registration
Applied to files:
src/app.ts
🧬 Code graph analysis (2)
src/dispatchers.ts (3)
src/app.ts (1)
ContextData(169-174)src/account/account.service.ts (1)
AccountService(58-870)src/activitypub/followers.service.ts (1)
FollowersService(7-48)
src/app.ts (3)
src/site/site.service.ts (1)
Site(9-13)src/account/account.entity.ts (1)
Account(16-48)src/activitypub/fedify-context.factory.ts (1)
FedifyContextFactory(5-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build / Build Docker Images (linux/amd64, amd64)
🔇 Additional comments (5)
.github/workflows/test.yml (1)
15-15: Clarify the motivation for the runner change.The change to
ubuntu-latest-4-coresappears operationally unrelated to the dispatcher/context refactoring objectives in this PR. If this change is necessary due to increased test resource requirements from the new code, please document that. Otherwise, consider deferring this as a separate optimization PR with clear justification (e.g., test timing baseline, resource bottleneck analysis).Could you clarify:
- Is this runner change required by the dispatcher/context changes, or is it an independent optimization?
- If required, what is the resource bottleneck or test performance concern?
Based on learnings from the codebase, maintaining focused PRs helps streamline review and reduces scope creep. If this is unrelated, a separate issue/PR would be preferred.
src/post/content.unit.test.ts (1)
246-263: LGTM! Test fixtures properly updated for new AccountEntity fields.The three new ActivityPub URL fields (
apFollowing,apOutbox,apLiked) have been correctly and consistently added to all account test fixtures. Setting these tonullis appropriate for these tests since they focus on content preparation and mention handling rather than ActivityPub URL functionality.Also applies to: 543-560, 562-579
src/helpers/fedify.ts (1)
21-22: LGTM!The changes correctly pass
siteandaccountfrom the context data to the Fedify context.src/app.ts (2)
726-743: Verify middleware always sets site and account before this point.This middleware reads
siteandaccountfrom the Hono context and passes them to the Fedify context. While thecreateHostDataContextMiddlewareat lines 720-724 should set these values, the type system shows a mismatch:HonoContextVariablesdeclares them as required (lines 513-514), butContextDatadeclares them as optional (lines 172-173).If
createHostDataContextMiddlewaredoesn't always set these values (e.g., when a site doesn't exist), passingundefinedvalues to the Fedify context could cause issues with dispatchers that use non-null assertions.Based on learnings
172-173: Fix non-null assertions that assume undefined properties.The review comment is correct. The code makes
siteandaccountoptional inContextData(lines 172-173 of src/app.ts) but then uses non-null assertions throughout dispatchers without ever setting these values. TheensureCorrectContextwrapper only initializesglobaldbandlogger, leavingsiteandaccountundefined when inbox handlers execute.Non-null assertions found in
src/dispatchers.tsat lines 50, 86, 196, 759, 781, 815, and 829 will fail at runtime when these properties remain undefined. These values are never assigned anywhere in the codebase, meaning the assumptions behind the assertions are not guaranteed.Either ensure
siteandaccountare properly initialized before dispatcher execution, or refactor handlers to gracefully handle undefined values instead of relying on non-null assertions.⛔ Skipped due to learnings
Learnt from: mike182uk Repo: TryGhost/ActivityPub PR: 1223 File: src/http/api/bluesky.controller.ts:0-0 Timestamp: 2025-08-21T13:32:58.582Z Learning: In the Ghost ActivityPub codebase, `ctx.get('account')` in controller methods should always return an account because the middleware guarantees this. Guards for missing account are unnecessary in this system.Learnt from: allouis Repo: TryGhost/ActivityPub PR: 566 File: src/account/account.entity.ts:110-123 Timestamp: 2025-04-24T10:41:15.124Z Learning: In the Account entity's `updateProfile` method, the current implementation using a helper function that checks for undefined values is type-safe at runtime, even though TypeScript might not perfectly infer this due to the function's return type declaration.
| 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, | ||
| ); | ||
| }; | ||
| } |
There was a problem hiding this comment.
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.
| 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).
6da77f9 to
ada1e7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
src/dispatchers.ts (2)
86-87: Contradictory non-null assertion and null check.Line 86 uses a non-null assertion (
ctx.data.site!), but line 87 immediately checks ifsiteis falsy. This pattern is contradictory and the non-null assertion should be removed.Apply this diff:
- const site = ctx.data.site!; - if (!site) return []; + const site = ctx.data.site; + if (!site) return [];
815-815: Non-null assertion on optional account field.
ctx.data.account!uses non-null assertion on an optional field. This could fail when the counter is called from queue context. Additionally, as noted in past review comments, this function should use the providedhandleparameter to resolve the account when available.Consider using the handle parameter to resolve the account:
- const siteDefaultAccount = ctx.data.account!; + const siteDefaultAccount = ctx.data.account; + if (!siteDefaultAccount) { + ctx.data.logger.warn('Account not available in followers counter context'); + return 0; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
features/step_definitions/follow_steps.js(6 hunks)src/account/account.entity.ts(4 hunks)src/account/account.repository.knex.ts(2 hunks)src/app.ts(3 hunks)src/dispatchers.ts(7 hunks)src/helpers/activitypub/activity.unit.test.ts(2 hunks)src/helpers/fedify.ts(1 hunks)src/http/host-data-context-loader.ts(2 hunks)src/post/content.unit.test.ts(3 hunks)src/post/post.repository.knex.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/post/content.unit.test.ts
- src/http/host-data-context-loader.ts
- src/helpers/fedify.ts
- src/post/post.repository.knex.ts
- src/account/account.entity.ts
🧰 Additional context used
📓 Path-based instructions (5)
features/step_definitions/**/*_steps.js
📄 CodeRabbit inference engine (AGENTS.md)
Group Cucumber step definitions by high-level feature and name files with the _steps.js suffix under features/step_definitions
Files:
features/step_definitions/follow_steps.js
**/*.unit.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use the .unit.test.ts file extension for unit tests
Files:
src/helpers/activitypub/activity.unit.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
All unit and integration test files should use the .test.ts suffix
Files:
src/helpers/activitypub/activity.unit.test.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Never use direct string comparisons for ActivityPub IDs; use SHA256 hash-based lookups instead
Use Result helper functions (isError, getError, getValue) and avoid destructuring Result values directly
Awilix uses CLASSIC injection: constructor parameter names must match their registration names
Define routes using decorators rather than direct registration
Prefer immutable entities that emit domain events over mutable entities with dirty flags
Use structured error objects with context in Result types instead of string literals
Prefer class-based handlers with dependency injection over function factories
Files:
src/helpers/activitypub/activity.unit.test.tssrc/app.tssrc/account/account.repository.knex.tssrc/dispatchers.ts
src/dispatchers.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not add new legacy factory handlers to dispatchers.ts (legacy file)
Files:
src/dispatchers.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1223
File: src/http/api/bluesky.controller.ts:0-0
Timestamp: 2025-08-21T13:32:58.582Z
Learning: In the Ghost ActivityPub codebase, `ctx.get('account')` in controller methods should always return an account because the middleware guarantees this. Guards for missing account are unnecessary in this system.
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 1010
File: src/app.ts:744-744
Timestamp: 2025-07-09T06:47:50.064Z
Learning: In the TryGhost/ActivityPub repository, allouis prefers to keep refactoring PRs focused on code restructuring only, without adding new functionality or changing behavior. When moving existing code that has issues, the issues should be preserved in the refactoring and addressed separately.
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 602
File: src/http/api/follow.ts:80-88
Timestamp: 2025-05-01T07:06:59.816Z
Learning: In the ActivityPub codebase, Account entities use the `apId` property when referencing ActivityPub IDs, not `ap_id`.
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 737
File: migrate/migrations/000054_populate-outboxes-table.up.sql:56-63
Timestamp: 2025-05-27T09:09:22.110Z
Learning: In the outboxes table migration for Ghost ActivityPub, the author_id field always refers to the author of the original post, not the person performing the outbox action. For reposts (outbox_type = 1), author_id should be posts.author_id (original author), while user_id captures who made the repost via the joins through accounts and users tables.
📚 Learning: 2025-05-01T07:06:59.816Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 602
File: src/http/api/follow.ts:80-88
Timestamp: 2025-05-01T07:06:59.816Z
Learning: In the ActivityPub codebase, Account entities use the `apId` property when referencing ActivityPub IDs, not `ap_id`.
Applied to files:
src/helpers/activitypub/activity.unit.test.ts
📚 Learning: 2025-08-21T13:32:58.582Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1223
File: src/http/api/bluesky.controller.ts:0-0
Timestamp: 2025-08-21T13:32:58.582Z
Learning: In the Ghost ActivityPub codebase, `ctx.get('account')` in controller methods should always return an account because the middleware guarantees this. Guards for missing account are unnecessary in this system.
Applied to files:
src/app.tssrc/dispatchers.ts
📚 Learning: 2025-09-22T13:57:08.390Z
Learnt from: CR
Repo: TryGhost/ActivityPub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T13:57:08.390Z
Learning: Applies to src/**/*.ts : Define routes using decorators rather than direct registration
Applied to files:
src/app.ts
📚 Learning: 2025-09-22T13:57:08.390Z
Learnt from: CR
Repo: TryGhost/ActivityPub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T13:57:08.390Z
Learning: Applies to src/dispatchers.ts : Do not add new legacy factory handlers to dispatchers.ts (legacy file)
Applied to files:
src/dispatchers.ts
📚 Learning: 2025-04-24T10:41:15.124Z
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 566
File: src/account/account.entity.ts:110-123
Timestamp: 2025-04-24T10:41:15.124Z
Learning: In the Account entity's `updateProfile` method, the current implementation using a helper function that checks for undefined values is type-safe at runtime, even though TypeScript might not perfectly infer this due to the function's return type declaration.
Applied to files:
src/dispatchers.ts
📚 Learning: 2025-07-31T16:35:35.813Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1136
File: src/instrumentation.ts:36-48
Timestamp: 2025-07-31T16:35:35.813Z
Learning: In the TryGhost/ActivityPub repository, the SQL queries captured by extractQueryInfoFromError from Knex's query-error event are already normalized with placeholders, so additional SQL normalization is not needed for Sentry fingerprinting.
Applied to files:
src/dispatchers.ts
🧬 Code graph analysis (3)
src/app.ts (4)
src/site/site.service.ts (1)
Site(9-13)src/account/account.entity.ts (1)
Account(16-48)src/configuration/container.ts (1)
container(3-6)src/activitypub/fedify-context.factory.ts (1)
FedifyContextFactory(5-25)
src/account/account.repository.knex.ts (1)
src/core/url.ts (1)
parseURL(1-7)
src/dispatchers.ts (3)
src/app.ts (1)
ContextData(169-174)src/account/account.service.ts (1)
AccountService(58-870)src/activitypub/followers.service.ts (1)
FollowersService(7-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build / Build Docker Images (linux/amd64, amd64)
🔇 Additional comments (5)
features/step_definitions/follow_steps.js (2)
28-29: Good addition for CI/CD stability.The 100ms delay addresses timing issues with wiremock registration in CI/CD environments. The duration is appropriate and won't significantly impact test execution time.
50-53: Excellent error reporting improvements.The enhanced error messages now include actor handle, status codes, and response bodies, which will significantly improve debugging when tests fail. The pattern is consistently applied across all failure scenarios.
Also applies to: 68-71, 89-92, 130-133, 151-154
src/helpers/activitypub/activity.unit.test.ts (1)
336-340: Test fixtures correctly updated for new AccountEntity fields.The additions of
apFollowing,apOutbox, andapLikedto the test fixtures properly reflect the extended AccountEntity shape introduced in this PR. The URL patterns are consistent with the existing test data.Also applies to: 355-359
src/app.ts (1)
726-743: Middleware placement is correct.The fedifyContext middleware that reads
ctx.get('site')andctx.get('account')is correctly placed after thecreateHostDataContextMiddleware(lines 720-724) that sets these values. This resolves the ordering issue mentioned in past reviews.src/account/account.repository.knex.ts (1)
34-37: Clean implementation of new AccountEntity fields.The new AP URL fields (
ap_following_url,ap_outbox_url,ap_liked_url) are properly:
- Typed as nullable in the
AccountRowinterface- Inserted from draft using optional chaining and
.hrefextraction- Mapped to AccountEntity using the
parseURLhelperThe implementation follows existing patterns and maintains consistency with other URL fields.
Also applies to: 62-64, 382-385
| site?: Site; | ||
| account?: Account; |
There was a problem hiding this comment.
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
| if (site === null) return null; | ||
|
|
||
| const account = await accountService.getDefaultAccountForSite(site); | ||
| const account = ctx.data.account!; |
There was a problem hiding this comment.
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.
| if (!site) { | ||
| throw new Error(`Site not found for host: ${ctx.host}`); | ||
| } | ||
| const site = ctx.data.site!; |
There was a problem hiding this comment.
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.
| 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.
| } | ||
|
|
||
| const account = await accountRepository.getBySite(site); | ||
| const account = ctx.data.account!; |
There was a problem hiding this comment.
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.
| 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.
| // @TODO: Get account by provided handle instead of default account? | ||
| const siteDefaultAccount = | ||
| await accountService.getDefaultAccountForSite(site); | ||
| const siteDefaultAccount = ctx.data.account!; |
There was a problem hiding this comment.
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.
|
|
||
| // @TODO: Get account by provided handle instead of default account? | ||
| const siteDefaultAccount = await accountService.getAccountForSite(site); | ||
| const siteDefaultAccount = ctx.data.account!; |
There was a problem hiding this comment.
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.
ada1e7f to
cc6b351
Compare
ref https://linear.app/ghost/issue/BER-2904 - The middleware already fetches `site` and `account` and sets it in the context variable. This makes dispatchers uses it instead of re-fetching from the database.
rmgpinto
force-pushed
the
dispatchers-n+1
branch
from
cc6b351 to
3364e1b
Compare
ref https://linear.app/ghost/issue/BER-2904
siteandaccountand sets it in the context variable. This makes dispatchers uses it instead of re-fetching from the database.