feat(twenty-slack): interactivity endpoint + native feedback buttons on assistant answers - #24107
feat(twenty-slack): interactivity endpoint + native feedback buttons on assistant answers#24107abdulrahmancodes wants to merge 13 commits into
Conversation
…s on assistant answers Add a slack-interactivity-resolver server route that verifies Slack's request signature, acknowledges unhandled interactions, and dispatches feedback block_actions to the resolved workspace. Assistant answers now end with Slack's native feedback_buttons element carrying the request record id, and a new slack-assistant-feedback function stores the Positive/Negative rating on a new feedbackRating select field of the slackAssistantRequest object. The app manifest enables interactivity with the new request URL.
|
👋 Thanks for contributing to Twenty! Your PR has been set to draft while you work on it. Once you're done, mark it as Ready for review and our automated checks will run. Looking forward to your contribution! |
Greptile SummaryThe PR adds Slack-native feedback controls to assistant answers and an authenticated interactivity route that stores the selected rating.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Sequence DiagramsequenceDiagram
participant User as Slack user
participant Slack
participant Resolver as Interactivity resolver
participant Queue as Server-route queue
participant Handler as Feedback handler
participant CRM as Workspace record API
User->>Slack: Click feedback button
Slack->>Resolver: Signed block_actions payload
Resolver->>Resolver: Verify signature and resolve workspace
Resolver->>Queue: Dispatch feedback payload
Resolver-->>Slack: HTTP 200
Queue->>Handler: Execute in target workspace
Handler->>CRM: Update assistant request rating
Reviews (2): Last reviewed commit: "fix(server): ack server route dispatches..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
✅ Standard review · no findings
High-level — Cohesive sub-1000-line additive Slack feedback-buttons feature that reuses existing seams (extracted shared webhook-verification util, teamId-narrowed resolveTargetWorkspaceId) and adds only an UPPER_CASE SELECT field with no migration, flag, or breaking change. Reviewed against the |
…oad path Validate the parsed interactivity payload with a type guard instead of an as cast, use isDefined for the feedback handler's nullish checks, drop a redundant === true in the resolver, and pin the feedback update mock to exactly one call in the handler tests.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-apps/public/slack/README.md">
<violation number="1" location="packages/twenty-apps/public/slack/README.md:13">
P3: The README states "Answers carry native thumbs up / thumbs down feedback buttons", but the assistant only attaches the feedback `context_actions` block when the answer fits within the markdown block limit. In the worker, `messageBlocks` is set to `undefined` (no buttons) whenever `responseText.length > SLACK_MARKDOWN_BLOCK_MAX_LENGTH` (12,000). Very long answers still deliver as plain markdown with no feedback UI, so the documentation overstates the guarantee. Qualify the claim, e.g. note that long answers keep plain text without buttons.</violation>
</file>
<file name="packages/twenty-apps/public/slack/src/logic-functions/__tests__/slack-assistant-feedback.test.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/__tests__/slack-assistant-feedback.test.ts:52">
P3: The `toHaveBeenCalledWith` assertion passes even if `updateSlackAssistantRequestFeedback` is called multiple times with matching args, so a regression causing duplicate writes wouldn't be caught. Add a `toHaveBeenCalledTimes(1)` assertion alongside `toHaveBeenCalledWith` for both the positive and negative rating tests.</violation>
</file>
<file name="packages/twenty-apps/public/slack/src/logic-functions/slack-interactivity-resolver.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/slack-interactivity-resolver.ts:50">
P2: When the interactivity body lacks a `payload` field or its JSON cannot be parsed, `parseSlackInteractivityPayload` throws and the handler surfaces a 5xx, which makes Slack retry the delivery. This contradicts the acknowledgement strategy used elsewhere in the same handler (200 `{ ok: true }` for unhandled actions) and the stated goal of not retrying junk payloads. Catch parse failures and return `new Response({ ok: true })` instead of throwing.</violation>
<violation number="2" location="packages/twenty-apps/public/slack/src/logic-functions/slack-interactivity-resolver.ts:64">
P1: When a user clicks a feedback button, this dispatch path responds with HTTP 202 instead of Slack’s required 200 acknowledgment. Slack can show an interaction failure and retry the action even though the feedback job was queued; make the interactivity route acknowledge with 200 after enqueueing.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
| return { | ||
| workspaceId: await resolveTargetWorkspaceId(payload.team?.id), |
There was a problem hiding this comment.
P1: When a user clicks a feedback button, this dispatch path responds with HTTP 202 instead of Slack’s required 200 acknowledgment. Slack can show an interaction failure and retry the action even though the feedback job was queued; make the interactivity route acknowledge with 200 after enqueueing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/logic-functions/slack-interactivity-resolver.ts, line 64:
<comment>When a user clicks a feedback button, this dispatch path responds with HTTP 202 instead of Slack’s required 200 acknowledgment. Slack can show an interaction failure and retry the action even though the feedback job was queued; make the interactivity route acknowledge with 200 after enqueueing.</comment>
<file context>
@@ -0,0 +1,81 @@
+ }
+
+ return {
+ workspaceId: await resolveTargetWorkspaceId(payload.team?.id),
+ targetLogicFunctionUniversalIdentifier:
+ SLACK_ASSISTANT_FEEDBACK_UNIVERSAL_IDENTIFIER,
</file context>
There was a problem hiding this comment.
Fixed in e6286cc, but at the platform level rather than here: a dispatching resolver has no way to set the response status — the 202 came from enqueueTargetFunction in server-route-trigger.service.ts. That ack is now 200, which Slack's interactivity docs require and every other webhook sender accepts. One nuance to the finding: Slack does not retry interaction payloads (retries are an Events API behavior), so the failure mode was a possible user-facing warning, not a duplicate action.
Generated by Claude Code
| throw new Error('Invalid Slack signature'); | ||
| } | ||
|
|
||
| const payload = parseSlackInteractivityPayload(routePayload.body); |
There was a problem hiding this comment.
P2: When the interactivity body lacks a payload field or its JSON cannot be parsed, parseSlackInteractivityPayload throws and the handler surfaces a 5xx, which makes Slack retry the delivery. This contradicts the acknowledgement strategy used elsewhere in the same handler (200 { ok: true } for unhandled actions) and the stated goal of not retrying junk payloads. Catch parse failures and return new Response({ ok: true }) instead of throwing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/public/slack/src/logic-functions/slack-interactivity-resolver.ts, line 50:
<comment>When the interactivity body lacks a `payload` field or its JSON cannot be parsed, `parseSlackInteractivityPayload` throws and the handler surfaces a 5xx, which makes Slack retry the delivery. This contradicts the acknowledgement strategy used elsewhere in the same handler (200 `{ ok: true }` for unhandled actions) and the stated goal of not retrying junk payloads. Catch parse failures and return `new Response({ ok: true })` instead of throwing.</comment>
<file context>
@@ -0,0 +1,81 @@
+ throw new Error('Invalid Slack signature');
+ }
+
+ const payload = parseSlackInteractivityPayload(routePayload.body);
+
+ const hasAssistantFeedbackAction =
</file context>
There was a problem hiding this comment.
Keeping the throw, deliberately. Slack does not retry interaction payloads on failure — the retry cycle is an Events API behavior — so a 5xx here cannot cause redelivery. And this branch is only reachable after signature verification: a signed request with a missing or unparseable payload means Slack's contract changed or something is wrong upstream, which should surface as an error rather than be swallowed by a 200. The ok: true ack is reserved for well-formed interactions we intentionally don't handle; the events resolver draws the same line by throwing on an empty verified body.
Generated by Claude Code
🔍 Automated Pre-Review✅ No issues detected - This PR is ready for human review. Automated pre-review — human approval still required. |
…vity Slack requires a plain HTTP 200 OK to acknowledge interactivity callbacks, and every webhook sender accepts 200, so the dispatch ack moves from 202 to 200. Also dedupe the SlackAssistantFeedbackRating type alias next to its constant and qualify the README feedback-buttons claim for long answers.
twenty-eng-sync
Bot
added
the
-PR: stale auto-closed
|
Auto-closed: this PR is draft and has had no update in over 24 hours. Please reopen it once you have the bandwidth to take it forward. |
abdulrahmancodes
removed
the
-PR: stale auto-closed
Move the feedback and interactivity resolver handlers into the existing handlers/ directory, extract SlackAssistantFeedbackRating into its own type file, and make SlackInteractionAction a local type so every new file has a single export.
parse-slack-assistant-feedback-rating and parse-slack-interactivity-payload are simple pure mappings whose behavior is already covered through the feedback handler tests; testing them directly added no signal.
twenty-ci-bot-public
Bot
added
external-needs-triage
and removed
external-needs-triage
labels
…rim the interactivity guard Extract verifySlackWebhookRequestOrThrow so the events and interactivity resolvers verify requests through one seam, and narrow the interactivity payload type and guard to the fields the handlers actually consume.
…and cover the interactivity parser
1 participant
Why
Assistant answers were one-way: a markdown block and a duration footer, with no way for the person who asked to say whether the answer was any good. The app also had no Slack interactivity surface at all — no request URL, no
block_actionshandling — so nothing interactive could be added to any message it posts.Closes twentyhq/core-team-issues#2767.
What it does
Interactivity endpoint. A new
slack-interactivity-resolverserver route (/webhooks/server/fd756b00-50a2-4816-a919-a1a959a2ed9a) receives Slack interactivity callbacks. It verifies the signature withSLACK_WEBHOOK_SECRETover the raw body, exactly like the events resolver, then parses the form-encodedpayloadfield and validates its shape with a type guard. Interactions it does not handle are acknowledged with a 200 so Slack neither retries nor shows the user a warning; feedback actions resolve the target workspace from the server-scoped team claim and dispatch to the handler there. The manifest enablesinteractivitywith the new request URL.Feedback buttons.
build-slack-assistant-answer-blocks.tsappends acontext_actionsblock with Slack's nativefeedback_buttonselement (thumbs up / thumbs down —@slack/web-api7.18 ships theFeedbackButtonsandContextActionsBlocktypes). The block'sblock_idcarries theslackAssistantRequestrecord id, so the interaction maps back to the record without any lookup by message timestamp. Long answers that already fall back to a plain markdown message (no blocks) keep that behavior and simply carry no buttons.Rating storage. A new
slack-assistant-feedbackfunction runs in the resolved workspace, maps thepositive_feedback/negative_feedbackbutton values to a newfeedbackRatingselect field (Positive / Negative) on theslackAssistantRequestobject, and updates the record. Repeated clicks overwrite — last click wins. Payloads without the feedback action, without a record id, or with an unknown value are skipped with a reason instead of erroring.Server dispatch ack is now 200 (was 202). Slack's interactivity docs require a plain HTTP 200 OK acknowledgment, and a dispatching resolver has no way to set the response status, so
enqueueTargetFunctioninserver-route-trigger.service.tsnow acks with 200. Every other webhook sender (including Slack's Events API, which accepts any 2xx) is unaffected beyond the code change; the integration spec is updated accordingly.resolveTargetWorkspaceIdnow takes the team id string instead of the whole events body, so both resolvers share it.New files follow one-export-per-file: the resolver and feedback entry files only default-export
defineLogicFunction, with their handlers in the existinghandlers/directory.Version
Bumped to 0.5.0 — 0.4.0 is claimed by #23985 and 0.3.1 by #24097, so the three compose in any merge order.
Existing Slack apps must enable Interactivity & Shortcuts with the new request URL (apps created from the updated manifest get it automatically), otherwise the buttons show a warning when clicked. No new scopes, so no re-authorization is needed. SETUP.md documents the step.
Screenshots