feat: Slack bot integration + mobile web UI#252
Open
phanisaimunipalli wants to merge 1 commit intompfaffenberger:mainfrom
Open
feat: Slack bot integration + mobile web UI#252phanisaimunipalli wants to merge 1 commit intompfaffenberger:mainfrom
phanisaimunipalli wants to merge 1 commit intompfaffenberger:mainfrom
Conversation
- code_puppy/integrations/slack_bot.py: Slack Bolt (Socket Mode) adapter that bridges Slack messages and /pup slash commands to the existing /ws/terminal PTY backend. Each Slack thread maps to a persistent PTY session so conversation context is preserved. - code_puppy/api/templates/mobile.html: chat-style, touch-optimised web UI served at /mobile. Connects to the same /ws/terminal WebSocket, strips ANSI codes, renders code blocks with copy buttons, and includes a bottom-sheet slash-command picker backed by /api/commands/. - code_puppy/api/app.py: adds GET /mobile route; adds Mobile UI link to the landing page. - pyproject.toml: adds optional [slack] extra (slack-bolt>=1.18.0). - docs/integrations/slack.md: step-by-step setup guide. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdded a new mobile chat interface endpoint and template serving static HTML. Introduced a Slack bot integration that forwards Slack messages and slash commands to the Code Puppy backend via WebSocket, maintaining persistent sessions per thread. Included Slack integration documentation and optional Changes
Sequence Diagram(s)sequenceDiagram
participant SlackUser as Slack User
participant SlackBolt as Slack Bolt<br/>(Socket Mode)
participant SlackBot as Code Puppy<br/>Slack Bot
participant PTY as Code Puppy<br/>PTY Backend
participant Slack as Slack API
SlackUser->>SlackBolt: mention/DM/@pup command
SlackBolt->>SlackBot: event/command handler
SlackBot->>SlackBot: get_or_create(thread_ts)
SlackBot->>PTY: WebSocket connect
PTY->>SlackBot: session handshake
SlackBot->>PTY: send_prompt(user input)
PTY->>PTY: execute in PTY
PTY->>SlackBot: output (base64-encoded)
SlackBot->>SlackBot: decode & strip ANSI
SlackBot->>Slack: post message to thread
Slack->>SlackUser: reply in thread
SlackUser->>SlackBolt: thread reply
SlackBolt->>SlackBot: message event
SlackBot->>SlackBot: reuse session from registry
SlackBot->>PTY: send_prompt(follow-up)
PTY->>SlackBot: output
SlackBot->>Slack: post reply
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
code_puppy/integrations/slack_bot.py (3)
119-122: Useasyncio.get_running_loop()instead of deprecatedasyncio.get_event_loop().
asyncio.get_event_loop()is deprecated since Python 3.10 and will emit aDeprecationWarningwhen called from a coroutine. Sincesend_promptis an async method, useasyncio.get_running_loop()instead.♻️ Proposed fix
- deadline = asyncio.get_event_loop().time() + _OUTPUT_HARD_TIMEOUT + loop = asyncio.get_running_loop() + deadline = loop.time() + _OUTPUT_HARD_TIMEOUT while True: - remaining = deadline - asyncio.get_event_loop().time() + remaining = deadline - loop.time()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/integrations/slack_bot.py` around lines 119 - 122, The code in send_prompt uses asyncio.get_event_loop() (used to compute deadline and remaining) which is deprecated; replace those calls with asyncio.get_running_loop() so the loop is retrieved from the currently running coroutine; update the two occurrences around the deadline and remaining calculations (where deadline = asyncio.get_event_loop().time() + _OUTPUT_HARD_TIMEOUT and remaining = deadline - asyncio.get_event_loop().time()) to call asyncio.get_running_loop().time() instead.
161-167: Potential race condition in session creation.The check-then-create pattern in
get_or_createis not atomic. If called concurrently for the samethread_ts, multiple sessions could be created. While current usage viaasyncio.run()serializes calls, this could become an issue if the code is refactored.🔒 Proposed fix using a lock
class SessionRegistry: """Maps Slack thread timestamps to persistent PTY sessions.""" def __init__(self, ws_url: str) -> None: self._ws_url = ws_url self._sessions: dict[str, PtySession] = {} + self._lock = asyncio.Lock() async def get_or_create(self, thread_ts: str) -> PtySession: - if thread_ts not in self._sessions: - session = PtySession(self._ws_url) - await session.connect() - self._sessions[thread_ts] = session - logger.info("New PTY session for thread %s", thread_ts) - return self._sessions[thread_ts] + async with self._lock: + if thread_ts not in self._sessions: + session = PtySession(self._ws_url) + await session.connect() + self._sessions[thread_ts] = session + logger.info("New PTY session for thread %s", thread_ts) + return self._sessions[thread_ts]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/integrations/slack_bot.py` around lines 161 - 167, get_or_create currently does a non-atomic check-then-create on self._sessions which can race; protect session creation by introducing and using an asyncio Lock: add a lock (either a single self._sessions_lock or per-key locks like self._session_locks keyed by thread_ts), acquire the lock before checking/creating the PtySession in get_or_create, await the lock, then perform a double-checked lookup (re-check self._sessions[thread_ts]) and only create/connect and assign if still missing, finally release the lock; reference get_or_create, self._sessions, and PtySession to locate where to add the locking.
269-287: Consider using Slack Bolt's async mode for better performance.Using
asyncio.run()inside each handler creates a new event loop per message, which is inefficient. Slack Bolt supports async handlers natively viaAsyncApp. This would allow sharing the event loop across handlers.♻️ Example using AsyncApp
from slack_bolt.async_app import AsyncApp app = AsyncApp(token=bot_token) `@app.event`("app_mention") async def handle_mention(event: dict, say): text = re.sub(r"<@[A-Z0-9]+>", "", event.get("text", "")).strip() thread_ts = event.get("thread_ts") or event["ts"] await _handle(text, thread_ts, say) # Use AsyncSocketModeHandler instead of SocketModeHandler from slack_bolt.adapter.socket_mode.async_handler import AsyncSocketModeHandler handler = AsyncSocketModeHandler(app, app_token) await handler.start_async()This requires more refactoring but would significantly improve performance under load.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/integrations/slack_bot.py` around lines 269 - 287, The handlers handle_mention and handle_dm currently call asyncio.run(_handle(...)) which creates a new event loop per message; replace the synchronous App with Slack Bolt's AsyncApp, convert handle_mention and handle_dm to async functions, remove asyncio.run and instead await _handle(text, thread_ts, say), and update the app startup to use AsyncSocketModeHandler (start_async) so the single shared event loop is used for all handlers.code_puppy/api/templates/mobile.html (1)
457-463: Consider exponential backoff for reconnection attempts.The fixed 3-second reconnect delay could cause rapid reconnection attempts if the server is temporarily unavailable, potentially overwhelming it when it recovers. Consider implementing exponential backoff with a cap.
♻️ Suggested exponential backoff
+let reconnectDelay = 3000; +const MAX_RECONNECT_DELAY = 30000; ws.onclose = () => { setStatus('disconnected'); document.getElementById('send-btn').disabled = true; finaliseResponse(); - addMessage('system', 'Disconnected. Reconnecting in 3 s…'); - setTimeout(connect, 3000); + addMessage('system', `Disconnected. Reconnecting in ${reconnectDelay / 1000} s…`); + setTimeout(() => { + connect(); + reconnectDelay = Math.min(reconnectDelay * 2, MAX_RECONNECT_DELAY); + }, reconnectDelay); }; +ws.onopen = () => { + reconnectDelay = 3000; // Reset on successful connection setStatus('connected'); // ...rest of onopen };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/api/templates/mobile.html` around lines 457 - 463, The fixed 3s reconnect in the ws.onclose handler should be replaced with an exponential backoff: introduce a retry counter (e.g., reconnectAttempts) and compute delay = Math.min(baseDelay * 2**reconnectAttempts, maxDelay), then call setTimeout(connect, delay) instead of setTimeout(connect, 3000); increment reconnectAttempts on each onclose and reset it to 0 on successful connection (in connect or ws.onopen), and ensure any existing timers are cleared if a manual disconnect or successful reconnect occurs; keep the existing setStatus('disconnected'), finaliseResponse(), and addMessage(...) calls in ws.onclose but swap the static timeout for the computed backoff logic so reconnect attempts are capped and reset on success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/api/templates/mobile.html`:
- Around line 544-555: The code inserts cmd.description via item.innerHTML
creating an XSS risk; update the cmds.forEach block (where item, cmdList,
item.innerHTML, cmd.name, cmd.description are used) to avoid innerHTML by
creating the span elements and setting their textContent (or otherwise escaping
cmd.description) before appending to item, then append item to cmdList; keep the
existing click handler logic (sheet, input, autoResize) unchanged.
- Around line 361-377: The innerHTML assignment for wrapper uses the unescaped
lang string (derived from codeRe match[1]) and can lead to XSS; fix by
sanitizing or avoiding direct HTML interpolation: compute const lang = match[1]
|| 'code' then either pass it through an escapeHtml function (escapeHtml(lang))
before interpolation or, better, replace the templated insertion with DOM
methods (createElement('span'), set className 'code-lang', set textContent to
lang, append to the .code-header) and only use innerHTML for static markup;
ensure wrapper.querySelector('.code-lang') or the newly created span uses
textContent so the language label cannot inject HTML.
In `@code_puppy/integrations/slack_bot.py`:
- Around line 297-298: The current assignment of thread_ts uses trigger_id
(thread_ts = str(command.get("trigger_id", "global"))), which prevents session
context from persisting across slash command invocations; change thread_ts to
use a channel-based key (e.g., thread_ts = str(command.get("channel_id",
"global")) or combine channel_id + user id) in the same spot to preserve context
across `/pup` calls, or if preserving is not desired, add a clear
comment/documentation next to the thread_ts assignment explaining that slash
commands intentionally create new PTY sessions by using trigger_id.
In `@docs/integrations/slack.md`:
- Around line 7-17: The fenced code block containing the ASCII diagram (the
block that starts with the lines "Slack message / /pup command" and ends after
"Output streamed back → posted to Slack thread") needs a language specifier to
satisfy linting; change the opening fence from ``` to ```text (or ```plaintext)
so the diagram is marked as plain text. Ensure you update the single fenced
block in docs/integrations/slack.md that wraps the ASCII diagram.
---
Nitpick comments:
In `@code_puppy/api/templates/mobile.html`:
- Around line 457-463: The fixed 3s reconnect in the ws.onclose handler should
be replaced with an exponential backoff: introduce a retry counter (e.g.,
reconnectAttempts) and compute delay = Math.min(baseDelay *
2**reconnectAttempts, maxDelay), then call setTimeout(connect, delay) instead of
setTimeout(connect, 3000); increment reconnectAttempts on each onclose and reset
it to 0 on successful connection (in connect or ws.onopen), and ensure any
existing timers are cleared if a manual disconnect or successful reconnect
occurs; keep the existing setStatus('disconnected'), finaliseResponse(), and
addMessage(...) calls in ws.onclose but swap the static timeout for the computed
backoff logic so reconnect attempts are capped and reset on success.
In `@code_puppy/integrations/slack_bot.py`:
- Around line 119-122: The code in send_prompt uses asyncio.get_event_loop()
(used to compute deadline and remaining) which is deprecated; replace those
calls with asyncio.get_running_loop() so the loop is retrieved from the
currently running coroutine; update the two occurrences around the deadline and
remaining calculations (where deadline = asyncio.get_event_loop().time() +
_OUTPUT_HARD_TIMEOUT and remaining = deadline - asyncio.get_event_loop().time())
to call asyncio.get_running_loop().time() instead.
- Around line 161-167: get_or_create currently does a non-atomic
check-then-create on self._sessions which can race; protect session creation by
introducing and using an asyncio Lock: add a lock (either a single
self._sessions_lock or per-key locks like self._session_locks keyed by
thread_ts), acquire the lock before checking/creating the PtySession in
get_or_create, await the lock, then perform a double-checked lookup (re-check
self._sessions[thread_ts]) and only create/connect and assign if still missing,
finally release the lock; reference get_or_create, self._sessions, and
PtySession to locate where to add the locking.
- Around line 269-287: The handlers handle_mention and handle_dm currently call
asyncio.run(_handle(...)) which creates a new event loop per message; replace
the synchronous App with Slack Bolt's AsyncApp, convert handle_mention and
handle_dm to async functions, remove asyncio.run and instead await _handle(text,
thread_ts, say), and update the app startup to use AsyncSocketModeHandler
(start_async) so the single shared event loop is used for all handlers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94851722-e904-4b11-bd55-73f322712f9d
📒 Files selected for processing (6)
code_puppy/api/app.pycode_puppy/api/templates/mobile.htmlcode_puppy/integrations/__init__.pycode_puppy/integrations/slack_bot.pydocs/integrations/slack.mdpyproject.toml
Comment on lines
+361
to
+377
| const wrapper = document.createElement('div'); | ||
| wrapper.className = 'code-block'; | ||
| wrapper.innerHTML = ` | ||
| <div class="code-header"> | ||
| <span class="code-lang">${lang}</span> | ||
| <button class="copy-btn" aria-label="Copy code">Copy</button> | ||
| </div> | ||
| <pre></pre>`; | ||
| wrapper.querySelector('pre').textContent = code; | ||
| wrapper.querySelector('.copy-btn').addEventListener('click', () => { | ||
| navigator.clipboard.writeText(code).then(() => { | ||
| const btn = wrapper.querySelector('.copy-btn'); | ||
| btn.textContent = 'Copied!'; | ||
| setTimeout(() => { btn.textContent = 'Copy'; }, 1500); | ||
| }); | ||
| }); | ||
| frag.appendChild(wrapper); |
There was a problem hiding this comment.
Potential XSS via unescaped lang variable in innerHTML.
The lang value extracted from the regex match is inserted directly into innerHTML without escaping. If malicious content appears in the language identifier (e.g., ```<img src=x onerror=alert(1)>), it could execute arbitrary JavaScript.
🛡️ Proposed fix to escape HTML
+function escapeHtml(str) {
+ const div = document.createElement('div');
+ div.textContent = str;
+ return div.innerHTML;
+}
+
function renderCodeBlocks(text) {
const frag = document.createDocumentFragment();
const codeRe = /```(\w*)\n?([\s\S]*?)```/g;
let last = 0, match;
while ((match = codeRe.exec(text)) !== null) {
if (match.index > last) {
const span = document.createElement('span');
span.textContent = text.slice(last, match.index);
frag.appendChild(span);
}
- const lang = match[1] || 'code';
+ const lang = escapeHtml(match[1] || 'code');
const code = match[2];
const wrapper = document.createElement('div');
wrapper.className = 'code-block';
wrapper.innerHTML = `
<div class="code-header">
<span class="code-lang">${lang}</span>
<button class="copy-btn" aria-label="Copy code">Copy</button>
</div>
<pre></pre>`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code_puppy/api/templates/mobile.html` around lines 361 - 377, The innerHTML
assignment for wrapper uses the unescaped lang string (derived from codeRe
match[1]) and can lead to XSS; fix by sanitizing or avoiding direct HTML
interpolation: compute const lang = match[1] || 'code' then either pass it
through an escapeHtml function (escapeHtml(lang)) before interpolation or,
better, replace the templated insertion with DOM methods (createElement('span'),
set className 'code-lang', set textContent to lang, append to the .code-header)
and only use innerHTML for static markup; ensure
wrapper.querySelector('.code-lang') or the newly created span uses textContent
so the language label cannot inject HTML.
Comment on lines
+544
to
+555
| cmds.forEach(cmd => { | ||
| const item = document.createElement('div'); | ||
| item.className = 'cmd-item'; | ||
| item.innerHTML = `<span class="cmd-name">/${cmd.name}</span><span class="cmd-desc">${cmd.description}</span>`; | ||
| item.addEventListener('click', () => { | ||
| sheet.classList.remove('open'); | ||
| input.value = `/${cmd.name} `; | ||
| input.focus(); | ||
| autoResize(input); | ||
| }); | ||
| cmdList.appendChild(item); | ||
| }); |
There was a problem hiding this comment.
XSS risk: cmd.description inserted via innerHTML without escaping.
The command description is inserted into the DOM using innerHTML. While command descriptions likely come from trusted sources (the command registry), it's safer to escape HTML or use textContent for defense in depth.
🛡️ Proposed fix
cmds.forEach(cmd => {
const item = document.createElement('div');
item.className = 'cmd-item';
- item.innerHTML = `<span class="cmd-name">/${cmd.name}</span><span class="cmd-desc">${cmd.description}</span>`;
+ const nameSpan = document.createElement('span');
+ nameSpan.className = 'cmd-name';
+ nameSpan.textContent = `/${cmd.name}`;
+ const descSpan = document.createElement('span');
+ descSpan.className = 'cmd-desc';
+ descSpan.textContent = cmd.description;
+ item.appendChild(nameSpan);
+ item.appendChild(descSpan);
item.addEventListener('click', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code_puppy/api/templates/mobile.html` around lines 544 - 555, The code
inserts cmd.description via item.innerHTML creating an XSS risk; update the
cmds.forEach block (where item, cmdList, item.innerHTML, cmd.name,
cmd.description are used) to avoid innerHTML by creating the span elements and
setting their textContent (or otherwise escaping cmd.description) before
appending to item, then append item to cmdList; keep the existing click handler
logic (sheet, input, autoResize) unchanged.
Comment on lines
+297
to
+298
| # Use command ts as thread key (each slash command is its own thread) | ||
| thread_ts = str(command.get("trigger_id", "global")) |
There was a problem hiding this comment.
Slash commands don't preserve session context across invocations.
Using trigger_id as the session key means each /pup command creates a new PTY session. Unlike threaded conversations where context persists, consecutive /pup commands won't share context. This may be intentional, but consider documenting this difference or using a channel-based key if context persistence is desired for slash commands.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code_puppy/integrations/slack_bot.py` around lines 297 - 298, The current
assignment of thread_ts uses trigger_id (thread_ts =
str(command.get("trigger_id", "global"))), which prevents session context from
persisting across slash command invocations; change thread_ts to use a
channel-based key (e.g., thread_ts = str(command.get("channel_id", "global")) or
combine channel_id + user id) in the same spot to preserve context across `/pup`
calls, or if preserving is not desired, add a clear comment/documentation next
to the thread_ts assignment explaining that slash commands intentionally create
new PTY sessions by using trigger_id.
Comment on lines
+7
to
+17
| ``` | ||
| Slack message / /pup command | ||
| ↓ | ||
| Slack Bolt (Socket Mode) | ||
| ↓ | ||
| WebSocket → /ws/terminal (existing PTY backend) | ||
| ↓ | ||
| Code Puppy agent runs | ||
| ↓ | ||
| Output streamed back → posted to Slack thread | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The ASCII diagram should have a language specified for the fenced code block to satisfy linting rules. Use text or plaintext for ASCII diagrams.
📝 Proposed fix
-```
+```text
Slack message / /pup command
↓
Slack Bolt (Socket Mode)🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/integrations/slack.md` around lines 7 - 17, The fenced code block
containing the ASCII diagram (the block that starts with the lines "Slack
message / /pup command" and ends after "Output streamed back → posted to Slack
thread") needs a language specifier to satisfy linting; change the opening fence
from ``` to ```text (or ```plaintext) so the diagram is marked as plain text.
Ensure you update the single fenced block in docs/integrations/slack.md that
wraps the ASCII diagram.
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
code_puppy/integrations/slack_bot.py): Slack Bolt adapter in Socket Mode. Bridges@mention, DMs, and/pup <prompt>slash commands to the existing/ws/terminalPTY backend. Each Slack thread maps to a persistent PTY session so conversation context carries across replies.code_puppy/api/templates/mobile.html): single-file, zero-build chat interface served at/mobile. Connects to the same/ws/terminalWebSocket, strips ANSI terminal codes, renders code fences with copy buttons, and includes a bottom-sheet slash-command picker backed by/api/commands/. Works on iPhone/Android — sticky input bar, safe-area insets, dynamic viewport height./mobileroute added toapp.py; landing page updated with a Mobile UI link.[slack]extra inpyproject.toml:pip install "code-puppy[slack]".docs/integrations/slack.md.No changes to existing CLI, API routes, or core agent logic.
Test plan
pup --api), openhttp://localhost:8765/mobileon a phone browser — verify chat UI loads, connects, and sends/receives a prompt/mobile404s gracefully when template missingcode-puppy[slack], setSLACK_BOT_TOKEN+SLACK_APP_TOKEN, runpython -m code_puppy.integrations.slack_bot— verify bot responds to/pup hello🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
/pupcommands to Code Puppy with persistent session management per thread.Documentation
Chores