feat: add Slack chat bot integration for Record Label Agent#296
Merged
sweetmantech merged 9 commits intotestfrom Mar 13, 2026
Merged
feat: add Slack chat bot integration for Record Label Agent#296sweetmantech merged 9 commits intotestfrom
sweetmantech merged 9 commits intotestfrom
Conversation
Adds a second Slack bot that runs the same AI chat generation logic as /api/chat/generate and posts responses back to Slack threads. The bot acts as a hardcoded account with conversational memory via roomId persisted in Redis thread state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds a Slack integration: new webhook route (GET/POST) with url_verification handling, a Redis-backed SlackChatBot factory and env validation, thread-state typing, message handlers that drive AI responses, and registration wiring to attach handlers to the bot. Changes
Sequence DiagramsequenceDiagram
participant Slack as Slack Service
participant Route as Webhook Route
participant Bot as SlackChatBot
participant Redis as Redis State
participant Handler as Message Handler
participant Chat as Chat/AI Agent
participant Thread as Slack Thread
Slack->>Route: POST webhook event
Route->>Route: Parse body (safe parse)
alt url_verification
Route->>Slack: Return { "challenge": ... } (200)
else message event
Route->>Bot: initialize()
Bot->>Redis: connect / load state
Route->>Bot: delegate -> webhooks.slack (after/waitUntil)
Bot->>Handler: onNewMention / onSubscribedMessage callback
Handler->>Redis: read thread.state
alt state == generating
Handler->>Thread: Post "Please wait..." reply
else
Handler->>Thread: Post "Thinking..."
Handler->>Chat: Build context & send ChatRequest
Chat->>Handler: Return assistant response
Handler->>Thread: Post assistant response
Handler->>Redis: update thread state (idle, prompt, roomId)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
lib/slack-chat/bot.ts (1)
23-27: Redis connection error handling follows existing pattern but has a subtle race condition.The
.catch()handler throws asynchronously, meaning subsequent code (likecreateIoRedisState) executes before the connection attempt completes. If Redis is unavailable, operations will fail with connection errors rather than the explicit error message. This matches thecoding-agentbot pattern (per context snippet 1), so it's consistent with the codebase architecture—but worth noting as a known limitation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack-chat/bot.ts` around lines 23 - 27, The current pattern calls redis.connect().catch(() => { throw new Error(...) }) which throws inside a promise handler and allows subsequent initialization (e.g., createIoRedisState) to run before connect completes; change this to perform a synchronous await or proper promise handling so the error is thrown before continuing: make the surrounding initializer function async (or return the promise) and await redis.connect(), wrapping it in try/catch to throw the explicit "[slack-chat] Redis failed to connect" error on failure; reference the redis.connect() call and any subsequent createIoRedisState usage to ensure connection is awaited before proceeding.lib/slack-chat/handlers/handleSlackChatMessage.ts (2)
32-36: Race condition potential in concurrency guard.The check-then-act pattern (read state → check status → set generating) isn't atomic. Under high concurrency, two messages could both read
status: "idle"before either sets"generating". This is likely low-risk for Slack's typical message rates, but worth noting.Redis transactions or atomic compare-and-set would make this bulletproof, though may be overkill for the current use case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack-chat/handlers/handleSlackChatMessage.ts` around lines 32 - 36, The current concurrency guard around currentState.status ("generating") is vulnerable to a race because it reads-then-acts; change this to an atomic compare-and-set using your backing store (e.g., Redis) so only one handler can transition the thread state to "generating". Specifically, replace the check-then-post block that reads currentState and calls thread.post with an atomic operation (Redis SETNX or a WATCH/MULTI or a small Lua script) that attempts to set the thread state key to "generating" only if it is not already set to "generating"; if the atomic set fails, call thread.post("I'm still working on a response...") and return, otherwise proceed and ensure you clear or update the state when generation completes or on error (refer to currentState and the code paths that set/clear the status).
10-10: Hardcoded account ID noted—consider making configurable.The PR summary mentions this is intentional for the MVP. For future maintainability, consider extracting this to an environment variable or configuration constant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack-chat/handlers/handleSlackChatMessage.ts` at line 10, The ACCOUNT_ID constant is hardcoded in handleSlackChatMessage.ts; make it configurable by reading from an environment variable or config module (e.g., process.env.SLACK_ACCOUNT_ID) and fall back to the existing literal for MVP. Update the declaration of ACCOUNT_ID to load from the env/config (preserve the current UUID as the default fallback) and ensure any tests or callers that import ACCOUNT_ID still work by adjusting their setup or using a test-specific env var. Also add a short comment noting why the default exists for the MVP and a TODO to remove the default once configuration is finalized.app/api/chat/slack/route.ts (1)
27-41: POST handler correctly handles Slack protocol requirements.The
request.clone()pattern preserves the body for signature verification in the downstream adapter—this is critical since the body can only be consumed once. The earlyurl_verificationcheck avoids unnecessary Redis/adapter initialization during Slack app setup.However, per coding guidelines, tests should cover success and error paths for new API endpoints.
Consider adding tests covering:
url_verificationchallenge response- Normal message webhook flow
- Error handling paths
Based on learnings: "Write tests for new API endpoints covering all success and error paths"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/chat/slack/route.ts` around lines 27 - 41, Add unit/integration tests for the POST handler to cover the url_verification challenge, normal webhook flow, and error paths: 1) test that when request body has type "url_verification" and a string challenge the handler returns JSON {challenge} and does not call slackChatBot.initialize (mock/stub slackChatBot.initialize and slackChatBot.webhooks.slack); 2) test the normal flow by providing a non-challenge body (use request.clone/json) and assert slackChatBot.initialize is called and slackChatBot.webhooks.slack is invoked with the original Request and the after-wrapper option (mock slackChatBot.webhooks.slack to resolve); 3) test error handling: simulate initialize throwing and simulate webhooks.slack throwing/rejecting and assert the handler surfaces/handles errors appropriately. Use request factory or NextRequest mock to control body consumption and ensure the clone pattern preserves body for the downstream mock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/slack-chat/handlers/handleSlackChatMessage.ts`:
- Around line 38-86: The thread state can remain "generating" if
setupConversation, setupChatRequest, or chatConfig.agent.generate throws; wrap
the main flow (from setting status "generating" through posting the response and
persisting it) in a try/catch/finally so that in finally you always call
thread.setState({ status: "idle", prompt: text, roomId }) to reset the thread
regardless of errors; inside catch log or rethrow the error (preserving existing
saveChatCompletion catch behavior) so failures are visible but do not leave the
thread stuck; locate and modify the block that calls setupConversation,
setupChatRequest, chatConfig.agent.generate, saveChatCompletion, and thread.post
to implement this try/catch/finally around those calls.
---
Nitpick comments:
In `@app/api/chat/slack/route.ts`:
- Around line 27-41: Add unit/integration tests for the POST handler to cover
the url_verification challenge, normal webhook flow, and error paths: 1) test
that when request body has type "url_verification" and a string challenge the
handler returns JSON {challenge} and does not call slackChatBot.initialize
(mock/stub slackChatBot.initialize and slackChatBot.webhooks.slack); 2) test the
normal flow by providing a non-challenge body (use request.clone/json) and
assert slackChatBot.initialize is called and slackChatBot.webhooks.slack is
invoked with the original Request and the after-wrapper option (mock
slackChatBot.webhooks.slack to resolve); 3) test error handling: simulate
initialize throwing and simulate webhooks.slack throwing/rejecting and assert
the handler surfaces/handles errors appropriately. Use request factory or
NextRequest mock to control body consumption and ensure the clone pattern
preserves body for the downstream mock.
In `@lib/slack-chat/bot.ts`:
- Around line 23-27: The current pattern calls redis.connect().catch(() => {
throw new Error(...) }) which throws inside a promise handler and allows
subsequent initialization (e.g., createIoRedisState) to run before connect
completes; change this to perform a synchronous await or proper promise handling
so the error is thrown before continuing: make the surrounding initializer
function async (or return the promise) and await redis.connect(), wrapping it in
try/catch to throw the explicit "[slack-chat] Redis failed to connect" error on
failure; reference the redis.connect() call and any subsequent
createIoRedisState usage to ensure connection is awaited before proceeding.
In `@lib/slack-chat/handlers/handleSlackChatMessage.ts`:
- Around line 32-36: The current concurrency guard around currentState.status
("generating") is vulnerable to a race because it reads-then-acts; change this
to an atomic compare-and-set using your backing store (e.g., Redis) so only one
handler can transition the thread state to "generating". Specifically, replace
the check-then-post block that reads currentState and calls thread.post with an
atomic operation (Redis SETNX or a WATCH/MULTI or a small Lua script) that
attempts to set the thread state key to "generating" only if it is not already
set to "generating"; if the atomic set fails, call thread.post("I'm still
working on a response...") and return, otherwise proceed and ensure you clear or
update the state when generation completes or on error (refer to currentState
and the code paths that set/clear the status).
- Line 10: The ACCOUNT_ID constant is hardcoded in handleSlackChatMessage.ts;
make it configurable by reading from an environment variable or config module
(e.g., process.env.SLACK_ACCOUNT_ID) and fall back to the existing literal for
MVP. Update the declaration of ACCOUNT_ID to load from the env/config (preserve
the current UUID as the default fallback) and ensure any tests or callers that
import ACCOUNT_ID still work by adjusting their setup or using a test-specific
env var. Also add a short comment noting why the default exists for the MVP and
a TODO to remove the default once configuration is finalized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15bb0945-29c0-4f89-83b0-40b26af5bf2d
📒 Files selected for processing (8)
app/api/chat/slack/route.tslib/slack-chat/bot.tslib/slack-chat/handlers/handleSlackChatMessage.tslib/slack-chat/handlers/onNewMention.tslib/slack-chat/handlers/onSubscribedMessage.tslib/slack-chat/handlers/registerHandlers.tslib/slack-chat/types.tslib/slack-chat/validateEnv.ts
Let the API key determine the account identity rather than hardcoding an account ID. This follows the existing auth pattern where account_id is always inferred from credentials, never passed explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/slack-chat/handlers/handleSlackChatMessage.ts (1)
49-97:⚠️ Potential issue | 🔴 CriticalCritical: Thread state remains "generating" indefinitely if any async operation throws.
This issue was flagged in a previous review but remains unaddressed. If
setupConversation,setupChatRequest, oragent.generatethrows an exception, the thread state stays stuck at"generating". All subsequent messages to this thread will receive "I'm still working on a response" forever, effectively breaking the thread.Wrap the main flow in a
try/finallyto guarantee state reset regardless of success or failure.🐛 Proposed fix: Add try-finally to ensure state reset
await thread.setState({ status: "generating", prompt: text, roomId: currentState?.roomId, }); await thread.post("Thinking..."); + let finalRoomId = currentState?.roomId; + try { // Get or create roomId from thread state const messages = getMessages(text, "user"); const uiMessages = convertToUiMessages(messages); const { lastMessage } = validateMessages(uiMessages); const { roomId } = await setupConversation({ accountId, roomId: currentState?.roomId, topic: text.slice(0, 100), promptMessage: lastMessage, memoryId: lastMessage.id, }); + finalRoomId = roomId; // Build ChatRequestBody — accountId inferred from API key const body: ChatRequestBody = { messages: uiMessages, accountId, orgId, roomId, authToken, }; const chatConfig = await setupChatRequest(body); const result = await chatConfig.agent.generate(chatConfig); // Persist assistant response try { await saveChatCompletion({ text: result.text, roomId }); } catch (error) { console.error("[slack-chat] Failed to persist assistant message:", error); } // Post response to Slack thread await thread.post(result.text); + } catch (error) { + console.error("[slack-chat] Error generating response:", error); + await thread.post("Sorry, something went wrong. Please try again."); + } finally { // Update thread state with roomId for conversational memory await thread.setState({ status: "idle", prompt: text, - roomId, + roomId: finalRoomId, }); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack-chat/handlers/handleSlackChatMessage.ts` around lines 49 - 97, The thread state is set to "generating" then several async calls (setupConversation, setupChatRequest, chatConfig.agent.generate, saveChatCompletion, thread.post) run without guaranteed cleanup, so any thrown error leaves the thread stuck; wrap the main flow that begins after the initial thread.setState({status: "generating", ...}) in a try/finally where the finally always calls thread.setState({status: "idle", prompt: text, roomId: /* use resolved roomId if available else currentState?.roomId */}) to ensure the state is reset regardless of success or failure, and keep existing error logging/handling inside the try block.
🧹 Nitpick comments (1)
lib/slack-chat/handlers/handleSlackChatMessage.ts (1)
21-98: Consider extracting helper functions to meet the 50-line guideline.The function body spans ~70 lines, exceeding the coding guideline of keeping functions under 50 lines. While the logic is cohesive, you could extract the message preparation (lines 57-68) and the ChatRequestBody construction + generation (lines 70-80) into focused helper functions. This would improve testability and align with SRP.
As per coding guidelines: "Keep functions under 50 lines."
♻️ Example extraction for message preparation
// Could extract to a separate file or inline helper function prepareUserMessage(text: string) { const messages = getMessages(text, "user"); const uiMessages = convertToUiMessages(messages); const { lastMessage } = validateMessages(uiMessages); return { uiMessages, lastMessage }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack-chat/handlers/handleSlackChatMessage.ts` around lines 21 - 98, The handleSlackChatMessage function is over 50 lines — extract the message preparation and chat request/build+generation into two helpers to reduce size and improve testability: create a prepareUserMessage(text: string) that runs getMessages, convertToUiMessages and validateMessages and returns { uiMessages, lastMessage }, and create buildAndGenerateChat({ accountId, orgId, roomId, uiMessages, lastMessage, authToken, topic }) which constructs the ChatRequestBody, calls setupChatRequest and returns the generation result (ensure it returns both result and roomId if setupConversation is called elsewhere). Replace the inlined blocks in handleSlackChatMessage with calls to prepareUserMessage and buildAndGenerateChat and update imports/types accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/slack-chat/handlers/handleSlackChatMessage.ts`:
- Around line 49-97: The thread state is set to "generating" then several async
calls (setupConversation, setupChatRequest, chatConfig.agent.generate,
saveChatCompletion, thread.post) run without guaranteed cleanup, so any thrown
error leaves the thread stuck; wrap the main flow that begins after the initial
thread.setState({status: "generating", ...}) in a try/finally where the finally
always calls thread.setState({status: "idle", prompt: text, roomId: /* use
resolved roomId if available else currentState?.roomId */}) to ensure the state
is reset regardless of success or failure, and keep existing error
logging/handling inside the try block.
---
Nitpick comments:
In `@lib/slack-chat/handlers/handleSlackChatMessage.ts`:
- Around line 21-98: The handleSlackChatMessage function is over 50 lines —
extract the message preparation and chat request/build+generation into two
helpers to reduce size and improve testability: create a
prepareUserMessage(text: string) that runs getMessages, convertToUiMessages and
validateMessages and returns { uiMessages, lastMessage }, and create
buildAndGenerateChat({ accountId, orgId, roomId, uiMessages, lastMessage,
authToken, topic }) which constructs the ChatRequestBody, calls setupChatRequest
and returns the generation result (ensure it returns both result and roomId if
setupConversation is called elsewhere). Replace the inlined blocks in
handleSlackChatMessage with calls to prepareUserMessage and buildAndGenerateChat
and update imports/types accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20fad3c5-e264-44b3-aebe-38d9592e5808
📒 Files selected for processing (1)
lib/slack-chat/handlers/handleSlackChatMessage.ts
The throw inside .catch() creates an unhandled rejection that crashes the Node process. Log the error instead since the shared Redis client may already be connected by the coding-agent bot. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep the throw inside .catch() to match lib/coding-agent/bot.ts. The Redis timeout is a pre-existing pattern, not specific to slack-chat. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Each Slack message was only sending the current message to the AI, with no prior context. Now loads all memories from the room before generating, giving the bot conversational memory within a thread. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cast memory content as UIMessage to satisfy strict parts type checking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/slack-chat/handlers/handleSlackChatMessage.ts (1)
50-122:⚠️ Potential issue | 🔴 CriticalThread can remain permanently stuck in
generatingon failures.Line 50 sets
status: "generating", but if any awaited call from Line 64 through Line 115 throws, Line 118 is skipped and the thread never returns to idle.🐛 Suggested fix (try/finally state reset)
- await thread.setState({ - status: "generating", - prompt: text, - roomId: currentState?.roomId, - }); - - await thread.post("Thinking..."); + let roomId = currentState?.roomId; + await thread.setState({ + status: "generating", + prompt: text, + roomId, + }); + await thread.post("Thinking..."); + + try { @@ - const { roomId } = await setupConversation({ + const conversation = await setupConversation({ accountId, - roomId: currentState?.roomId, + roomId, topic: text.slice(0, 100), promptMessage: lastMessage, memoryId: lastMessage.id, }); + roomId = conversation.roomId; @@ - await thread.setState({ - status: "idle", - prompt: text, - roomId, - }); + } finally { + await thread.setState({ + status: "idle", + prompt: text, + roomId, + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack-chat/handlers/handleSlackChatMessage.ts` around lines 50 - 122, The thread status is set to "generating" but any exception between setupConversation and the final thread.setState will leave it stuck; wrap the main work (from after setting status through posting the assistant response and saving it) in a try/finally so thread.setState({ status: "idle", prompt: text, roomId }) always runs in the finally block. Specifically, keep the initial thread.setState({ status: "generating", ... }) and thread.post("Thinking..."), then run setupConversation, selectMemories, setupChatRequest, chatConfig.agent.generate, saveChatCompletion and thread.post(result.text) inside try, and move the terminal thread.setState call into finally (using the roomId captured from setupConversation when available) to guarantee reset on errors.
🧹 Nitpick comments (2)
lib/slack-chat/bot.ts (1)
31-35: Use a bot-specific Redis key namespace.Line 33 uses
keyPrefix: "chat", which is very broad. A scoped prefix (e.g.,slack-chat) reduces cross-feature state collision risk.♻️ Suggested change
- keyPrefix: "chat", + keyPrefix: "slack-chat",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack-chat/bot.ts` around lines 31 - 35, Replace the broad Redis key prefix used in the createIoRedisState call to a bot-specific namespace: update the keyPrefix parameter in the createIoRedisState({ client: redis, keyPrefix: "chat", logger }) invocation (the createIoRedisState call) to a scoped value like "slack-chat" (or "slack_chat"); ensure any other code that expects the previous "chat" prefix (e.g., lookups or tests referencing that prefix) is updated to the new prefix so state keys remain consistent and avoid cross-feature collisions.lib/slack-chat/handlers/handleSlackChatMessage.ts (1)
22-123: Split this handler into smaller units (currently too many responsibilities).This function does concurrency gating, auth/account resolution, memory loading/transformation, model invocation, persistence, and Slack posting in one flow. It’s hard to reason about and exceeds the size guideline.
As per coding guidelines,
lib/**/*.ts: “Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well” and “Keep functions under 50 lines”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack-chat/handlers/handleSlackChatMessage.ts` around lines 22 - 123, The handler handleSlackChatMessage is doing too much; split it into small single-responsibility helpers and make handleSlackChatMessage an orchestrator under 50 lines that wires them together. Create functions like ensureNoConcurrentGeneration(thread), resolveAccount(authToken) (wraps getApiKeyDetails), persistUserMessageAndEnsureRoom({accountId, roomId, topic, promptMessage, memoryId}) (wraps setupConversation), loadAndTransformHistory(roomId) (uses selectMemories, filters and maps to historyMessages, then convertToUiMessages and validateMessages), buildAndRunChat(body) (calls setupChatRequest and chatConfig.agent.generate), persistAssistantResponse(text, roomId) (wraps saveChatCompletion), and postAndUpdateState(thread, text, roomId). Replace the large inline blocks in handleSlackChatMessage with calls to these helpers and keep handleSlackChatMessage focused on orchestration and error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/slack-chat/bot.ts`:
- Around line 25-29: The bot factory functions (createSlackChatBot and the
analogous createCodingAgentBot) call redis.connect() without awaiting it, then
immediately call createIoRedisState with the client; make these factory
functions async, await redis.connect() when redis.status === "wait", and
propagate any connect error (e.g., throw or rethrow the caught error) before
calling createIoRedisState so the state adapter is only created after a
successful connection.
---
Duplicate comments:
In `@lib/slack-chat/handlers/handleSlackChatMessage.ts`:
- Around line 50-122: The thread status is set to "generating" but any exception
between setupConversation and the final thread.setState will leave it stuck;
wrap the main work (from after setting status through posting the assistant
response and saving it) in a try/finally so thread.setState({ status: "idle",
prompt: text, roomId }) always runs in the finally block. Specifically, keep the
initial thread.setState({ status: "generating", ... }) and
thread.post("Thinking..."), then run setupConversation, selectMemories,
setupChatRequest, chatConfig.agent.generate, saveChatCompletion and
thread.post(result.text) inside try, and move the terminal thread.setState call
into finally (using the roomId captured from setupConversation when available)
to guarantee reset on errors.
---
Nitpick comments:
In `@lib/slack-chat/bot.ts`:
- Around line 31-35: Replace the broad Redis key prefix used in the
createIoRedisState call to a bot-specific namespace: update the keyPrefix
parameter in the createIoRedisState({ client: redis, keyPrefix: "chat", logger
}) invocation (the createIoRedisState call) to a scoped value like "slack-chat"
(or "slack_chat"); ensure any other code that expects the previous "chat" prefix
(e.g., lookups or tests referencing that prefix) is updated to the new prefix so
state keys remain consistent and avoid cross-feature collisions.
In `@lib/slack-chat/handlers/handleSlackChatMessage.ts`:
- Around line 22-123: The handler handleSlackChatMessage is doing too much;
split it into small single-responsibility helpers and make
handleSlackChatMessage an orchestrator under 50 lines that wires them together.
Create functions like ensureNoConcurrentGeneration(thread),
resolveAccount(authToken) (wraps getApiKeyDetails),
persistUserMessageAndEnsureRoom({accountId, roomId, topic, promptMessage,
memoryId}) (wraps setupConversation), loadAndTransformHistory(roomId) (uses
selectMemories, filters and maps to historyMessages, then convertToUiMessages
and validateMessages), buildAndRunChat(body) (calls setupChatRequest and
chatConfig.agent.generate), persistAssistantResponse(text, roomId) (wraps
saveChatCompletion), and postAndUpdateState(thread, text, roomId). Replace the
large inline blocks in handleSlackChatMessage with calls to these helpers and
keep handleSlackChatMessage focused on orchestration and error handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cee60133-4f66-499c-83ac-76b1df4cf60f
📒 Files selected for processing (2)
lib/slack-chat/bot.tslib/slack-chat/handlers/handleSlackChatMessage.ts
Comment on lines
+25
to
+29
| if (redis.status === "wait") { | ||
| redis.connect().catch(() => { | ||
| throw new Error("[slack-chat] Redis failed to connect"); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Redis init pattern and whether connect is awaited before state creation.
rg -nP --type=ts -C3 'redis\.status|redis\.connect\(|createIoRedisState\(' lib/slack-chat/bot.ts lib/coding-agent/bot.tsRepository: recoupable/api
Length of output: 1507
🏁 Script executed:
#!/bin/bash
# Get the full createSlackChatBot function and see if it's async and how errors are handled
head -50 lib/slack-chat/bot.ts | cat -nRepository: recoupable/api
Length of output: 1776
🏁 Script executed:
#!/bin/bash
# Check if there's a createIoRedisState function and how it uses the redis client
rg -nP --type=ts 'createIoRedisState|export.*createSlackChatBot' lib/slack-chat/ -A 20 | head -100Repository: recoupable/api
Length of output: 2844
🏁 Script executed:
#!/bin/bash
# Look for imports to understand where createIoRedisState comes from
rg -nP --type=ts 'import.*createIoRedisState|from.*state' lib/slack-chat/bot.ts lib/coding-agent/bot.tsRepository: recoupable/api
Length of output: 239
Await Redis connection before creating state adapter.
createSlackChatBot() is a synchronous function that calls redis.connect() without awaiting it. The error thrown in the .catch() handler (line 27) won't propagate to the caller; the function returns immediately with a state adapter backed by a potentially-unconnected Redis client. Make the function async and await the connection to guarantee Redis is ready before initialization completes:
Current problematic pattern
if (redis.status === "wait") {
redis.connect().catch(() => {
throw new Error("[slack-chat] Redis failed to connect");
});
}
const state = createIoRedisState({
client: redis,
keyPrefix: "chat",
logger,
});
Same issue exists in lib/coding-agent/bot.ts at lines 26–30.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/slack-chat/bot.ts` around lines 25 - 29, The bot factory functions
(createSlackChatBot and the analogous createCodingAgentBot) call redis.connect()
without awaiting it, then immediately call createIoRedisState with the client;
make these factory functions async, await redis.connect() when redis.status ===
"wait", and propagate any connect error (e.g., throw or rethrow the caught
error) before calling createIoRedisState so the state adapter is only created
after a successful connection.
Both the coding-agent and slack-chat routes had identical url_verification challenge logic. Extract to lib/slack/handleUrlVerification.ts so both routes share a single implementation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consolidate all Slack-related libs under lib/slack/ for simpler organization. Shared utils like handleUrlVerification live at lib/slack/, bot-specific code lives at lib/slack/chat/. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wrap generation logic in try/catch/finally so thread state always resets to "idle", even if an exception occurs mid-flow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cebcc866-34c3-451c-8cd7-f63309acff0awith conversational memory viaroomIdpersisted in Redis thread stateNew Files
lib/slack-chat/bot.ts— Chat bot singleton withSlackAdapter+ Redis state (key prefix:"chat")lib/slack-chat/types.ts—SlackChatThreadStatetypelib/slack-chat/validateEnv.ts— Env validation forSLACK_CHAT_BOT_TOKEN,SLACK_CHAT_SIGNING_SECRET,SLACK_CHAT_API_KEY,REDIS_URLlib/slack-chat/handlers/handleSlackChatMessage.ts— Core logic:setupConversation()→setupChatRequest()→agent.generate()→saveChatCompletion()→ post to threadlib/slack-chat/handlers/onNewMention.ts— Subscribes to thread + generates responselib/slack-chat/handlers/onSubscribedMessage.ts— Follow-up messages reuseroomIdfor memorylib/slack-chat/handlers/registerHandlers.ts— Handler registryapp/api/chat/slack/route.ts— Webhook endpoint (GET verification + POST message handling)Env Vars Required
Test plan
.env.localapp_mentions:read,chat:write,channels:history)https://<host>/api/chat/slackpnpm dev→ mention the bot in Slack → verify it responds with AI-generated text🤖 Generated with Claude Code
Summary by CodeRabbit