fix(slack): add missing QStash endpoint for DM messages#1411
Conversation
The event router was dispatching DM messages to /api/integrations/slack/jobs/process-assistant-message via QStash, but no route handler existed — every DM job hit a 404. Also adds resolveUserMentions to the DM handler to match the mention handler, so <@u12345> refs get resolved to display names.
📝 WalkthroughWalkthroughThe PR adds user mention resolution preprocessing to Slack message handling and introduces a new server route that validates Slack webhook requests via QStash signature verification before triggering message processing. Changes
Sequence Diagram(s)sequenceDiagram
actor Client as HTTP Client
participant Route as POST Route Handler
participant QStash as QStash Receiver
participant Processor as Process Assistant<br/>Message
participant Slack as Slack Agent
Client->>Route: POST /slack/jobs/process-assistant-message
Route->>QStash: Create Receiver & Validate Signature
alt Invalid Signature
QStash-->>Route: Signature Invalid
Route-->>Client: 401 Unauthorized
else Valid Signature
QStash-->>Route: Signature Valid
Route->>Route: Parse & Validate Payload (Zod)
alt Invalid Payload
Route-->>Client: 400 Bad Request
else Valid Payload
Route->>Processor: Invoke with event, teamId, eventId
Processor->>Processor: resolveUserMentions(event.text)
Processor->>Slack: runSlackAgent(resolvedText, ...)
Slack-->>Processor: Response
Processor-->>Route: Complete
Route-->>Client: { success: true }
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@apps/api/src/app/api/integrations/slack/jobs/process-assistant-message/route.ts`:
- Line 45: The code calls payloadSchema.safeParse(JSON.parse(body)) which lets
JSON.parse throw on malformed input; wrap the JSON.parse in a try-catch (or use
a safe JSON parse helper) before passing the result to payloadSchema.safeParse
and handle the parse error by returning a 400/invalid-body response and logging
the error; update the code around payloadSchema.safeParse, JSON.parse and body
in route.ts to first attempt a safe parse, then call
payloadSchema.safeParse(parsedBody).
- Around line 27-61: The POST handler should skip QStash signature verification
in development: introduce isDev (e.g., const isDev = env.NODE_ENV ===
"development") at the top of POST and wrap the existing signature checks and the
receiver.verify(...) call (and its signature presence check) in if (!isDev) {
... } so that in development the route proceeds without requiring
upstash-signature or calling receiver.verify, while keeping the payloadSchema
parsing and processAssistantMessage(...) call unchanged.
- Around line 35-43: receiver.verify throws a SignatureError on invalid
signatures so the existing isValid check is unreachable; wrap the call to
receiver.verify(...) in a try/catch inside route.ts (the handler around
receiver.verify), catch SignatureError (or any verification error) and return
Response.json({ error: "Invalid signature" }, { status: 401 }) from the catch
block to prevent a 500, otherwise proceed when verification succeeds.
🧹 Nitpick comments (2)
apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts (1)
67-73: Mention resolution integration looks correct.The two-phase pattern (batch-fetch user info, then apply resolution) mirrors the existing mention handler. One minor observation:
event.text ?? ""is evaluated twice — once to seed the resolver and once to apply it. Consider extracting to a local constant to avoid repetition.♻️ Optional: extract raw text to a local variable
+ const rawText = event.text ?? ""; const resolve = await resolveUserMentions({ - texts: [event.text ?? ""], + texts: [rawText], slack, }); const result = await runSlackAgent({ - prompt: resolve(event.text ?? ""), + prompt: resolve(rawText), channelId: event.channel,apps/api/src/app/api/integrations/slack/jobs/process-assistant-message/route.ts (1)
54-58: Unhandled rejection fromprocessAssistantMessagewould crash the route.If
processAssistantMessagethrows, the POST handler has no try-catch, so it will return a 500 to QStash. QStash will then retry the message, which could cause duplicate "Thinking…" messages and repeated failures. Consider wrapping in a try-catch to log the error and return a controlled response (or let it propagate intentionally if you want QStash retries — in which case, add a comment explaining that).♻️ Proposed: wrap in try-catch with logging
- await processAssistantMessage({ - event: { ...parsed.data.event, subtype: undefined }, - teamId: parsed.data.teamId, - eventId: parsed.data.eventId, - }); - - return Response.json({ success: true }); + try { + await processAssistantMessage({ + event: { ...parsed.data.event, subtype: undefined }, + teamId: parsed.data.teamId, + eventId: parsed.data.eventId, + }); + return Response.json({ success: true }); + } catch (err) { + console.error("[slack/process-assistant-message] Processing failed:", err); + return Response.json({ error: "Processing failed" }, { status: 500 }); + }
| export async function POST(request: Request) { | ||
| const body = await request.text(); | ||
| const signature = request.headers.get("upstash-signature"); | ||
|
|
||
| if (!signature) { | ||
| return Response.json({ error: "Missing signature" }, { status: 401 }); | ||
| } | ||
|
|
||
| const isValid = await receiver.verify({ | ||
| body, | ||
| signature, | ||
| url: `${env.NEXT_PUBLIC_API_URL}/api/integrations/slack/jobs/process-assistant-message`, | ||
| }); | ||
|
|
||
| if (!isValid) { | ||
| return Response.json({ error: "Invalid signature" }, { status: 401 }); | ||
| } | ||
|
|
||
| const parsed = payloadSchema.safeParse(JSON.parse(body)); | ||
| if (!parsed.success) { | ||
| console.error( | ||
| "[slack/process-assistant-message] Invalid payload:", | ||
| parsed.error, | ||
| ); | ||
| return Response.json({ error: "Invalid payload" }, { status: 400 }); | ||
| } | ||
|
|
||
| await processAssistantMessage({ | ||
| event: { ...parsed.data.event, subtype: undefined }, | ||
| teamId: parsed.data.teamId, | ||
| eventId: parsed.data.eventId, | ||
| }); | ||
|
|
||
| return Response.json({ success: true }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all route.ts files under jobs directories to identify QStash job handlers
echo "=== QStash Job Routes ==="
find . -path "*/jobs/*/route.ts" -type f | head -20
echo -e "\n=== Content of similar job routes ==="
find . -path "*/jobs/*/route.ts" -type f -exec sh -c 'echo "File: $1"; head -80 "$1"; echo "---"' _ {} \;Repository: superset-sh/superset
Length of output: 11813
🏁 Script executed:
#!/bin/bash
# Search for receiver.verify patterns to see how signature verification is done elsewhere
echo "=== Receiver.verify usages ==="
rg -n 'receiver\.verify' --type ts --type tsx -B2 -A5Repository: superset-sh/superset
Length of output: 139
🏁 Script executed:
#!/bin/bash
# Also search for other patterns of Upstash/QStash integration patterns
echo "=== QStash/Upstash patterns ==="
rg -n 'upstash|QStash|Receiver' --type ts --type tsx -l | head -20Repository: superset-sh/superset
Length of output: 140
Add development environment check to skip signature verification in development.
This route lacks the development environment check present in other QStash job handlers (github/initial-sync, linear/initial-sync). In development, QStash cannot reach localhost, making signature verification impossible. Wrap the receiver.verify() call in an isDev check:
const isDev = env.NODE_ENV === "development";
if (!isDev) {
if (!signature) {
return Response.json({ error: "Missing signature" }, { status: 401 });
}
const isValid = await receiver.verify({
body,
signature,
url: `${env.NEXT_PUBLIC_API_URL}/api/integrations/slack/jobs/process-assistant-message`,
});
if (!isValid) {
return Response.json({ error: "Invalid signature" }, { status: 401 });
}
}🤖 Prompt for AI Agents
In
`@apps/api/src/app/api/integrations/slack/jobs/process-assistant-message/route.ts`
around lines 27 - 61, The POST handler should skip QStash signature verification
in development: introduce isDev (e.g., const isDev = env.NODE_ENV ===
"development") at the top of POST and wrap the existing signature checks and the
receiver.verify(...) call (and its signature presence check) in if (!isDev) {
... } so that in development the route proceeds without requiring
upstash-signature or calling receiver.verify, while keeping the payloadSchema
parsing and processAssistantMessage(...) call unchanged.
| const isValid = await receiver.verify({ | ||
| body, | ||
| signature, | ||
| url: `${env.NEXT_PUBLIC_API_URL}/api/integrations/slack/jobs/process-assistant-message`, | ||
| }); | ||
|
|
||
| if (!isValid) { | ||
| return Response.json({ error: "Invalid signature" }, { status: 401 }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@upstash/qstash Receiver verify method throws invalid signature documentation
💡 Result:
The Upstash QStash docs indicate Receiver.verify(...) will fail (often surfacing as “Invalid signature”) when any input used for verification doesn’t exactly match what QStash signed, especially:
- You didn’t use the raw request body: many frameworks parse JSON into an object; re-stringifying it (e.g.
JSON.stringify(req.body)) can change spacing/key order and cause verification to fail. The docs explicitly warn to use the raw body string “as is.” [1] - The
urlyou pass doesn’t exactly match the JWTsubclaim (scheme/host/path). The manual verification example shows the verifier checksp.sub !== urland throws (subject mismatch). So passing only a host, missinghttps://, missing path, or using a different public URL than QStash is calling will fail verification. [2] - Wrong signing keys (or keys were rolled and your app didn’t update). QStash uses “current” and “next” signing keys; your receiver should verify with both. [1]
Helpful cross-check: QStash delivers the body unchanged (“passed as is”), so your handler must read it in that same raw form for hashing/verification. [3]
Sources: Upstash “Verify Signatures” doc (raw body + Receiver usage) [1], Upstash AWS Lambda quickstart (shows sub/url and body-hash checks) [2], Upstash “Receiving Messages” (body passed as-is) [3].
🌐 Web query:
@upstash/qstash Receiver.verify() return type throws error documentation examples
💡 Result:
In @upstash/qstash (TypeScript), Receiver.verify(...) is async and effectively works like:
- Return type:
Promise<boolean>(it resolves totruewhen verification succeeds). - Failure mode: it throws a
SignatureError(it does not returnfalsefor invalid signatures). The SDK throws on JWT verification errors, subject (url) mismatch, or body-hash mismatch. [1]
So usage patterns in docs/examples differ, but both are valid:
Pattern A: try/catch (recommended, since failures throw)
try {
await receiver.verify({ signature, body, url });
// verified
} catch (err) {
// invalid signature (SignatureError)
}Example shown in Upstash’s AWS Lambda quickstart. [2]
Pattern B: assign to isValid (works, but false is not expected)
const isValid = await receiver.verify({ signature, body, url });
if (!isValid) {
// typically unreachable; invalid cases throw instead
}Shown in Upstash docs and quickstarts. [3]
Docs/examples: Receiver verify examples are in the “Verify Signatures” doc and the Receiver SDK example page. [3]
Sources:
[1] UNPKG compiled source for @upstash/qstash showing SignatureError and verify() implementation/behavior.
[2] Upstash AWS Lambda (Node) quickstart demonstrating try/catch around verify().
[3] Upstash docs pages showing const isValid = await receiver.verify(...) usage.
receiver.verify() throws on invalid signatures — the isValid check is unreachable.
The QStash Receiver.verify() method throws a SignatureError when the signature is invalid rather than returning false. This means the if (!isValid) block is dead code, and an invalid signature will produce an unhandled exception (500 error) instead of the intended 401 response. Wrap the verify call in a try-catch to properly handle verification failures.
Proposed fix
- const isValid = await receiver.verify({
- body,
- signature,
- url: `${env.NEXT_PUBLIC_API_URL}/api/integrations/slack/jobs/process-assistant-message`,
- });
-
- if (!isValid) {
- return Response.json({ error: "Invalid signature" }, { status: 401 });
- }
+ try {
+ await receiver.verify({
+ body,
+ signature,
+ url: `${env.NEXT_PUBLIC_API_URL}/api/integrations/slack/jobs/process-assistant-message`,
+ });
+ } catch (err) {
+ console.error("[slack/process-assistant-message] Invalid QStash signature:", err);
+ return Response.json({ error: "Invalid signature" }, { status: 401 });
+ }🤖 Prompt for AI Agents
In
`@apps/api/src/app/api/integrations/slack/jobs/process-assistant-message/route.ts`
around lines 35 - 43, receiver.verify throws a SignatureError on invalid
signatures so the existing isValid check is unreachable; wrap the call to
receiver.verify(...) in a try/catch inside route.ts (the handler around
receiver.verify), catch SignatureError (or any verification error) and return
Response.json({ error: "Invalid signature" }, { status: 401 }) from the catch
block to prevent a 500, otherwise proceed when verification succeeds.
| return Response.json({ error: "Invalid signature" }, { status: 401 }); | ||
| } | ||
|
|
||
| const parsed = payloadSchema.safeParse(JSON.parse(body)); |
There was a problem hiding this comment.
JSON.parse(body) can throw on malformed input, resulting in an unhandled 500.
If QStash delivers a corrupted or non-JSON body that passes signature verification, JSON.parse will throw before the Zod validation can catch the issue. Wrap it or use a safe parse.
🛡️ Proposed fix — combine with the Zod parse in a single try-catch
- const parsed = payloadSchema.safeParse(JSON.parse(body));
- if (!parsed.success) {
+ let jsonBody: unknown;
+ try {
+ jsonBody = JSON.parse(body);
+ } catch {
+ return Response.json({ error: "Invalid JSON" }, { status: 400 });
+ }
+
+ const parsed = payloadSchema.safeParse(jsonBody);
+ if (!parsed.success) {📝 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.
| const parsed = payloadSchema.safeParse(JSON.parse(body)); | |
| let jsonBody: unknown; | |
| try { | |
| jsonBody = JSON.parse(body); | |
| } catch { | |
| return Response.json({ error: "Invalid JSON" }, { status: 400 }); | |
| } | |
| const parsed = payloadSchema.safeParse(jsonBody); | |
| if (!parsed.success) { |
🤖 Prompt for AI Agents
In
`@apps/api/src/app/api/integrations/slack/jobs/process-assistant-message/route.ts`
at line 45, The code calls payloadSchema.safeParse(JSON.parse(body)) which lets
JSON.parse throw on malformed input; wrap the JSON.parse in a try-catch (or use
a safe JSON parse helper) before passing the result to payloadSchema.safeParse
and handle the parse error by returning a 400/invalid-body response and logging
the error; update the code around payloadSchema.safeParse, JSON.parse and body
in route.ts to first attempt a safe parse, then call
payloadSchema.safeParse(parsedBody).
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
jobs/process-assistant-message/route.tsQStash handler — the event router was dispatching DM messages to this endpoint, but no route existed (404)resolveUserMentionsto the DM handler to match the mention handler patternTest plan
@mention, verify the user's display name appears in the agent's promptSummary by CodeRabbit
Bug Fixes
New Features