feat(slack): split responses into summary + side effects messages#1101
Merged
saddlepaddle merged 1 commit intoFeb 1, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughReplaces Slack thread status updates with posting an initial "Thinking..." message whose timestamp is stored and edited as progress arrives via an onProgress callback; final text updates that message or posts a threaded message; side effects are posted separately using a new formatter; run-agent emits progress and a task_deleted action. Changes
Sequence Diagram(s)sequenceDiagram
participant SlackHandler as Slack Handler
participant Agent as Agent Executor
participant SlackAPI as Slack API
participant ProgressCB as Progress Callback
SlackHandler->>SlackAPI: Post initial message ("Thinking...")
SlackAPI-->>SlackHandler: Return message_ts
SlackHandler->>Agent: runSlackAgent(..., onProgress=ProgressCB)
Agent->>Agent: execute tools / pause_turn
Agent->>ProgressCB: onProgress(status)
ProgressCB->>SlackAPI: Update initial message text (best-effort)
Note over SlackAPI: Failures ignored (non-critical)
Agent-->>SlackHandler: Return result { text, actions }
alt message_ts exists
SlackHandler->>SlackAPI: Update initial message with result.text
else
SlackHandler->>SlackAPI: Post threaded message with result.text
end
alt actions exist
SlackHandler->>SlackAPI: Post separate message with formatSideEffectsMessage(actions)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 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)
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
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts`:
- Around line 46-53: Wrap the slack.chat.postMessage() call in a try/catch
inside process-assistant-message so failures are logged (use a prefixed message
like "[slack/postMessage] ...") and do not throw; on error set messageTs to
undefined or leave it falsy so the existing logic that handles undefined
messageTs (the later update path) runs instead of crashing; reference the
initialMsg variable and messageTs assignment to locate where to add the
try/catch and logging.
- Around line 61-71: The onProgress handler currently swallows errors in the
catch block when calling slack.chat.update; replace the silent catch with a
logged error that includes context (event.channel, messageTs, and the status
string) and the caught error object to aid observability. Update the catch in
the onProgress async function (the block around slack.chat.update) to call your
project's logger (use the existing logger instance, e.g., processLogger or
logger) and log a descriptive message plus the error, ensuring progress update
failures remain non-fatal but are recorded.
In
`@apps/api/src/app/api/integrations/slack/events/process-mention/process-mention.ts`:
- Around line 71-81: The onProgress async handler currently swallows errors from
slack.chat.update; replace the empty catch with a logged error using the
repository's logger (e.g., logger or processLogger) and the
[slack/process-mention] prefix so failures are visible; specifically, in the
onProgress block (the function passed to onProgress that calls slack.chat.update
with channel: event.channel, ts: messageTs, text: status) catch the error and
call logger.error with a message like "[slack/process-mention] Failed to update
progress for message {messageTs} in channel {event.channel} (status={status})"
and include the caught error object, then continue without rethrowing.
- Around line 56-63: The initial slack.chat.postMessage call (creating
initialMsg and messageTs) is executed before the surrounding try/finally that
removes the reaction, so a failure will skip the finally cleanup; wrap the
postMessage call in its own try/catch that logs the error using the existing
logger (or processLogger) and leaves messageTs undefined (or null) so the rest
of processMention can continue and the later try/finally will still execute to
remove the reaction; update references to initialMsg/messageTs (used later in
send/update logic and in the final cleanup) to handle the undefined case as
currently expected.
In `@apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts`:
- Around line 341-351: The try/catch around invoking params.onProgress currently
swallows failures; update the catch to log the error instead of ignoring it:
when calling params.onProgress(progressStatus) (in the run-agent function that
uses parseToolName, TOOL_PROGRESS_STATUS and computes progressStatus), catch any
thrown error and call the logger (or console) with a prefixed message like
"[slack/run-agent onProgress]" including progressStatus,
toolUse.name/rawToolName and the caught error details so failures are recorded
for diagnostics.
Comment on lines
+61
to
+71
| onProgress: messageTs | ||
| ? async (status) => { | ||
| try { | ||
| await slack.chat.update({ | ||
| channel: event.channel, | ||
| ts: messageTs, | ||
| text: status, | ||
| }); | ||
| } catch { | ||
| // Non-critical: progress updates are best-effort | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the specific file
find . -path "*/apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts" -type fRepository: superset-sh/superset
Length of output: 167
🏁 Script executed:
# If found, read the file to verify the code at lines 61-71
if [ -f "apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts" ]; then
wc -l "apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts"
echo "---"
sed -n '50,80p' "apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts"
fiRepository: superset-sh/superset
Length of output: 881
🏁 Script executed:
# Search for the file using fd
fd "process-assistant-message.ts" -type fRepository: superset-sh/superset
Length of output: 234
🏁 Script executed:
# Read the full file to understand logging patterns and other error handling
cat -n "apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts"Repository: superset-sh/superset
Length of output: 3803
🏁 Script executed:
# Check for other console/logging patterns in the same directory or file
grep -r "console\." "apps/api/src/app/api/integrations/slack/" --include="*.ts" --include="*.tsx" | head -20Repository: superset-sh/superset
Length of output: 2766
🏁 Script executed:
# Search for error handling patterns in the file
grep -n "catch" "apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts"Repository: superset-sh/superset
Length of output: 102
Log progress update failures for observability.
The silent catch block at line 69 violates the coding guideline requiring all errors to be logged with context. While progress updates are best-effort, failures should still be logged at minimum for Slack API observability.
💡 Suggested fix
- } catch {
- // Non-critical: progress updates are best-effort
- }
+ } catch (error) {
+ console.warn(
+ "[slack/process-assistant-message] Failed to update progress:",
+ error,
+ );
+ }📝 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
| onProgress: messageTs | |
| ? async (status) => { | |
| try { | |
| await slack.chat.update({ | |
| channel: event.channel, | |
| ts: messageTs, | |
| text: status, | |
| }); | |
| } catch { | |
| // Non-critical: progress updates are best-effort | |
| } | |
| onProgress: messageTs | |
| ? async (status) => { | |
| try { | |
| await slack.chat.update({ | |
| channel: event.channel, | |
| ts: messageTs, | |
| text: status, | |
| }); | |
| } catch (error) { | |
| console.warn( | |
| "[slack/process-assistant-message] Failed to update progress:", | |
| error, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
`@apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts`
around lines 61 - 71, The onProgress handler currently swallows errors in the
catch block when calling slack.chat.update; replace the silent catch with a
logged error that includes context (event.channel, messageTs, and the status
string) and the caught error object to aid observability. Update the catch in
the onProgress async function (the block around slack.chat.update) to call your
project's logger (use the existing logger instance, e.g., processLogger or
logger) and log a descriptive message plus the error, ensuring progress update
failures remain non-fatal but are recorded.
Comment on lines
+71
to
+81
| onProgress: messageTs | ||
| ? async (status) => { | ||
| try { | ||
| await slack.chat.update({ | ||
| channel: event.channel, | ||
| ts: messageTs, | ||
| text: status, | ||
| }); | ||
| } catch { | ||
| // Non-critical: progress updates are best-effort | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "process-mention.ts" --type fRepository: superset-sh/superset
Length of output: 145
🏁 Script executed:
cat -n apps/api/src/app/api/integrations/slack/events/process-mention/process-mention.ts | sed -n '65,85p'Repository: superset-sh/superset
Length of output: 695
Log progress update failures instead of silently swallowing them.
The silent catch block violates the coding guidelines requiring that errors are always logged with context. Add logging using the [domain/operation] prefix pattern.
💡 Suggested fix
- } catch {
- // Non-critical: progress updates are best-effort
- }
+ } catch (error) {
+ console.warn(
+ "[slack/process-mention] Failed to update progress:",
+ error,
+ );
+ }🤖 Prompt for AI Agents
In
`@apps/api/src/app/api/integrations/slack/events/process-mention/process-mention.ts`
around lines 71 - 81, The onProgress async handler currently swallows errors
from slack.chat.update; replace the empty catch with a logged error using the
repository's logger (e.g., logger or processLogger) and the
[slack/process-mention] prefix so failures are visible; specifically, in the
onProgress block (the function passed to onProgress that calls slack.chat.update
with channel: event.channel, ts: messageTs, text: status) catch the error and
call logger.error with a message like "[slack/process-mention] Failed to update
progress for message {messageTs} in channel {event.channel} (status={status})"
and include the caught error object, then continue without rethrowing.
Comment on lines
+341
to
+351
| const { toolName: rawToolName } = parseToolName(toolUse.name); | ||
| const progressStatus = | ||
| TOOL_PROGRESS_STATUS[toolUse.name] ?? | ||
| TOOL_PROGRESS_STATUS[rawToolName] ?? | ||
| "Working..."; | ||
|
|
||
| try { | ||
| await params.onProgress?.(progressStatus); | ||
| } catch { | ||
| // Non-critical: don't fail the agent if progress update fails | ||
| } |
Contributor
There was a problem hiding this comment.
Log onProgress failures instead of swallowing them silently.
This currently hides callback failures, making Slack update issues hard to diagnose.
💡 Suggested fix
- try {
- await params.onProgress?.(progressStatus);
- } catch {
- // Non-critical: don't fail the agent if progress update fails
- }
+ try {
+ await params.onProgress?.(progressStatus);
+ } catch (error) {
+ console.warn(
+ "[slack-agent/on-progress] Failed to emit progress:",
+ error,
+ );
+ }As per coding guidelines, "Never swallow errors silently; at minimum log them with context" and "Use prefixed console logging with pattern [domain/operation] message".
📝 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
| const { toolName: rawToolName } = parseToolName(toolUse.name); | |
| const progressStatus = | |
| TOOL_PROGRESS_STATUS[toolUse.name] ?? | |
| TOOL_PROGRESS_STATUS[rawToolName] ?? | |
| "Working..."; | |
| try { | |
| await params.onProgress?.(progressStatus); | |
| } catch { | |
| // Non-critical: don't fail the agent if progress update fails | |
| } | |
| const { toolName: rawToolName } = parseToolName(toolUse.name); | |
| const progressStatus = | |
| TOOL_PROGRESS_STATUS[toolUse.name] ?? | |
| TOOL_PROGRESS_STATUS[rawToolName] ?? | |
| "Working..."; | |
| try { | |
| await params.onProgress?.(progressStatus); | |
| } catch (error) { | |
| console.warn( | |
| "[slack-agent/on-progress] Failed to emit progress:", | |
| error, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts`
around lines 341 - 351, The try/catch around invoking params.onProgress
currently swallows failures; update the catch to log the error instead of
ignoring it: when calling params.onProgress(progressStatus) (in the run-agent
function that uses parseToolName, TOOL_PROGRESS_STATUS and computes
progressStatus), catch any thrown error and call the logger (or console) with a
prefixed message like "[slack/run-agent onProgress]" including progressStatus,
toolUse.name/rawToolName and the caught error details so failures are recorded
for diagnostics.
saddlepaddle
force-pushed
the
split-slack-responses-into-summary-side-effects-me
branch
from
7cdfd7f to
fb1420b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts (1)
468-472:⚠️ Potential issue | 🟡 MinorLog cleanup failures instead of silently swallowing.
The MCP client cleanup error is silently ignored, which could mask resource leak issues or connection problems.
💡 Suggested fix
} finally { if (cleanupSuperset) { try { await cleanupSuperset(); - } catch {} + } catch (error) { + console.warn("[slack-agent/cleanup] Failed to cleanup MCP client:", error); + } } }As per coding guidelines, "Never silently swallow errors with
catch(() => {})orcatch(e) { return null }".apps/api/src/app/api/integrations/slack/events/process-mention/process-mention.ts (1)
127-134:⚠️ Potential issue | 🟡 MinorLog reaction removal failures instead of silently swallowing.
The silent catch at line 133 hides failures that could indicate permission or API issues.
💡 Suggested fix
} finally { try { await slack.reactions.remove({ channel: event.channel, timestamp: event.ts, name: "eyes", }); - } catch {} + } catch (error) { + console.warn("[slack/process-mention] Failed to remove reaction:", error); + } }As per coding guidelines, "Never silently swallow errors with
catch(() => {})orcatch(e) { return null }".
🤖 Fix all issues with AI agents
In
`@apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts`:
- Around line 99-115: The current catch block logs the original error but calls
slack.chat.update/postMessage directly, which can throw and mask the original
error; wrap the calls to slack.chat.update (when messageTs) and
slack.chat.postMessage (otherwise) in their own try-catch inside the existing
catch so any Slack-send failure is caught and logged (e.g., console.error with
both the original error id and the secondary Slack error) without overwriting or
rethrowing the original error; reference the catch block handling messageTs,
slack.chat.update, slack.chat.postMessage, event.channel, and threadTs to locate
and modify the code.
In
`@apps/api/src/app/api/integrations/slack/events/process-mention/process-mention.ts`:
- Around line 109-125: The current catch in process-mention.ts logs the original
error and then calls slack.chat.update or slack.chat.postMessage directly, which
can throw and mask the original error; wrap the notification calls (the branches
that call slack.chat.update and slack.chat.postMessage that use messageTs and
threadTs) in their own try-catch so any failure to send the error notification
is caught and logged (e.g., console.error with a clear tag like
"[slack/process-mention] Error sending error notification") without overriding
or swallowing the original err; after attempting the notification, rethrow or
propagate the original err as before so the original error context is preserved.
In `@apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts`:
- Around line 345-349: The pause_turn path currently swallows errors from
params.onProgress (the try { await params.onProgress?.("Searching the web...");
} catch { /* Non-critical */ }) — change the catch to log the error instead of
dropping it: catch (err) { console.error("[slack/run-agent pause_turn]
onProgress failed:", err); } so failures in params.onProgress are observable and
include the error object and a clear prefix for context.
🧹 Nitpick comments (1)
apps/api/src/app/api/integrations/slack/events/utils/slack-blocks/slack-blocks.ts (1)
62-94: Consider extracting shared formatting logic to reduce duplication.
formatSideEffectsMessageduplicates almost all logic fromformatActionsAsText, differing only in the bullet prefix and header. A shared helper could format lines, then callers decide on prefix/header.♻️ Suggested refactor
+function formatActionLines( + actions: AgentAction[], + options: { bulletPrefix?: string } = {}, +): string[] { + const prefix = options.bulletPrefix ?? ""; + const lines: string[] = []; + + for (const action of actions) { + if (action.type === "task_created") { + for (const task of action.tasks) { + const url = `${env.NEXT_PUBLIC_WEB_URL}/tasks/${task.slug}`; + lines.push(`${prefix}Created task <${url}|${task.slug}>`); + } + } else if (action.type === "task_updated") { + for (const task of action.tasks) { + const url = `${env.NEXT_PUBLIC_WEB_URL}/tasks/${task.slug}`; + lines.push(`${prefix}Updated task <${url}|${task.slug}>`); + } + } else if (action.type === "task_deleted") { + for (const task of action.tasks) { + lines.push(`${prefix}Deleted task ${task.slug}`); + } + } else if (action.type === "workspace_created") { + for (const ws of action.workspaces) { + lines.push( + `${prefix}Created workspace *${ws.name}*${ws.branch ? ` on branch \`${ws.branch}\`` : ""}`, + ); + } + } else if (action.type === "workspace_switched") { + for (const ws of action.workspaces) { + lines.push(`${prefix}Switched to workspace *${ws.name}*`); + } + } + } + return lines; +} + export function formatActionsAsText(actions: AgentAction[]): string { - const lines: string[] = []; - // ... existing logic ... - return lines.join("\n"); + return formatActionLines(actions).join("\n"); } export function formatSideEffectsMessage(actions: AgentAction[]): string { - const lines: string[] = []; - // ... existing logic ... - return `*Changes:*\n${lines.join("\n")}`; + const lines = formatActionLines(actions, { bulletPrefix: "• " }); + return `*Changes:*\n${lines.join("\n")}`; }
Comment on lines
99
to
+115
| } catch (err) { | ||
| console.error("[slack/process-assistant-message] Agent error:", err); | ||
|
|
||
| await slack.chat.postMessage({ | ||
| channel: event.channel, | ||
| thread_ts: threadTs, | ||
| text: `Sorry, something went wrong: ${err instanceof Error ? err.message : "Unknown error"}`, | ||
| }); | ||
| } finally { | ||
| try { | ||
| await slack.assistant.threads.setStatus({ | ||
| channel_id: event.channel, | ||
| const errorText = `Sorry, something went wrong: ${err instanceof Error ? err.message : "Unknown error"}`; | ||
| if (messageTs) { | ||
| await slack.chat.update({ | ||
| channel: event.channel, | ||
| ts: messageTs, | ||
| text: errorText, | ||
| }); | ||
| } else { | ||
| await slack.chat.postMessage({ | ||
| channel: event.channel, | ||
| thread_ts: threadTs, | ||
| status: "", | ||
| text: errorText, | ||
| }); | ||
| } catch {} | ||
| } |
Contributor
There was a problem hiding this comment.
Wrap error-handler Slack calls in try-catch to prevent masking original error.
If chat.update or chat.postMessage throws in the error handler, the original error context is lost. Consider wrapping these calls.
💡 Suggested fix
} catch (err) {
console.error("[slack/process-assistant-message] Agent error:", err);
const errorText = `Sorry, something went wrong: ${err instanceof Error ? err.message : "Unknown error"}`;
- if (messageTs) {
- await slack.chat.update({
- channel: event.channel,
- ts: messageTs,
- text: errorText,
- });
- } else {
- await slack.chat.postMessage({
- channel: event.channel,
- thread_ts: threadTs,
- text: errorText,
- });
+ try {
+ if (messageTs) {
+ await slack.chat.update({
+ channel: event.channel,
+ ts: messageTs,
+ text: errorText,
+ });
+ } else {
+ await slack.chat.postMessage({
+ channel: event.channel,
+ thread_ts: threadTs,
+ text: errorText,
+ });
+ }
+ } catch (postError) {
+ console.error(
+ "[slack/process-assistant-message] Failed to post error message:",
+ postError,
+ );
}
}📝 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
| } catch (err) { | |
| console.error("[slack/process-assistant-message] Agent error:", err); | |
| await slack.chat.postMessage({ | |
| channel: event.channel, | |
| thread_ts: threadTs, | |
| text: `Sorry, something went wrong: ${err instanceof Error ? err.message : "Unknown error"}`, | |
| }); | |
| } finally { | |
| try { | |
| await slack.assistant.threads.setStatus({ | |
| channel_id: event.channel, | |
| const errorText = `Sorry, something went wrong: ${err instanceof Error ? err.message : "Unknown error"}`; | |
| if (messageTs) { | |
| await slack.chat.update({ | |
| channel: event.channel, | |
| ts: messageTs, | |
| text: errorText, | |
| }); | |
| } else { | |
| await slack.chat.postMessage({ | |
| channel: event.channel, | |
| thread_ts: threadTs, | |
| status: "", | |
| text: errorText, | |
| }); | |
| } catch {} | |
| } | |
| } catch (err) { | |
| console.error("[slack/process-assistant-message] Agent error:", err); | |
| const errorText = `Sorry, something went wrong: ${err instanceof Error ? err.message : "Unknown error"}`; | |
| try { | |
| if (messageTs) { | |
| await slack.chat.update({ | |
| channel: event.channel, | |
| ts: messageTs, | |
| text: errorText, | |
| }); | |
| } else { | |
| await slack.chat.postMessage({ | |
| channel: event.channel, | |
| thread_ts: threadTs, | |
| text: errorText, | |
| }); | |
| } | |
| } catch (postError) { | |
| console.error( | |
| "[slack/process-assistant-message] Failed to post error message:", | |
| postError, | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts`
around lines 99 - 115, The current catch block logs the original error but calls
slack.chat.update/postMessage directly, which can throw and mask the original
error; wrap the calls to slack.chat.update (when messageTs) and
slack.chat.postMessage (otherwise) in their own try-catch inside the existing
catch so any Slack-send failure is caught and logged (e.g., console.error with
both the original error id and the secondary Slack error) without overwriting or
rethrowing the original error; reference the catch block handling messageTs,
slack.chat.update, slack.chat.postMessage, event.channel, and threadTs to locate
and modify the code.
Comment on lines
109
to
+125
| } catch (err) { | ||
| console.error("[slack/process-mention] Agent error:", err); | ||
|
|
||
| await slack.chat.postMessage({ | ||
| channel: event.channel, | ||
| thread_ts: threadTs, | ||
| text: `Sorry, something went wrong: ${err instanceof Error ? err.message : "Unknown error"}`, | ||
| }); | ||
| const errorText = `Sorry, something went wrong: ${err instanceof Error ? err.message : "Unknown error"}`; | ||
| if (messageTs) { | ||
| await slack.chat.update({ | ||
| channel: event.channel, | ||
| ts: messageTs, | ||
| text: errorText, | ||
| }); | ||
| } else { | ||
| await slack.chat.postMessage({ | ||
| channel: event.channel, | ||
| thread_ts: threadTs, | ||
| text: errorText, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Wrap error-handler Slack calls in try-catch to prevent masking the original error.
Same issue as in process-assistant-message.ts. If the error notification fails, the original error context is lost.
💡 Suggested fix
} catch (err) {
console.error("[slack/process-mention] Agent error:", err);
const errorText = `Sorry, something went wrong: ${err instanceof Error ? err.message : "Unknown error"}`;
- if (messageTs) {
- await slack.chat.update({
- channel: event.channel,
- ts: messageTs,
- text: errorText,
- });
- } else {
- await slack.chat.postMessage({
- channel: event.channel,
- thread_ts: threadTs,
- text: errorText,
- });
+ try {
+ if (messageTs) {
+ await slack.chat.update({
+ channel: event.channel,
+ ts: messageTs,
+ text: errorText,
+ });
+ } else {
+ await slack.chat.postMessage({
+ channel: event.channel,
+ thread_ts: threadTs,
+ text: errorText,
+ });
+ }
+ } catch (postError) {
+ console.error(
+ "[slack/process-mention] Failed to post error message:",
+ postError,
+ );
}
}🤖 Prompt for AI Agents
In
`@apps/api/src/app/api/integrations/slack/events/process-mention/process-mention.ts`
around lines 109 - 125, The current catch in process-mention.ts logs the
original error and then calls slack.chat.update or slack.chat.postMessage
directly, which can throw and mask the original error; wrap the notification
calls (the branches that call slack.chat.update and slack.chat.postMessage that
use messageTs and threadTs) in their own try-catch so any failure to send the
error notification is caught and logged (e.g., console.error with a clear tag
like "[slack/process-mention] Error sending error notification") without
overriding or swallowing the original err; after attempting the notification,
rethrow or propagate the original err as before so the original error context is
preserved.
Comment on lines
+345
to
+349
| try { | ||
| await params.onProgress?.("Searching the web..."); | ||
| } catch { | ||
| // Non-critical | ||
| } |
Contributor
There was a problem hiding this comment.
Log onProgress failures in the pause_turn path.
This silent catch mirrors the issue already flagged in the tool execution loop. Progress failures should be logged for observability.
💡 Suggested fix
try {
await params.onProgress?.("Searching the web...");
- } catch {
- // Non-critical
+ } catch (error) {
+ console.warn(
+ "[slack-agent/on-progress] Failed to emit progress:",
+ error,
+ );
}As per coding guidelines, "Never swallow errors silently; at minimum log them with context" and "Use prefixed console logging with pattern [domain/operation] message".
🤖 Prompt for AI Agents
In `@apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts`
around lines 345 - 349, The pause_turn path currently swallows errors from
params.onProgress (the try { await params.onProgress?.("Searching the web...");
} catch { /* Non-critical */ }) — change the catch to log the error instead of
dropping it: catch (err) { console.error("[slack/run-agent pause_turn]
onProgress failed:", err); } so failures in params.onProgress are observable and
include the error object and a clear prefix for context.
fb1420b to
3517fd4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts (1)
467-472:⚠️ Potential issue | 🟡 MinorLog cleanup failures instead of swallowing them silently.
The silent catch in the finally block makes cleanup failures invisible, which can hide resource leaks or configuration issues.
💡 Suggested fix
} finally { if (cleanupSuperset) { try { await cleanupSuperset(); - } catch {} + } catch (error) { + console.warn("[slack-agent/cleanup] Failed to cleanup MCP client:", error); + } } }As per coding guidelines, "Never swallow errors silently; at minimum log them with context".
apps/api/src/app/api/integrations/slack/events/process-mention/process-mention.ts (1)
141-149:⚠️ Potential issue | 🟡 MinorLog reaction removal failures instead of swallowing them silently.
The silent catch block makes cleanup failures invisible.
💡 Suggested fix
} finally { try { await slack.reactions.remove({ channel: event.channel, timestamp: event.ts, name: "eyes", }); - } catch {} + } catch (error) { + console.warn("[slack/process-mention] Failed to remove reaction:", error); + } }As per coding guidelines, "Never swallow errors silently; at minimum log them with context".
Post an initial "Thinking..." message that updates in-place via chat.update as the agent executes tools, then finalizes with Claude's text response. Side effects (task created/updated/deleted, workspace created) are posted as a separate second message with unfurlable links. Also fix web search: use last text block from response instead of first, since server-side tools produce preamble + synthesis text blocks.
saddlepaddle
force-pushed
the
split-slack-responses-into-summary-side-effects-me
branch
from
3517fd4 to
32de5b4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts`:
- Around line 69-95: In formatErrorForSlack, the catch block currently swallows
the failure and incorrectly inspects the outer `error`; change it to `catch
(err)` and log the caught error with a prefixed message (e.g.
console.error(`[slack/run-agent] formatErrorForSlack fallback failed:`, err, {
originalErrorMessage: message })) for observability, then use the caught error
when checking Anthropic.APIError/status 429 to return the rate-limit fallback,
otherwise return the static fallback string as before.
🧹 Nitpick comments (1)
apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts (1)
329-332: Extractmax_usesto a named constant.
Avoid the inline magic number for easier tuning and consistency.♻️ Suggested refactor
+const WEB_SEARCH_MAX_USES = 1; + const SLACK_GET_CHANNEL_HISTORY_TOOL: Anthropic.Tool = { name: "slack_get_channel_history", description: "Get recent messages from the current Slack channel. Use this to understand what the team has been discussing.", @@ { type: "web_search_20250305" as const, name: "web_search" as const, - max_uses: 1, + max_uses: WEB_SEARCH_MAX_USES, }, ];As per coding guidelines, "Avoid magic numbers by extracting them to named constants at module top".
Comment on lines
+69
to
+95
| export async function formatErrorForSlack(error: unknown): Promise<string> { | ||
| const message = | ||
| error instanceof Error ? error.message : "Unknown error occurred"; | ||
| try { | ||
| const anthropic = new Anthropic({ apiKey: env.ANTHROPIC_API_KEY }); | ||
| const response = await anthropic.messages.create({ | ||
| model: "claude-3-5-haiku-latest", | ||
| max_tokens: 256, | ||
| messages: [ | ||
| { | ||
| role: "user", | ||
| content: `Rewrite this API error as a brief, friendly Slack message (1-2 sentences). No technical jargon, no JSON. If it's a rate limit, tell them to try again shortly.\n\nError: ${message}`, | ||
| }, | ||
| ], | ||
| }); | ||
| const text = response.content.find( | ||
| (b): b is Anthropic.TextBlock => b.type === "text", | ||
| ); | ||
| return text?.text ?? "Sorry, something went wrong. Please try again."; | ||
| } catch { | ||
| // Haiku itself failed (possibly also rate limited) — use static fallback | ||
| if (error instanceof Anthropic.APIError && error.status === 429) { | ||
| return "I'm a bit overloaded right now — please try again in a moment."; | ||
| } | ||
| return "Sorry, something went wrong. Please try again."; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Log Haiku fallback failures for observability.
The catch currently swallows errors, making formatting failures invisible.
💡 Suggested fix
- } catch {
- // Haiku itself failed (possibly also rate limited) — use static fallback
+ } catch (err) {
+ console.warn(
+ "[slack-agent/format-error] Failed to format error message:",
+ err,
+ );
+ // Haiku itself failed (possibly also rate limited) — use static fallback
if (error instanceof Anthropic.APIError && error.status === 429) {
return "I'm a bit overloaded right now — please try again in a moment.";
}
return "Sorry, something went wrong. Please try again.";
}As per coding guidelines, "Never swallow errors silently; at minimum log them with context" and "Use prefixed console logging with pattern [domain/operation] message for all logging".
🤖 Prompt for AI Agents
In `@apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts`
around lines 69 - 95, In formatErrorForSlack, the catch block currently swallows
the failure and incorrectly inspects the outer `error`; change it to `catch
(err)` and log the caught error with a prefixed message (e.g.
console.error(`[slack/run-agent] formatErrorForSlack fallback failed:`, err, {
originalErrorMessage: message })) for observability, then use the caught error
when checking Anthropic.APIError/status 429 to return the rate-limit fallback,
otherwise return the static fallback string as before.
4 tasks
Kitenite
deleted the
split-slack-responses-into-summary-side-effects-me
branch
This was referenced Feb 12, 2026
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
chat.update) with tool progress status ("Creating task...", "Searching tasks...", etc.) as the agent works, then finalizes with Claude's text response*Changes:*header and bullet-pointed, unfurlable task linksChanges
process-mention.ts/process-assistant-message.ts— Post initial message, update in-place during agent execution, post separate side effects messagerun-agent.ts— AddedonProgresscallback,TOOL_PROGRESS_STATUSlookup map, anddelete_taskaction trackingslack-blocks.ts— AddedformatSideEffectsMessage()with header + bullet formattingTest plan
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.