feat: add bidirectional slack integration#63
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds Slack slash-command support: raw-body HMAC verification, command parsing (list/status/cancel/help), formatting, handlers that interact with the run registry and cancellation logic, response-url posting, environment validation for ChangesSlack Slash Commands
Sequence DiagramsequenceDiagram
actor Operator as Operator (Slack)
participant Slack as Slack API
participant Route as /webhooks/slack
participant Verify as verifySlackSignature
participant Parse as parseCommand
participant Handler as Handlers
participant Registry as Run Registry
participant Cancel as cancelRun
participant Respond as postToResponseUrl
Operator->>Slack: /ai-workflow list
Slack->>Route: POST (signed form body)
Route->>Verify: verify signature + timestamp
Verify-->>Route: valid
Route->>Parse: parseCommand(text)
Parse-->>Route: { kind: "list" }
Route-->>Operator: ephemeral "Working on …"
Route->>Handler: handleList() [deferred]
Handler->>Registry: listAll()
Registry-->>Handler: rows
Handler-->>Respond: formatted result
Respond->>Slack: POST to response_url
Slack->>Operator: in_channel result
Operator->>Slack: /ai-workflow cancel AWT-1
Slack->>Route: POST (signed form body)
Route->>Verify: verify signature
Verify-->>Route: valid
Route->>Parse: parseCommand("cancel AWT-1")
Parse-->>Route: { kind: "cancel", ticketKey: "AWT-1" }
Route-->>Operator: ephemeral "Working on …"
Route->>Handler: handleCancel(AWT-1) [deferred]
Handler->>Registry: getRunId("AWT-1")
Registry-->>Handler: runId
Handler->>Cancel: cancelRun("AWT-1", runId)
Cancel-->>Handler: success/failure
Handler-->>Respond: cancellation result
Respond->>Slack: POST to response_url
Slack->>Operator: cancellation outcome
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/routes/webhooks/slack.post.test.ts (1)
190-203: ⚡ Quick winAssert response post count before indexing
postedToResponseUrl[0].At Line 202 and Line 221, indexing without first asserting array length can fail with a noisy
undefinedaccess instead of a clear expectation failure. Addexpect(postedToResponseUrl).toHaveLength(1)before reading index0.Proposed diff
await handler(makeRequest(body)); await flushDeferred(); expect(cancelRunFn).not.toHaveBeenCalled(); + expect(postedToResponseUrl).toHaveLength(1); expect(postedToResponseUrl[0]!.payload.text).toContain("No active run for AWT-1"); @@ await handler(makeRequest(body)); await flushDeferred(); expect(cancelRunFn).toHaveBeenCalledTimes(1); expect(cancelRunFn).toHaveBeenCalledWith("AWT-1", "run_a", runRegistry); + expect(postedToResponseUrl).toHaveLength(1); expect(postedToResponseUrl[0]!.payload.text).toContain("Cancelled AWT-1");Also applies to: 205-222
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/webhooks/slack.post.test.ts` around lines 190 - 203, The test accesses postedToResponseUrl[0] without first asserting the array length which can produce an unclear undefined access; before any indexing of postedToResponseUrl in the "/ai-workflow cancel AWT-1 with no entry..." test and the other affected test (around where cancelRunFn, handler, makeRequest, flushDeferred are used), add an explicit assertion like expect(postedToResponseUrl).toHaveLength(1) immediately after flushDeferred() and before reading postedToResponseUrl[0] so the test fails with a clear message if nothing was posted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/init-slack/references/slash-commands.md:
- Around line 28-30: The markdown code fences around the examples for
SLACK_ALLOWED_USER_IDS and the `/ai-workflow list` command lack a language tag
and trigger MD040; update those fenced blocks to use the bash language (e.g.,
change ``` to ```bash) for the blocks containing
"SLACK_ALLOWED_USER_IDS=U0123ABCD,U4567WXYZ" and the "/ai-workflow list" example
so markdownlint no longer reports MD040.
In @.claude/skills/init-slack/SKILL.md:
- Around line 57-59: The fenced code blocks for the environment variables lack a
language identifier; update the backticks surrounding CHAT_SDK_BOT_NAME and
SLACK_ALLOWED_USER_IDS to use a language tag (e.g., ```bash) so Markdown lint
MD040 is satisfied; locate the blocks containing the symbols CHAT_SDK_BOT_NAME
and SLACK_ALLOWED_USER_IDS in SKILL.md and add the "bash" language after the
opening triple backticks for each block.
In `@docs/superpowers/plans/2026-05-01-slack-slash-commands.md`:
- Line 123: The docs currently reference a machine-specific absolute path
"/Users/kacper/.claude/skills/init-slack" which is not portable; update the
Slack docs line to use a repo-relative or generic path (e.g.,
"./skills/init-slack" or "~/.claude/skills/init-slack") and ensure the mention
appears alongside the `init-slack` skill name, and add the one-paragraph
operator note under the existing Slack section in `README.md` referencing the
generic path instead of the absolute one.
In `@src/lib/slack/handlers.ts`:
- Around line 51-54: The current cleanup swallows errors from stopSandboxes and
registry.unregister but always returns a success message; change this so you
attempt each operation (registry.getSandboxId, stopSandboxes(ticketKey,
sandboxId), registry.unregister(ticketKey)) while capturing their errors, log or
collect the actual error details, and only return the “claim cleared” success
string if both stopSandboxes and registry.unregister succeeded; if either failed
return a clear failure message including which operation(s) failed and include
the captured error details (use the ticketKey, sandboxId, stopSandboxes, and
registry.unregister identifiers to locate and report the failures).
- Around line 30-33: The current Promise.all([registry.getRunId(ticketKey),
registry.getSandboxId(ticketKey)]) causes the whole read to fail if getSandboxId
rejects; change this to treat the sandbox lookup as best-effort by awaiting
registry.getRunId(ticketKey) normally and then attempting
registry.getSandboxId(ticketKey) inside a try/catch (or use Promise.allSettled)
so you still return status when runId resolves; update handling of the sandboxId
variable (e.g., default to undefined on error) and ensure subsequent logic that
computes status uses runId and tolerates a missing sandboxId.
In `@src/lib/slack/respond.ts`:
- Around line 26-30: The outbound fetch to Slack's response_url (the await
fetch(responseUrl, {...}) call) lacks a timeout—wrap the request with an
AbortController: create an AbortController, pass controller.signal into fetch,
start a setTimeout that calls controller.abort() after a reasonable bound (e.g.,
3–10s), and clear that timeout after fetch completes; update the same fetch
invocation in src/lib/slack/respond.ts (referencing responseUrl, payload, and
the const res variable) so errors from abort are handled (try/catch) and the
timeout is always cleared.
In `@src/routes/webhooks/slack.post.ts`:
- Around line 126-137: The deferred promise from runHandler(parsed, responseUrl)
can fail silently when using event.waitUntil; ensure you attach error logging in
both paths by passing a promise that has a catch handler to event.waitUntil (or
otherwise call promise.catch) so failures are logged via logger.error with the
same metadata/tag (e.g., "slack_handler_unhandled_error"); update the code
around runHandler, event.waitUntil, promise, and the existing promise.catch(...)
usage to uniformly handle/log errors.
---
Nitpick comments:
In `@src/routes/webhooks/slack.post.test.ts`:
- Around line 190-203: The test accesses postedToResponseUrl[0] without first
asserting the array length which can produce an unclear undefined access; before
any indexing of postedToResponseUrl in the "/ai-workflow cancel AWT-1 with no
entry..." test and the other affected test (around where cancelRunFn, handler,
makeRequest, flushDeferred are used), add an explicit assertion like
expect(postedToResponseUrl).toHaveLength(1) immediately after flushDeferred()
and before reading postedToResponseUrl[0] so the test fails with a clear message
if nothing was posted.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e5c0bc6-a187-4e15-b2b7-feae86bbea1c
📒 Files selected for processing (17)
.claude/skills/init-slack/SKILL.md.claude/skills/init-slack/references/slash-commands.mdREADME.mddocs/superpowers/plans/2026-05-01-slack-slash-commands.mdenv.test.tsenv.tssrc/lib/slack/commands.test.tssrc/lib/slack/commands.tssrc/lib/slack/format.test.tssrc/lib/slack/format.tssrc/lib/slack/handlers.test.tssrc/lib/slack/handlers.tssrc/lib/slack/respond.tssrc/lib/slack/verify.test.tssrc/lib/slack/verify.tssrc/routes/webhooks/slack.post.test.tssrc/routes/webhooks/slack.post.ts
| - Reinstall app (`commands` scope is already granted on most chat-adapter installs; verify). | ||
| - Copy the Signing Secret into Vercel env (`SLACK_SIGNING_SECRET`) for Production + Preview. | ||
|
|
||
| **Task 5.2:** Update `init-slack` skill (`/Users/kacper/.claude/skills/init-slack`) to also prompt for `SLACK_SIGNING_SECRET` and mention the slash-command URL. Add a one-paragraph operator note in `README.md` under the existing Slack section. |
There was a problem hiding this comment.
Replace machine-specific absolute path in docs.
At Line 123, /Users/kacper/... is not portable and will be broken for other contributors. Please switch to a repo-relative or generic path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-05-01-slack-slash-commands.md` at line 123, The
docs currently reference a machine-specific absolute path
"/Users/kacper/.claude/skills/init-slack" which is not portable; update the
Slack docs line to use a repo-relative or generic path (e.g.,
"./skills/init-slack" or "~/.claude/skills/init-slack") and ensure the mention
appears alongside the `init-slack` skill name, and add the one-paragraph
operator note under the existing Slack section in `README.md` referencing the
generic path instead of the absolute one.
Summary by CodeRabbit
New Features
Documentation