fix: Slack AI 429 에러 수정 및 스레드 대댓글 기능 추가#380
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughClaudeClient에 멀티턴 도구 호출 흐름과 새 chat(List) 오버로드가 추가되고, Slack 쪽은 스레드 인식 처리(스레드 조회/스레드 답글 게시), 중복 event_id 캐시, 대화 이력 구성 로직이 도입되었습니다. 설정에는 Slack botToken과 Hibernate 타임존 설정이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Slack as Slack Event
participant Controller as SlackEventController
participant Service as SlackAIService
participant Client as SlackClient
participant Claude as ClaudeClient
participant MCP as MCPClient
participant API as Claude API
User->>Slack: Slack 메시지 발송
Slack->>Controller: handleEvent 호출
Controller->>Controller: 중복 event_id 확인
alt 중복 이벤트
Controller-->>Slack: 200 OK (처리 안 함)
else 신규 이벤트
Controller->>Client: getThreadReplies(channelId, threadTs)
Client->>API: Slack API 호출
API-->>Client: 스레드 메시지 조회
Client-->>Service: 캐시된 스레드 replies 반환
Service->>Service: buildConversationHistory(replies)
Service->>Service: 대화 이력 구성 및 병합
Service->>Claude: chat(List<Map> messages)
Claude->>Claude: buildRequest(messages)
Claude->>API: Claude API 호출
API-->>Claude: 응답 수신
alt stop_reason == "tool_use"
Claude->>Claude: processToolCalls(content)
Claude->>MCP: executeToolCall(toolName, input)
MCP-->>Claude: 도구 실행 결과
Claude->>Claude: 결과를 메시지에 추가
Claude->>API: 업데이트된 메시지로 재호출
API-->>Claude: 최종 응답
else stop_reason == "end_turn"
Claude->>Claude: extractTextResponse(content)
end
Claude-->>Service: 최종 텍스트 응답
Service->>Client: postThreadReply(channelId, threadTs, text)
Client->>API: Slack API 호출
API-->>Client: 메시지 게시 완료
Client-->>Service: 성공
Service-->>Controller: 처리 완료
Controller-->>Slack: 200 OK
end
sequenceDiagram
participant App as ClaudeClient
participant Cache as Tool Cache
participant API as Claude API
participant Tool as MCPClient
loop MAX_TOOL_ITERATIONS까지
App->>App: buildRequest(messages)
App->>API: callClaudeApi(request)
API-->>App: 응답 수신
App->>App: stop_reason 파싱
alt stop_reason == "tool_use"
App->>App: processToolCalls(content)
loop 각 tool_use 블록
App->>App: executeToolCall(toolName, input)
App->>Tool: 도구 실행 요청
Tool-->>App: 실행 결과 반환
App->>Cache: 결과 캐싱
App->>App: 대화에 도구 결과 추가
end
Note over App: 루프 계속
else stop_reason == "end_turn"
App->>App: extractTextResponse(content)
App-->>App: 텍스트 응답 반환
Note over App: 루프 종료
else 기타 stop_reason
App->>App: 로그 기록 및 반환
Note over App: 루프 종료
end
end
alt 최대 반복 초과
App->>App: ClaudeException 발생
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60분 Possibly related PRs
시
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Slack AI 이벤트 처리에서 중복 이벤트로 인한 재처리(재시도) 문제를 완화하고, 스레드 대댓글 기반 멀티턴 대화를 지원하기 위한 변경입니다. 또한 Slack Bot Token 설정과 Hibernate JDBC 타임존 설정을 추가합니다.
Changes:
- Slack 이벤트 중복 처리 방지를 위한
event_id캐시 추가 및 스레드 컨텍스트(channel/thread_ts) 기반 AI 처리로 확장 - Slack Webhook 전송 대신 Bot Token 기반
chat.postMessage/conversations.replies사용하여 스레드 대댓글/히스토리 조회 기능 추가 - Claude 멀티턴 입력(
List<Map<String,Object>>) 지원 및 Hibernate JDBC 타임존 설정 추가
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/application-infrastructure.yml | Slack Bot Token 설정(slack.bot-token) 추가 |
| src/main/resources/application-db.yml | Hibernate JDBC 타임존 설정 추가(현재는 YAML 키 중복 이슈 존재) |
| src/main/java/gg/agit/konect/infrastructure/slack/config/SlackProperties.java | botToken 프로퍼티 바인딩 추가 |
| src/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.java | 스레드 대댓글 전송 및 스레드 replies 조회 API 호출 추가 |
| src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java | 이벤트 중복 처리 방지 캐시 + 스레드 기반 AI 처리 플로우로 확장 |
| src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java | 스레드 히스토리 기반 멀티턴 메시지 구성 및 응답을 스레드로 전송 |
| src/main/java/gg/agit/konect/infrastructure/claude/client/ClaudeClient.java | 멀티턴 메시지 입력을 받는 chat(List<...>) 오버로드 추가 |
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.java
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.java (1)
35-44: 🛠️ Refactor suggestion | 🟠 Major[LEVEL: medium]
postThreadReply에@Retryable누락으로 sendMessage와 일관성이 없습니다.
sendMessage는@Retryable을 사용하여 일시적 네트워크 오류에 재시도하지만,postThreadReply는 재시도 없이 실패합니다. 429 에러 수정이 PR 목적인 점을 감안하면, 재시도 로직 추가가 필요합니다.제안: `@Retryable` 추가
+ `@Retryable` public void postThreadReply(String channelId, String threadTs, String text) {// Recover 메서드도 추가 `@Recover` public void postThreadReplyRecover(Exception e, String channelId, String threadTs, String text) { log.error("Slack 스레드 응답 전송 최종 실패: channelId={}, threadTs={}", channelId, threadTs, e); }Also applies to: 52-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.java` around lines 35 - 44, postThreadReply is missing `@Retryable` making it inconsistent with sendMessage and vulnerable to transient errors (e.g., 429); add `@Retryable` to the postThreadReply method and implement a corresponding `@Recover` handler (e.g., postThreadReplyRecover(Exception e, String channelId, String threadTs, String text)) that logs the final failure including channelId, threadTs and the exception; update any method signatures or annotations in SlackClient (postThreadReply and add postThreadReplyRecover) so retries behave like sendMessage and the recover method captures and logs the failure context.
🤖 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/main/java/gg/agit/konect/infrastructure/claude/client/ClaudeClient.java`:
- Around line 162-164: The current chat() implementation in ClaudeClient.java
returns extractTextResponse(content) for unexpected stop_reason values, which
incorrectly treats truncated responses (e.g., "max_tokens" and
"model_context_window_exceeded") as complete; update the else branch in chat()
to explicitly handle stop_reason == "max_tokens" and
"model_context_window_exceeded" by either (a) triggering a continuation request
flow, (b) reducing/truncating context and retrying, or (c) returning an explicit
error/partial-response flag so callers know the response is incomplete; apply
the same changes to the other chat() overloads that use extractTextResponse so
all truncated/Context-window stop_reason cases are consistently handled instead
of being returned as successful full responses.
- Around line 158-160: The code is appending raw toolResults into the messages
list (see messages.add(Map.of("role","user","content", toolResults)) in
ClaudeClient) without size limits, causing request token bloat; before adding
toolResults, truncate by character/line count or compress to a concise summary
(implement a helper like summarizeOrTruncateToolResults(String toolResults, int
maxChars) or limit to N lines), ensure the result includes an explicit marker
(e.g., "[TRUNCATED]" or a brief summary header) so the model knows data was
shortened, and use that sanitized string in place of toolResults when calling
messages.add to keep input tokens under ITPM limits.
In `@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java`:
- Around line 140-144: The subList call on merged (result of
mergeConsecutiveRoles) returns a view into the original list which can cause
side-effects; replace the view with a defensive copy by assigning merged = new
ArrayList<>(merged.subList(merged.size() - MAX_HISTORY_MESSAGES,
merged.size())); so that downstream callers (e.g., ClaudeClient.chat / any code
using merged) get an independent list instead of a view.
- Around line 66-74: The current logic in SlackAIService (variables replies,
rootMessage, rootText) returns replies when any message has a bot_id, which
allows buildConversationHistory to start with an "assistant" role and violate
Claude's requirement that the first message must be "user"; update the check so
that if the root message is from a bot (rootMessage contains "bot_id" or
rootText indicates a bot) you do not return replies (return empty list) OR
modify buildConversationHistory to remove a leading assistant message (e.g., if
merged is non-empty and merged.get(0).get("role") == "assistant" then remove
index 0) so the conversation sent to Claude always starts with a "user" role;
apply the fix to the code paths that return replies to ensure no assistant-first
message is sent to the API.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java`:
- Around line 142-147: The app_mention branch in SlackEventController ignores
thread context and passes null; update the handler to compute an effective
thread timestamp the same way the message branch does (use event.thread_ts if
present else fallback to event.ts) and pass that effectiveThreadTs into
slackAIService.processAIQuery so prior thread history can be loaded (or, if you
rely on explicit loading, call slackAIService.fetchAIThreadReplies when
thread_ts exists before calling processAIQuery); reference the methods
normalizeAppMentionText and processAIQuery and add a brief comment if omitting
thread context was intentional.
- Around line 36-44: processedEventIds is not thread-safe causing race
conditions; replace its initialization with a concurrent set by changing the
field to: private final Set<String> processedEventIds =
ConcurrentHashMap.newKeySet(); and keep EVENT_CACHE_MAX_SIZE in mind — because
ConcurrentHashMap.newKeySet() has no LRU eviction, add a simple concurrent
trimming strategy after adds (e.g., if (processedEventIds.size() >
EVENT_CACHE_MAX_SIZE) remove oldest via an iterator inside a synchronized block)
or switch to a bounded concurrent cache (Caffeine/Guava) for true LRU eviction;
update usages of processedEventIds.add/remove accordingly in
SlackEventController.
In `@src/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.java`:
- Around line 75-83: The getThreadReplies method in SlackClient currently treats
missing "messages" the same as an API error; update the parsing after
objectMapper.readValue(...) to first check the "ok" field (e.g., Boolean ok =
(Boolean) parsed.get("ok")), and if ok is Boolean.FALSE then log the error
detail from parsed.get("error") (and rethrow or return an error result per
project convention) instead of silently returning an empty list; keep the
existing flow to return (List<Map<String,Object>>) parsed.get("messages") only
when ok is true and messages is a List.
- Around line 52-64: The postThreadReply method in SlackClient currently ignores
the Slack API response; update it to capture the response from
restTemplate.postForEntity (call to SLACK_API_BASE + "/chat.postMessage"),
deserialize/inspect the body for the Slack fields (e.g. "ok" and "error"), and
handle non-ok results by logging the error and throwing or returning a failure
so callers can react (include channelId/threadTs in logs); also treat HTTP
non-2xx responses as failures and optionally surface rate-limit headers for
retry logic. Ensure this validation is implemented inside postThreadReply and
uses the existing restTemplate and slackProperties context.
In `@src/main/resources/application-db.yml`:
- Around line 20-23: The YAML contains a duplicated top-level mapping key
"properties" (one at line 14 and another at line 20) which breaks parsing; to
fix, merge the entries under the two "properties" mappings into a single
"properties" mapping and ensure nested keys like "hibernate.jdbc.time_zone:
Asia/Seoul" are placed under that single "properties" block (verify there is
only one "properties" key and combine any other child keys from both
occurrences).
---
Outside diff comments:
In `@src/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.java`:
- Around line 35-44: postThreadReply is missing `@Retryable` making it
inconsistent with sendMessage and vulnerable to transient errors (e.g., 429);
add `@Retryable` to the postThreadReply method and implement a corresponding
`@Recover` handler (e.g., postThreadReplyRecover(Exception e, String channelId,
String threadTs, String text)) that logs the final failure including channelId,
threadTs and the exception; update any method signatures or annotations in
SlackClient (postThreadReply and add postThreadReplyRecover) so retries behave
like sendMessage and the recover method captures and logs the failure context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 09d6ca2e-dcab-40a9-b237-9c22ded42cd2
📒 Files selected for processing (7)
src/main/java/gg/agit/konect/infrastructure/claude/client/ClaudeClient.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackProperties.javasrc/main/resources/application-db.ymlsrc/main/resources/application-infrastructure.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.javasrc/main/java/gg/agit/konect/infrastructure/claude/client/ClaudeClient.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackProperties.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.javasrc/main/resources/application-infrastructure.ymlsrc/main/resources/application-db.ymlsrc/main/java/gg/agit/konect/infrastructure/claude/client/ClaudeClient.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackProperties.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java
🪛 YAMLlint (1.38.0)
src/main/resources/application-db.yml
[error] 20-20: duplication of key "properties" in mapping
(key-duplicates)
🔇 Additional comments (1)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (1)
82-86: [LEVEL: medium]
add()와 조건 검사가 원자적이지 않아 TOCTOU 취약점이 있습니다.Line 83의
processedEventIds.add(eventId)호출은synchronized래퍼를 적용해도 check-then-act 패턴이 아닌add()반환값을 사용하므로 원자적입니다. 하지만 위의 동기화 수정이 적용되어야 이 패턴이 정상 동작합니다.
src/main/java/gg/agit/konect/infrastructure/claude/client/ClaudeClient.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/claude/client/ClaudeClient.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/client/SlackClient.java
Show resolved
Hide resolved
Merged
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🔍 개요
🚀 주요 변경 내용
💬 참고 사항
✅ Checklist (완료 조건)