Conversation
feat: 引用メッセージを展開するように
fix: 引用メッセージの展開が正しく行われるように
fix: チャンネルによるバイパスができなかったのを修正
📝 WalkthroughWalkthroughbot APIを通じてチャネルとユーザー情報をキャッシュする機構、メッセージ内の埋め込みと引用をフォーマットする機能を追加し、メッセージハンドラーでこれらの新しいフォーマッター機能を統合しました。 Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as Message Handler
participant Formatter as Formatter
participant Cache as Cache (Channel/User/Message)
participant API as TraQ API
Handler->>Formatter: FormatEmbeds(message.text)
Formatter-->>Handler: textWithEmbed
Handler->>Formatter: FormatQuotedMessage(userID, textWithEmbed)
Formatter->>Cache: Extract message IDs & search for quotes
loop For each quoted message
Formatter->>Cache: GetChannelPath(channelID)
Cache->>API: Fetch channel if not cached
API-->>Cache: Return channel path
Cache-->>Formatter: Return cached path
Formatter->>Cache: GetUser(userID)
Cache->>API: Fetch user if not cached
API-->>Cache: Return user detail
Cache-->>Formatter: Return cached user
Formatter->>Cache: GetMessage(messageID)
Cache->>API: Fetch message if not cached
API-->>Cache: Return message
Cache-->>Formatter: Return cached message
Formatter->>Formatter: Check if channel/user allows quoting
Formatter->>Formatter: Build Markdown blockquote (truncate if needed)
end
Formatter-->>Handler: formattedMessage or error
Handler->>Handler: messageReceived(text, formattedMessage)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
internal/handler/OnMessageCreated.go (1)
21-27: 整形パイプラインがDM側と重複しているため、共通化を推奨します。Line 21-27 の処理は
internal/handler/OnDirectMessageCreated.goの同等ブロックと実質同一です。ヘルパー化して1か所に寄せると、今後のフォーマット仕様変更で差分不整合を防げます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/handler/OnMessageCreated.go` around lines 21 - 27, The text formatting pipeline (formatter.FormatEmbeds -> bot.RemoveFirstBotID -> formatter.FormatQuotedMessage) is duplicated between OnMessageCreated.go and OnDirectMessageCreated.go; extract this sequence into a single helper (e.g., FormatIncomingMessage or NormalizeAndFormatMessage) that returns the final formattedMessage and any error, and replace the inline block in both handlers to call that helper; ensure the helper accepts the original message text and user ID and preserves the current fallback behavior (return the intermediate text when FormatQuotedMessage fails) so callers keep existing semantics.internal/pkg/formatter/embed_test.go (1)
5-35: エスケープ文字を含むrawのケースを1件追加すると、回帰耐性がさらに上がります。Line 18/23 のような正常系に加えて、
rawに\"や\\nを含む入力の期待値を固定しておくと、今後の正規表現変更時に壊れにくくなります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/pkg/formatter/embed_test.go` around lines 5 - 35, Add a test case to TestFormatEmbeds that covers a raw field containing escaped characters so we catch regressions in JSON-unescaping; specifically, extend the tests slice used by FormatEmbeds with an entry (e.g., name "Escaped raw") whose input is an embed JSON containing raw with escaped quote and newline sequences (e.g., raw contains \" and \\n) and set expected to the unescaped result (i.e., actual quote character and a real newline where \\n was). This ensures FormatEmbeds correctly unescapes JSON string contents like in the new test case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/bot/channel.go`:
- Around line 23-25: The current GetChannelPath uses context.Background() which
prevents callers from supplying timeouts; change GetChannelPath(channelID
string) to GetChannelPath(ctx context.Context, channelID string) and use ctx
when calling channelPathCache.Get; propagate the context through
isChannelAllowingQuotes(ctx context.Context, channelID string) and
FormatQuotedMessage(ctx context.Context, userID, content string) and update all
callers (including WebSocket event handlers MessageReceived and
DirectMessageReceived) to accept and forward the ctx so timeouts/cancellation
are honored; ensure channelPathCache.Get(ctx, channelID) is used and update any
tests and call sites accordingly.
In `@internal/bot/user.go`:
- Around line 24-26: Change the GetUser function to accept and propagate a
caller-provided context instead of using context.Background(): update the
signature from GetUser(userID string) to GetUser(ctx context.Context, userID
string), pass that ctx into userCache.Get and any internal helper like
getUserInternal(), and ensure all callers are adjusted to provide their upstream
context so cancellations/timeouts propagate correctly.
In `@internal/pkg/formatter/quotes.go`:
- Around line 16-26: The current isChannelAllowingQuotes uses allowingPrefixes
and strings.HasPrefix on channelPath which is too permissive; update
isChannelAllowingQuotes (and allowingPrefixes) so that "event", "general",
"random", and "services" are matched by exact equality with channelPath, while
"team/SysAd" is matched either by exact equality or by HasPrefix on
"team/SysAd/"; use bot.GetChannelPath to obtain channelPath as before and only
return true,nil when one of these stricter conditions is met.
---
Nitpick comments:
In `@internal/handler/OnMessageCreated.go`:
- Around line 21-27: The text formatting pipeline (formatter.FormatEmbeds ->
bot.RemoveFirstBotID -> formatter.FormatQuotedMessage) is duplicated between
OnMessageCreated.go and OnDirectMessageCreated.go; extract this sequence into a
single helper (e.g., FormatIncomingMessage or NormalizeAndFormatMessage) that
returns the final formattedMessage and any error, and replace the inline block
in both handlers to call that helper; ensure the helper accepts the original
message text and user ID and preserves the current fallback behavior (return the
intermediate text when FormatQuotedMessage fails) so callers keep existing
semantics.
In `@internal/pkg/formatter/embed_test.go`:
- Around line 5-35: Add a test case to TestFormatEmbeds that covers a raw field
containing escaped characters so we catch regressions in JSON-unescaping;
specifically, extend the tests slice used by FormatEmbeds with an entry (e.g.,
name "Escaped raw") whose input is an embed JSON containing raw with escaped
quote and newline sequences (e.g., raw contains \" and \\n) and set expected to
the unescaped result (i.e., actual quote character and a real newline where \\n
was). This ensures FormatEmbeds correctly unescapes JSON string contents like in
the new test case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a900744-ef66-4c04-a9cd-c65a991516f1
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
go.modinternal/bot/channel.gointernal/bot/user.gointernal/handler/OnDirectMessageCreated.gointernal/handler/OnMessageCreated.gointernal/pkg/formatter/embed.gointernal/pkg/formatter/embed_test.gointernal/pkg/formatter/quotes.go
Comment on lines
+23
to
+25
| func GetChannelPath(channelID string) (string, error) { | ||
| return channelPathCache.Get(context.Background(), channelID) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verification script from review comment
set -euo pipefail
echo "=== Checking context.Background() usage ==="
rg -n 'context\.Background\(\)' internal/bot/channel.go internal/bot/user.go -C2 2>/dev/null || echo "No matches in specified files"
echo -e "\n=== Checking GetChannelPath function definition ==="
rg -n 'func\s+GetChannelPath\s*\(' internal/bot/channel.go -C2 2>/dev/null || echo "No matches found"
echo -e "\n=== Checking all GetChannelPath call sites ==="
rg -n '\bGetChannelPath\s*\(' --type=go -C2 2>/dev/null | head -50 || echo "No matches found"Repository: traPtitech/BOT_GPT
Length of output: 1801
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking isChannelAllowingQuotes call sites ==="
rg -n '\bisChannelAllowingQuotes\s*\(' --type=go -B2 -A2
echo -e "\n=== Checking function signatures that call GetChannelPath or isChannelAllowingQuotes ==="
rg -n 'func.*GetChannelPath|func.*isChannelAllowingQuotes|func.*quotes' --type=go -A1 | head -30
echo -e "\n=== Checking cache.Get method signature ==="
rg -n 'type.*Cache|func.*\.Get\(' --type=go internal/pkg/cache -A2 2>/dev/null || echo "Cache package info not found"
echo -e "\n=== Looking for cache implementation ==="
fd -t f '.*cache.*' --type=go 2>/dev/null || echo "No cache files found"Repository: traPtitech/BOT_GPT
Length of output: 1393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Function containing isChannelAllowingQuotes call at line 86 ==="
sed -n '75,95p' internal/pkg/formatter/quotes.go | cat -n
echo -e "\n=== Full function signature around line 86 ==="
sed -n '70,100p' internal/pkg/formatter/quotes.go | cat -n
echo -e "\n=== Search for cache package in internal/pkg ==="
find internal/pkg -name '*cache*' -o -name '*Cache*' 2>/dev/null
echo -e "\n=== Search for 'sc.' or cache-like pattern ==="
rg -n 'import|sc\.' internal/bot/channel.go | head -20
echo -e "\n=== Check imports in channel.go to find cache package ==="
head -20 internal/bot/channel.goRepository: traPtitech/BOT_GPT
Length of output: 2410
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find the function containing line 86 (isChannelAllowingQuotes call) ==="
rg -B50 'isChannelAllowingQuotes\(message.ChannelId\)' internal/pkg/formatter/quotes.go | head -60 | tail -20
echo -e "\n=== Get full function signature ==="
rg -n 'func.*Format.*Quote|^func' internal/pkg/formatter/quotes.go
echo -e "\n=== Check the actual formatter function with context ==="
sed -n '50,75p' internal/pkg/formatter/quotes.go | cat -n
echo -e "\n=== Check motoki317/sc package documentation or usage ==="
rg -n 'sc\.' internal/ -A1 -B1 | head -30Repository: traPtitech/BOT_GPT
Length of output: 2381
🏁 Script executed:
#!/bin/bash
set -eulo pipefail
echo "=== Find all calls to FormatQuotedMessage ==="
rg -n '\bFormatQuotedMessage\s*\(' --type=go -B2 -A2
echo -e "\n=== Check the function signatures that call FormatQuotedMessage ==="
rg -B20 'FormatQuotedMessage\(' internal/ --type=go | grep -E 'func |ctx|context' | head -20
echo -e "\n=== Check if there's a handler function calling FormatQuotedMessage ==="
rg -n 'Handler|HandleMessage' internal/ --type=go -A3 | head -40Repository: traPtitech/BOT_GPT
Length of output: 3466
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check full OnMessageCreated handler signature ==="
head -30 internal/handler/OnMessageCreated.go | cat -n
echo -e "\n=== Check full OnDirectMessageCreated handler signature ==="
head -30 internal/handler/OnDirectMessageCreated.go | cat -n
echo -e "\n=== Check where these handlers are registered/called ==="
rg -n 'MessageReceived|DirectMessageReceived' --type=go -B2 -A2 | grep -E '(main|func|Register|http|Handler)' | head -30
echo -e "\n=== Check if handlers are HTTP handlers or event handlers ==="
rg -n 'type.*Handler|func.*Handler' internal/handler/ --type=goRepository: traPtitech/BOT_GPT
Length of output: 3610
context.Background() 固定では、外部API呼び出しのタイムアウト制御ができません。
Line 24 の実装は、呼び出し元がタイムアウトを設定しても反映できません。ネットワーク遅延時にハンドラが長時間ブロックされるリスクがあります。
ただし、修正には複数レイヤーの変更が必要です：
GetChannelPath(channelID string)→GetChannelPath(ctx context.Context, channelID string)isChannelAllowingQuotes(channelID string)→isChannelAllowingQuotes(ctx context.Context, channelID string)FormatQuotedMessage(userID, content string)→FormatQuotedMessage(ctx context.Context, userID, content string)- WebSocket イベントハンドラ（
MessageReceived、DirectMessageReceived）にも context を追加
現在のハンドラは WebSocket 接続に基づくイベントコールバックであり、context がないため、この変更は各レイヤーで実施が必要です。
💡 変更の概要
-func GetChannelPath(channelID string) (string, error) {
- return channelPathCache.Get(context.Background(), channelID)
+func GetChannelPath(ctx context.Context, channelID string) (string, error) {
+ return channelPathCache.Get(ctx, channelID)
}
-func isChannelAllowingQuotes(channelID string) (bool, error) {
- channelPath, err := bot.GetChannelPath(channelID)
+func isChannelAllowingQuotes(ctx context.Context, channelID string) (bool, error) {
+ channelPath, err := bot.GetChannelPath(ctx, channelID)
}
-func FormatQuotedMessage(userID string, content string) (string, error) {
+func FormatQuotedMessage(ctx context.Context, userID string, content string) (string, error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/bot/channel.go` around lines 23 - 25, The current GetChannelPath
uses context.Background() which prevents callers from supplying timeouts; change
GetChannelPath(channelID string) to GetChannelPath(ctx context.Context,
channelID string) and use ctx when calling channelPathCache.Get; propagate the
context through isChannelAllowingQuotes(ctx context.Context, channelID string)
and FormatQuotedMessage(ctx context.Context, userID, content string) and update
all callers (including WebSocket event handlers MessageReceived and
DirectMessageReceived) to accept and forward the ctx so timeouts/cancellation
are honored; ensure channelPathCache.Get(ctx, channelID) is used and update any
tests and call sites accordingly.
Comment on lines
+24
to
+26
| func GetUser(userID string) (*traq.UserDetail, error) { | ||
| return userCache.Get(context.Background(), userID) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the structure and find the relevant files
find . -name "user.go" -o -name "channel.go" | grep -E "(user|channel)\.go"Repository: traPtitech/BOT_GPT
Length of output: 110
🏁 Script executed:
# Now let's examine the GetUser function in user.go at lines 24-26
cat -n internal/bot/user.go | head -35Repository: traPtitech/BOT_GPT
Length of output: 754
🏁 Script executed:
# Let's also check the channel.go file to understand the similar pattern
cat -n internal/bot/channel.go | head -35Repository: traPtitech/BOT_GPT
Length of output: 755
🏁 Script executed:
# Search for all usages of GetUser to understand the impact
rg "GetUser\(" -A 2 -B 2Repository: traPtitech/BOT_GPT
Length of output: 1135
GetUser は context を非伝播のため、API遅延時の制御ができません。
Line 25で context.Background() が固定されているため、呼び出し元のキャンセルやタイムアウトが機能しません。GetUser(ctx context.Context, userID string) に変更し、上流の context を渡すようにしてください。これにより、内部の getUserInternal() に context が正しく伝播されます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/bot/user.go` around lines 24 - 26, Change the GetUser function to
accept and propagate a caller-provided context instead of using
context.Background(): update the signature from GetUser(userID string) to
GetUser(ctx context.Context, userID string), pass that ctx into userCache.Get
and any internal helper like getUserInternal(), and ensure all callers are
adjusted to provide their upstream context so cancellations/timeouts propagate
correctly.
Comment on lines
+16
to
+26
| var allowingPrefixes = []string{"event", "general", "random", "services", "team/SysAd"} | ||
|
|
||
| func isChannelAllowingQuotes(channelID string) (bool, error) { | ||
| channelPath, err := bot.GetChannelPath(channelID) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| for _, prefix := range allowingPrefixes { | ||
| if strings.HasPrefix(channelPath, prefix) { | ||
| return true, nil |
There was a problem hiding this comment.
許可チャンネル判定が広すぎ、意図しないチャンネルを通す可能性があります。
Line 25 の HasPrefix 判定だと、general / random / event / services が前方一致で通るため、要件外チャンネルまで展開対象になるリスクがあります。
少なくとも event|random|general|services は厳密一致、team/SysAd のみ team/SysAd または team/SysAd/ 配下一致に分けるのが安全です。
🔧 判定ロジックの例
-var allowingPrefixes = []string{"event", "general", "random", "services", "team/SysAd"}
+var exactAllowingChannels = map[string]struct{}{
+ "event": {},
+ "general": {},
+ "random": {},
+ "services": {},
+}
func isChannelAllowingQuotes(channelID string) (bool, error) {
channelPath, err := bot.GetChannelPath(channelID)
if err != nil {
return false, err
}
- for _, prefix := range allowingPrefixes {
- if strings.HasPrefix(channelPath, prefix) {
- return true, nil
- }
- }
+ if _, ok := exactAllowingChannels[channelPath]; ok {
+ return true, nil
+ }
+ if channelPath == "team/SysAd" || strings.HasPrefix(channelPath, "team/SysAd/") {
+ return true, nil
+ }
return false, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var allowingPrefixes = []string{"event", "general", "random", "services", "team/SysAd"} | |
| func isChannelAllowingQuotes(channelID string) (bool, error) { | |
| channelPath, err := bot.GetChannelPath(channelID) | |
| if err != nil { | |
| return false, err | |
| } | |
| for _, prefix := range allowingPrefixes { | |
| if strings.HasPrefix(channelPath, prefix) { | |
| return true, nil | |
| var exactAllowingChannels = map[string]struct{}{ | |
| "event": {}, | |
| "general": {}, | |
| "random": {}, | |
| "services": {}, | |
| } | |
| func isChannelAllowingQuotes(channelID string) (bool, error) { | |
| channelPath, err := bot.GetChannelPath(channelID) | |
| if err != nil { | |
| return false, err | |
| } | |
| if _, ok := exactAllowingChannels[channelPath]; ok { | |
| return true, nil | |
| } | |
| if channelPath == "team/SysAd" || strings.HasPrefix(channelPath, "team/SysAd/") { | |
| return true, nil | |
| } | |
| return false, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/pkg/formatter/quotes.go` around lines 16 - 26, The current
isChannelAllowingQuotes uses allowingPrefixes and strings.HasPrefix on
channelPath which is too permissive; update isChannelAllowingQuotes (and
allowingPrefixes) so that "event", "general", "random", and "services" are
matched by exact equality with channelPath, while "team/SysAd" is matched either
by exact equality or by HasPrefix on "team/SysAd/"; use bot.GetChannelPath to
obtain channelPath as before and only return true,nil when one of these stricter
conditions is met.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ただし展開されるには次の条件のいずれかを満たす必要がある
引用メッセージの投稿者が自分
引用メッセージの投稿者がBot
引用メッセージのチャンネルが特定のチャンネル
#event / #random / #general / #services / #team/SysAd 以下全てのチャンネル
さらに1つの引用メッセージで10000文字を超えた分は省略される
Summary by CodeRabbit
新機能
テスト