Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughバックエンド、ボット、ダッシュボードに跨る「グローバルチャット」機能を追加。DBモデル・APIルータ・ボットのメッセージリレー・ダッシュボードUI/ルートを導入し、インフラ用の Changes
Sequence Diagram(s)sequenceDiagram
actor User as Discord User
participant Bot as Discord Bot
participant API as Backend API
participant DB as Database
participant Webhook as Target Webhook
User->>Bot: チャンネルへメッセージ送信
Bot->>API: GET /globalchat/channels/:id(チャネル設定取得)
API->>DB: GlobalChatRoom/Connect/Restriction を照会
DB-->>API: ルーム設定・接続情報
API-->>Bot: チャネル設定と接続リスト
alt 制限に該当(ban/mute/slowmode/account age)
Bot->>Bot: リレーを中止(反応やログ)
else リレー実行
Bot->>API: GET /globalchat/rooms/:name(アクティブ接続取得)
API->>DB: GlobalChatConnect を取得
DB-->>API: webhook URL 等
API-->>Bot: 接続情報
par 各宛先に並列送信
Bot->>Webhook: (必要なら) webhook 作成/修復
Webhook-->>Bot: webhook URL
Bot->>Webhook: メッセージを webhook で送信(embed 等)
Webhook->>Webhook: 宛先チャンネルへ表示
end
Bot->>Bot: 最大 5s 待機、失敗は無視して残タスクをキャンセル
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (6)
run.sh-1-1 (1)
1-1:⚠️ Potential issue | 🟡 Minorファイル末尾に改行がありません。
POSIX 標準では、テキストファイルは改行で終わることが推奨されています。一部のツールやエディタで警告が表示される可能性があります。
📝 改行を追加する修正案
sudo docker compose up -d --build +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@run.sh` at line 1, Add a trailing newline at the end of run.sh so the file ends with a newline character; specifically ensure the final line containing "sudo docker compose up -d --build" is followed by a newline (i.e., the file ends with a blank line terminator) to satisfy POSIX text file conventions and avoid editor/tool warnings.src/api/src/internal/model/GlobalChatSetting.go-57-66 (1)
57-66:⚠️ Potential issue | 🟡 MinorWebhookURL のセキュリティに関する考慮が必要です。
WebhookURLは機密情報として扱う必要があります。現在の実装では JSON レスポンスに含まれるため、クライアント側に露出します。
- Bot 内部でのみ使用する場合は
json:"-"で非公開にすることを検討してください- または、ダッシュボードでの表示が必要な場合はマスキング処理を追加してください
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/model/GlobalChatSetting.go` around lines 57 - 66, The WebhookURL field on the GlobalChatConnect struct is sensitive and must not be returned raw in API JSON; update the struct definition for GlobalChatConnect by removing or replacing the json tag on WebhookURL (e.g., change to json:"-" to prevent automatic exposure) or implement a safe serialization path that returns a masked value for WebhookURL when needed (e.g., add a method like MaskedWebhook() or a DTO/serializer that emits a masked string) and ensure all JSON encoding paths use the new tag or DTO instead of the raw struct.src/dashboard/src/lib/api/requests.ts-327-333 (1)
327-333:⚠️ Potential issue | 🟡 Minor
encodeURIではなくencodeURIComponentを使用してください。
encodeURIは/,?,#などの特殊文字をエンコードしないため、特殊文字を含むルーム名で問題が発生する可能性があります。また、エラーメッセージに$の誤りがあります。🐛 提案する修正
export async function getGlobalChatConfig(name: string) { - const res = await fetch(`${RESOURCE_API_BASE_URL}/globalchat/rooms/${encodeURI(name)}`) + const res = await fetch(`${RESOURCE_API_BASE_URL}/globalchat/rooms/${encodeURIComponent(name)}`) if (!res.ok) { - throw new Error(`$Failed to get settings: ${res.statusText}`) + throw new Error(`Failed to get settings: ${res.statusText}`) } return await res.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 327 - 333, In getGlobalChatConfig, replace encodeURI(name) with encodeURIComponent(name) to correctly encode reserved characters in room names when building the fetch URL (RESOURCE_API_BASE_URL/.../globalchat/rooms/); also fix the thrown error string by removing the stray `$` so the throw uses "Failed to get settings: ${res.statusText}" or a proper concatenation to include res.statusText.src/api/src/internal/router/globalchat.go-230-235 (1)
230-235:⚠️ Potential issue | 🟡 Minor204 No Content では JSON ボディを送信すべきではありません。
HTTP 204 はレスポンスボディを持たないことを意味します。
c.JSON(http.StatusNoContent, nil)ではなく、ステータスのみを設定してください。🐛 提案する修正
- c.JSON(http.StatusNoContent, nil) + c.Status(http.StatusNoContent)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/globalchat.go` around lines 230 - 235, The handler currently sends a JSON body with a 204 response; update the response to send no body by replacing the c.JSON(http.StatusNoContent, nil) call with a status-only response (e.g., c.Status(http.StatusNoContent)) after the delete succeeds; locate the block that calls db.Where(...).Delete(&model.GlobalChatConnect{}) and change the final response to use c.Status on the gin.Context instead of c.JSON.src/dashboard/src/app/dashboard/[guildId]/globalchat/page.tsx-42-44 (1)
42-44:⚠️ Potential issue | 🟡 MinorUI テキストがこのページの機能と一致していません。
「埋め込み」に関するテキストがありますが、このページはグローバルチャット機能用です。コピー&ペーストの誤りと思われます。
🐛 提案する修正
<p className="mt-10 text-center text-xs text-slate-400"> - ※ 同じ名前の埋め込みを保存すると、上書きされます。 + ※ グローバルチャットの設定は即座に反映されます。 </p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/page.tsx around lines 42 - 44, The paragraph text "※ 同じ名前の埋め込みを保存すると、上書きされます。" is a copy/paste leftover and doesn’t match the Global Chat page; locate the JSX <p className="mt-10 text-center text-xs text-slate-400"> in the page.tsx component for the global chat and replace the message with copy relevant to global chat (for example, a short note about global messages visibility or chat persistence), ensuring the wording matches the page purpose and tone.src/bot/cogs/globalchat.py-73-73 (1)
73-73:⚠️ Potential issue | 🟡 Minorログレベルが不適切です。
デバッグ情報に
logging.errorを使用しています。logging.debugまたはlogging.infoに変更するか、本番環境では削除してください。🐛 提案する修正
- logging.error(config) + logging.debug(f"Relaying message with config: {config}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` at line 73, The call logging.error(config) in src/bot/cogs/globalchat.py is using an inappropriate log level for debug data; replace it with logging.debug(config) (or logging.info(config) if you need higher visibility) or remove the statement for production. Locate the logging.error call (in the globalchat cog—e.g., the function or method that currently logs the `config` variable) and update the logging level accordingly, ensuring any sensitive data in `config` is not logged in production.
🧹 Nitpick comments (8)
run.sh (1)
1-1:sudoの使用について検討してください。スクリプト内で
sudoを使用すると、セキュリティやファイル権限の問題が発生する可能性があります。代わりに、ユーザーをdockerグループに追加することで、sudoなしで Docker コマンドを実行できるようになります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@run.sh` at line 1, The script run.sh currently invokes "sudo docker compose up -d --build"; remove the hardcoded sudo from that command and instead document or enforce adding the user to the docker group so the script can run without elevated privileges (e.g., instruct to run usermod -aG docker <user> and re-login or check group membership before running). Update run.sh to call "docker compose up -d --build" and optionally add a brief pre-check that the current user is in the docker group (or print a clear message instructing how to add the user to the docker group) so callers aren’t required to use sudo.src/api/src/internal/model/GlobalChatSetting.go (1)
31-37: JSON タグの不整合があります。
RoomNameフィールドの JSON タグがjson:"name"になっていますが、GlobalChatConnectでは同じフィールドがjson:"room_name"で定義されています。一貫性のためにjson:"room_name"に変更することを推奨します。また、
(RoomName, UserID)の組み合わせに対するユニーク制約がないため、同じユーザーが同じルームに複数回登録される可能性があります。♻️ 提案する修正
type GlobalChatRoomUser struct { - ID uint `gorm:"primaryKey"` - RoomName string `gorm:"index;not null" json:"name"` - UserID string `gorm:"index;not null" json:"user_id"` + ID uint `gorm:"primaryKey"` + RoomName string `gorm:"uniqueIndex:idx_room_user;not null" json:"room_name"` + UserID string `gorm:"uniqueIndex:idx_room_user;not null" json:"user_id"` Role string `gorm:"type:varchar(20);default:'member'" json:"role"` JoinedAt time.Time `gorm:"autoCreateTime" json:"joined_at"` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/model/GlobalChatSetting.go` around lines 31 - 37, The JSON tag for GlobalChatRoomUser.RoomName is inconsistent with GlobalChatConnect; change the tag from json:"name" to json:"room_name" in the GlobalChatRoomUser struct to match GlobalChatConnect, and add a unique composite constraint on (RoomName, UserID) (e.g., via GORM tag `uniqueIndex` or equivalent) to prevent the same user being registered multiple times in the same room; update the struct definition for GlobalChatRoomUser accordingly.src/api/src/internal/router/globalchat.go (1)
160-177: Discord ID のバリデーションがありません。
ChannelID、GuildID、CreatorIDが Discord スノーフレーク ID として有効かどうかの検証がありません。不正な形式のデータがデータベースに保存される可能性があります。// Discord IDのバリデーション例 func isValidDiscordID(id string) bool { if len(id) < 17 || len(id) > 20 { return false } for _, c := range id { if c < '0' || c > '9' { return false } } return true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/globalchat.go` around lines 160 - 177, connectChannel lacks validation for Discord snowflake IDs (ChannelID, GuildID, CreatorID); add a helper like isValidDiscordID and call it after binding JSON to reject requests with non-numeric or out-of-range lengths, returning http.StatusBadRequest with an explanatory message; update connectChannel to validate each of input.ChannelID, input.GuildID, and input.CreatorID (and optionally log which field failed) before proceeding to any DB operations or room creation.src/dashboard/src/lib/api/requests.ts (1)
342-348: GET リクエストにContent-Typeヘッダーは不要です。GET リクエストにはリクエストボディがないため、
Content-Typeヘッダーは意味がありません。削除しても問題ありません。♻️ 提案する修正
const res = await fetch(url, { method: "GET", - headers: { - "Content-Type": "application/json", - }, cache: 'no-store' });🤖 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 342 - 348, Remove the unnecessary "Content-Type" header from the GET fetch call: locate the fetch invocation that creates const res = await fetch(url, { method: "GET", headers: { "Content-Type": "application/json" }, cache: 'no-store' }) in requests.ts and delete the headers entry (or the "Content-Type" key) so the GET request does not send a Content-Type header.src/bot/cogs/globalchat.py (3)
9-11:loggingを使用してください。他の箇所では
loggingを使用していますが、ここではloggingに統一してください。♻️ 提案する修正
def __init__(self, bot): self.bot = bot - print("init -> GlobalChatCog") + logging.info("GlobalChatCog initialized")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` around lines 9 - 11, Replace the direct print in GlobalChatCog.__init__ with the module logger: import logging if not present, get a logger (e.g., logging.getLogger(__name__) or reuse an existing logger) and call an appropriate level method (info/debug) instead of print for the "init -> GlobalChatCog" message; update the line containing print("init -> GlobalChatCog") to use that logger so logging is consistent with the rest of the project.
63-65: 例外を無視せずにログに記録してください。Webhook の作成/取得に失敗した場合、例外を無視するとデバッグが困難になります。
♻️ 提案する修正
except Exception as e: + logging.error(f"[GlobalChat] Failed to setup webhook for channel {channel_id}: {e}") await self.bot.api.globalchat_disconnect_channel(channel_id=str(channel_id)) return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` around lines 63 - 65, In the except block that currently does "except Exception as e: await self.bot.api.globalchat_disconnect_channel(channel_id=str(channel_id)); return None", record the caught exception before returning by calling the module/class logger (e.g., self.logger.exception or self.bot.logger.error(..., exc_info=True)) and include context like channel_id and the exception variable e; keep the existing call to self.bot.api.globalchat_disconnect_channel(channel_id=str(channel_id)) and then return None as before.
91-100:asyncio.waitの結果処理が冗長です。
doneセット内のタスクは既に完了しているため、再度awaitする必要はありません。例外を取得したい場合はtask.result()またはtask.exception()を使用してください。♻️ 提案する修正
done, pending = await asyncio.wait(tasks, timeout=5.0) for task in done: - try: - await task - except Exception as e: - pass + if task.exception(): + logging.warning(f"[GlobalChat] Relay task failed: {task.exception()}") for p in pending: p.cancel()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` around lines 91 - 100, The loop awaiting completed tasks is redundant: in the asyncio.wait block that assigns done, pending from tasks, do not await tasks in the done set again; instead iterate done and call task.exception() (or task.result() if you need the return value) to surface any exceptions and handle/log them, and leave pending tasks to be cancelled (call p.cancel() and optionally await their cancellation via asyncio.gather(*pending, return_exceptions=True)); update the code around the asyncio.wait call that uses variables tasks, done, pending to replace "await task" with task.exception()/task.result() handling.src/dashboard/src/app/dashboard/[guildId]/globalchat/page.tsx (1)
2-2: 未使用のインポートがあります。
fetchEmbedSettingsがインポートされていますが、このファイルでは使用されていません。♻️ 提案する修正
-import { fetchEmbedSettings, isModuleEnabled } from "@/lib/api/requests"; +import { isModuleEnabled } from "@/lib/api/requests";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/page.tsx at line 2, The import list in the module includes an unused symbol fetchEmbedSettings alongside isModuleEnabled; remove fetchEmbedSettings from the import statement in page.tsx (leave isModuleEnabled) or, if the settings are intended to be used, replace its usage point to call fetchEmbedSettings where needed (e.g., within any data-fetching function or useEffect). Locate the import line that references fetchEmbedSettings and isModuleEnabled and either delete fetchEmbedSettings from that import or add the appropriate usage of fetchEmbedSettings so the import is no longer unused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@run.sh`:
- Line 1: Add a proper shebang as the very first line of run.sh (e.g.
#!/usr/bin/env bash) so the file executes under a defined shell rather than
relying on the caller; place it above the existing sudo docker compose up -d
--build line and, if needed, ensure the script is executable (chmod +x run.sh).
In `@src/api/src/go.mod`:
- Line 43: Replace the non-existent module version in the go.mod dependency
entry: change the line referencing github.com/lib/pq v1.12.0 to the available
version github.com/lib/pq v1.11.1 so the module resolve succeeds (update the
dependency string in go.mod where github.com/lib/pq is declared).
In `@src/api/src/internal/router/globalchat.go`:
- Around line 133-135: The query in the transaction uses the wrong column name
("name") for the GlobalChatRoomUser model; update the tx.Where call in the
function handling room lookup to reference the correct DB column/field
(room_name) or use a struct predicate (e.g., GlobalChatRoomUser{RoomName:
input.Name, UserID: input.CreatorID}) so the WHERE clause matches the model's
RoomName and user_id columns when calling tx.Where(...). Ensure you still check
res.Error against gorm.ErrRecordNotFound afterwards.
- Around line 95-101: The code incorrectly checks res.Error ==
gorm.ErrRecordNotFound after calling db.Where(...).Find(&connections) — Find
never returns ErrRecordNotFound for no rows. Change the logic to: first check if
res.Error != nil and handle as a 500 (use res.Error in the log/response), and
then check if len(connections) == 0 to return 404 ("Data NotFound"); keep the
successful path returning c.JSON(http.StatusOK, connections). Use the existing
symbols db.Where, Find(&connections), res.Error, connections, guildID and c.JSON
to locate and update the code.
- Line 116: Remove the direct user-data log call log.Print(input) in
globalchat.go and either delete it or replace it with a safe alternative: use
structured logging that avoids embedding raw user input (e.g., log a sanitized
summary, a length/count, a non-reversible hash, or specific non-sensitive
fields) or log at a debug level behind a feature flag; ensure the change touches
the log.Print(input) occurrence and any surrounding handler function that
receives the input variable so no raw user string is written to logs.
In `@src/bot/cogs/globalchat.py`:
- Around line 82-86: The code currently calls send_webhook with url that may be
None because check_and_fix_webhook can return None; update the block around
dest.get("webhook_url") so that after calling await
self.check_and_fix_webhook(dest) you verify url is truthy (not None or empty)
before creating and appending asyncio.create_task(self.send_webhook(url,
message)) — if url is falsy, skip creating the task (and optionally log/warn);
this ensures send_webhook is never invoked with a None url.
In `@src/bot/lib/api.py`:
- Around line 478-484: The method globalchat_get_active_channel_ids_byname
currently returns raw JSON on success and {} on failure, breaking the declared
List[str] contract; change it to always return a List[str] by: 1) on non-200
responses return an empty list ([]); 2) on 200 responses parse and coerce the
JSON into a list of strings (e.g., if data is a list, filter to str elements; if
data is a dict, pull a well-known key like "ids" or "channel_ids" and validate
it is a list of strings), and 3) ensure the function returns that validated
List[str] in all code paths (refer to globalchat_get_active_channel_ids_byname
and the local variables url, resp, data).
- Around line 497-499: The method globalchat_disconnect_channel references
self.headers which doesn't exist (only session and base_url are set in
__init__); either remove the headers argument from the request call or add an
explicit headers attribute initialization in __init__ (e.g., self.headers = {}
or populate with auth/content-type as required) so the DELETE request uses a
valid headers object; update __init__ and/or globalchat_disconnect_channel to
consistently use the chosen approach and keep symbols: __init__, self.headers,
session, base_url, and globalchat_disconnect_channel in sync.
- Around line 486-493: In globalchat_connect_channel, the payload uses the wrong
JSON key "room_name"; change the payload entry in the payload dict to use "name"
instead (i.e., replace the "room_name": room_name pair with "name": room_name)
so the backend connectChannel handler receives the expected key; update the
payload construction in globalchat_connect_channel accordingly.
In
`@src/dashboard/src/app/api/guilds/`[guildId]/modules/globalchat/rooms/[name]/route.ts:
- Around line 62-64: The fetch URL currently injects the user-derived variable
name directly into `/globalchat/rooms/${name}` which breaks for spaces and
reserved characters; update the fetch call that builds the URL (the line using
fetch in route.ts) to use encodeURIComponent(name) when interpolating the room
name so the backend receives a properly URL-encoded room identifier.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/globalchat/route.ts:
- Around line 60-62: The catch blocks in globalchat route handlers are
collapsing all errors into 401/403 or letting non-auth errors surface as 500
inconsistently; update the catch logic in route.ts (the try/catch around
validateAdmin() and checkAdminPermission() used by GET/POST/DELETE handlers) to
detect explicit auth errors (e.g., error.message or error.name values thrown by
validateAdmin() like "Unauthorized" and "Forbidden") and return 401 or 403
respectively, while any other exceptions should return a 500 with the error
message; apply the same pattern to the analogous catch blocks in
rooms/[name]/route.ts so only authentication/authorization errors map to 401/403
and all other failures map to 500.
- Around line 75-80: Remove the debug log that prints the user identifier:
delete the console.log(discordUser.accountId) in the route handler so user IDs
are not written to server logs; if you still need a trace, replace it with a
non-identifying note (e.g., log only that a request was received or use a
masked/hashed form) and avoid logging discordUser.accountId or any direct PII.
Ensure any other stray console.log calls (e.g., console.log(room_name)) are
reviewed and either removed or replaced with safe, non-identifying logs.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx:
- Around line 151-169: Add accessible names to the icon-only buttons by adding
aria-label attributes describing their action; update the three buttons using
handleInfoRoom, checkEditRoom, and handleDelete (and the other similar buttons
around lines 228-232 and 287-291) to include appropriate aria-labels such as
"Room info", "Edit room settings", and "Delete room" (or localized equivalents)
so screen readers can announce the button purpose while keeping existing title
attributes.
- Around line 108-119: The info button currently calls handleInfoRoom which hits
the edit-only GET /rooms/[name] and returns 403 for non-owners; fix by either
calling a read-only info endpoint (e.g. GET
/api/guilds/{guildId}/modules/globalchat/rooms/{name}/info) from handleInfoRoom
or by hiding/disabled the info button for non-owners before calling the API.
Locate handleInfoRoom and the JSX that renders the info button (the same area
referenced around lines 151-157) and: (A) if adding an endpoint, change the
fetch URL in handleInfoRoom to the read-only path and keep
setInfoData/setInfoModal behavior; or (B) if restricting UI, add an
ownership/permission check (use existing guild role/props or isOwner flag) and
conditionally render or disable the info button so non-owners cannot trigger
handleInfoRoom. Ensure error handling remains for network errors.
- Around line 219-275: The modal markup for infoModal in GlobalChatClient.tsx
lacks accessibility attributes and focus management; update the modal container
(where infoModal is used) to include role="dialog", aria-modal="true" and an
aria-labelledby pointing at the heading (give the <h3> an id), ensure the close
button (onClick={() => setInfoModal(false)}) has a ref and is used as the
initial focus target, implement a focus-trap and restore focus to the previously
focused element on close, and add an Escape key handler (via useEffect) that
calls setInfoModal(false); apply the same changes to the other modal block
(lines ~278-351) so both modals use aria labels, initial focus, focus trapping,
Escape handling, and restore focus on close.
---
Minor comments:
In `@run.sh`:
- Line 1: Add a trailing newline at the end of run.sh so the file ends with a
newline character; specifically ensure the final line containing "sudo docker
compose up -d --build" is followed by a newline (i.e., the file ends with a
blank line terminator) to satisfy POSIX text file conventions and avoid
editor/tool warnings.
In `@src/api/src/internal/model/GlobalChatSetting.go`:
- Around line 57-66: The WebhookURL field on the GlobalChatConnect struct is
sensitive and must not be returned raw in API JSON; update the struct definition
for GlobalChatConnect by removing or replacing the json tag on WebhookURL (e.g.,
change to json:"-" to prevent automatic exposure) or implement a safe
serialization path that returns a masked value for WebhookURL when needed (e.g.,
add a method like MaskedWebhook() or a DTO/serializer that emits a masked
string) and ensure all JSON encoding paths use the new tag or DTO instead of the
raw struct.
In `@src/api/src/internal/router/globalchat.go`:
- Around line 230-235: The handler currently sends a JSON body with a 204
response; update the response to send no body by replacing the
c.JSON(http.StatusNoContent, nil) call with a status-only response (e.g.,
c.Status(http.StatusNoContent)) after the delete succeeds; locate the block that
calls db.Where(...).Delete(&model.GlobalChatConnect{}) and change the final
response to use c.Status on the gin.Context instead of c.JSON.
In `@src/bot/cogs/globalchat.py`:
- Line 73: The call logging.error(config) in src/bot/cogs/globalchat.py is using
an inappropriate log level for debug data; replace it with logging.debug(config)
(or logging.info(config) if you need higher visibility) or remove the statement
for production. Locate the logging.error call (in the globalchat cog—e.g., the
function or method that currently logs the `config` variable) and update the
logging level accordingly, ensuring any sensitive data in `config` is not logged
in production.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/page.tsx:
- Around line 42-44: The paragraph text "※ 同じ名前の埋め込みを保存すると、上書きされます。" is a
copy/paste leftover and doesn’t match the Global Chat page; locate the JSX <p
className="mt-10 text-center text-xs text-slate-400"> in the page.tsx component
for the global chat and replace the message with copy relevant to global chat
(for example, a short note about global messages visibility or chat
persistence), ensuring the wording matches the page purpose and tone.
In `@src/dashboard/src/lib/api/requests.ts`:
- Around line 327-333: In getGlobalChatConfig, replace encodeURI(name) with
encodeURIComponent(name) to correctly encode reserved characters in room names
when building the fetch URL (RESOURCE_API_BASE_URL/.../globalchat/rooms/); also
fix the thrown error string by removing the stray `$` so the throw uses "Failed
to get settings: ${res.statusText}" or a proper concatenation to include
res.statusText.
---
Nitpick comments:
In `@run.sh`:
- Line 1: The script run.sh currently invokes "sudo docker compose up -d
--build"; remove the hardcoded sudo from that command and instead document or
enforce adding the user to the docker group so the script can run without
elevated privileges (e.g., instruct to run usermod -aG docker <user> and
re-login or check group membership before running). Update run.sh to call
"docker compose up -d --build" and optionally add a brief pre-check that the
current user is in the docker group (or print a clear message instructing how to
add the user to the docker group) so callers aren’t required to use sudo.
In `@src/api/src/internal/model/GlobalChatSetting.go`:
- Around line 31-37: The JSON tag for GlobalChatRoomUser.RoomName is
inconsistent with GlobalChatConnect; change the tag from json:"name" to
json:"room_name" in the GlobalChatRoomUser struct to match GlobalChatConnect,
and add a unique composite constraint on (RoomName, UserID) (e.g., via GORM tag
`uniqueIndex` or equivalent) to prevent the same user being registered multiple
times in the same room; update the struct definition for GlobalChatRoomUser
accordingly.
In `@src/api/src/internal/router/globalchat.go`:
- Around line 160-177: connectChannel lacks validation for Discord snowflake IDs
(ChannelID, GuildID, CreatorID); add a helper like isValidDiscordID and call it
after binding JSON to reject requests with non-numeric or out-of-range lengths,
returning http.StatusBadRequest with an explanatory message; update
connectChannel to validate each of input.ChannelID, input.GuildID, and
input.CreatorID (and optionally log which field failed) before proceeding to any
DB operations or room creation.
In `@src/bot/cogs/globalchat.py`:
- Around line 9-11: Replace the direct print in GlobalChatCog.__init__ with the
module logger: import logging if not present, get a logger (e.g.,
logging.getLogger(__name__) or reuse an existing logger) and call an appropriate
level method (info/debug) instead of print for the "init -> GlobalChatCog"
message; update the line containing print("init -> GlobalChatCog") to use that
logger so logging is consistent with the rest of the project.
- Around line 63-65: In the except block that currently does "except Exception
as e: await
self.bot.api.globalchat_disconnect_channel(channel_id=str(channel_id)); return
None", record the caught exception before returning by calling the module/class
logger (e.g., self.logger.exception or self.bot.logger.error(...,
exc_info=True)) and include context like channel_id and the exception variable
e; keep the existing call to
self.bot.api.globalchat_disconnect_channel(channel_id=str(channel_id)) and then
return None as before.
- Around line 91-100: The loop awaiting completed tasks is redundant: in the
asyncio.wait block that assigns done, pending from tasks, do not await tasks in
the done set again; instead iterate done and call task.exception() (or
task.result() if you need the return value) to surface any exceptions and
handle/log them, and leave pending tasks to be cancelled (call p.cancel() and
optionally await their cancellation via asyncio.gather(*pending,
return_exceptions=True)); update the code around the asyncio.wait call that uses
variables tasks, done, pending to replace "await task" with
task.exception()/task.result() handling.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/page.tsx:
- Line 2: The import list in the module includes an unused symbol
fetchEmbedSettings alongside isModuleEnabled; remove fetchEmbedSettings from the
import statement in page.tsx (leave isModuleEnabled) or, if the settings are
intended to be used, replace its usage point to call fetchEmbedSettings where
needed (e.g., within any data-fetching function or useEffect). Locate the import
line that references fetchEmbedSettings and isModuleEnabled and either delete
fetchEmbedSettings from that import or add the appropriate usage of
fetchEmbedSettings so the import is no longer unused.
In `@src/dashboard/src/lib/api/requests.ts`:
- Around line 342-348: Remove the unnecessary "Content-Type" header from the GET
fetch call: locate the fetch invocation that creates const res = await
fetch(url, { method: "GET", headers: { "Content-Type": "application/json" },
cache: 'no-store' }) in requests.ts and delete the headers entry (or the
"Content-Type" key) so the GET request does not send a Content-Type header.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c67530a7-b0cf-41eb-94a4-2baad375d1b3
⛔ Files ignored due to path filters (1)
src/api/src/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
run.shsrc/api/src/cmd/main.gosrc/api/src/go.modsrc/api/src/internal/model/GlobalChatSetting.gosrc/api/src/internal/router/globalchat.gosrc/bot/cogs/globalchat.pysrc/bot/cogs/levels.pysrc/bot/lib/api.pysrc/dashboard/src/app/api/guilds/[guildId]/modules/globalchat/rooms/[name]/route.tssrc/dashboard/src/app/api/guilds/[guildId]/modules/globalchat/route.tssrc/dashboard/src/app/dashboard/[guildId]/globalchat/GlobalChatClient.tsxsrc/dashboard/src/app/dashboard/[guildId]/globalchat/page.tsxsrc/dashboard/src/app/dashboard/globalchat/page.tsxsrc/dashboard/src/lib/api/requests.tssrc/dashboard/src/lib/modules.ts
💤 Files with no reviewable changes (1)
- src/bot/cogs/levels.py
| @@ -0,0 +1 @@ | |||
| sudo docker compose up -d --build No newline at end of file | |||
There was a problem hiding this comment.
シェバンが欠落しています。
スクリプトにシェバン(#!/bin/bash など)がないため、どのシェルで実行されるかが不明確です。これは移植性と正確性に影響します。静的解析ツール (Shellcheck SC2148) でも指摘されています。
🔧 シェバンを追加する修正案
+#!/bin/bash
sudo docker compose up -d --build📝 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.
| sudo docker compose up -d --build | |
| #!/bin/bash | |
| sudo docker compose up -d --build |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@run.sh` at line 1, Add a proper shebang as the very first line of run.sh
(e.g. #!/usr/bin/env bash) so the file executes under a defined shell rather
than relying on the caller; place it above the existing sudo docker compose up
-d --build line and, if needed, ensure the script is executable (chmod +x
run.sh).
| var connections []model.GlobalChatConnect | ||
| res := db.Where("guild_id = ?", guildID).Find(&connections) | ||
| if res.Error == gorm.ErrRecordNotFound { | ||
| c.JSON(http.StatusNotFound, gin.H{"error": "Data NotFound"}) | ||
| return | ||
| } | ||
| c.JSON(http.StatusOK, connections) |
There was a problem hiding this comment.
Find() は空の結果で ErrRecordNotFound を返しません。
GORM の Find() メソッドは、レコードが見つからない場合でもエラーを返さず、空のスライスを返します。この条件は常に false になります。
🐛 提案する修正
func getGuildConnections(c *gin.Context) {
guildID := c.Param("guild_id")
db := c.MustGet("db").(*gorm.DB)
var connections []model.GlobalChatConnect
res := db.Where("guild_id = ?", guildID).Find(&connections)
- if res.Error == gorm.ErrRecordNotFound {
- c.JSON(http.StatusNotFound, gin.H{"error": "Data NotFound"})
- return
+ if res.Error != nil {
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "DB error"})
+ return
}
c.JSON(http.StatusOK, connections)
}📝 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.
| var connections []model.GlobalChatConnect | |
| res := db.Where("guild_id = ?", guildID).Find(&connections) | |
| if res.Error == gorm.ErrRecordNotFound { | |
| c.JSON(http.StatusNotFound, gin.H{"error": "Data NotFound"}) | |
| return | |
| } | |
| c.JSON(http.StatusOK, connections) | |
| var connections []model.GlobalChatConnect | |
| res := db.Where("guild_id = ?", guildID).Find(&connections) | |
| if res.Error != nil { | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": "DB error"}) | |
| return | |
| } | |
| c.JSON(http.StatusOK, connections) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/src/internal/router/globalchat.go` around lines 95 - 101, The code
incorrectly checks res.Error == gorm.ErrRecordNotFound after calling
db.Where(...).Find(&connections) — Find never returns ErrRecordNotFound for no
rows. Change the logic to: first check if res.Error != nil and handle as a 500
(use res.Error in the log/response), and then check if len(connections) == 0 to
return 404 ("Data NotFound"); keep the successful path returning
c.JSON(http.StatusOK, connections). Use the existing symbols db.Where,
Find(&connections), res.Error, connections, guildID and c.JSON to locate and
update the code.
| } catch (error: any) { | ||
| const status = error.message === "Forbidden" ? 403 : 401; | ||
| return NextResponse.json({ error: error.message }, { status }); |
There was a problem hiding this comment.
例外のステータス変換が一貫していません。
validateAdmin() は Unauthorized / Forbidden を投げますが、checkAdminPermission() の上流障害は通常の例外として飛んできます。今の実装だと GET はそれら全部を 401 に潰し、POST/DELETE は逆に認証失敗まで 500 にしてしまいます。明示的な認証・認可エラーだけ 401/403 にし、それ以外は 500 に分けてください。rooms/[name]/route.ts の catch も同じパターンです。
💡 修正例
+function toErrorResponse(error: unknown) {
+ const message =
+ error instanceof Error ? error.message : "Internal Server Error";
+ const status =
+ message === "Unauthorized" ? 401 :
+ message === "Forbidden" ? 403 :
+ 500;
+
+ return NextResponse.json(
+ { error: status === 500 ? "Internal Server Error" : message },
+ { status }
+ );
+}
+
export async function GET(
_request: Request,
{ params }: { params: Promise<{ guildId: string }> }
) {
@@
- } catch (error: any) {
- const status = error.message === "Forbidden" ? 403 : 401;
- return NextResponse.json({ error: error.message }, { status });
+ } catch (error) {
+ return toErrorResponse(error);
}
}
@@
- } catch (error: any) {
- return NextResponse.json({ error: error.message }, { status: 500 });
+ } catch (error) {
+ return toErrorResponse(error);
}
}
@@
- } catch (error: any) {
- return NextResponse.json({ error: error.message }, { status: 500 });
+ } catch (error) {
+ return toErrorResponse(error);
}
}Also applies to: 97-98, 124-125
🤖 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/globalchat/route.ts
around lines 60 - 62, The catch blocks in globalchat route handlers are
collapsing all errors into 401/403 or letting non-auth errors surface as 500
inconsistently; update the catch logic in route.ts (the try/catch around
validateAdmin() and checkAdminPermission() used by GET/POST/DELETE handlers) to
detect explicit auth errors (e.g., error.message or error.name values thrown by
validateAdmin() like "Unauthorized" and "Forbidden") and return 401 or 403
respectively, while any other exceptions should return a 500 with the error
message; apply the same pattern to the analogous catch blocks in
rooms/[name]/route.ts so only authentication/authorization errors map to 401/403
and all other failures map to 500.
| const handleInfoRoom = async (roomName: string) => { | ||
| try { | ||
| const res = await fetch(`/api/guilds/${guildId}/modules/globalchat/rooms/${encodeURIComponent(roomName)}`); | ||
| if (!res.ok) throw new Error("Forbidden"); | ||
|
|
||
| const data = await res.json(); | ||
|
|
||
| setInfoData(data); | ||
| setInfoModal(true); | ||
| } catch (error) { | ||
| alert("エラーが発生しました。"); | ||
| } |
There was a problem hiding this comment.
情報ボタンが owner 以外では必ず失敗します。
ここも編集時と同じ /rooms/[name] を叩いていますが、サーバー側 GET は owner 以外を 403 にしています。共有ルームへ参加しているだけの管理者は情報モーダルを開けないのに、UI では常にボタンが表示されます。閲覧専用 endpoint を分けるか、少なくとも非 owner では情報ボタンを出さない方がよいです。
Also applies to: 151-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx
around lines 108 - 119, The info button currently calls handleInfoRoom which
hits the edit-only GET /rooms/[name] and returns 403 for non-owners; fix by
either calling a read-only info endpoint (e.g. GET
/api/guilds/{guildId}/modules/globalchat/rooms/{name}/info) from handleInfoRoom
or by hiding/disabled the info button for non-owners before calling the API.
Locate handleInfoRoom and the JSX that renders the info button (the same area
referenced around lines 151-157) and: (A) if adding an endpoint, change the
fetch URL in handleInfoRoom to the read-only path and keep
setInfoData/setInfoModal behavior; or (B) if restricting UI, add an
ownership/permission check (use existing guild role/props or isOwner flag) and
conditionally render or disable the info button so non-owners cannot trigger
handleInfoRoom. Ensure error handling remains for network errors.
feat: クールダウンとルールを機能するように
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
src/dashboard/src/app/dashboard/[guildId]/globalchat/GlobalChatClient.tsx (2)
92-93:parseIntに基数を指定してください。
parseIntを基数なしで使用すると、先頭が0の文字列で予期しない結果が生じる可能性があります。明示的に基数 10 を指定してください。♻️ 提案する修正
- slowmode: parseInt(editingData.slowmode) || 0, - min_account_age: parseInt(editingData.min_account_age) || 0, + slowmode: parseInt(editingData.slowmode, 10) || 0, + min_account_age: parseInt(editingData.min_account_age, 10) || 0,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx around lines 92 - 93, The calls to parseInt for editingData.slowmode and editingData.min_account_age omit the radix which can yield incorrect results for strings with leading zeros; update both parseInt(editingData.slowmode) and parseInt(editingData.min_account_age) to parseInt(..., 10) so they explicitly parse as base-10 numbers (locate these in GlobalChatClient.tsx where slowmode and min_account_age are set).
11-13: Props の型定義を改善してください。
any[]の使用は型安全性を低下させます。チャンネルと設定の型を定義することで、IDE のサポートとバグの早期発見が向上します。♻️ 提案する型定義
interface Channel { id: string; name: string; // 他の必要なフィールド } interface ConnectionSetting { channel_id: string; room_name: string; // 他の必要なフィールド } interface Props { guildId: string; initChannels: Channel[]; settings: ConnectionSetting[]; userId: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx around lines 11 - 13, Replace the loose any[] props with explicit interfaces and use them in the component Props: define a Channel interface (e.g., id: string, name: string, ...), a ConnectionSetting interface (e.g., channel_id: string, room_name: string, ...), then update the component's Props type so initChannels: Channel[] and settings: ConnectionSetting[] and keep userId and guildId typed as string; update any references to initChannels or settings in GlobalChatClient.tsx to use the new types.src/api/src/internal/router/globalchat.go (1)
183-186:errors.Is()を使用してエラー比較を行ってください。Line 51 と 81 では
errors.Is(err, gorm.ErrRecordNotFound)を使用していますが、ここでは直接比較res.Error == gorm.ErrRecordNotFoundを使用しています。一貫性とラップされたエラーの正しい処理のためにerrors.Is()を使用してください。♻️ 提案する修正
- if res.Error == gorm.ErrRecordNotFound { + if errors.Is(res.Error, gorm.ErrRecordNotFound) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/globalchat.go` around lines 183 - 186, Replace the direct comparison res.Error == gorm.ErrRecordNotFound with errors.Is(res.Error, gorm.ErrRecordNotFound) in the tx.Where("name = ?", input.RoomName).First(&room) block so wrapped errors are handled consistently (like other occurrences that already use errors.Is); ensure you import the errors package if not already imported and keep the same control flow that creates newRoom when the record-not-found condition is true.src/dashboard/src/app/api/guilds/[guildId]/modules/globalchat/route.ts (1)
23-54:validateAdminが重複しています。
validateAdmin関数はrooms/[name]/route.tsにも同一のコードで存在します。共通のユーティリティモジュールに抽出することで、保守性が向上します。例えば
@/lib/api/auth.tsに以下のように抽出できます:// `@/lib/api/auth.ts` export async function validateAdmin(guildId: string) { // 共通の認証・権限チェックロジック }🤖 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/globalchat/route.ts around lines 23 - 54, The validateAdmin function is duplicated; extract it into a shared utility by exporting a single async function validateAdmin(guildId: string) and have both route handlers import it. Keep the same logic (calls to auth.api.listUserAccounts, auth.api.getAccessToken, and checkAdminPermission) and return shape ({ token, discordUser }); replace the in-file implementations in both modules with an import of the shared validateAdmin so callers continue to receive the same value and errors.src/bot/cogs/globalchat.py (2)
55-55: デバッグ情報にlogging.errorを使用しないでください。
logging.error(data)とlogging.error(config)はデバッグ目的のようです。エラーレベルではなくlogging.debugを使用してください。本番環境でログが肥大化し、実際のエラーの発見が困難になります。♻️ 提案する修正
- logging.error(data) + logging.debug(f"Join member response: {data}")- logging.error(config) + logging.debug(f"Relay config: {config}")Also applies to: 121-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` at line 55, Replace the two debug-only error logs that call logging.error(data) and logging.error(config) with logging.debug(...) so they don't clutter error-level logs; locate these calls in src/bot/cogs/globalchat.py (the occurrences that log the incoming `data` and the `config`) and change them to logging.debug(data) and logging.debug(config), respectively, ensuring any surrounding context or f-strings remain unchanged.
144-151: 例外を無視せず、少なくともログに記録してください。
try-except-passパターンは例外を完全に無視しており、問題のデバッグを困難にします。♻️ 提案する修正
for task in done: try: await task except Exception as e: - pass + logging.warning(f"Webhook task failed: {e}") for p in pending: p.cancel()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` around lines 144 - 151, In the for task in done loop in src/bot/cogs/globalchat.py, don't swallow exceptions with "except Exception: pass"; instead log them with a proper logger (e.g., logging.getLogger(__name__).exception or an existing module logger) so you capture the exception message and stack trace; replace the pass in the except block with a logger.exception call (including a short contextual message referencing the task) and ensure the logger is imported/available in the module.src/bot/lib/api.py (1)
46-68: 定義されたデータクラスが使用されていません。
GlobalChatRoomRestriction,GlobalChatRoom,GlobalChatConfigが定義されていますが、API メソッドは生の辞書を返しています。型安全性を向上させるため、これらのデータクラスを活用するか、使用しない場合は削除を検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/lib/api.py` around lines 46 - 68, The dataclasses GlobalChatRoomRestriction, GlobalChatRoom and GlobalChatConfig are defined but not used; update the API methods that currently return raw dicts so they construct and return these typed objects (or remove the dataclass defs if you prefer not to use them). Specifically, in the functions that build the global chat response, instantiate GlobalChatRoomRestriction for each restriction dict and then GlobalChatRoom (passing restrictions=[...]) and finally GlobalChatConfig (with room=the GlobalChatRoom or None), and update the function return type hints to GlobalChatConfig | dict as appropriate; alternatively delete the three classes if you choose to keep returning plain dicts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/src/internal/router/cooldowns.go`:
- Around line 33-39: The current parsing of secondsStr/hoursStr ignores
strconv.Atoi errors which can set sec/hours to 0 and bypass cooldowns; update
the block that computes limitDuration (the strconv.Atoi calls for secondsStr and
hoursStr) to validate the Atoi error and return a 400 Bad Request on invalid
input (or apply a documented safe default), e.g. if strconv.Atoi returns an
error for secondsStr or hoursStr respond with an error to the client instead of
using the zero value, and only set limitDuration when parsing succeeds.
In `@src/api/src/internal/router/globalchat.go`:
- Around line 237-243: GetUserRoomRole reads user_id with c.Query("user_id") but
the route defines user_id as a path parameter; change to c.Param("user_id") so
userID is taken from the path, i.e. replace the call that sets userID in
GetUserRoomRole to use c.Param("user_id") and ensure any downstream DB query
(db.Where(..., userID)) continues to use that value (or alternatively update the
route to remove the path param and keep c.Query if you intend a query
parameter).
In `@src/bot/cogs/globalchat.py`:
- Around line 62-63: The description string is mistakenly an f-string without
placeholders; change the description from
description=f"ルールと説明を確認してください。\n再度送信するとルールに同意したものとします。" to a plain string
(remove the leading f) while keeping the title line (title=f"{room['name']}
へようこそ!") as-is; ensure only the description literal is modified in the
globalchat embed construction.
In `@src/bot/lib/api.py`:
- Line 481: URL に生の部屋名を埋め込んでいるためエンコードが必要です: URL を作る箇所(変数 name を使っている箇所、例: url =
f"{self.base_url}/globalchat/rooms/{name}")と同様に
globalchat_get_active_channel_ids_byname でも name を urllib.parse.quote
でエンコードしてから埋め込むよう修正してください(もしくは requests.utils.quote 相当を使う)。エンコード処理は name
を直接変更せずローカル変数 encoded_name に入れて URL 組み立て時に使用し、既存の URL 文字列作成箇所(url 変数と
globalchat_get_active_channel_ids_byname の URL 作成)を置き換えてください。
- Line 509: The POST URL building uses raw room_name which can break for
spaces/special chars; update the code where self.session.post is called (the
line with f"{self.base_url}/globalchat/rooms/{room_name}/members") to URL-encode
room_name using urllib.parse.quote before interpolation (same approach as
check_globalchat_cooldown), e.g., compute an encoded_room_name variable with
urllib.parse.quote(room_name, safe='') and use that in the f-string so the
request works for names containing spaces or reserved characters.
In
`@src/dashboard/src/app/api/guilds/`[guildId]/modules/globalchat/rooms/[name]/route.ts:
- Line 82: Remove the duplicate semicolon at the end of the conditional return
in the route handler: locate the statement using role.role and NextResponse.json
(the line with if (role.role !== "owner") return NextResponse.json(...);; ) and
delete the extra trailing semicolon so the line ends with a single semicolon.
- Around line 65-68: The catch block in the GET handler incorrectly assumes
authentication errors and maps all exceptions to 401/403; update the error
mapping in the GET export so it doesn't force non-auth errors into 401. In the
catch for the GET handler, check for an explicit status on the thrown error
(e.g., error.status || error.statusCode) and return that, otherwise default to
500, and include error.message in NextResponse.json; remove the special-case
check for "Forbidden" that assumes validateAdmin was called.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/globalchat/route.ts:
- Around line 91-95: getValidatedChannelInServer can throw different errors
(e.g. "Invalid Guild or Channel ID format" vs Discord-not-found), but the
current catch always returns 404; update the try/catch around
getValidatedChannelInServer(guildId, channel_id) to inspect the thrown error
(error.message or a custom error type) and map "Invalid Guild or Channel ID
format" to a 400 Bad Request response while keeping Discord-not-found/other
not-found errors returning 404; adjust the catch in route.ts to check
error.message (or instanceof if getValidatedChannelInServer exports a specific
Error class) and return NextResponse.json({ error: "Invalid ID format" }, {
status: 400 }) for that case, otherwise return the existing 404 response.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx:
- Line 220: Replace the redundant boolean check in the JSX conditional inside
GlobalChatClient.tsx — change the duplicated `infoModal && infoModal &&` to
check `infoModal && infoData &&` so it matches the `editingData` pattern used
later (see the `editingData` conditional around Line 279) and prevents rendering
when `infoData` is missing; update the conditional that renders the info modal
accordingly.
---
Nitpick comments:
In `@src/api/src/internal/router/globalchat.go`:
- Around line 183-186: Replace the direct comparison res.Error ==
gorm.ErrRecordNotFound with errors.Is(res.Error, gorm.ErrRecordNotFound) in the
tx.Where("name = ?", input.RoomName).First(&room) block so wrapped errors are
handled consistently (like other occurrences that already use errors.Is); ensure
you import the errors package if not already imported and keep the same control
flow that creates newRoom when the record-not-found condition is true.
In `@src/bot/cogs/globalchat.py`:
- Line 55: Replace the two debug-only error logs that call logging.error(data)
and logging.error(config) with logging.debug(...) so they don't clutter
error-level logs; locate these calls in src/bot/cogs/globalchat.py (the
occurrences that log the incoming `data` and the `config`) and change them to
logging.debug(data) and logging.debug(config), respectively, ensuring any
surrounding context or f-strings remain unchanged.
- Around line 144-151: In the for task in done loop in
src/bot/cogs/globalchat.py, don't swallow exceptions with "except Exception:
pass"; instead log them with a proper logger (e.g.,
logging.getLogger(__name__).exception or an existing module logger) so you
capture the exception message and stack trace; replace the pass in the except
block with a logger.exception call (including a short contextual message
referencing the task) and ensure the logger is imported/available in the module.
In `@src/bot/lib/api.py`:
- Around line 46-68: The dataclasses GlobalChatRoomRestriction, GlobalChatRoom
and GlobalChatConfig are defined but not used; update the API methods that
currently return raw dicts so they construct and return these typed objects (or
remove the dataclass defs if you prefer not to use them). Specifically, in the
functions that build the global chat response, instantiate
GlobalChatRoomRestriction for each restriction dict and then GlobalChatRoom
(passing restrictions=[...]) and finally GlobalChatConfig (with room=the
GlobalChatRoom or None), and update the function return type hints to
GlobalChatConfig | dict as appropriate; alternatively delete the three classes
if you choose to keep returning plain dicts.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/globalchat/route.ts:
- Around line 23-54: The validateAdmin function is duplicated; extract it into a
shared utility by exporting a single async function validateAdmin(guildId:
string) and have both route handlers import it. Keep the same logic (calls to
auth.api.listUserAccounts, auth.api.getAccessToken, and checkAdminPermission)
and return shape ({ token, discordUser }); replace the in-file implementations
in both modules with an import of the shared validateAdmin so callers continue
to receive the same value and errors.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx:
- Around line 92-93: The calls to parseInt for editingData.slowmode and
editingData.min_account_age omit the radix which can yield incorrect results for
strings with leading zeros; update both parseInt(editingData.slowmode) and
parseInt(editingData.min_account_age) to parseInt(..., 10) so they explicitly
parse as base-10 numbers (locate these in GlobalChatClient.tsx where slowmode
and min_account_age are set).
- Around line 11-13: Replace the loose any[] props with explicit interfaces and
use them in the component Props: define a Channel interface (e.g., id: string,
name: string, ...), a ConnectionSetting interface (e.g., channel_id: string,
room_name: string, ...), then update the component's Props type so initChannels:
Channel[] and settings: ConnectionSetting[] and keep userId and guildId typed as
string; update any references to initChannels or settings in
GlobalChatClient.tsx to use the new types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b701b57-9297-4331-9bbc-fb986340a57c
📒 Files selected for processing (7)
src/api/src/internal/router/cooldowns.gosrc/api/src/internal/router/globalchat.gosrc/bot/cogs/globalchat.pysrc/bot/lib/api.pysrc/dashboard/src/app/api/guilds/[guildId]/modules/globalchat/rooms/[name]/route.tssrc/dashboard/src/app/api/guilds/[guildId]/modules/globalchat/route.tssrc/dashboard/src/app/dashboard/[guildId]/globalchat/GlobalChatClient.tsx
| if secondsStr != "" { | ||
| sec, _ := strconv.Atoi(secondsStr) | ||
| limitDuration = time.Duration(sec) * time.Second | ||
| } else { | ||
| hours, _ := strconv.Atoi(hoursStr) | ||
| limitDuration = time.Duration(hours) * time.Hour | ||
| } |
There was a problem hiding this comment.
strconv.Atoi のエラーを無視すると、不正な入力でクールダウンがバイパスされる可能性があります。
strconv.Atoi のエラーを無視した場合、seconds=abc のような不正な入力では sec が 0 になり、クールダウンが事実上無効化されます。エラー時には 400 Bad Request を返すか、適切なデフォルト値を使用してください。
🛡️ 提案する修正
if secondsStr != "" {
- sec, _ := strconv.Atoi(secondsStr)
+ sec, err := strconv.Atoi(secondsStr)
+ if err != nil {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid seconds parameter"})
+ return
+ }
limitDuration = time.Duration(sec) * time.Second
} else {
- hours, _ := strconv.Atoi(hoursStr)
+ hours, err := strconv.Atoi(hoursStr)
+ if err != nil {
+ hours = 24 // デフォルト値にフォールバック
+ }
limitDuration = time.Duration(hours) * time.Hour
}📝 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.
| if secondsStr != "" { | |
| sec, _ := strconv.Atoi(secondsStr) | |
| limitDuration = time.Duration(sec) * time.Second | |
| } else { | |
| hours, _ := strconv.Atoi(hoursStr) | |
| limitDuration = time.Duration(hours) * time.Hour | |
| } | |
| if secondsStr != "" { | |
| sec, err := strconv.Atoi(secondsStr) | |
| if err != nil { | |
| c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid seconds parameter"}) | |
| return | |
| } | |
| limitDuration = time.Duration(sec) * time.Second | |
| } else { | |
| hours, err := strconv.Atoi(hoursStr) | |
| if err != nil { | |
| hours = 24 // デフォルト値にフォールバック | |
| } | |
| limitDuration = time.Duration(hours) * time.Hour | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/src/internal/router/cooldowns.go` around lines 33 - 39, The current
parsing of secondsStr/hoursStr ignores strconv.Atoi errors which can set
sec/hours to 0 and bypass cooldowns; update the block that computes
limitDuration (the strconv.Atoi calls for secondsStr and hoursStr) to validate
the Atoi error and return a 400 Bad Request on invalid input (or apply a
documented safe default), e.g. if strconv.Atoi returns an error for secondsStr
or hoursStr respond with an error to the client instead of using the zero value,
and only set limitDuration when parsing succeeds.
| func GetUserRoomRole(c *gin.Context) { | ||
| roomName := c.Param("name") | ||
| userID := c.Query("user_id") | ||
| db := c.MustGet("db").(*gorm.DB) | ||
|
|
||
| var roomUser model.GlobalChatRoomUser | ||
| err := db.Where("room_name = ? AND user_id = ?", roomName, userID).First(&roomUser).Error |
There was a problem hiding this comment.
パスパラメータとクエリパラメータの不一致があります。
ルート定義 (Line 29) では /rooms/:name/:user_id と user_id がパスパラメータとして定義されていますが、c.Query("user_id") を使用しています。c.Param("user_id") を使用するか、ルート定義を GET /rooms/:name に変更してクエリパラメータとして受け取る必要があります。
🐛 提案する修正(パスパラメータを使用する場合)
func GetUserRoomRole(c *gin.Context) {
roomName := c.Param("name")
- userID := c.Query("user_id")
+ userID := c.Param("user_id")
db := c.MustGet("db").(*gorm.DB)📝 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.
| func GetUserRoomRole(c *gin.Context) { | |
| roomName := c.Param("name") | |
| userID := c.Query("user_id") | |
| db := c.MustGet("db").(*gorm.DB) | |
| var roomUser model.GlobalChatRoomUser | |
| err := db.Where("room_name = ? AND user_id = ?", roomName, userID).First(&roomUser).Error | |
| func GetUserRoomRole(c *gin.Context) { | |
| roomName := c.Param("name") | |
| userID := c.Param("user_id") | |
| db := c.MustGet("db").(*gorm.DB) | |
| var roomUser model.GlobalChatRoomUser | |
| err := db.Where("room_name = ? AND user_id = ?", roomName, userID).First(&roomUser).Error |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/src/internal/router/globalchat.go` around lines 237 - 243,
GetUserRoomRole reads user_id with c.Query("user_id") but the route defines
user_id as a path parameter; change to c.Param("user_id") so userID is taken
from the path, i.e. replace the call that sets userID in GetUserRoomRole to use
c.Param("user_id") and ensure any downstream DB query (db.Where(..., userID))
continues to use that value (or alternatively update the route to remove the
path param and keep c.Query if you intend a query parameter).
| title=f"{room['name']} へようこそ!", | ||
| description=f"ルールと説明を確認してください。\n再度送信するとルールに同意したものとします。", |
There was a problem hiding this comment.
f-string にプレースホルダがありません。
Line 63 の f-string には変数が含まれていません。通常の文字列に変更してください。
🐛 提案する修正
embed = discord.Embed(
title=f"{room['name']} へようこそ!",
- description=f"ルールと説明を確認してください。\n再度送信するとルールに同意したものとします。",
+ description="ルールと説明を確認してください。\n再度送信するとルールに同意したものとします。",
color=discord.Color.blue()
)📝 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.
| title=f"{room['name']} へようこそ!", | |
| description=f"ルールと説明を確認してください。\n再度送信するとルールに同意したものとします。", | |
| embed = discord.Embed( | |
| title=f"{room['name']} へようこそ!", | |
| description="ルールと説明を確認してください。\n再度送信するとルールに同意したものとします。", | |
| color=discord.Color.blue() | |
| ) |
🧰 Tools
🪛 Ruff (0.15.7)
[warning] 62-62: String contains ambiguous ! (FULLWIDTH EXCLAMATION MARK). Did you mean ! (EXCLAMATION MARK)?
(RUF001)
[error] 63-63: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/cogs/globalchat.py` around lines 62 - 63, The description string is
mistakenly an f-string without placeholders; change the description from
description=f"ルールと説明を確認してください。\n再度送信するとルールに同意したものとします。" to a plain string
(remove the leading f) while keeping the title line (title=f"{room['name']}
へようこそ!") as-is; ensure only the description literal is modified in the
globalchat embed construction.
| return [] | ||
|
|
||
| async def globalchat_get_active_channel_ids_byname(self, name: str): | ||
| url = f"{self.base_url}/globalchat/rooms/{name}" |
There was a problem hiding this comment.
name パラメータも URL エンコードが必要です。
globalchat_get_active_channel_ids_byname でも部屋名を直接 URL に埋め込んでいます。
🐛 提案する修正
async def globalchat_get_active_channel_ids_byname(self, name: str):
- url = f"{self.base_url}/globalchat/rooms/{name}"
+ url = f"{self.base_url}/globalchat/rooms/{urllib.parse.quote(name)}"
async with self.session.get(url) as resp:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/lib/api.py` at line 481, URL に生の部屋名を埋め込んでいるためエンコードが必要です: URL を作る箇所(変数
name を使っている箇所、例: url = f"{self.base_url}/globalchat/rooms/{name}")と同様に
globalchat_get_active_channel_ids_byname でも name を urllib.parse.quote
でエンコードしてから埋め込むよう修正してください(もしくは requests.utils.quote 相当を使う)。エンコード処理は name
を直接変更せずローカル変数 encoded_name に入れて URL 組み立て時に使用し、既存の URL 文字列作成箇所(url 変数と
globalchat_get_active_channel_ids_byname の URL 作成)を置き換えてください。
| "user_id": str(user_id), | ||
| "role": role | ||
| } | ||
| async with self.session.post(f"{self.base_url}/globalchat/rooms/{room_name}/members", json=payload) as resp: |
There was a problem hiding this comment.
room_name を URL エンコードしてください。
room_name にスペースや特殊文字(#, %, / など)が含まれる場合、URL が壊れる可能性があります。check_globalchat_cooldown (Line 515) のように urllib.parse.quote を使用してください。
🐛 提案する修正
async def globalchat_join_member(self, room_name: str, user_id: str, role: str = "member"):
payload = {
"user_id": str(user_id),
"role": role
}
- async with self.session.post(f"{self.base_url}/globalchat/rooms/{room_name}/members", json=payload) as resp:
+ async with self.session.post(f"{self.base_url}/globalchat/rooms/{urllib.parse.quote(room_name)}/members", json=payload) as resp:
if resp.status == 200:
return await resp.json()
return None📝 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.
| async with self.session.post(f"{self.base_url}/globalchat/rooms/{room_name}/members", json=payload) as resp: | |
| async with self.session.post(f"{self.base_url}/globalchat/rooms/{urllib.parse.quote(room_name)}/members", json=payload) as resp: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/lib/api.py` at line 509, The POST URL building uses raw room_name
which can break for spaces/special chars; update the code where
self.session.post is called (the line with
f"{self.base_url}/globalchat/rooms/{room_name}/members") to URL-encode room_name
using urllib.parse.quote before interpolation (same approach as
check_globalchat_cooldown), e.g., compute an encoded_room_name variable with
urllib.parse.quote(room_name, safe='') and use that in the f-string so the
request works for names containing spaces or reserved characters.
| } catch (error: any) { | ||
| const status = error.message === "Forbidden" ? 403 : 401; | ||
| return NextResponse.json({ error: error.message }, { status }); | ||
| } |
There was a problem hiding this comment.
GET のエラーハンドリングが認証エラー前提になっています。
GET ハンドラは validateAdmin() を呼び出していないため、"Unauthorized" や "Forbidden" エラーは発生しません。fetch エラーやその他の例外が 401 としてマッピングされるのは不適切です。
♻️ 提案する修正
} catch (error: any) {
- const status = error.message === "Forbidden" ? 403 : 401;
- return NextResponse.json({ error: error.message }, { status });
+ return NextResponse.json({ error: "Internal Server 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]/modules/globalchat/rooms/[name]/route.ts
around lines 65 - 68, The catch block in the GET handler incorrectly assumes
authentication errors and maps all exceptions to 401/403; update the error
mapping in the GET export so it doesn't force non-auth errors into 401. In the
catch for the GET handler, check for an explicit status on the thrown error
(e.g., error.status || error.statusCode) and return that, otherwise default to
500, and include error.message in NextResponse.json; remove the special-case
check for "Forbidden" that assumes validateAdmin was called.
| try { | ||
| const role = await getGlobalChatRole(name, discordUser.accountId); | ||
|
|
||
| if (role.role !== "owner") return NextResponse.json({ error: "Forbidden" }, { status: 403 });; |
There was a problem hiding this comment.
セミコロンが重複しています。
行末に ;; が2つあります。
- if (role.role !== "owner") return NextResponse.json({ error: "Forbidden" }, { status: 403 });;
+ if (role.role !== "owner") return NextResponse.json({ error: "Forbidden" }, { status: 403 });📝 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.
| if (role.role !== "owner") return NextResponse.json({ error: "Forbidden" }, { status: 403 });; | |
| if (role.role !== "owner") return NextResponse.json({ error: "Forbidden" }, { status: 403 }); |
🤖 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/globalchat/rooms/[name]/route.ts
at line 82, Remove the duplicate semicolon at the end of the conditional return
in the route handler: locate the statement using role.role and NextResponse.json
(the line with if (role.role !== "owner") return NextResponse.json(...);; ) and
delete the extra trailing semicolon so the line ends with a single semicolon.
| try { | ||
| const check = await getValidatedChannelInServer(guildId, channel_id); | ||
| } catch { | ||
| return NextResponse.json({ error: "Not Found" }, { status: 404 }); | ||
| } |
There was a problem hiding this comment.
getValidatedChannelInServer のエラーハンドリングが不完全です。
コンテキストスニペットによると、getValidatedChannelInServer は無効な ID フォーマットで throw new Error("Invalid Guild or Channel ID format") を投げます。この場合、内側の catch で 404 を返しますが、これは「見つからない」ではなく「無効な入力」です。Discord API の失敗も 404 として扱われます。
♻️ より正確なエラーハンドリング
try {
const check = await getValidatedChannelInServer(guildId, channel_id);
+ if (!check) {
+ return NextResponse.json({ error: "Channel not found" }, { status: 404 });
+ }
} catch {
- return NextResponse.json({ error: "Not Found" }, { status: 404 });
+ return NextResponse.json({ error: "Invalid channel ID" }, { status: 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/globalchat/route.ts
around lines 91 - 95, getValidatedChannelInServer can throw different errors
(e.g. "Invalid Guild or Channel ID format" vs Discord-not-found), but the
current catch always returns 404; update the try/catch around
getValidatedChannelInServer(guildId, channel_id) to inspect the thrown error
(error.message or a custom error type) and map "Invalid Guild or Channel ID
format" to a 400 Bad Request response while keeping Discord-not-found/other
not-found errors returning 404; adjust the catch in route.ts to check
error.message (or instanceof if getValidatedChannelInServer exports a specific
Error class) and return NextResponse.json({ error: "Invalid ID format" }, {
status: 400 }) for that case, otherwise return the existing 404 response.
| </div> | ||
| </CollapsibleSection> | ||
|
|
||
| {infoModal && infoModal && ( |
There was a problem hiding this comment.
条件が重複しています。
infoModal && infoModal && は冗長です。infoModal && infoData && とすべきではないでしょうか? editingData を確認する Line 279 のパターンと一致させてください。
🐛 提案する修正
- {infoModal && infoModal && (
+ {infoModal && infoData && (📝 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.
| {infoModal && infoModal && ( | |
| {infoModal && infoData && ( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx at
line 220, Replace the redundant boolean check in the JSX conditional inside
GlobalChatClient.tsx — change the duplicated `infoModal && infoModal &&` to
check `infoModal && infoData &&` so it matches the `editingData` pattern used
later (see the `editingData` conditional around Line 279) and prevents rendering
when `infoData` is missing; update the conditional that renders the info modal
accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/bot/cogs/globalchat.py (4)
11-11:loggingを使用してください。他の箇所では
loggingを使用しているため、一貫性を保つためにlogging.infoまたはlogging.debugに変更することを推奨します。♻️ 提案する修正
- print("init -> GlobalChatCog") + logging.debug("init -> GlobalChatCog")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` at line 11, GlobalChatCog の初期化で使われている print("init -> GlobalChatCog") を標準の logging に置き換えてください: グローバルに既存のロガーを使っていれば logging.getLogger(__name__) から取得した logger.info か logger.debug を使い、まだロガーがない場合はモジュール冒頭で import logging と logger = logging.getLogger(__name__) を追加してから logger.info("init -> GlobalChatCog") のように変更してください。
190-190:loggingを使用してください。他のエラーログと一貫性を保つため、
logging.errorを使用してください。♻️ 提案する修正
except discord.NotFound: pass except Exception as e: - print(f"[GlobalChat] Webhook send error: {e}") + logging.error(f"[GlobalChat] Webhook send error: {e}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` at line 190, Replace the direct print call print(f"[GlobalChat] Webhook send error: {e}") with a structured logging call using the logging module (e.g., logging.error("GlobalChat webhook send error: %s", e, exc_info=True)) to match other error logs; ensure the logging module is imported (import logging) or use the existing module-level logger variable if present (e.g., logger.error(...)) so error messages and stack traces are recorded consistently.
143-147: 例外を握り潰さずにログ出力を検討してください。
try-except-passパターンでは問題発生時のデバッグが困難になります。少なくともlogging.debugでエラー内容を記録することを推奨します。♻️ 提案する修正
for task in done: try: await task except Exception as e: - pass + logging.debug(f"Webhook task failed: {e}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` around lines 143 - 147, The loop that awaits completed tasks currently swallows all exceptions ("for task in done: try: await task except Exception as e: pass"); change this to log the exception instead of ignoring it—use an appropriate logger (e.g., module-level logging or an instance logger) and call logger.exception or logger.error including the task identity (task, repr(task) or task.get_name()) and context so the stacktrace and error message are recorded; update the block around the "for task in done" / "await task" to replace the bare pass with a logging call that includes the caught exception and task info.
88-94: 戻り値のスタイルが不統一です。Line 90 では暗黙的に
Noneを返し、Line 94 では明示的にreturn Noneとしています。一貫性のため、どちらかに統一することを推奨します。♻️ 提案する修正
if not guild: logging.error(f"{guild_id} is NotFound") - return + return None channel_obj = guild.get_channel(channel_id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` around lines 88 - 94, The early-return style is inconsistent: one branch uses an implicit return and the other uses explicit `return None`; make them consistent by returning `None` explicitly in both places—update the branch that checks `if not guild:` to use `return None` (the other branch already returns `None`), keeping the `logging.error` calls for `guild_id` and `channel_id` and the variable names `guild`, `channel_obj`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bot/cogs/globalchat.py`:
- Line 120: Remove the leftover debug-level error log call logging.error(config)
in src/bot/cogs/globalchat.py: either delete it if not needed in production or
change it to logging.debug(config) so it becomes a non-error debug message;
update the call site where logging.error(config) appears to use logging.debug or
remove it entirely.
- Around line 14-18: The on_message handler uses self.bot.api before ensuring it
exists; add the same guard used in other cogs (e.g. welcome.py) near the top of
on_message to return early if the API isn't ready—check either hasattr(self.bot,
"api") and self.bot.api is not None (or simply if not getattr(self.bot, "api",
None): return) before calling self.bot.api.globalchat_get_channel_config; this
ensures setup_hook initialization timing (from setup_hook) won't cause an
AttributeError in on_message.
---
Nitpick comments:
In `@src/bot/cogs/globalchat.py`:
- Line 11: GlobalChatCog の初期化で使われている print("init -> GlobalChatCog") を標準の logging
に置き換えてください: グローバルに既存のロガーを使っていれば logging.getLogger(__name__) から取得した logger.info か
logger.debug を使い、まだロガーがない場合はモジュール冒頭で import logging と logger =
logging.getLogger(__name__) を追加してから logger.info("init -> GlobalChatCog")
のように変更してください。
- Line 190: Replace the direct print call print(f"[GlobalChat] Webhook send
error: {e}") with a structured logging call using the logging module (e.g.,
logging.error("GlobalChat webhook send error: %s", e, exc_info=True)) to match
other error logs; ensure the logging module is imported (import logging) or use
the existing module-level logger variable if present (e.g., logger.error(...))
so error messages and stack traces are recorded consistently.
- Around line 143-147: The loop that awaits completed tasks currently swallows
all exceptions ("for task in done: try: await task except Exception as e:
pass"); change this to log the exception instead of ignoring it—use an
appropriate logger (e.g., module-level logging or an instance logger) and call
logger.exception or logger.error including the task identity (task, repr(task)
or task.get_name()) and context so the stacktrace and error message are
recorded; update the block around the "for task in done" / "await task" to
replace the bare pass with a logging call that includes the caught exception and
task info.
- Around line 88-94: The early-return style is inconsistent: one branch uses an
implicit return and the other uses explicit `return None`; make them consistent
by returning `None` explicitly in both places—update the branch that checks `if
not guild:` to use `return None` (the other branch already returns `None`),
keeping the `logging.error` calls for `guild_id` and `channel_id` and the
variable names `guild`, `channel_obj`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb2a6920-b15a-4fe9-8729-1a77ba3fb27e
📒 Files selected for processing (1)
src/bot/cogs/globalchat.py
| async def on_message(self, message: discord.Message): | ||
| if message.author.bot or message.is_system(): | ||
| return | ||
|
|
||
| config = await self.bot.api.globalchat_get_channel_config(message.channel.id) |
There was a problem hiding this comment.
bot.api の存在確認が必要です。
setup_hook() ではコグのロード後に bot.api が初期化されます(src/bot/main.py 参照)。起動直後にメッセージが到着すると AttributeError が発生する可能性があります。他のコグ(welcome.py など)と同様のガードを追加してください。
🐛 提案する修正
`@commands.Cog.listener`()
async def on_message(self, message: discord.Message):
if message.author.bot or message.is_system():
return
+ if not self.bot.api:
+ return
+
config = await self.bot.api.globalchat_get_channel_config(message.channel.id)📝 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.
| async def on_message(self, message: discord.Message): | |
| if message.author.bot or message.is_system(): | |
| return | |
| config = await self.bot.api.globalchat_get_channel_config(message.channel.id) | |
| async def on_message(self, message: discord.Message): | |
| if message.author.bot or message.is_system(): | |
| return | |
| if not self.bot.api: | |
| return | |
| config = await self.bot.api.globalchat_get_channel_config(message.channel.id) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/cogs/globalchat.py` around lines 14 - 18, The on_message handler uses
self.bot.api before ensuring it exists; add the same guard used in other cogs
(e.g. welcome.py) near the top of on_message to return early if the API isn't
ready—check either hasattr(self.bot, "api") and self.bot.api is not None (or
simply if not getattr(self.bot, "api", None): return) before calling
self.bot.api.globalchat_get_channel_config; this ensures setup_hook
initialization timing (from setup_hook) won't cause an AttributeError in
on_message.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/bot/cogs/globalchat.py (1)
14-18:⚠️ Potential issue | 🟠 Major
bot.apiの初期化前アクセスが残っています。Line 18 は
self.bot.apiを無条件に参照しているので、起動直後にメッセージが来るとAttributeErrorになり得ます。API クライアントの準備ができるまで早期 return を入れてください。🐛 提案する修正
async def on_message(self, message: discord.Message): if message.author.bot or message.is_system(): return + if not getattr(self.bot, "api", None): + return + config = await self.bot.api.globalchat_get_channel_config(message.channel.id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` around lines 14 - 18, on_message 内で self.bot.api を無条件参照しているため起動直後に AttributeError になる可能性があります。on_message メソッドで最初に self.bot.api の存在・準備状態をチェックし(例: if not getattr(self.bot, "api", None): return)、API クライアントが未初期化なら早期 return するようにして globalchat_get_channel_config 呼び出しを保護してください。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bot/cogs/globalchat.py`:
- Around line 50-59: globalchat_get_active_channel_ids_byname() が非200時に空の dict
を返すため、取得結果が空(falsy または members キー欠落)だった場合は処理を続けずに早期リターンまたはスキップするよう修正してください:
チェック対象は変数 channels とその members(現在の member_ids を作る前)、該当箇所は
globalchat_get_active_channel_ids_byname 呼び出し直後の channels 取り扱い部分で、channels が
falsy または "members" を持たない場合は globalchat_join_member もウェルカム処理(rule/description
ブロック)も実行しないようにしてください。
- Around line 96-112: The current broad except in the webhook setup (around
channel_obj.webhooks(), channel_obj.create_webhook(), and
self.bot.api.globalchat_connect_channel()) always calls
self.bot.api.globalchat_disconnect_channel() on any error; change it to only
call globalchat_disconnect_channel() when the channel is confirmed deleted
(catch discord.NotFound or an HTTP 404 from Discord) and for all other
exceptions just log the error and return None without removing the backend
connection; keep the normal flow that returns webhook.url on success and ensure
you reference the webhook setup block and the API calls (channel_obj.webhooks,
channel_obj.create_webhook, self.bot.api.globalchat_connect_channel,
self.bot.api.globalchat_disconnect_channel) when editing.
---
Duplicate comments:
In `@src/bot/cogs/globalchat.py`:
- Around line 14-18: on_message 内で self.bot.api を無条件参照しているため起動直後に AttributeError
になる可能性があります。on_message メソッドで最初に self.bot.api の存在・準備状態をチェックし(例: if not
getattr(self.bot, "api", None): return)、API クライアントが未初期化なら早期 return するようにして
globalchat_get_channel_config 呼び出しを保護してください。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85c736ec-125a-425a-b07d-c86a9f72449e
📒 Files selected for processing (1)
src/bot/cogs/globalchat.py
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
src/dashboard/src/app/dashboard/[guildId]/globalchat/GlobalChatClient.tsx (2)
222-223:⚠️ Potential issue | 🟡 Minor情報モーダルは
infoDataも揃った時だけ描画してください。今の条件は
infoModalの重複チェックだけなので、infoDataが未設定のまま開いた時にinfoData.name参照で落ちます。editingData側と同じガードにしておく方が安全です。🐛 提案する修正
- {infoModal && infoModal && ( + {infoModal && infoData && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx around lines 222 - 223, The modal render checks only infoModal and can crash when infoData is undefined; change the conditional to require both infoModal and infoData (e.g., replace "{infoModal && infoModal && (" with "{infoModal && infoData && (" or mirror the same guard used for editingData) so the component only renders when infoData is present.
223-277:⚠️ Potential issue | 🟠 Majorモーダルにダイアログ属性とフォーカス制御が必要です。
どちらのモーダルも単なる
divなので、スクリーンリーダーとキーボード操作で文脈を失います。role="dialog",aria-modal, 見出しとの関連付け、閉じるボタンのaria-label、初期フォーカス、Escape、フォーカス復元/トラップは最低限入れてください。Also applies to: 282-353
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx around lines 223 - 277, The modal markup lacks proper ARIA and keyboard/focus handling; update the outer modal container div to include role="dialog" aria-modal="true" and add an id on the heading (e.g., id="info-modal-title") then reference it with aria-labelledby; give the close button an aria-label (e.g., "閉じる") and a ref for initial focus. In GlobalChatClient.tsx, implement a useRef (e.g., infoModalRef, closeBtnRef) and a useEffect that when the modal opens: saves document.activeElement, sets focus to the close button (or first focusable), adds keydown handler to close on Escape (calling setInfoModal(false)), implements a basic focus trap (wrap Tab/Shift+Tab to stay within infoModalRef), and on close restores focus to the previously focused element; ensure these changes are applied to the info modal and the other modal block referenced in the comment.src/api/src/internal/router/globalchat.go (2)
264-266:⚠️ Potential issue | 🟠 Major
user_idの取得元がルート定義と一致していません。Line 30 では
/rooms/:name/:user_idを登録しているのに、ここではクエリ文字列を読んでいます。現状の path 形式で呼ぶと空user_idで検索されるため、ロール判定が壊れます。🐛 提案する修正
func GetUserRoomRole(c *gin.Context) { roomName := c.Param("name") - userID := c.Query("user_id") + userID := c.Param("user_id") db := c.MustGet("db").(*gorm.DB)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/globalchat.go` around lines 264 - 266, GetUserRoomRole reads user_id from the query but the route is defined as /rooms/:name/:user_id, causing empty user_id and broken role checks; change the retrieval to use c.Param("user_id") in GetUserRoomRole (and adjust any subsequent parsing/conversion logic that expects a string/ID) so the handler uses the path parameter consistent with the route definition.
159-162:⚠️ Potential issue | 🔴 Critical
GlobalChatRoomUserの検索列が誤っています。
src/api/src/internal/model/GlobalChatSetting.go:30-37のGlobalChatRoomUserはRoomNameをroom_name列にマップするので、ここでname = ?を使うと既存 owner を見つけられません。しかも今は想定外の DB エラーも握りつぶしているため、ルーム作成直後に owner 登録が欠落します。🐛 提案する修正
- res := tx.Where("name = ? AND user_id = ?", input.Name, input.CreatorID).First(&user) - if res.Error == gorm.ErrRecordNotFound { + res := tx.Where("room_name = ? AND user_id = ?", input.Name, input.CreatorID).First(&user) + if res.Error != nil && !errors.Is(res.Error, gorm.ErrRecordNotFound) { + return res.Error + } + if errors.Is(res.Error, gorm.ErrRecordNotFound) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/globalchat.go` around lines 159 - 162, The query is using the wrong column and swallowing DB errors: replace the WHERE clause that currently uses "name = ? AND user_id = ?" with the correct column name "room_name = ? AND user_id = ?" to match the GlobalChatRoomUser struct (RoomName -> room_name), and change the error handling on the gorm result (res) so that you explicitly handle gorm.ErrRecordNotFound as the "not found" case but return or propagate any other non-nil res.Error instead of ignoring it, ensuring the owner lookup/registration doesn't get silently skipped; update references around GlobalChatRoomUser and the tx.Where(...) call accordingly.src/bot/lib/api.py (1)
480-526:⚠️ Potential issue | 🟠 Major部屋名を path に生で埋め込まないでください。
globalchat_get_active_channel_ids_bynameとglobalchat_join_memberは未エンコード、check_globalchat_cooldownはquote()のデフォルトで/が残ります。部屋名に空白・%・/が入ると 404 や別ルート扱いになるので、ここはすべて path-segment 用に統一してエンコードする必要があります。🐛 提案する修正
async def globalchat_get_active_channel_ids_byname(self, name: str): - url = f"{self.base_url}/globalchat/rooms/{name}" + encoded_name = urllib.parse.quote(name, safe="") + url = f"{self.base_url}/globalchat/rooms/{encoded_name}" async with self.session.get(url) as resp: if resp.status == 200: data = await resp.json() return data return {} @@ async def globalchat_join_member(self, room_name: str, user_id: str, role: str = "member"): payload = { "user_id": str(user_id), "role": role } - async with self.session.post(f"{self.base_url}/globalchat/rooms/{room_name}/members", json=payload) as resp: + encoded_room_name = urllib.parse.quote(room_name, safe="") + async with self.session.post(f"{self.base_url}/globalchat/rooms/{encoded_room_name}/members", json=payload) as resp: if resp.status == 200: return await resp.json() return None @@ async def check_globalchat_cooldown(self, room_name: str, user_id: str, seconds: int) -> Dict[str, Any]: - url = f"{self.base_url}/cooldowns/globalchat_{urllib.parse.quote(room_name)}/{user_id}" + url = f"{self.base_url}/cooldowns/globalchat_{urllib.parse.quote(room_name, safe='')}/{user_id}"検証では
quote()のデフォルトが/を残すことを確認できます。#!/bin/bash python - <<'PY' import urllib.parse for value in ("main", "my room", "a/b", "50% room"): print(f"{value!r}: default={urllib.parse.quote(value)!r}, strict={urllib.parse.quote(value, safe='')!r}") PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/lib/api.py` around lines 480 - 526, The room name is being inserted into URL path segments without proper path-segment encoding which breaks on spaces, %, or slashes; update globalchat_get_active_channel_ids_byname, globalchat_join_member, and check_globalchat_cooldown to encode room_name using urllib.parse.quote(room_name, safe='') (i.e., no safe chars) before using it in f-strings so the path segment is correctly escaped; ensure any other places that interpolate room_name into path segments follow the same pattern.src/bot/cogs/globalchat.py (1)
66-70:⚠️ Potential issue | 🟠 Major
self.bot.apiの初期化前アクセスを防いでください。起動直後にメッセージが来ると、ここで
AttributeErrorが起きます。API クライアント未準備なら早期 return しておく方が安全です。🐛 提案する修正
async def on_message(self, message: discord.Message): if message.author.bot or message.is_system(): return + + if not getattr(self.bot, "api", None): + return config = await self.bot.api.globalchat_get_channel_config(message.channel.id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/globalchat.py` around lines 66 - 70, Prevent AttributeError by guarding access to the API client in on_message: check that self.bot.api (or a readiness flag like self.bot.api_ready) is initialized and truthy before calling self.bot.api.globalchat_get_channel_config; if it's not ready, return early. Update on_message to perform this guard at the top (before calling globalchat_get_channel_config) and optionally log/debug when skipping processing due to an uninitialized API client.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/src/internal/router/globalchat.go`:
- Around line 208-212: The DB error handling around tx.Where("name = ?",
input.RoomName).First(&room) is treating all errors as success; update the
db.Transaction block in GlobalChatConnect so that after calling
tx.Where(...).First(&room) you check if res.Error != nil and if it's
gorm.ErrRecordNotFound then create the room, but if res.Error is any other error
return that error immediately (so the transaction aborts) instead of proceeding
to save the connection and returning 200. Ensure you return the underlying
res.Error from the Transaction callback to surface real DB failures.
In `@src/bot/cogs/globalchat.py`:
- Around line 76-80: The current loop in globalchat.py that checks
room.get("restrictions", []) ignores the restriction.expires_at field so
time-limited mutes/bans act permanent; update the logic inside the loop (where
you check res["target_id"] and res["type"]) to first skip any restriction whose
expires_at is present and already past (parse res["expires_at"] from the
restriction DTO in src/bot/lib/api.py and compare to now using timezone-aware
UTC datetime), then proceed to check the target_id/type (for "mute_user",
"ban_user", "ban_server") against str(message.author.id) / str(message.guild.id)
as currently done so only active (unexpired) restrictions block messages.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/page.tsx:
- Around line 80-87: The current fetch in page.tsx silently falls back to an
empty array (settings = []) on non-OK responses which conflates "no results"
with "request failed" and causes GlobalChatClient to show "not connected";
instead, detect non-OK responses and surface an explicit error state (e.g.,
throw an Error or return a distinct value) so the caller (GlobalChatClient) can
differentiate network/HTTP errors from an empty settings array. Update the fetch
block around BACKEND_URL and the settings variable to either throw when !res.ok
(including status/message) or return { settings: null, fetchError: true } so
GlobalChatClient can check fetchError vs empty settings and render
appropriately.
---
Duplicate comments:
In `@src/api/src/internal/router/globalchat.go`:
- Around line 264-266: GetUserRoomRole reads user_id from the query but the
route is defined as /rooms/:name/:user_id, causing empty user_id and broken role
checks; change the retrieval to use c.Param("user_id") in GetUserRoomRole (and
adjust any subsequent parsing/conversion logic that expects a string/ID) so the
handler uses the path parameter consistent with the route definition.
- Around line 159-162: The query is using the wrong column and swallowing DB
errors: replace the WHERE clause that currently uses "name = ? AND user_id = ?"
with the correct column name "room_name = ? AND user_id = ?" to match the
GlobalChatRoomUser struct (RoomName -> room_name), and change the error handling
on the gorm result (res) so that you explicitly handle gorm.ErrRecordNotFound as
the "not found" case but return or propagate any other non-nil res.Error instead
of ignoring it, ensuring the owner lookup/registration doesn't get silently
skipped; update references around GlobalChatRoomUser and the tx.Where(...) call
accordingly.
In `@src/bot/cogs/globalchat.py`:
- Around line 66-70: Prevent AttributeError by guarding access to the API client
in on_message: check that self.bot.api (or a readiness flag like
self.bot.api_ready) is initialized and truthy before calling
self.bot.api.globalchat_get_channel_config; if it's not ready, return early.
Update on_message to perform this guard at the top (before calling
globalchat_get_channel_config) and optionally log/debug when skipping processing
due to an uninitialized API client.
In `@src/bot/lib/api.py`:
- Around line 480-526: The room name is being inserted into URL path segments
without proper path-segment encoding which breaks on spaces, %, or slashes;
update globalchat_get_active_channel_ids_byname, globalchat_join_member, and
check_globalchat_cooldown to encode room_name using
urllib.parse.quote(room_name, safe='') (i.e., no safe chars) before using it in
f-strings so the path segment is correctly escaped; ensure any other places that
interpolate room_name into path segments follow the same pattern.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/GlobalChatClient.tsx:
- Around line 222-223: The modal render checks only infoModal and can crash when
infoData is undefined; change the conditional to require both infoModal and
infoData (e.g., replace "{infoModal && infoModal && (" with "{infoModal &&
infoData && (" or mirror the same guard used for editingData) so the component
only renders when infoData is present.
- Around line 223-277: The modal markup lacks proper ARIA and keyboard/focus
handling; update the outer modal container div to include role="dialog"
aria-modal="true" and add an id on the heading (e.g., id="info-modal-title")
then reference it with aria-labelledby; give the close button an aria-label
(e.g., "閉じる") and a ref for initial focus. In GlobalChatClient.tsx, implement a
useRef (e.g., infoModalRef, closeBtnRef) and a useEffect that when the modal
opens: saves document.activeElement, sets focus to the close button (or first
focusable), adds keydown handler to close on Escape (calling
setInfoModal(false)), implements a basic focus trap (wrap Tab/Shift+Tab to stay
within infoModalRef), and on close restores focus to the previously focused
element; ensure these changes are applied to the info modal and the other modal
block referenced in the comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b73a0ef7-3eef-43c6-b3ea-a3a48c5f2e3d
📒 Files selected for processing (8)
src/api/src/internal/router/globalchat.gosrc/bot/cogs/globalchat.pysrc/bot/lib/api.pysrc/dashboard/src/app/api/guilds/[guildId]/commands/route.tsxsrc/dashboard/src/app/dashboard/[guildId]/globalchat/GlobalChatClient.tsxsrc/dashboard/src/app/dashboard/[guildId]/globalchat/page.tsxsrc/dashboard/src/constants/commands/globalchat.tssrc/dashboard/src/lib/commands.ts
✅ Files skipped from review due to trivial changes (1)
- src/dashboard/src/lib/commands.ts
| err := db.Transaction(func(tx *gorm.DB) error { | ||
| var room model.GlobalChatRoom | ||
| res := tx.Where("name = ?", input.RoomName).First(&room) | ||
|
|
||
| if res.Error == gorm.ErrRecordNotFound { |
There was a problem hiding this comment.
部屋取得の DB エラーを成功扱いにしないでください。
First(&room) が ErrRecordNotFound 以外で失敗しても、そのまま接続保存に進んで 200 を返します。一時的な DB 障害でも GlobalChatConnect だけ残りうるので、not found 以外は即 return した方が安全です。
🐛 提案する修正
var room model.GlobalChatRoom
res := tx.Where("name = ?", input.RoomName).First(&room)
+ if res.Error != nil && !errors.Is(res.Error, gorm.ErrRecordNotFound) {
+ return res.Error
+ }
- if res.Error == gorm.ErrRecordNotFound {
+ if errors.Is(res.Error, gorm.ErrRecordNotFound) {📝 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.
| err := db.Transaction(func(tx *gorm.DB) error { | |
| var room model.GlobalChatRoom | |
| res := tx.Where("name = ?", input.RoomName).First(&room) | |
| if res.Error == gorm.ErrRecordNotFound { | |
| err := db.Transaction(func(tx *gorm.DB) error { | |
| var room model.GlobalChatRoom | |
| res := tx.Where("name = ?", input.RoomName).First(&room) | |
| if res.Error != nil && !errors.Is(res.Error, gorm.ErrRecordNotFound) { | |
| return res.Error | |
| } | |
| if errors.Is(res.Error, gorm.ErrRecordNotFound) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/src/internal/router/globalchat.go` around lines 208 - 212, The DB
error handling around tx.Where("name = ?", input.RoomName).First(&room) is
treating all errors as success; update the db.Transaction block in
GlobalChatConnect so that after calling tx.Where(...).First(&room) you check if
res.Error != nil and if it's gorm.ErrRecordNotFound then create the room, but if
res.Error is any other error return that error immediately (so the transaction
aborts) instead of proceeding to save the connection and returning 200. Ensure
you return the underlying res.Error from the Transaction callback to surface
real DB failures.
| for res in room.get("restrictions", []): | ||
| if res["target_id"] == str(message.author.id) and res["type"] in ["ban_user", "mute_user"]: | ||
| return | ||
| if res["target_id"] == str(message.guild.id) and res["type"] == "ban_server": | ||
| return |
There was a problem hiding this comment.
期限付き制限が失効しません。
src/bot/lib/api.py:47-51 の restriction DTO には expires_at がありますが、ここでは一度も見ていません。このままだと期限付きの mute_user / ban_user / ban_server も永久ブロックになります。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/cogs/globalchat.py` around lines 76 - 80, The current loop in
globalchat.py that checks room.get("restrictions", []) ignores the
restriction.expires_at field so time-limited mutes/bans act permanent; update
the logic inside the loop (where you check res["target_id"] and res["type"]) to
first skip any restriction whose expires_at is present and already past (parse
res["expires_at"] from the restriction DTO in src/bot/lib/api.py and compare to
now using timezone-aware UTC datetime), then proceed to check the target_id/type
(for "mute_user", "ban_user", "ban_server") against str(message.author.id) /
str(message.guild.id) as currently done so only active (unexpired) restrictions
block messages.
| const res = await fetch(`${BACKEND_URL}/globalchat/guilds/${guildId}`, { | ||
| cache: 'no-store' | ||
| }); | ||
|
|
||
| let settings = []; | ||
| if (res.ok) { | ||
| settings = await res.json(); | ||
| } |
There was a problem hiding this comment.
取得失敗を『接続なし』に見せないでください。
ここで 500/401 を [] に落とすと、src/dashboard/src/app/dashboard/[guildId]/globalchat/GlobalChatClient.tsx:137-140 では未接続表示になります。通信失敗と空結果は分けて扱った方が安全です。
🐛 提案する修正
const res = await fetch(`${BACKEND_URL}/globalchat/guilds/${guildId}`, {
cache: 'no-store'
});
- let settings = [];
- if (res.ok) {
- settings = await res.json();
+ if (!res.ok) {
+ return (
+ <Alert
+ text="グローバルチャット設定の取得に失敗しました。時間をおいて再度お試しください。"
+ redirectUrl={`/dashboard/${guildId}`}
+ />
+ );
}
+ const settings = await res.json();
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 res = await fetch(`${BACKEND_URL}/globalchat/guilds/${guildId}`, { | |
| cache: 'no-store' | |
| }); | |
| let settings = []; | |
| if (res.ok) { | |
| settings = await res.json(); | |
| } | |
| const res = await fetch(`${BACKEND_URL}/globalchat/guilds/${guildId}`, { | |
| cache: 'no-store' | |
| }); | |
| if (!res.ok) { | |
| return ( | |
| <Alert | |
| text="グローバルチャット設定の取得に失敗しました。時間をおいて再度お試しください。" | |
| redirectUrl={`/dashboard/${guildId}`} | |
| /> | |
| ); | |
| } | |
| const settings = await res.json(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/src/app/dashboard/`[guildId]/globalchat/page.tsx around lines
80 - 87, The current fetch in page.tsx silently falls back to an empty array
(settings = []) on non-OK responses which conflates "no results" with "request
failed" and causes GlobalChatClient to show "not connected"; instead, detect
non-OK responses and surface an explicit error state (e.g., throw an Error or
return a distinct value) so the caller (GlobalChatClient) can differentiate
network/HTTP errors from an empty settings array. Update the fetch block around
BACKEND_URL and the settings variable to either throw when !res.ok (including
status/message) or return { settings: null, fetchError: true } so
GlobalChatClient can check fetchError vs empty settings and render
appropriately.
グローバルチャットを追加
ダッシュボードによる設定を追加
Summary by CodeRabbit
新機能
バグ修正
Chores