feat(plugins): implement event bus foundation for plugin system#134
Merged
feat(plugins): implement event bus foundation for plugin system#134
Conversation
Contributor
📝 WalkthroughWalkthroughEventBus を導入して DI コンテナへ配線し、型安全なプラグインイベント（before/after）を追加。AuthService、NoteService、FollowService のライフサイクルでイベント発行を組み込み、プラグイン型定義、ユーティリティ、テスト、ミドルウェア注入を追加しました。 Changes
Sequence DiagramssequenceDiagram
participant Client
participant Route as Note Route
participant NoteService
participant EventBus
participant BeforePlugins as Before Hooks (Plugins)
participant Repository
participant AfterPlugins as After Hooks (Plugins)
Client->>Route: POST /notes (create)
Route->>NoteService: create(payload)
NoteService->>EventBus: emitBefore("note:beforeCreate", { author, note })
EventBus->>BeforePlugins: 呼び出し（優先度順）
BeforePlugins-->>EventBus: { cancelled?, cancelReason?, modifiedPayload? }
alt cancelled
EventBus-->>NoteService: { cancelled: true }
NoteService-->>Route: throw Error (中止)
Route-->>Client: 4xx
else proceed
EventBus-->>NoteService: { cancelled: false, modifiedPayload? }
NoteService->>Repository: save(modified or original note)
Repository-->>NoteService: saved note
NoteService->>EventBus: emit("note:afterCreate", { note, author }) (async)
EventBus->>AfterPlugins: 呼び出し（fire-and-forget）
NoteService-->>Route: return note
Route-->>Client: 2xx
end
sequenceDiagram
participant Client
participant Route as Auth Route
participant AuthService
participant EventBus
participant BeforePlugins as Before Hooks (Plugins)
participant Repository
participant AfterPlugins as After Hooks (Plugins)
Client->>Route: POST /auth/register
Route->>AuthService: register(input)
AuthService->>EventBus: emitBefore("user:beforeRegister", { user })
EventBus->>BeforePlugins: 呼び出し（優先度順）
BeforePlugins-->>EventBus: { cancelled?, cancelReason?, modifiedPayload? }
alt cancelled
EventBus-->>AuthService: { cancelled: true }
AuthService-->>Route: throw Error (中止)
Route-->>Client: 4xx
else proceed
AuthService->>Repository: create user
Repository-->>AuthService: created user
AuthService->>EventBus: emit("user:afterRegister", { user }) (async)
EventBus->>AfterPlugins: 呼び出し（fire-and-forget）
AuthService-->>Route: { user, session }
Route-->>Client: 2xx
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/backend/src/routes/auth.ts (1)
311-312: ログアウト時のAuthService初期化の一貫性を統一してください。登録・ログインフローではeventBusを渡していますが、ログアウトでは渡していません。現在、logout()メソッドはセッションの削除のみを行い、イベントを発火していないため機能的には問題ありませんが、将来
user:logoutイベントが必要になった場合に修正を要します。一貫性のため、次のいずれかの対応を検討してください：
- eventBusを常に渡すようにする（将来のイベント対応に備える）
- イベント発火の必要性が明確になるまで、eventBusはイベント発火時のみ渡すパターンに統一する
🧹 Nitpick comments (5)
packages/backend/src/lib/events.ts (2)
10-10: 未使用のEventEmitterを削除してください。
EventEmitterをインポートしてインスタンス化していますが、実際には使用されていません。ハンドラの登録と呼び出しはすべてMapを使って独自に実装されています。setMaxListeners(100)の設定も効果がありません。♻️ 提案する修正
-import { EventEmitter } from "events"; import type { PluginEvents, PluginEventPayload } from "../plugins/types.js"; import { logger } from "./logger.js";export class EventBus { - private emitter: EventEmitter; private handlers: Map<string, HandlerEntry<unknown>[]>; private beforeHandlers: Map<string, HandlerEntry<unknown>[]>; constructor() { - this.emitter = new EventEmitter(); - this.emitter.setMaxListeners(100); this.handlers = new Map(); this.beforeHandlers = new Map(); }Also applies to: 79-87
212-252:emitBeforeの戻り値の仕様を確認してください。ハンドラが存在するが変更がなかった場合でも
{ modifiedPayload: currentPayload }を返しています。これは意図的な設計と思われますが、呼び出し側が「変更なし」と「同じ値に変更」を区別できません。現在の設計で問題ない場合は、TSDocコメントに「ハンドラが存在する場合は常に
modifiedPayloadを返す」旨を明記すると、利用者にとって分かりやすくなります。packages/backend/src/tests/unit/EventBus.test.ts (1)
25-52: テストペイロードのヘルパー関数を検討してください。同じ構造のペイロードオブジェクトが複数のテストで繰り返されています。テストヘルパー関数やファクトリを作成すると、コードの重複を減らし、変更時のメンテナンスが容易になります。
♻️ 例：ヘルパー関数の作成
// テストファイルの先頭に追加 function createTestNotePayload(overrides?: Partial<NoteAfterCreatePayload>): NoteAfterCreatePayload { return { note: { id: "note1", userId: "user1", text: "Hello", cw: null, visibility: "public", localOnly: false, replyId: null, renoteId: null, fileIds: [], mentions: [], uri: null, createdAt: new Date(), }, author: { id: "user1", username: "alice", displayName: null, avatarUrl: null, host: null, isAdmin: false, isSuspended: false, isSystemUser: false, uri: null, createdAt: new Date(), }, ...overrides, }; }packages/backend/src/services/FollowService.ts (1)
214-227:followIdの複合ID形式について確認してください。削除済みのフォロー関係では元のIDが利用できないため、
${followerId}:${followeeId}という複合IDを使用していますが、プラグイン開発者がこの形式を期待するか確認が必要です。プラグイン型定義（
PluginEvents）でこのIDフォーマットがドキュメント化されているか確認することをお勧めします。#!/bin/bash # follow:afterDelete イベントのペイロード型定義を確認 rg -n -A 10 "follow:afterDelete" packages/backend/src/plugins/types.tspackages/backend/src/services/NoteService.ts (1)
113-149:toPluginUserとtoPluginNoteヘルパーの重複について検討してください。これらの変換ヘルパーはAuthService（そしておそらくFollowService）にも存在する可能性があります。共通のユーティリティに抽出することで、変換ロジックの一貫性を保ち、メンテナンス性を向上させることができます。
♻️ 提案: 共通ユーティリティへの抽出
packages/backend/src/plugins/utils.tsのような共有ファイルを作成することを検討してください:import type { User } from "shared"; import type { Note } from "shared"; import type { PluginUser, PluginNote } from "./types.js"; export function toPluginUser(user: User): PluginUser { return { id: user.id, username: user.username, displayName: user.displayName, avatarUrl: user.avatarUrl, host: user.host, isAdmin: user.isAdmin, isSuspended: user.isSuspended, isSystemUser: user.isSystemUser, uri: user.uri, createdAt: user.createdAt, }; } export function toPluginNote(note: Note): PluginNote { return { id: note.id, userId: note.userId, text: note.text, cw: note.cw, visibility: note.visibility, localOnly: note.localOnly, replyId: note.replyId, renoteId: note.renoteId, fileIds: note.fileIds, mentions: note.mentions, uri: note.uri, createdAt: note.createdAt, }; }
4 tasks
sasagar
force-pushed
the
feature/plugin-event-bus
branch
2 times, most recently
from
ad0b799 to
1979651
Compare
Contributor
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)
packages/backend/src/services/AuthService.ts (1)
212-262: beforeLogin の modifiedPayload を無視している
Line 215-219 でmodifiedPayloadを受け取れる設計ですが、input.usernameとcontextをそのまま使っており変更が反映されません。プラグインで補正できる仕様なら、修正後の値を認証・afterLogin の payload に適用してください。🐛 修正案（例）
- if (this.eventBus) { - const beforeResult = await this.eventBus.emitBefore("user:beforeLogin", { - usernameOrEmail: input.username, - ipAddress: context?.ipAddress || null, - userAgent: context?.userAgent || null, - }); + let usernameOrEmail = input.username; + let ipAddress = context?.ipAddress || null; + let userAgent = context?.userAgent || null; + if (this.eventBus) { + const beforeResult = await this.eventBus.emitBefore("user:beforeLogin", { + usernameOrEmail, + ipAddress, + userAgent, + }); if (beforeResult.cancelled) { throw new Error(beforeResult.cancelReason || "Login cancelled by plugin"); } + if (beforeResult.modifiedPayload) { + usernameOrEmail = beforeResult.modifiedPayload.usernameOrEmail ?? usernameOrEmail; + ipAddress = beforeResult.modifiedPayload.ipAddress ?? ipAddress; + userAgent = beforeResult.modifiedPayload.userAgent ?? userAgent; + } } @@ - const user = await this.userRepository.findByUsername(input.username); + const user = await this.userRepository.findByUsername(usernameOrEmail); @@ - ipAddress: context?.ipAddress || null, - userAgent: context?.userAgent || null, + ipAddress, + userAgent,
🤖 Fix all issues with AI agents
In `@packages/backend/src/plugins/types.ts`:
- Around line 140-147: The UserBeforeLoginPayload currently exposes possible PII
via usernameOrEmail, ipAddress, and userAgent; update the payload to match the
UserBeforeRegisterPayload policy by either (a) removing/renaming usernameOrEmail
to username (and enforce that it cannot contain an email) in the
UserBeforeLoginPayload interface, or (b) move ipAddress and userAgent out of
UserBeforeLoginPayload into a separate context/event interface (e.g.,
UserBeforeLoginContext) that is only emitted to authorized listeners; modify
references to UserBeforeLoginPayload accordingly and add validation/typing to
ensure emails are not accepted in the username field if you choose option (a).
In `@packages/backend/src/services/AuthService.ts`:
- Around line 108-119: The eventBus.emitBefore call returns
beforeResult.modifiedPayload but the code still continues using the original
input for duplicate checks and user creation; update the AuthService flow so
that after calling this.eventBus.emitBefore("user:beforeRegister", ...) you
merge/apply beforeResult.modifiedPayload (when present) into the local
registration data (e.g., replace or merge into the input or a new
registrationPayload variable) and use that modified payload for the subsequent
duplicate checks and the user creation routine (the logic that performs
uniqueness validation and calls whatever creates the user record), while
preserving the existing cancellation handling via beforeResult.cancelled.
In `@packages/backend/src/services/NoteService.ts`:
- Around line 189-211: Change the immutable input variables to mutable and apply
any plugin modifications returned by emitBefore: when calling
this.eventBus.emitBefore("note:beforeCreate", ...) capture
beforeResult.modifiedPayload and, if present, overwrite the local variables used
to create the note (e.g., text, cw, visibility, localOnly, replyId, renoteId,
fileIds) before proceeding; ensure you change their declarations from const to
let so they can be updated, and if fileIds is changed re-run the file
ownership/permission checks (same logic used elsewhere in NoteService) to
validate the new file IDs; keep the existing cancellation handling using
beforeResult.cancelled and return beforeResult.cancelReason when cancelled.
🧹 Nitpick comments (6)
packages/backend/src/routes/auth.ts (1)
245-259: x-forwarded-for を先頭 IP に正規化したい
Line 257 でx-forwarded-forをそのまま渡すと複数 IP がカンマ区切りで入る可能性があり、プラグイン/監査側の扱いが不安定になります。先頭 IP のみ抽出して渡すのが安全です。♻️ 提案修正
- ipAddress: c.req.header("x-forwarded-for") || c.req.header("x-real-ip") || undefined, + ipAddress: (() => { + const forwarded = c.req.header("x-forwarded-for"); + if (forwarded) return forwarded.split(",")[0].trim(); + return c.req.header("x-real-ip") || undefined; + })(),packages/backend/src/tests/unit/EventBus.test.ts (1)
108-126: ペイロード変更テストの型安全性を確認してください。Line 114で
payload.text?.toUpperCase()を使用していますが、NoteBeforeCreatePayloadのtextはstring | null型です。オプショナルチェーンは適切ですが、戻り値がnullの場合の処理を確認してください。🔧 より明示的な null ハンドリングの提案
eventBus.onBefore( "note:beforeCreate", async (payload) => ({ modifiedPayload: { ...payload, - text: payload.text?.toUpperCase() || null, + text: payload.text !== null ? payload.text.toUpperCase() : null, }, }), "modifier-plugin" );packages/backend/src/services/FollowService.ts (1)
197-210: follow:afterDeleteの複合IDについてドキュメント化を検討してください。Line 203で
${followerId}:${followeeId}という複合IDを使用していますが、プラグイン開発者がこの形式を期待していない可能性があります。PluginFollowのidフィールドとは異なる形式になるため、プラグインAPIドキュメントでこの動作を明記することを推奨します。📝 コメントの追加提案
// Emit follow:afterDelete event (async, non-blocking) // Only emit if the relationship existed and we have valid user info if (this.eventBus && exists && follower && followee) { this.eventBus .emit("follow:afterDelete", { - followId: `${followerId}:${followeeId}`, // Composite ID since we don't have the original + // Note: Using composite ID format "followerId:followeeId" since the original + // follow record ID is not available after deletion. Plugin developers should + // be aware that this differs from the `follow.id` in afterCreate events. + followId: `${followerId}:${followeeId}`, follower: toPluginUser(follower), followee: toPluginUser(followee), })packages/backend/src/tests/helpers/pluginTestHelpers.ts (1)
53-107: 他のイベントペイロード用のファクトリ追加を検討してください。現在、Note関連の
beforeCreateとafterCreateのペイロードヘルパーのみ実装されています。将来的に以下のヘルパーも追加すると、テストの一貫性が向上します：
createNoteBeforeDeletePayloadcreateNoteAfterDeletePayloadcreateFollowAfterCreatePayloadcreateFollowAfterDeletePayloadpackages/backend/src/lib/events.ts (1)
211-251: emitBefore()のエラーハンドリングについて確認してください。現在の実装では、beforeハンドラーでエラーが発生してもキャンセルせずに処理を続行します。これは意図的な設計かもしれませんが、セキュリティ関連のプラグイン（スパムフィルターなど）がエラーで失敗した場合、操作が続行されるリスクがあります。
この動作が意図的であれば、JSDocコメントにその旨を明記することを推奨します。
📝 エラー動作のドキュメント化
/** * Emit a "before" event and collect cancellation/modification results * * Handlers are executed in priority order. If any handler cancels, * the operation should be aborted. + * + * `@remarks` + * If a handler throws an error, it is logged but processing continues. + * This means security-critical plugins should handle their own errors + * and return `{ cancelled: true }` explicitly rather than throwing. * * `@param` event - Before event namepackages/backend/src/plugins/types.ts (1)
35-38: 可視性フィールドをユニオン型にして型安全性を強化する
PluginNote.visibilityとNoteBeforeCreatePayload.visibilityがstringだと無効値が混入しやすいです。既存のVisibility型（shared/src/types/common.js）が"public" | "home" | "followers" | "specified"として定義されており、NoteServiceやScheduledNoteServiceで既に使用されています。このプラグイン型でも同じVisibility型を import して使用することで、型安全性と一貫性が向上します。
sasagar
force-pushed
the
feature/plugin-event-bus
branch
from
1979651 to
d83663b
Compare
- Add typed EventBus class with before/after event patterns - Define plugin type interfaces (PluginUser, PluginNote, PluginFollow, PluginActivity) - Use Visibility union type for type safety in note visibility - Implement event payload types for all lifecycle events - Add note lifecycle events: beforeCreate, afterCreate, beforeDelete, afterDelete - Add user lifecycle events: beforeRegister, afterRegister, beforeLogin, afterLogin, afterLogout - Add follow lifecycle events: afterCreate, afterDelete - Add ActivityPub events: beforeInbox, afterInbox, beforeDelivery, afterDelivery - Add moderation events: userSuspended, noteDeleted - Integrate EventBus into DI container and middleware - Emit events in NoteService, AuthService, FollowService - Apply modifiedPayload from beforeCreate/beforeRegister events - Remove PII (email) from user:beforeRegister payload - Remove PII (ipAddress, userAgent) from user:beforeLogin payload - Add shared toPluginUser/toPluginNote utility functions - Add test helper factory functions - Document composite followId format for delete events - Document emitBefore error handling behavior
sasagar
force-pushed
the
feature/plugin-event-bus
branch
from
d83663b to
e1b6e9d
Compare
4 tasks
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements Phase 1 of the plugin system: the EventBus foundation for plugin event hooks.
Changes
EventBus class (
packages/backend/src/lib/events.ts)on()/emit()for post-action eventsonBefore()/emitBefore()for pre-action events with cancellation supportoffPlugin()Plugin type definitions (
packages/backend/src/plugins/types.ts)RoxPlugininterface for plugin registrationPluginContextfor plugins to access core servicesPluginUser,PluginNote,PluginFollow) that exclude sensitive dataEvent hooks integrated into services
NoteService:note:beforeCreate,note:afterCreate,note:beforeDelete,note:afterDeleteAuthService:user:beforeRegister,user:afterRegister,user:beforeLogin,user:afterLoginFollowService:follow:afterCreate,follow:afterDeleteDI container integration
Architecture
Test plan
Summary by CodeRabbit
新機能
テスト
ツール
✏️ Tip: You can customize this high-level summary in your review settings.