feat(slack): add support for custom bot configuration #1407
Conversation
WalkthroughAdds a dual-path Slack integration: a bot selection UI (deco.chat or Custom Bot), short-lived session-backed credential storage, state-aware OAuth start/callback handling, dynamic bot identifier usage across channel flows, debug-only tool-call rendering, scope updates, and README/UI template additions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant B as Browser (Selection Page)
participant S as Server (start.ts)
participant SH as StateHelpers
participant Slack as Slack OAuth
participant CB as Callback (callback.ts)
participant Cfg as Config Store
U->>S: GET /oauth/start
alt no query flags
S->>B: Serve bot selection HTML
B->>S: GET /oauth/start?useDecoChatBot=true
S->>Slack: Redirect to OAuth (state without sessionToken)
else choose custom bot
B->>S: POST credentials (store)
S->>SH: storeCustomBotSession(clientId, clientSecret, botName, debugMode)
SH-->>S: sessionToken
S-->>B: { sessionToken }
B->>S: GET /oauth/start?useCustomBot=true&sessionToken=...
S->>SH: retrieveCustomBotSession(sessionToken)
SH-->>S: { clientId, clientSecret, botName?, debugMode? }
S->>Slack: Redirect to OAuth (state includes isCustomBot + sessionToken + customBotName)
end
Slack-->>CB: code + state
CB->>SH: decodeCustomBotState(state)
alt isCustomBot with sessionToken
CB->>SH: retrieveCustomBotSession(sessionToken)
SH-->>CB: { clientId, clientSecret, botName? }
CB->>SH: invalidateSession(sessionToken)
end
CB->>Slack: Token exchange (using final clientId/secret)
Slack-->>CB: tokens + bot_info
CB->>Cfg: Save credentials, customBotName
CB-->>U: Install result (installId, name, botInfo)
sequenceDiagram
autonumber
participant User as Slack User
participant App as App Server
participant Slack as Slack API
Note over App: ctx.customBotName may be set
User->>App: Request channel list
App->>App: botIdentifier = ctx.customBotName ? "@<name>" : DECO_CHAT_CHANNEL_ID
App-->>User: List includes botIdentifier
User->>App: Join IM channel
App->>Slack: Join using botIdentifier
Slack-->>App: Ack
App-->>User: Welcome message mentioning botIdentifier
Note over App: If config.debugMode === true
App->>Slack: Post debug tool-call message
App->>Slack: Update message with tool result when available
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Tagging OptionsShould a new tag be published when this PR is merged?
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
slack/loaders/deco-chat/channels/list.ts (1)
7-37: Keep the discriminator invariant for the pseudo “bot channel”.We still advertise
DECO_CHAT_CHANNEL_IDelsewhere as the sentinel value meaning “talk to the bot directly”. After this change thelabelnow reflects the custom bot name (great), but thevaluediverges from that sentinel when a custom name is present. Any existing logic that branches onprops.discriminator === DECO_CHAT_CHANNEL_IDwill quietly stop firing for custom bots. Please keep thevalueatDECO_CHAT_CHANNEL_ID(only the label needs to change), or update every downstream check accordingly.- const botIdentifier = ctx.customBotName ? `@${ctx.customBotName}` : DECO_CHAT_CHANNEL_ID; + const botLabel = ctx.customBotName ? `@${ctx.customBotName}` : DECO_CHAT_CHANNEL_ID; … - label: botIdentifier, - value: botIdentifier, + label: botLabel, + value: DECO_CHAT_CHANNEL_ID,
🧹 Nitpick comments (2)
slack/actions/deco-chat/channels/join.ts (1)
24-33: Normalize custom bot names before mentioning.If the stored
customBotNamealready includes a leading@, we’ll emit@@mybot, which is noisy for users and breaks auto-linking in Slack. Strip any leading@before interpolating.- const botMention = config.customBotName ? `@${config.customBotName}` : "@deco.chat"; + const rawName = config.customBotName?.replace(/^@+/, ""); + const botMention = rawName ? `@${rawName}` : "@deco.chat";slack/README.md (1)
128-159: Document the “no @” convention forbotName.The runtime now prepends
@when rendering mentions, so callers should pass the bare name (my-bot) rather than@my-bot. Please spell that out here to prevent the double‑@UX your change guards against.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
slack/README.md(4 hunks)slack/actions/deco-chat/channels/join.ts(1 hunks)slack/actions/oauth/callback.ts(4 hunks)slack/loaders/deco-chat/channels/list.ts(2 hunks)slack/loaders/oauth/start.ts(1 hunks)slack/mod.ts(1 hunks)slack/utils/constants.ts(0 hunks)
💤 Files with no reviewable changes (1)
- slack/utils/constants.ts
🧰 Additional context used
🧬 Code graph analysis (1)
slack/actions/oauth/callback.ts (2)
slack/loaders/oauth/start.ts (1)
Props(3-12)slack/mod.ts (2)
Props(44-88)AppContext(98-98)
🔇 Additional comments (2)
slack/mod.ts (1)
74-88: Custom bot metadata fields LGTMThe new
customBotNameandappInfoprops integrate cleanly with the existing state plumbing. Looks good.slack/loaders/oauth/start.ts (1)
7-11: Clarify how the newbotNameprop is consumed
botNameis now part of the loader props but never used withinstart, so the value won’t reach Slack or the callback unless another layer handles it. Please double-check the flow and confirm whether we need to weave the value into the OAuth request (e.g., encoded instate) so the callback can persist it.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
slack/actions/deco-chat/channels/join.ts (1)
28-33: Prefer user‑ID mentions and sanitize custom names to avoid '@@' and ensure notificationsUsing @name may not reliably notify the bot; Slack-recommended format is
<@BOT_USER_ID>. Also strip a leading '@' if users provide it.- const botMention = config.customBotName - ? `@${config.customBotName}` - : "@deco.chat"; + const normalizedName = config.customBotName?.replace(/^@+/, ""); + const botMention = ctx.botUserId + ? `<@${ctx.botUserId}>` + : (normalizedName ? `@${normalizedName}` : "@deco.chat"); @@ - `<${props.agentLink}|${props.agentName}> has joined the channel! To interact with me, just mention ${botMention} in your messages!`, + `<${props.agentLink}|${props.agentName}> has joined the channel! To interact with me, just mention ${botMention} in your messages!`,slack/loaders/deco-chat/channels/list.ts (1)
20-23: Normalize custom bot name to prevent '@@' and keep display consistentStrip any leading '@' before composing the identifier. Behavior unchanged otherwise.
- // Use custom bot name or fallback to default - const botIdentifier = ctx.customBotName - ? `@${ctx.customBotName}` - : DECO_CHAT_CHANNEL_ID; + // Use custom bot name (normalized) or fallback to default + const normalizedName = ctx.customBotName?.replace(/^@+/, ""); + const botIdentifier = normalizedName ? `@${normalizedName}` : DECO_CHAT_CHANNEL_ID; @@ - label: botIdentifier, - value: botIdentifier, + label: botIdentifier, + value: botIdentifier,Also applies to: 35-37
slack/actions/oauth/callback.ts (1)
86-94: botInfo in return payload — LGTM; optional: include teamId for convenienceThe enriched return is useful. Consider adding
teamIdfor callers that need to bind UI/links immediately after OAuth.return { installId, name: `${effectiveBotName || "Slack"} | ${tokenData.team.name}`, botInfo: { id: tokenData.bot_user_id, appId: tokenData.app_id, name: effectiveBotName || "deco.chat", + // teamId: tokenData.team.id, // optional }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
slack/README.md(4 hunks)slack/actions/deco-chat/channels/join.ts(1 hunks)slack/actions/oauth/callback.ts(5 hunks)slack/loaders/deco-chat/channels/list.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- slack/README.md
🧰 Additional context used
🧬 Code graph analysis (1)
slack/actions/oauth/callback.ts (2)
slack/loaders/oauth/start.ts (1)
Props(3-12)slack/mod.ts (2)
Props(44-88)AppContext(98-98)
🔇 Additional comments (2)
slack/actions/oauth/callback.ts (2)
10-15: Add optional botName in Props — LGTMInterface extension is clear and backward compatible.
61-77: Preserve prior custom name — resolved; consider normalizing and avoiding empty valuesGood fix with
effectiveBotName = botName ?? currentCtx?.customBotName. To harden:
- Normalize input by trimming and removing a leading '@'.
- Avoid persisting empty strings (use fallback when blank).
- const effectiveBotName = botName ?? currentCtx?.customBotName; + const normalizedIncoming = botName?.trim()?.replace(/^@+/, "") || undefined; + const normalizedExisting = currentCtx?.customBotName?.trim()?.replace(/^@+/, "") || undefined; + const effectiveBotName = normalizedIncoming ?? normalizedExisting; @@ - customBotName: effectiveBotName ?? "deco.chat", + // Persist a non-empty identifier + customBotName: (effectiveBotName && effectiveBotName.length > 0) ? effectiveBotName : "deco.chat", @@ - name: effectiveBotName ?? currentCtx?.appInfo?.name ?? - tokenData.team.name, + name: (effectiveBotName && effectiveBotName.length > 0) + ? effectiveBotName + : (currentCtx?.appInfo?.name ?? tokenData.team.name),
…x/apps-mcp into feat/slack-custom-bot
…eat/slack-custom-bot
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
slack/README.md (1)
122-126: Add language identifier to fenced code block.The flow diagram should specify a language for syntax highlighting, or use
textif no specific language applies.Apply this diff:
-``` +```text User → Start OAuth → Selection UI → [Choose Option] → Slack Authorization → Callback → Complete ↓ ↓ [deco.chat bot] [Custom bot + credentials]</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 133fa855fb4c0fc49c813a14a84e7c6d3440bee5 and 80ebc4ee596724b5b64d8e502ccbf088e3fb7f4b. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `slack/README.md` (5 hunks) * `slack/actions/deco-chat/channels/invoke.ts` (1 hunks) * `slack/actions/oauth/callback.ts` (4 hunks) * `slack/loaders/oauth/start.ts` (1 hunks) * `slack/mod.ts` (1 hunks) * `slack/utils/ui-templates/page-generator.ts` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * slack/mod.ts </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code graph analysis (3)</summary> <details> <summary>slack/loaders/oauth/start.ts (3)</summary><blockquote> <details> <summary>slack/actions/oauth/callback.ts (1)</summary> * `Props` (4-20) </details> <details> <summary>slack/utils/ui-templates/page-generator.ts (1)</summary> * `generateBotSelectionPage` (5-709) </details> <details> <summary>slack/utils/constants.ts (2)</summary> * `SCOPES` (1-17) * `OAUTH_URL_AUTH` (21-21) </details> </blockquote></details> <details> <summary>slack/actions/oauth/callback.ts (2)</summary><blockquote> <details> <summary>slack/loaders/oauth/start.ts (1)</summary> * `Props` (4-13) </details> <details> <summary>slack/mod.ts (2)</summary> * `Props` (44-79) * `AppContext` (89-89) </details> </blockquote></details> <details> <summary>slack/actions/deco-chat/channels/invoke.ts (2)</summary><blockquote> <details> <summary>slack/loaders/deco-chat/channels/list.ts (1)</summary> * `DECO_CHAT_CHANNEL_ID` (7-7) </details> <details> <summary>slack/client.ts (1)</summary> * `joinChannel` (331-351) </details> </blockquote></details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>slack/README.md</summary> 122-122: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (5)</summary><blockquote> <details> <summary>slack/actions/deco-chat/channels/invoke.ts (1)</summary><blockquote> `19-26`: **LGTM! Custom bot identifier logic is well-implemented.** The conditional logic correctly handles direct message channels by using the custom bot name when available, with a proper fallback to the default identifier. </blockquote></details> <details> <summary>slack/actions/oauth/callback.ts (2)</summary><blockquote> `22-29`: **Robust error handling in state decoding.** The try-catch block with empty object fallback ensures the OAuth flow doesn't break on malformed state. --- `90-92`: **Correct fallback chain for bot name preservation.** The effectiveBotName calculation properly implements the requirement: new name → existing name → default. This addresses the re-authentication concern from the previous review. </blockquote></details> <details> <summary>slack/loaders/oauth/start.ts (1)</summary><blockquote> `30-45`: **Appropriate security headers for HTML response.** The CSP, X-Content-Type-Options, and X-Frame-Options headers provide good baseline security for the selection page. </blockquote></details> <details> <summary>slack/utils/ui-templates/page-generator.ts (1)</summary><blockquote> `705-708`: **Proper XSS prevention with JSON encoding.** Using `JSON.stringify()` to safely embed the callback URL prevents XSS injection through the template parameter. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
slack/actions/oauth/callback.ts (1)
48-54: Security issue: Extracting secret from OAuth state.This code retrieves
customClientSecretfrom the decoded state parameter, perpetuating the credential exposure issue identified inslack/utils/state-helpers.tsandslack/loaders/oauth/start.ts. The secret has already been exposed through URL transmission by the time it reaches this point.This issue was flagged in previous reviews and should be addressed holistically across the OAuth flow by implementing server-side credential storage.
🧹 Nitpick comments (2)
slack/utils/state-helpers.ts (2)
6-13: Consider logging decoding failures.Silently returning an empty object masks OAuth state corruption or tampering. While the fallback prevents crashes, it makes debugging difficult when state is malformed.
Add error logging:
export function decodeState(state: string): Record<string, unknown> { try { const decoded = atob(decodeURIComponent(state)); return JSON.parse(decoded); - } catch { + } catch (error) { + console.error("Failed to decode OAuth state:", error); return {}; } }
25-27: Type assertion without runtime validation.The cast to
CustomBotStatedoesn't verify the decoded object's shape. While all properties are optional, unexpected data structures could cause subtle bugs.Consider adding runtime validation:
export function decodeCustomBotState(state: string): CustomBotState { - return decodeState(state) as CustomBotState; + const decoded = decodeState(state); + // Validate expected properties exist and have correct types + if (decoded.customClientSecret !== undefined && typeof decoded.customClientSecret !== 'string') { + console.warn('Invalid customClientSecret in state'); + } + return decoded as CustomBotState; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
slack/actions/oauth/callback.ts(4 hunks)slack/loaders/oauth/start.ts(1 hunks)slack/utils/state-helpers.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
slack/loaders/oauth/start.ts (4)
slack/actions/oauth/callback.ts (1)
Props(5-21)slack/utils/ui-templates/page-generator.ts (1)
generateBotSelectionPage(5-709)slack/utils/state-helpers.ts (1)
decodeState(6-13)slack/utils/constants.ts (2)
SCOPES(1-17)OAUTH_URL_AUTH(21-21)
slack/actions/oauth/callback.ts (2)
slack/loaders/oauth/start.ts (1)
Props(5-14)slack/mod.ts (1)
Props(44-79)
🔇 Additional comments (9)
slack/loaders/oauth/start.ts (4)
5-16: LGTM - Props interface extension is clean.The optional
botNameparameter is well-documented and the function signature correctly accepts theRequestobject needed for the new bot selection flow.
17-37: Good security headers for the selection page.The CSP header with
'unsafe-inline'is a necessary trade-off for the template-based UI, and is acceptable here since the page handles no user data. The additionalX-Frame-OptionsandX-Content-Type-Optionsheaders provide appropriate protection.
70-82: LGTM - Default OAuth flow is secure.The deco.chat flow correctly implements the standard OAuth authorization code flow with only public client_id in the URL. This is the secure pattern that should be applied to the custom bot flow as well.
58-66: State encoding/decoding is correct; no changes required.
The included decodeURIComponent call is redundant but harmless, and the state round-trip functions as intended.slack/actions/oauth/callback.ts (5)
5-21: LGTM - Props interface is well-documented.The addition of optional
botNameandstateparameters with clear JSDoc annotations supports the custom bot flow without breaking existing usage.
28-39: Good extension of return type.Adding
botInfoto the response provides callers with the bot's ID, app ID, and name, which is useful for verification and display purposes.
56-70: LGTM - Token exchange follows OAuth 2.0 spec.The token exchange correctly uses the authorization code flow with appropriate headers and body encoding. The HTTP-to-HTTPS replacement for the redirect URI is a good security practice.
80-101: Good preservation of custom bot name during re-auth.Lines 82-83 correctly implement a fallback chain that preserves the existing
customBotNamewhen no newfinalBotNameis provided. This addresses the concern from previous reviews about re-auth overwriting custom bot branding.The configuration update comprehensively stores all necessary OAuth metadata.
103-113: LGTM - Response structure is clear and complete.The display name composition and
botInfoobject provide all necessary information for callers to identify and work with the configured bot.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
slack/README.md (1)
115-123: Rename duplicate “Custom Bot Configuration” heading.
This second H3 repeats the earlier title, tripping MD024 and making the outline harder to scan. Rename it to reflect the credentials section instead.(app.unpkg.com)-### Custom Bot Configuration +### Custom Bot Credentials
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
slack/README.md(6 hunks)slack/actions/oauth/callback.ts(4 hunks)slack/actions/oauth/store-credentials.ts(1 hunks)slack/loaders/oauth/start.ts(1 hunks)slack/manifest.gen.ts(2 hunks)slack/utils/state-helpers.ts(1 hunks)slack/utils/ui-templates/page-generator.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
slack/actions/oauth/store-credentials.ts (3)
slack/actions/oauth/callback.ts (1)
Props(9-25)slack/loaders/oauth/start.ts (1)
Props(8-17)slack/utils/state-helpers.ts (1)
storeCustomBotSession(62-78)
slack/loaders/oauth/start.ts (5)
slack/actions/oauth/callback.ts (1)
Props(9-25)slack/actions/oauth/store-credentials.ts (1)
Props(3-28)slack/utils/ui-templates/page-generator.ts (1)
generateBotSelectionPage(5-799)slack/utils/state-helpers.ts (2)
retrieveCustomBotSession(83-107)decodeState(6-13)slack/utils/constants.ts (2)
SCOPES(1-17)OAUTH_URL_AUTH(21-21)
slack/actions/oauth/callback.ts (2)
slack/mod.ts (2)
Props(44-79)AppContext(89-89)slack/utils/state-helpers.ts (3)
decodeCustomBotState(26-28)retrieveCustomBotSession(83-107)invalidateSession(112-114)
🪛 markdownlint-cli2 (0.18.1)
slack/README.md
115-115: Multiple headings with the same content
(MD024, no-duplicate-heading)
139-139: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (10)
slack/utils/ui-templates/page-generator.ts (3)
566-580: LGTM! Past issues resolved.The duplicate
DOMContentLoadedlistener flagged in previous reviews has been fixed. The validation setup is now correctly integrated into a single listener with proper null checks.
639-641: LGTM! URL construction issue resolved.The URL construction correctly uses
new URL()API which properly handles query parameter separators. The previous concern about blindly concatenating&has been addressed.Also applies to: 695-698
667-710: LGTM! Secure credential handling.Credentials are correctly transmitted via POST body (JSON) rather than URL parameters, with
same-origincredentials policy. Only the session token (not the secret) is included in the redirect URL, which is the intended secure pattern.slack/manifest.gen.ts (1)
14-15: LGTM! Auto-generated manifest update.The manifest correctly reflects the new
store-credentialsaction. As this is an auto-generated file, the import renumbering is expected.Also applies to: 52-53
slack/loaders/oauth/start.ts (2)
46-79: LGTM! Security issues from previous reviews resolved.The custom bot flow now correctly:
- Accepts only a
sessionTokenin query parameters (not raw credentials)- Retrieves credentials server-side via
retrieveCustomBotSession- Applies
encodeURIComponentto the base64-encoded state (line 68)- Stores only the session token reference in OAuth state, never the actual secret
This addresses all critical security concerns flagged in previous reviews.
24-40: LGTM! Good security headers.The CSP headers appropriately restrict script and style sources while allowing necessary inline content for the selection page UI.
slack/actions/oauth/store-credentials.ts (1)
35-43: LGTM! Secure credential storage.Credentials are stored server-side with a time-limited session token. This prevents credential exposure in URLs or browser history.
slack/actions/oauth/callback.ts (3)
47-69: LGTM! Security issue from previous review resolved.Credentials are now retrieved server-side using the session token (line 56) rather than being embedded in OAuth state. The session is properly invalidated after use (line 64). This addresses the critical security concern flagged in previous reviews.
95-109: LGTM! Custom bot name preservation fixed.Lines 97-98 correctly preserve the existing
customBotNamewhen no new name is provided, preventing the regression flagged in previous reviews where re-authentication would overwrite custom branding.
120-128: LGTM! Improved response structure.The addition of
botInfoprovides useful context about the configured bot, supporting the custom bot identification feature introduced in this PR.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…tart loader for custom bot handling, and improve UI for bot selection page
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
slack/actions/deco-chat/channels/invoke.ts (1)
68-71: Consider reading the environment variable at module initialization.The current implementation reads
DEBUG_TOOLSon every invocation. For better performance, consider moving this to module scope:+const isDebugMode = Deno.env.get("DEBUG_TOOLS") === "true"; + /** * @name DECO_CHAT_CHANNELS_INVOKE * @title Deco Chat Channel Invoke * @description This action is triggered when slack sends a webhook event */ export default async function invoke( props: SlackWebhookPayload & { challenge?: string }, _req: Request, ctx: AppContext, ) { // ... existing code ... - const isDebugMode = Deno.env.get("DEBUG_TOOLS") === "true";This avoids repeated environment variable lookups on the hot path, though the current implementation is functionally correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
slack/README.md(6 hunks)slack/actions/deco-chat/channels/invoke.ts(5 hunks)slack/loaders/oauth/start.ts(1 hunks)slack/utils/state-helpers.ts(1 hunks)slack/utils/ui-templates/page-generator.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
slack/loaders/oauth/start.ts (4)
slack/mod.ts (2)
Props(44-79)AppContext(89-89)slack/utils/state-helpers.ts (3)
storeCustomBotSession(74-90)retrieveCustomBotSession(95-119)decodeState(6-13)slack/utils/ui-templates/page-generator.ts (1)
generateBotSelectionPage(4-224)slack/utils/constants.ts (2)
SCOPES(1-17)OAUTH_URL_AUTH(21-21)
slack/actions/deco-chat/channels/invoke.ts (2)
slack/loaders/deco-chat/channels/list.ts (1)
DECO_CHAT_CHANNEL_ID(7-7)slack/client.ts (1)
joinChannel(331-351)
🪛 markdownlint-cli2 (0.18.1)
slack/README.md
114-114: Multiple headings with the same content
(MD024, no-duplicate-heading)
🔇 Additional comments (4)
slack/actions/deco-chat/channels/invoke.ts (4)
5-14: LGTM! Clean type definitions.The ToolCall and ToolResult interfaces provide clear typing for the tool invocation flow. The use of
unknownfor args values and result is appropriate given the dynamic nature of tool parameters and outputs.
31-36: LGTM! Dynamic bot identification logic is sound.The botIdentifier computation correctly handles both custom bots and the default @deco.chat bot. The
@prefix is consistently applied for custom bots, matching the format of DECO_CHAT_CHANNEL_ID. This enables the multi-bot support described in the PR objectives while maintaining backward compatibility.
90-139: LGTM! Well-structured debug output for tool calls.The handler correctly:
- Guards against execution in non-debug mode
- Formats tool call information in readable Slack blocks
- Stores message metadata for subsequent updates
- Handles the postMessage response appropriately
The toolCallMessageTs map is function-scoped, ensuring proper cleanup after each invocation.
140-198: LGTM! Tool result updates are handled consistently.The handler:
- Maintains the same debug-mode guard pattern
- Safely handles missing tool call metadata
- Formats results consistently with arguments
- Relies on the outer error handler (line 224) for failure cases
The lack of result size limits is acceptable for debug-only functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
slack/utils/ui-templates/page-generator.ts (2)
147-152: Consider adding validation feedback.The validation silently disables/enables the submit button based on required field presence. Consider adding visual feedback (e.g., border color changes, validation messages) to help users understand why the button is disabled.
Example enhancement:
function validateCustomForm() { const clientId = document.getElementById('clientId').value.trim(); const clientSecret = document.getElementById('clientSecret').value.trim(); + const isValid = clientId && clientSecret; + + // Visual feedback + [document.getElementById('clientId'), document.getElementById('clientSecret')].forEach(input => { + if (input.value.trim()) { + input.style.borderColor = '#999'; + } else { + input.style.borderColor = '#ef4444'; + } + }); + - submitBtn.disabled = !clientId || !clientSecret; + submitBtn.disabled = !isValid; updateFormAction(); }
183-197: Consider adding request timeout.The fetch request to store credentials doesn't include a timeout. If the server is slow or unresponsive, users may wait indefinitely with a "Processing..." button.
Add a timeout wrapper:
try { submitBtn.textContent = 'Processing...'; submitBtn.disabled = true; + // Add timeout + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); // 10 second timeout + // Store credentials via POST request - const response = await fetch(sanitizedJSCurrentUrl, { + const response = await fetch(sanitizedJSCurrentUrl, { method: 'POST', headers: { 'Content-Type': 'application/json' }, + signal: controller.signal, body: JSON.stringify({ action: 'storeCredentials', clientId: clientId, clientSecret: clientSecret, botName: botName }) }); + + clearTimeout(timeoutId); const result = await response.json();And update the catch block:
} catch (error) { - showError('Network error: ' + error.message); + if (error.name === 'AbortError') { + showError('Request timeout. Please try again.'); + } else { + showError('Network error: ' + error.message); + } submitBtn.textContent = 'Continue'; submitBtn.disabled = false; }slack/README.md (1)
142-150: Environment variables documentation could be clearer.The documentation states these variables are "optional" for "default custom bot" but doesn't explain what "default custom bot" means or when these would be used vs. the selection interface approach.
Consider clarifying:
## Environment Variables -For custom bot configurations, you may optionally set: +For custom bot configurations, you may optionally pre-configure default credentials: ```bash -SLACK_CLIENT_ID=your_slack_client_id # Optional: Default custom bot -SLACK_CLIENT_SECRET=your_slack_client_secret # Optional: Default custom bot +SLACK_CLIENT_ID=your_slack_client_id # Optional: Pre-fill custom bot Client ID +SLACK_CLIENT_SECRET=your_slack_client_secret # Optional: Pre-fill custom bot Client Secret
+Note: These credentials can be overridden through the bot selection interface. When set, they provide default values but users can still enter different credentials during OAuth.
</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 3bc9dd8707bd916ed0908a36df9e5d596a414156 and be948dcaad29870ea8ad1f54d4841b8f591edbcb. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `slack/README.md` (6 hunks) * `slack/utils/ui-templates/page-generator.ts` (1 hunks) </details> <details> <summary>🔇 Additional comments (18)</summary><blockquote> <details> <summary>slack/utils/ui-templates/page-generator.ts (8)</summary><blockquote> `1-10`: **LGTM! Clean function signature and documentation.** The function is well-documented with a clear JSDoc comment, and the signature appropriately accepts the current URL as a parameter for generating the bot selection page. --- `11-76`: **Well-structured CSS with good UX considerations.** The styles provide: - Responsive design with proper box-sizing - Clean visual hierarchy with appropriate spacing - Hover states for interactive elements - Focus states for accessibility - Disabled button styling - Error message styling The design is minimal and functional. --- `85-122`: **Form structure is logical and accessible.** The form includes: - Proper semantic HTML with labels and form elements - Required attribute on mandatory fields - Clear visual hierarchy with option cards - Hidden custom form that expands when selected - Proper input types (text, password) - Disabled submit button by default (enabled by validation) --- `124-127`: **Excellent XSS mitigation.** The sanitization of `currentUrl` using `JSON.stringify()` prevents reflected XSS attacks. This addresses the critical security concern from the previous review and ensures that any special characters in the URL (quotes, backticks, `</script>` tags) are properly escaped. Based on learnings. --- `165-168`: **Input validation listeners are properly attached.** The code correctly attaches input event listeners to validate the form as users type. This provides immediate feedback and resolves the duplicate listener issue from the previous review. Based on learnings. --- `170-218`: **Robust custom bot submission with proper error handling.** The form submission handler: - Prevents default for custom bot to use fetch - Validates required fields before submission - Shows loading state during processing - Makes secure POST request with credentials - Handles success with redirect including session token - Provides clear error messages on failure - Restores button state after errors The error handling and user feedback are well-implemented. --- `220-224`: **Error display function is simple and effective.** The error handling properly shows error messages inline with appropriate styling. --- `134-163`: **Form method `"get"` is correct.** The custom flow’s `submit` handler calls `e.preventDefault()` and issues a POST `fetch()`, so the static `method="get"` applies only to the deco flow and requires no change. > Likely an incorrect or invalid review comment. </blockquote></details> <details> <summary>slack/README.md (10)</summary><blockquote> `1-16`: **Comprehensive feature list with new capabilities.** The README clearly documents the app's features including the new flexible bot configuration and file upload V2 support. The emoji icons improve readability and visual appeal. --- `17-36`: **Clear comparison of integration options.** The two integration paths (deco.chat Bot vs Custom Bot) are well-explained with clear benefits for each approach. This helps users make informed decisions about which option suits their needs. --- `49-88`: **Detailed setup instructions for both bot types.** The dual-path setup instructions are thorough and well-organized: - Option 1 (deco.chat Bot): Simple 4-step process - Option 2 (Custom Bot): Detailed 3-part guide with OAuth configuration - Clear scope requirements listed The instructions match the implementation in the PR. --- `89-119`: **Bot Selection Interface documentation is accurate.** The documentation covers: - Interface features and selection options - Custom bot credential flow with security details - Required credentials with proper sourcing information The security features section correctly states the 10-minute token expiration, addressing the previous review comment. Based on learnings. --- `127-141`: **Flow diagram is clear and informative.** The text-based flow diagram effectively illustrates the OAuth process with both bot options. The code block now has a language specifier (`text`), resolving the previous markdownlint issue. Based on learnings. --- `185-207`: **Excellent documentation of bot name behavior.** The Custom Bot Configuration Details section thoroughly explains: - Where bot names are used - Bot name preservation during re-auth - Multiple bot support This level of detail is helpful for understanding the system behavior. --- `218-225`: **Security section is comprehensive and accurate.** The security documentation covers: - Token storage - Automatic refresh - Scope limitations - HTTPS requirements - Client secret handling All points align with the implementation. --- `226-246`: **Thorough troubleshooting guide.** The troubleshooting section covers: - Selection interface issues - Common OAuth problems - Custom bot specific issues This provides good support for users encountering problems. --- `252-263`: **Development section properly documents new components.** The development section lists all relevant files including the new state helpers and UI templates, making it easy for contributors to understand the codebase structure. --- `264-273`: **Contributing guidelines emphasize dual-bot testing.** The contributing section appropriately highlights the need to test both bot flows and UI considerations, ensuring quality contributions. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
slack/actions/deco-chat/channels/invoke.ts(5 hunks)slack/actions/oauth/callback.ts(4 hunks)slack/loaders/oauth/start.ts(1 hunks)slack/mod.ts(1 hunks)slack/utils/state-helpers.ts(1 hunks)slack/utils/ui-templates/page-generator.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- slack/actions/deco-chat/channels/invoke.ts
- slack/actions/oauth/callback.ts
- slack/mod.ts
🧰 Additional context used
🧬 Code graph analysis (1)
slack/loaders/oauth/start.ts (4)
slack/actions/oauth/callback.ts (1)
Props(9-25)slack/mod.ts (2)
Props(44-86)AppContext(96-96)slack/utils/state-helpers.ts (3)
storeCustomBotSession(76-94)decodeState(6-13)retrieveCustomBotSession(99-125)slack/utils/ui-templates/page-generator.ts (1)
generateBotSelectionPage(4-253)
🔇 Additional comments (1)
slack/utils/ui-templates/page-generator.ts (1)
139-179: SanitizedcurrentUrllooks good.
Capturing the URL once viaJSON.stringifykeeps every downstreamnew URL(...)/fetch(...)call safe from injection. Nicely done.
| const sessionStore = new Map<string, { | ||
| clientId: string; | ||
| clientSecret: string; | ||
| botName?: string; | ||
| debugMode?: boolean; | ||
| expiresAt: number; | ||
| }>(); | ||
|
|
||
| /** | ||
| * Store custom bot credentials securely with session token | ||
| */ | ||
| export function storeCustomBotSession( | ||
| clientId: string, | ||
| clientSecret: string, | ||
| botName?: string, | ||
| debugMode?: boolean, | ||
| ): string { | ||
| const sessionToken = generateSessionToken(); | ||
| const expiresAt = Date.now() + (10 * 60 * 1000); // 10 minutes expiry | ||
|
|
||
| sessionStore.set(sessionToken, { | ||
| clientId, | ||
| clientSecret, | ||
| botName, | ||
| debugMode, | ||
| expiresAt, | ||
| }); | ||
|
|
||
| return sessionToken; | ||
| } | ||
|
|
||
| /** | ||
| * Retrieve custom bot credentials using session token | ||
| */ | ||
| export function retrieveCustomBotSession(sessionToken: string): { | ||
| clientId: string; | ||
| clientSecret: string; | ||
| botName?: string; | ||
| debugMode?: boolean; | ||
| } | null { | ||
| const session = sessionStore.get(sessionToken); | ||
|
|
||
| if (!session) { | ||
| return null; | ||
| } | ||
|
|
||
| // Check expiration | ||
| if (Date.now() > session.expiresAt) { | ||
| sessionStore.delete(sessionToken); | ||
| return null; | ||
| } | ||
|
|
||
| // Don't delete on retrieval - allow multiple uses during OAuth flow | ||
| // Will be cleaned up by expiration | ||
| return { | ||
| clientId: session.clientId, | ||
| clientSecret: session.clientSecret, | ||
| botName: session.botName, | ||
| debugMode: session.debugMode, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Persist custom-bot sessions outside of process memory.
sessionStore is a module-level Map, so the POST that writes credentials and the subsequent GET that reads them must land on the same isolate. In our Deploy/edge setup that’s not guaranteed—requests are routinely routed across workers or cold-started—so retrieveCustomBotSession will often return null, causing the “Session expired or invalid” error path and making the custom-bot flow flaky once we scale past a single instance.
Please move this storage into a shared backend (e.g., the existing KV/DB used for installations) or at least inject a storage interface from AppContext so every worker sees the same data. Until the session token is backed by durable storage, the new flow will break in production.
2 participants
🚀 Support for Custom Slack Bots
Summary
This PR introduces support for custom/private Slack bots in addition to the default
deco.chatbot.Users can now create and configure their own Slack apps with unique bot names while maintaining full compatibility with existing installations.
Key Changes
@deco.chatinstallations remain unaffectedImplementation Details
customBotNameduring OAuth callback@deco.chatbotUserId,appId,customBotName, etc.)Example Usage
Bot configured via OAuth
Compatibility
Use Cases
Troubleshooting
✅ Conclusion
This PR enables full support for custom/private Slack bots with personalized names, while preserving compatibility with the default deco.chat bot. It allows organizations to set up branded or environment-specific bots with no loss of functionality.
Reference: deco-sites/mcp#92
Summary by CodeRabbit
New Features
Improvements
Security
Documentation