Open
Conversation
📝 WalkthroughWalkthroughこのプルリクエストは、SharkBotの大幅な機能拡張であり、埋め込み設定とメッセージ設定の新しいAPI エンドポイント、リアクションロールと検索機能の新しいBotコグ、MongoDB からバックエンドAPIへの移行を含むダッシュボード再設計、および関連するインフラストラクチャと初期化ロジックの更新を導入しています。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/dashboard/src/app/api/guilds/[guildId]/commands/route.tsx (1)
53-55:⚠️ Potential issue | 🟠 Major
deleteアクションでcommandIdの検証が欠落しています。
commandIdが未定義の場合、deleteSlashCommandが不正なパラメータで呼び出され、予期しないエラーが発生する可能性があります。🐛 検証を追加する修正案
case "delete": + if (!commandId) { + return NextResponse.json( + { error: "Missing commandId" }, + { status: 400 }, + ); + } await deleteSlashCommand(guildId, commandId); return NextResponse.json({ success: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/api/guilds/`[guildId]/commands/route.tsx around lines 53 - 55, In the case "delete" branch, add validation for commandId before calling deleteSlashCommand: check that commandId is present and well-formed (non-empty string/expected type) and if missing or invalid return an early NextResponse.json/NextResponse with an error status (e.g., 400) instead of calling deleteSlashCommand; update the logic around the case "delete" branch so deleteSlashCommand(guildId, commandId) is only invoked after the validation passes.
🟠 Major comments (19)
src/api/src/internal/model/MessageSetting.go-12-13 (1)
12-13:⚠️ Potential issue | 🟠 Major
OnDelete:CASCADEをおOnDelete:SET NULLに変更してください。現在の設定では、
EmbedSettingが削除されると参照している全てのMessageSettingがカスケード削除されてしまいます。EmbedIDが nullable であり、一つのEmbedSettingが複数のMessageSettingから参照される可能性があるため、OnDelete:SET NULLを使用すると、埋め込みが削除されてもメッセージ設定は保持され、EmbedIDが NULL にリセットされます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/model/MessageSetting.go` around lines 12 - 13, MessageSetting の GORM 外部キー制約で現在 EmbedSetting の削除時に関連する MessageSetting を全削除してしまうため、struct の Embed フィールド定義（EmbedID と EmbedSetting）にある gorm タグの constraint を OnDelete:CASCADE から OnDelete:SET NULL に変更してください（EmbedID が nullable であることを前提に、EmbedID フィールドと EmbedSetting フィールドの gorm:"foreignKey:EmbedID;constraint:OnUpdate:CASCADE,OnDelete:SET NULL;" といった形に修正して、Embed が削除されたときに EmbedID を NULL にセットするようにしてください）。src/api/src/internal/router/guilds.go-181-185 (1)
181-185:⚠️ Potential issue | 🟠 Major
moduleの空文字バリデーションを追加してください。空の
moduleを許可するとEnabledModules[""]が保存され、設定破損の原因になります。修正案（例）
var req dto.UpdateModuleRequest if err := c.ShouldBindJSON(&req); err != nil { c.JSON(400, gin.H{"error": err.Error()}) return } + if req.Module == "" { + c.JSON(400, gin.H{"error": "module is required"}) + return + }Also applies to: 221-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/guilds.go` around lines 181 - 185, Add a validation step after binding dto.UpdateModuleRequest (the variable req) to reject empty req.Module so we never set EnabledModules[""]; specifically, after c.ShouldBindJSON(&req) check if req.Module == "" and return c.JSON(400, gin.H{"error":"module must not be empty"}) (or similar) instead of proceeding; apply the same validation in the other handler handling the same dto.UpdateModuleRequest at the second occurrence (around line 221) so both code paths prevent saving an empty module key.src/api/src/internal/router/message.go-25-26 (1)
25-26:⚠️ Potential issue | 🟠 Major
typeパラメータの許可値チェックがありません。
welcome/goodbye以外も通るため、想定外レコードが作成・削除対象になります。各ハンドラ入口で拒否してください。修正案（例）
+func isValidMessageType(t string) bool { + return t == "welcome" || t == "goodbye" +} @@ msgType := c.Param("type") + if !isValidMessageType(msgType) { + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid message type"}) + return + }Also applies to: 47-48, 93-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/message.go` around lines 25 - 26, 各ハンドラ入口で取得している msgType := c.Param("type")（c.Param("type") を参照している箇所）に対して許可値チェックを追加し、"welcome" と "goodbye" のどちらでもない場合は早期に 400 を返して拒否してください（例: 条件で比較して c.JSON/AbortWithStatusJSON などでエラーレスポンス）。該当する箇所は msgType := c.Param("type") を使っている全て（コメントで指摘された箇所と create/delete 相当のハンドラ入口）に同様のバリデーションを入れて、不正な値が下流ロジック（レコード作成・削除）へ進まないようにしてください。src/bot/cogs/help.py-50-51 (1)
50-51:⚠️ Potential issue | 🟠 Major
guildパラメータを指定するとグローバルコマンドが除外されます。
fetch_commands(guild=interaction.guild)はそのguild固有のコマンドのみを取得し、グローバルコマンドは返されません。ヘルプコマンドが全コマンドを表示すべき場合、以下のように修正してください：global_cmds = await self.bot.tree.fetch_commands() guild_cmds = await self.bot.tree.fetch_commands(guild=interaction.guild) all_cmds = global_cmds + [cmd for cmd in guild_cmds if cmd.id not in {c.id for c in global_cmds}] for cmd in all_cmds: cmd_list.append(f"`/{cmd.name}` - {cmd.description}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/help.py` around lines 50 - 51, The current help builder calls self.bot.tree.fetch_commands(guild=interaction.guild) which returns only guild-specific commands and omits global commands; update the logic in the help command to call self.bot.tree.fetch_commands() to get global_cmds and also fetch guild_cmds via fetch_commands(guild=interaction.guild), then merge them into all_cmds while deduplicating by command id (use cmd.id) before appending to cmd_list so both global and guild commands appear in the help output.src/api/src/internal/router/message.go-73-85 (1)
73-85:⚠️ Potential issue | 🟠 MajorDB取得失敗時に更新処理へ進む分岐があります。
ErrRecordNotFound以外の取得エラー時もelse側でSaveが実行されるため、障害時の挙動が壊れます。取得エラーを先に500で返してください。修正案（例）
+import "errors" @@ - if result.Error == gorm.ErrRecordNotFound { + if result.Error != nil && !errors.Is(result.Error, gorm.ErrRecordNotFound) { + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to load setting"}) + return + } + + if errors.Is(result.Error, gorm.ErrRecordNotFound) { // 新規作成 if err := db.Create(&setting).Error; err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create setting"}) return } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/message.go` around lines 73 - 85, The current logic treats any db query error as an update path because it only checks for gorm.ErrRecordNotFound; change the branching around result.Error so that: if result.Error is gorm.ErrRecordNotFound then perform db.Create(&setting), else if result.Error != nil return a 500 with an error (do not call db.Save), and only when result.Error == nil proceed to db.Save(&setting). Update the error handling in the code that uses variables result, db, and setting in message.go accordingly so non-NotFound DB errors are returned as HTTP 500 instead of falling through to the Save branch.src/api/src/internal/router/message.go-98-103 (1)
98-103:⚠️ Potential issue | 🟠 Major削除結果の判定順が逆です。
RowsAffectedを先に見ると、DBエラー時に404を返す可能性があります。result.Errorを先に評価してください。修正案（例）
- if result.RowsAffected == 0 { - c.JSON(http.StatusNotFound, gin.H{"error": "Setting not found"}) - return - } if result.Error != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "Internal server error"}) return } + if result.RowsAffected == 0 { + c.JSON(http.StatusNotFound, gin.H{"error": "Setting not found"}) + return + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/message.go` around lines 98 - 103, The handler currently checks result.RowsAffected before result.Error which can return 404 on DB errors; change the logic in the message handler that uses the variable result (the DB operation result) so you first evaluate if result.Error != nil and return the 500 response, then check if result.RowsAffected == 0 and return 404; update the branches that call c.JSON(http.StatusInternalServerError, ...) and c.JSON(http.StatusNotFound, ...) accordingly to use this new order.src/bot/lib/embed.py-7-12 (1)
7-12:⚠️ Potential issue | 🟠 MajorID比較の型不一致でembedが見つからない可能性があります。
メソッド自体は現在コード上で呼び出されていませんが、11行目の
e['ID']はAPI側から返される数値型IDと、文字列型のembed_idパラメータを直接比較しているため、呼び出された場合にマッチしません。Go APIがuint型のIDを返すため、JSON解析後は整数として受け取られます。str(...)で正規化して比較し、キー参照も.get()で防御することをお勧めします。修正例
async def getEmbed(self, guild_id: str, embed_id: str): embed = await self.bot.api.fetch_embed_settings(guild_id) if embed: for e in embed: - if e['ID'] == embed_id: - return e['data'] + if str(e.get("ID")) == embed_id: + return e.get("data")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/lib/embed.py` around lines 7 - 12, In getEmbed, e['ID'] (from fetch_embed_settings) may be an integer while embed_id is a string, causing mismatches; update the comparison to normalize types (e.g., compare str(e.get('ID')) to embed_id) and use e.get('ID') / e.get('data') to avoid KeyError, then return the data when matched; ensure function still returns None when no match is found.src/dashboard/src/lib/Discord/RolePanel.ts-7-20 (1)
7-20:⚠️ Potential issue | 🟠 Majorボタン総数の上限ガードがないため、入力次第で送信失敗します。
現在のコードは5件ずつのチャンク処理を行っていますが、総ボタン数の上限チェックがないため、26件以上のボタンを設定するとAction Rowが6個以上生成され、Discord APIの上限（Legacy: 最大5行）を超えてメッセージ送信時にエラーが発生します。
入力値の検証は早期に行うべきです。
修正例
export function buildActionRowsFromMap(rolesMap: ButtonRoleMap) { const entries = Object.entries(rolesMap); + const MAX_BUTTONS = 25; + if (entries.length > MAX_BUTTONS) { + throw new Error(`roles は最大 ${MAX_BUTTONS} 件までです。受信: ${entries.length}`); + } const rows = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/lib/Discord/RolePanel.ts` around lines 7 - 20, The code that builds ActionRows from entries must validate the total number of buttons before chunking: check entries.length and if it exceeds Discord's limit (25 buttons / 5 rows) either throw an error or return a validation failure; update the function in RolePanel.ts where entries and rows are used (the loop that slices entries and creates components with custom_id `reaction_role:${config.roleId}`) to perform this guard early and return a clear error/validation result instead of attempting to build more than 5 rows.src/api/src/internal/router/embed.go-71-89 (1)
71-89:⚠️ Potential issue | 🟠 MajorUpsert ロジックでデータベースエラーが適切に処理されていません。
Line 71 の
db.First()がErrRecordNotFound以外のエラー（例：DB 接続エラー）を返した場合、コードは else ブロック（更新処理）に進みます。これにより、予期しない動作やデータ不整合が発生する可能性があります。🐛 修正案
var setting model.EmbedSetting // 既存の設定があるか確認 result := db.Where("guild_id = ? AND name = ?", id, input.Name).First(&setting) setting.GuildID = id setting.Name = input.Name setting.Data = input.Data if result.Error == gorm.ErrRecordNotFound { // 新規作成 if err := db.Create(&setting).Error; err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create"}) return } - } else { + } else if result.Error != nil { + // その他のDBエラー + c.JSON(http.StatusInternalServerError, gin.H{"error": "Database error"}) + return + } else { // 更新 if err := db.Save(&setting).Error; err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update"}) return } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/embed.go` around lines 71 - 89, The upsert currently treats any error from db.Where(...).First(&setting) as "not found" and proceeds to update; change the logic in the block that calls db.Where(...).First(&setting) to first check result.Error: if result.Error != nil && result.Error != gorm.ErrRecordNotFound, return a 500 via c.JSON with an appropriate message; then if result.Error == gorm.ErrRecordNotFound perform db.Create(&setting) and handle its error, otherwise perform db.Save(&setting) and handle its error. Ensure you reference the same variables (result, setting) and use gorm.ErrRecordNotFound to distinguish not-found vs other DB errors.src/dashboard/src/app/api/guilds/[guildId]/modules/embed/route.ts-58-60 (1)
58-60:⚠️ Potential issue | 🟠 Major認証エラーとバックエンド障害の HTTP ステータスを混同しないでください。
Line 58-60 では
fetch/jsonの失敗まで 401 に寄り、Line 87-89 と Line 113-115 ではvalidateAdmin()のUnauthorized/Forbiddenまで 500 に潰れます。これだとフロントが未認証・権限不足・バックエンド障害を誤判定するので、認証/認可だけ 401/403 に分けて、それ以外は 5xx に切り分けてください。Also applies to: 87-89, 113-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/embed/route.ts around lines 58 - 60, The catch blocks that currently map all non-"Forbidden" errors to 401 (at the NextResponse.json return) and collapse validateAdmin() failures into 500 should be changed to distinguish auth/authorization errors from backend failures: update each catch (including the one that returns NextResponse.json(...) and the catches around validateAdmin() usages) to check for error.message === "Unauthorized" -> return 401, error.message === "Forbidden" -> return 403, and for any other error return a 5xx (500) response; keep the response body shape { error: error.message } but ensure NextResponse.json is passed the appropriate status (401, 403, or 500) so the frontend can differentiate unauthenticated, unauthorized, and server errors.src/dashboard/src/app/dashboard/[guildId]/welcome/WelcomeGoodbyeEditor.tsx-90-95 (1)
90-95:⚠️ Potential issue | 🟠 Majorgoodbye 設定が welcome 側のチャンネル ID を送っています。
Line 92 で
goodbye.channelIdではなくwelcome.channelIdを POST しているので、退出メッセージだけ保存しても送信先が参加メッセージ側に引っ張られます。ここはgoodbye.channelIdを送る必要があります。🐛 修正例
const body = { goodbye: { - channel_id: welcome.channelId, + channel_id: goodbye.channelId, content: goodbye.content, embed_id: goodbye.embed_id ? Number(goodbye.embed_id) : null, } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/welcome/WelcomeGoodbyeEditor.tsx around lines 90 - 95, The goodbye payload is incorrectly using welcome.channelId when constructing body.goodbye; update the object in WelcomeGoodbyeEditor (the body, goodbye block) to send goodbye.channelId instead of welcome.channelId so the farewell message stores the correct target channel, keeping the existing embed_id conversion (embed_id ? Number(embed_id) : null) and content fields unchanged.src/bot/cogs/search.py-29-46 (1)
29-46:⚠️ Potential issue | 🟠 MajorImgur API呼び出しにタイムアウトと明示的なHTTPエラー処理を追加してください。
Line 30-35 の
aiohttp.ClientSession()はデフォルトで 300 秒のタイムアウトを設定しており、Imgur API からの応答がない場合は 5 分間ブロックされます。また、raise_for_status()がないため HTTP エラーコード (4xx, 5xx) が検出されず、Line 45 の bareexcept:で全ての例外が一括捕捉されるため、エラーの種別が失われデバッグが困難になります。明示的に短いタイムアウトを設定し、raise_for_status()で HTTP エラーを検出し、具体的な例外型のみを捕捉することで、信頼性を向上させられます。修正例
await interaction.response.defer() try: - async with aiohttp.ClientSession() as session: + timeout = aiohttp.ClientTimeout(total=10) + async with aiohttp.ClientSession(timeout=timeout) as session: async with session.get( - f"https://api.imgur.com/3/gallery/search", + "https://api.imgur.com/3/gallery/search", params={"q": search}, headers={"Authorization": f"Client-ID {token}"}, ) as resp: + resp.raise_for_status() data = await resp.json() @@ - except: - return await interaction.followup.send(f"検索に失敗しました。") + except (aiohttp.ClientError, asyncio.TimeoutError, ValueError): + return await interaction.followup.send("検索に失敗しました。")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/search.py` around lines 29 - 46, The Imgur call currently uses a default 300s timeout and a bare except; change to create a ClientSession with an explicit short timeout (e.g., timeout = aiohttp.ClientTimeout(total=10)) and use it in aiohttp.ClientSession(timeout=timeout), call resp.raise_for_status() after the GET to surface HTTP 4xx/5xx as exceptions, and replace the bare except with explicit except blocks (e.g., except aiohttp.ClientResponseError, asyncio.TimeoutError, aiohttp.ClientError as e) that send a clear message via interaction.followup.send including the error string; update the logic around session/resp (the async with session, async with session.get(...)) and keep references to token, search, resp.json(), and interaction.followup.send when implementing these changes.src/dashboard/src/app/dashboard/[guildId]/reaction_role/page.tsx-60-64 (1)
60-64:⚠️ Potential issue | 🟠 Majorロール取得失敗が 500 に直結します。
getGuildRoles(guildId)の例外が未処理なので、Discord 側の一時障害だけでこのページ全体が落ちます。ここもisModuleEnabledと同じレベルでフォールバックを返した方が安全です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/reaction_role/page.tsx around lines 60 - 64, ReactionRoleDataLoader currently awaits getGuildRoles(guildId) without handling errors, so any exception from Discord will surface as a 500; wrap the getGuildRoles call in a try/catch (similar to the isModuleEnabled fallback) and on error return a safe fallback (e.g., an empty roles array or null) while logging the error, then render ReactionRoleClient with that fallback roles value to avoid the whole page crashing.src/dashboard/src/app/dashboard/[guildId]/page.tsx-32-37 (1)
32-37:⚠️ Potential issue | 🟠 MajorAPI 失敗時のフォールバックが効いていません。
dataPromiseが reject するとif (!data)に入る前に Server Component ごと落ちます。await dataPromiseをtry/catchして/dashboardに寄せないと、一時的なバックエンド障害で guild ページが 500 になります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/page.tsx around lines 32 - 37, The ModuleListWrapper server component awaits dataPromise without handling rejections, so a rejected promise will crash the component; wrap the await in a try/catch around the await dataPromise inside ModuleListWrapper, and on either an error or null/undefined data call redirect("/dashboard") (or the same fallback) to ensure transient API failures don't produce a 500; reference ModuleListWrapper, dataPromise and redirect("/dashboard") when making the change.src/dashboard/src/app/dashboard/[guildId]/welcome/page.tsx-49-52 (1)
49-52:⚠️ Potential issue | 🟠 Major読込失敗を「未設定」に変換しないでください。
catch(() => null)で API エラーと本当に未設定のケースを同一視しているので、バックエンド障害時に既存設定が空に見えます。この状態で保存すると正常な設定を上書きしかねないため、失敗時はエラー UI へ分岐した方が安全です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/welcome/page.tsx around lines 49 - 52, The current Promise.all call masks API failures by using catch(() => null) for fetchMessageSetting, conflating real "unset" values with backend errors; update the logic around fetchMessageSetting (the welcomeRaw and goodbyeRaw variables) to stop swallowing errors — either remove the catch handlers so exceptions propagate to the page-level loader/error boundary or replace the catch with explicit error handling that distinguishes network/API errors from a legitimately empty/undefined response, and ensure the page branches to an error UI when fetchMessageSetting throws instead of treating failures as "not set."src/dashboard/src/app/api/guilds/[guildId]/modules/goodbye/route.ts-61-64 (1)
61-64:⚠️ Potential issue | 🟠 Major認可以外の障害まで 401/403 に潰れています。
fetchMessageSetting/saveMessageSetting/deleteMessageSettingの失敗やrequest.json()の parse error まで全部 401/403 として返すので、実際はバックエンド障害でも UI からは「未認証」に見えます。auth 由来の例外だけを個別にマッピングし、それ以外は 500 へフォールバックしてください。Also applies to: 84-87, 102-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/goodbye/route.ts around lines 61 - 64, The catch blocks in route.ts currently map every error to 401/403, hiding non-auth failures; update the three catch sites (the blocks handling fetchMessageSetting/saveMessageSetting/deleteMessageSetting and request.json() parsing) to detect only authentication errors (e.g., error.message === "Forbidden" -> 403 or error.message === "Unauthorized" -> 401) and otherwise return a 500 status for unexpected/backend errors, returning the error.message in the JSON body so the UI can distinguish auth failures from server errors.src/dashboard/src/components/ButtonRole.tsx-99-101 (1)
99-101:⚠️ Potential issue | 🟠 Major削除ボタンにアクセシブルネームがありません。
どちらの削除ボタンもアイコンしか持っていないので、
aria-labelなしだと操作名が伝わりません。aria-label={ボタン ${index + 1} を削除}のような名前を付けてください。Also applies to: 154-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/components/ButtonRole.tsx` around lines 99 - 101, Add an accessible name to the icon-only delete buttons by adding an aria-label that includes the button index; update the <button onClick={() => onDelete(id)} ...> in ButtonRole.tsx to include aria-label={`ボタン ${index + 1} を削除`} (and do the same for the second delete button around lines 154–159), ensuring you use the same index prop/variable used in the component and keep onDelete(id) and the existing className/props unchanged.src/dashboard/src/app/dashboard/[guildId]/embed/EmbedEditorClient.tsx-40-49 (1)
40-49:⚠️ Potential issue | 🟠 Major保存直後の埋め込みは削除できません。
成功時に state へ積む
newSettingにIDが入っていないため、Line 72 のString(e.ID)が"undefined"になります。結果として次の DELETE はembed_id: NaNを送り、リロード前の新規保存/更新分を削除できません。保存 API のレスポンスからIDを反映するか、保存後に一覧を再取得してください。Also applies to: 71-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/embed/EmbedEditorClient.tsx around lines 40 - 49, Saved embeds lack their returned ID so String(e.ID) becomes "undefined" and subsequent DELETEs send embed_id: NaN; update the code that handles the save response (the logic building newSetting used in setSavedEmbeds) to include the ID returned by the save API in the new EmbedSetting (or alternatively trigger a refetch of the embed list after a successful save), ensuring references like String(e.ID) and the DELETE payload use the real numeric ID (check newSetting, setSavedEmbeds, EmbedSetting and the code paths around the save response and the DELETE at lines referenced as 71-72).src/dashboard/src/app/api/guilds/[guildId]/modules/reactionRole/button/route.ts-48-60 (1)
48-60:⚠️ Potential issue | 🟠 Major
rolesの各要素を route 側でも検証してください。今は
channelIdとrolesの存在しか見ていないので、UI が追加直後に持つroleId: ""のエントリや改ざん payload までそのままbuildActionRowsFromMapに流れます。少なくとも各ボタンのroleId/label/styleの shape と空値はここで 400 にしないと、壊れ方が送信時まで遅延します。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/reactionRole/button/route.ts around lines 48 - 60, Validate the incoming roles array in the route handler before calling buildActionRowsFromMap: ensure request.json()'s roles is an array and that each entry has non-empty roleId, label, and style of the expected types/shape (e.g., roleId and label are non-empty strings; style is one of the allowed values), and return NextResponse.json({ error: "Invalid roles" }, { status: 400 }) if any entry fails; place this check after getValidatedChannelInServer(guildId, channelId) succeeds and before buildActionRowsFromMap(roles) is invoked so malformed or empty-button entries are rejected early.
🟡 Minor comments (11)
src/dashboard/src/components/Alert.tsx-13-20 (1)
13-20:⚠️ Potential issue | 🟡 Minor
useEffectの依存配列にtextとredirectUrlが欠落しています。React Hooksのルールに従い、エフェクト内で使用されるすべての値を依存配列に含める必要があります。現在の実装では、propsが変更されてもエフェクトが再実行されません。
🔧 依存配列を修正
useEffect(() => { alert(text); if (redirectUrl) { router.push(redirectUrl); } else { router.push("/dashboard"); } - }, [router]); + }, [router, text, redirectUrl]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/components/Alert.tsx` around lines 13 - 20, The useEffect in the Alert component references text and redirectUrl but only lists router in its dependency array; update the effect's dependencies to include text and redirectUrl (i.e., useEffect(..., [router, text, redirectUrl])) so the effect re-runs when those props change and continue using router.push/alert as before.src/dashboard/src/components/ModuleCard.tsx-32-37 (1)
32-37:⚠️ Potential issue | 🟡 Minor
onToggleのPromiseが処理されていません。エラーが見逃される可能性があります。
onToggleはPromise<void>を返しますが、awaitされておらず、エラーハンドリングもありません。トグル中のローディング状態や、エラー発生時のフィードバックがユーザーに提供されません。🔧 エラーハンドリングを追加する修正案
onClick={(e) => { e.stopPropagation(); if (onToggle) { - onToggle(mod.id); + onToggle(mod.id).catch((err) => { + console.error("Failed to toggle module:", err); + }); } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/components/ModuleCard.tsx` around lines 32 - 37, onToggle (a Promise-returning prop) is invoked in the onClick handler without awaiting or handling errors; update the ModuleCard component so the click handler becomes async, introduce a local loading state (e.g., isToggling via useState) and set it true before awaiting onToggle(mod.id) and false in a finally block, wrap the await in try/catch to handle and surface errors (log via console or process logger and show user feedback/toast or set an error state), and guard double-clicks by returning early when isToggling is true; ensure references to onToggle, mod.id and the ModuleCard onClick handler are updated accordingly.src/api/src/internal/router/guilds.go-136-136 (1)
136-136:⚠️ Potential issue | 🟡 MinorSwagger のルート注釈が実装と不一致です。
実装は
GET /guilds/:id/moduleですが、コメントは/module/checkになっています。公開API仕様として揃えてください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/guilds.go` at line 136, Swagger のルート注釈が実装と不一致なので、コメントの `@Router` を実装されているエンドポイントに合わせて修正してください — つまり現在の実装が GET /guilds/:id/module であるため、該当する注釈行（現在は "// `@Router` /guilds/{id}/module/check [get]"）を "/guilds/{id}/module [get]" に直し、パスパラメータ {id} と HTTP メソッド get が正しく表記されていることを確認してください（該当箇所は guilds.go のルート注釈行）。src/dashboard/src/app/api/guilds/[guildId]/roles/route.tsx-56-62 (1)
56-62:⚠️ Potential issue | 🟡 Minor変数名とエラーメッセージが不正確です。
- Line 57: 変数名が
channelsになっていますが、実際はrolesを取得しています。- Line 61: エラーメッセージが「DB接続エラー」となっていますが、この処理は Discord API を呼び出しており、データベース接続ではありません。
🐛 修正案
try { - const channels = await getGuildRoles(guildId); + const roles = await getGuildRoles(guildId); - return NextResponse.json(channels); + return NextResponse.json(roles); } catch { - return NextResponse.json({ error: "DB接続エラー" }, { status: 500 }); + return NextResponse.json({ error: "ロール取得エラー" }, { status: 500 }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/api/guilds/`[guildId]/roles/route.tsx around lines 56 - 62, The response handler incorrectly names the fetched data and mislabels the error: rename the local variable channels to roles where getGuildRoles(guildId) is called and return NextResponse.json(roles) on success; in the catch block change the error message from "DB接続エラー" to something like "Discord APIエラー" (keeping status 500) so it correctly reflects the failure source (refer to getGuildRoles and the try/catch around it in route.tsx).src/dashboard/src/lib/Discord/Bot.ts-83-83 (1)
83-83:⚠️ Potential issue | 🟡 MinorPOST リクエストに
next: { revalidate }オプションは不適切です。
next: { revalidate }は GET リクエストのキャッシュ制御用です。POST リクエストはキャッシュされないため、このオプションは効果がなく削除すべきです。🐛 修正案
const response = await fetch(`${DISCORD_API_BASE_URL}/channels/${channelId}/messages`, { method: "POST", headers: headers, body: JSON.stringify({ content: message, embeds: embed ? [embed] : [], components: Array.isArray(components) ? components : (components ? [components] : undefined), }), - next: { revalidate: 10 }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/lib/Discord/Bot.ts` at line 83, The fetch/HTTP POST call in Bot.ts mistakenly includes the Next.js cache option next: { revalidate: 10 } which only applies to GET requests; remove the next: { revalidate: 10 } entry from the options object used in the POST request (locate the fetch/axios call that sets method: 'POST' and the options containing next: { revalidate: 10 }) so the POST options no longer include any Next-specific cache fields.src/dashboard/src/lib/Discord/Bot.ts-52-62 (1)
52-62:⚠️ Potential issue | 🟡 Minortry-catch ブロックのエラーハンドリングが不整合です。
Line 56 で
throw new Error(...)しているエラーが、Line 60-62 の catch ブロックで捕捉されnullが返されます。これにより、チャンネルが見つからない場合にエラーメッセージが失われ、呼び出し元はエラーの原因を特定できません。🐛 修正案
export async function getValidatedChannelInServer(guildId: string, channelId: string): Promise<any> { if (!isValidDiscordId(guildId) || !isValidDiscordId(channelId)) { throw new Error("Invalid Guild or Channel ID format"); } const channels: any[] = await getGuildChannels(guildId); - - try { - const channel = channels.find((c) => c.id === channelId); - - if (!channel) { - throw new Error(`Channel with ID ${channelId} not found in guild ${guildId}`); - } - - return channel; - } catch { - return null; - } + const channel = channels.find((c) => c.id === channelId); + + if (!channel) { + return null; + } + + return channel; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/lib/Discord/Bot.ts` around lines 52 - 62, 現在の try-catch は内部で throw したエラーを即座に catch して null を返しているため、Channel が見つからない原因が失われています；修正は Channel 検索ロジック (channels.find(...) を使う箇所、参照している variables: channelId, guildId, channels) から不要な throw/catch を取り除き、見つかった場合は channel を返し、見つからなければ明示的に null を返す（またはエラーを投げして呼び出し元に伝搬させる方針に統一）するようにして、エラー情報が失われないようにしてください。src/bot/cogs/reaction_role.py-27-28 (1)
27-28:⚠️ Potential issue | 🟡 Minor
custom_idを数値化する前に形式チェックを入れてください。Line 27 は
reaction_role:の後ろが空文字や非数値だとValueErrorになり、listener 全体が失敗します。int()の前に形式チェックを入れて、不正なcustom_idはフォローアップで弾いた方が安全です。🧩 修正例
- role_id = int(custom_id.split(":")[1]) + _, _, raw_role_id = custom_id.partition(":") + if not raw_role_id.isdigit(): + await interaction.followup.send("ロール情報が不正です。", ephemeral=True) + return + role_id = int(raw_role_id) role = interaction.guild.get_role(role_id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/reaction_role.py` around lines 27 - 28, custom_id を int に変換する前に形式チェックを追加してください: listener 内で custom_id が "reaction_role:<id>" 形式かを確認し、split(":") の結果が期待どおりで、id 部分が空でなく数字のみ（str.isdigit() など）であることを検証してから role_id = int(...) を実行し、検証に失敗した場合は interaction.followup（または適切な返信）で不正な custom_id を弾いて処理を中断するようにしてください；参照箇所は変数名 custom_id、role_id と interaction.guild.get_role を使って探してください。src/bot/cogs/welcome.py-26-60 (1)
26-60:⚠️ Potential issue | 🟡 Minor
Exceptionを丸ごと握りつぶさないでください。ここで包括捕捉すると、API schema 破壊や Discord 権限エラーまで同じログ 1 行に潰れて、welcome/goodbye が静かに停止します。少なくとも
logging.exception(...)で traceback を残すか、想定済み例外だけを個別に捕捉した方が追跡しやすいです。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/welcome.py` around lines 26 - 60, The current broad try/except around the welcome/goodbye handler swallows all exceptions (ending with "except Exception as e")—replace it so errors preserve tracebacks and only expected errors are handled: use logging.exception(...) in the except block to log full traceback for unexpected failures, or explicitly catch anticipated exceptions (e.g. discord.Forbidden, discord.HTTPException, KeyError, ValueError) around calls like self.bot.api.fetch_message_setting, self.welcome_parse, member.guild.get_channel, and channel.send and handle or return for those, while re-raising or logging.exception for anything else; ensure event_type remains part of the log context so you can trace which handler failed.src/dashboard/src/app/api/guilds/[guildId]/modules/welcome/route.ts-101-104 (1)
101-104:⚠️ Potential issue | 🟡 MinorDELETEハンドラーのエラーログメッセージが不正確です。
console.errorのメッセージが "Settings POST Error" となっていますが、これは DELETE ハンドラー内にあります。また、status || 500のフォールバックは冗長です（statusは必ず 401 か 403 が設定されます）。🐛 修正案
} catch (error: any) { - console.error("Settings POST Error:", error); - const status = error.message === "Forbidden" ? 403 : 401; - return NextResponse.json({ error: error.message }, { status: status || 500 }); + console.error("Settings DELETE Error:", error); + const status = error.message === "Forbidden" ? 403 : 401; + return NextResponse.json({ error: error.message }, { status }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/welcome/route.ts around lines 101 - 104, The catch block in the DELETE handler incorrectly logs "Settings POST Error" and uses a redundant status fallback; update the console.error call to a correct message like "Settings DELETE Error" (located in the catch block of the DELETE handler in route.ts) and remove the unnecessary "|| 500" fallback so the return uses NextResponse.json({ error: error.message }, { status }) directly; ensure you reference the same error variable and NextResponse.json call when making the change.src/dashboard/src/lib/api/requests.ts-43-56 (1)
43-56:⚠️ Potential issue | 🟡 Minor
fetchGuildSettingsの戻り値が一貫していません。
createGuildEntryを呼び出した後、return;でundefinedを返していますが、新しく作成されたギルドデータを返すべきです。- Lines 47-49 のチェックは冗長です。
isValidDiscordIdが通過した時点でguildIdは必ず truthy です。🐛 修正案
export async function fetchGuildSettings(guildId: string) { if (!isValidDiscordId(guildId)) { throw new Error("Invalid Guild ID"); } - if (!guildId) { - throw new Error("Guild ID is required"); - } const data = await fetch(`${RESOURCE_API_BASE_URL}/guilds/${guildId}`); if (!data.ok) { - await createGuildEntry(guildId); - return; + return await createGuildEntry(guildId); } return data.json(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/lib/api/requests.ts` around lines 43 - 56, fetchGuildSettings currently does a redundant truthy check after isValidDiscordId and returns undefined after calling createGuildEntry; remove the unnecessary "if (!guildId)" check (isValidDiscordId guarantees truthiness) and change the failure branch to return the newly created guild data instead of undefined by either returning the result of createGuildEntry(guildId) if that returns the created object or performing a follow-up fetch to `${RESOURCE_API_BASE_URL}/guilds/${guildId}` and returning its JSON; update only the logic in fetchGuildSettings and reference isValidDiscordId, createGuildEntry and RESOURCE_API_BASE_URL when making the change.src/dashboard/src/lib/api/requests.ts-109-112 (1)
109-112:⚠️ Potential issue | 🟡 Minor到達不能コードがあります。
throw文の後にreturn文がありますが、これは決して実行されません。また、セミコロンの配置により構文的にも問題があります。🐛 修正案
if (!response.ok) { throw new Error(`Failed to save ${type} settings: ${response.statusText}`); - return - }; + } return response.json();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/lib/api/requests.ts` around lines 109 - 112, Remove the unreachable `return` after the `throw` inside the response check and fix the stray semicolon placement so the block ends cleanly; specifically, in the function where `if (!response.ok) { throw new Error(\`Failed to save ${type} settings: ${response.statusText}\`); return }` (references: `response`, `type`), delete the `return` and ensure the `if` block is closed without an extra semicolon so the syntax is valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 198a92cf-ffa8-4695-9312-8ef04c938ad5
⛔ Files ignored due to path filters (1)
src/dashboard/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (54)
.gitignoreREADME.mddocker-compose.yamlsrc/api/scripts/run.ps1src/api/src/cmd/main.gosrc/api/src/internal/dto/Guild.gosrc/api/src/internal/model/EmbedSetting.gosrc/api/src/internal/model/MessageSetting.gosrc/api/src/internal/router/embed.gosrc/api/src/internal/router/guilds.gosrc/api/src/internal/router/message.gosrc/bot/cogs/batch.pysrc/bot/cogs/help.pysrc/bot/cogs/reaction_role.pysrc/bot/cogs/search.pysrc/bot/cogs/test.pysrc/bot/cogs/welcome.pysrc/bot/lib/api.pysrc/bot/lib/command.pysrc/bot/lib/embed.pysrc/bot/main.pysrc/dashboard/package.jsonsrc/dashboard/src/app/api/guilds/[guildId]/channels/route.tsxsrc/dashboard/src/app/api/guilds/[guildId]/commands/route.tsxsrc/dashboard/src/app/api/guilds/[guildId]/modules/embed/route.tssrc/dashboard/src/app/api/guilds/[guildId]/modules/goodbye/route.tssrc/dashboard/src/app/api/guilds/[guildId]/modules/reactionRole/button/route.tssrc/dashboard/src/app/api/guilds/[guildId]/modules/route.tssrc/dashboard/src/app/api/guilds/[guildId]/modules/welcome/route.tssrc/dashboard/src/app/api/guilds/[guildId]/roles/route.tsxsrc/dashboard/src/app/dashboard/[guildId]/client.tsxsrc/dashboard/src/app/dashboard/[guildId]/embed/EmbedEditorClient.tsxsrc/dashboard/src/app/dashboard/[guildId]/embed/page.tsxsrc/dashboard/src/app/dashboard/[guildId]/help/page.tsxsrc/dashboard/src/app/dashboard/[guildId]/page.tsxsrc/dashboard/src/app/dashboard/[guildId]/reaction_role/Client.tsxsrc/dashboard/src/app/dashboard/[guildId]/reaction_role/page.tsxsrc/dashboard/src/app/dashboard/[guildId]/search/page.tsxsrc/dashboard/src/app/dashboard/[guildId]/test/page.tsxsrc/dashboard/src/app/dashboard/[guildId]/welcome/WelcomeGoodbyeEditor.tsxsrc/dashboard/src/app/dashboard/[guildId]/welcome/page.tsxsrc/dashboard/src/app/page.tsxsrc/dashboard/src/components/Alert.tsxsrc/dashboard/src/components/ButtonRole.tsxsrc/dashboard/src/components/EmbedSelecter.tsxsrc/dashboard/src/components/LoadingSkeleton.tsxsrc/dashboard/src/components/ModuleCard.tsxsrc/dashboard/src/constants/api/endpoints.tssrc/dashboard/src/constants/reaction_role/rolesmap.tssrc/dashboard/src/lib/Discord/Bot.tssrc/dashboard/src/lib/Discord/RolePanel.tssrc/dashboard/src/lib/api/requests.tssrc/dashboard/src/lib/commands.tssrc/dashboard/src/lib/modules.ts
Comment on lines
+55
to
+59
| bot.session = aiohttp.ClientSession() | ||
|
|
||
| base_url = os.environ.get("RESOURCE_API_BASE_URL", "http://localhost:8080") | ||
| bot.api = ResourceAPIClient(bot.session, base_url) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the repository structure
git ls-files | grep -E "\.py$" | head -20Repository: SharkBot-Dev/NewSharkBot
Length of output: 332
🏁 Script executed:
# Look at the src/bot/main.py file to understand the context
wc -l src/bot/main.pyRepository: SharkBot-Dev/NewSharkBot
Length of output: 86
🏁 Script executed:
# Read the relevant section and surrounding context
cat -n src/bot/main.py | sed -n '40,75p'Repository: SharkBot-Dev/NewSharkBot
Length of output: 1162
🏁 Script executed:
# Search for close() method implementations in the bot file
rg -n "async def close|def close" src/bot/main.pyRepository: SharkBot-Dev/NewSharkBot
Length of output: 50
🏁 Script executed:
# Search for ClientSession cleanup or close calls in the entire bot directory
rg -n "session.*close|close.*session" src/bot/Repository: SharkBot-Dev/NewSharkBot
Length of output: 50
🏁 Script executed:
# Check the class definition and inheritance
rg -n "class.*Bot|class NewSharkBot" src/bot/main.py -A 3Repository: SharkBot-Dev/NewSharkBot
Length of output: 205
🏁 Script executed:
# Read the entire main.py file to see the complete structure
cat -n src/bot/main.pyRepository: SharkBot-Dev/NewSharkBot
Length of output: 2333
🏁 Script executed:
# Search for any cleanup/close related code more broadly
rg -n "close\|cleanup\|shutdown" src/bot/main.py -iRepository: SharkBot-Dev/NewSharkBot
Length of output: 50
🏁 Script executed:
# Check if there's a close method defined anywhere in the bot class
ast-grep --pattern 'class NewSharkBot {
$$$
}'Repository: SharkBot-Dev/NewSharkBot
Length of output: 50
🏁 Script executed:
# Look for bot.run or how the bot is started/stopped
rg -n "bot.run\|bot.close" src/bot/Repository: SharkBot-Dev/NewSharkBot
Length of output: 50
🏁 Script executed:
# Check if there's any other file handling the bot lifecycle/cleanup
rg -n "setup_hook\|close\|on_error" src/bot/ -lRepository: SharkBot-Dev/NewSharkBot
Length of output: 50
🏁 Script executed:
# Look at ResourceAPIClient to understand if it handles session cleanup
cat src/bot/lib/api.pyRepository: SharkBot-Dev/NewSharkBot
Length of output: 5711
aiohttp.ClientSession のクローズ処理が不足しています。
setup_hook() で生成した ClientSession (line 55) が終了時に閉じられていません。Botシャットダウン時にリソースリークが発生します。NewSharkBot クラスで close() メソッドをオーバーライドし、セッションを明示的に閉じてください。
修正案（例）
class NewSharkBot(commands.AutoShardedBot):
def __init__(self):
super().__init__(
command_prefix="!",
help_command=None,
intents=discord.Intents.all(),
tree_cls=tree.CustomTree,
)
print("InitDone")
self.api = None
+ self.session = None
self.slashcommands: Dict[str, Command] = {}
self.embed = customEmbed(self)
def add_slashcommand(self, command: Command):
self.slashcommands[command.name] = command
+ async def close(self):
+ if self.session is not None and not self.session.closed:
+ await self.session.close()
+ await super().close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/main.py` around lines 55 - 59, The aiohttp.ClientSession created in
setup_hook is never closed; add an override of NewSharkBot.close to properly
close bot.session and then call the parent close: implement async def
close(self): if self.session: await self.session.close(); await super().close()
(or use try/except to ignore if already closed) so the ClientSession opened in
setup_hook is explicitly awaited closed on shutdown.
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
2 participants
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 by CodeRabbit
リリースノート
新機能
改善