You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat: post review requests to slack with approve and reject buttons (CM-1841) - #4919
PR 6 of the onboarding-core series (after #4908, #4909, #4910, #4913, #4914, #4916). Rows routed to review by the evaluation worker now land in a Slack channel as a card with Approve / Reject buttons, and the decision is recorded on the projectCatalog row.
@crowd/slack
New reviewRequest.ts: buildReviewRequestBlocks(card) (Block Kit: repo, project name, origin link, classification kind, proposed action, two buttons with a confirm dialog, value = catalog id), buildReviewDecisionMessage(card, decision, reviewedBy) (same summary, buttons replaced by the outcome) and postReviewRequest(card) (bot token, posts to the review channel).
getSlackBotConfig() also reads CROWD_SLACK_REVIEW_CHANNEL_ID.
Action ids approve_review / reject_review live here so worker and backend share them.
Evaluation worker
classifyAndRouteProjects collects rows routed to review that have no reviewMessageTs yet (reviewRequests), so a retried batch or a row routed by a previous attempt is requested once.
New activity notifyReviewRequest(project): posts the card and stores the message ts with setProjectCatalogReviewMessageTs. Runs with the notify proxy (1 attempt, no retry, same reasoning as CM-1791); a failure is logged and the row stays in review, so the next run requests it again.
Backend
interactivity.ts now handles approve_review / reject_review alongside force_onboarding; the TODO(CM-1791) warning branch is gone. The handler is a factory (createInteractivityHandler(temporal)), mounted with the Temporal client like the events route.
New reviewDecisionCommand.ts: validates the catalog id, resolveProjectCatalogReview (review → onboard / review → skip, guarded on action = 'review'), then replaces the card through response_url with the decision. On approve it starts onboardProjects for that id right away (onboard-review-<catalogId>, REJECT_DUPLICATE, already-started tolerated); if Temporal is not configured the row waits for the 08:00 schedule.
postToResponseUrl accepts the decision message type next to the block-builder DTO.
Not in scope
/crowd onboard-project, force onboarding, runPrecheck.ts.
Thread replies in the bot channel (PR 8).
Deployment notes
Add CROWD_SLACK_REVIEW_CHANNEL_ID (channel id, the bot must be a member) to the projects_evaluation_worker deployment. Without it the activity logs a warning and nothing is posted.
services/libs/slack: block builder, decision message, postReviewRequest without channel / with channel
projects_evaluation_worker: reviewRequests populated for new review rows, not for rows with a message, populated for previous-attempt rows without a message
backend: interactivity dispatch for both buttons, failure replacement, missing user, unknown action; review decision command (approve starts workflow, reject does not, already-started tolerated, no Temporal, row no longer in review, invalid id)
Staging: evaluate a known LF repo via the bot (expect card in the review channel), click Approve (expect card replaced and onboardProjects started), click Reject on another (expect skip, skipReason = rejected in review by <user>)
Medium Risk
Changes project-catalog lifecycle and can start onboardProjects from Slack clicks; deployment depends on CROWD_SLACK_REVIEW_CHANNEL_ID and coordinating with follow-up onboarding behavior in prod.
Overview
Adds a Slack human-review loop for projectCatalog rows in review: the evaluation worker posts Block Kit cards (Approve/Reject) to a configured review channel, and button clicks record the decision and update the message.
Worker & data layer: After classification, the workflow fetches rows in review with no reviewMessageTs, posts via postReviewRequest, and persists the message ts inside a transaction with lockProjectCatalogReviewRequest so duplicate cards are avoided. setProjectCatalogReviewMessageTs now only sets when still null.
@crowd/slack: New reviewRequest helpers (card blocks, decision replacement message, postReviewRequest), shared action ids, escapeSlackText in mrkdwn, and CROWD_SLACK_REVIEW_CHANNEL_ID on bot config.
Backend: Slack interactivity is a createInteractivityHandler(temporal) factory wired with the Temporal client; approve_review / reject_review call runReviewDecisionCommand, which validates the catalog id, runs resolveProjectCatalogReview (review → onboard or skip), replaces the card via response_url, and on approve starts onboardProjects immediately (or leaves the row for the schedule if Temporal is missing or workflow start fails).
Reviewed by Cursor Bugbot for commit 4c89ecb. Bugbot is set up for automated code reviews on this repo. Configure here.
Adds Slack-based human review for project-catalog onboarding decisions, connecting the evaluation worker, shared Slack library, backend interactivity, and Temporal onboarding.
Changes:
Posts review cards with Approve/Reject actions.
Records decisions and dispatches approved onboarding.
Failed review posts never retried (activities.ts / evaluateProjects.ts): the workflow now calls a new fetchReviewRequests activity backed by findProjectCatalogReviewWithoutMessage (action = 'review' AND "reviewMessageTs" IS NULL), so a row whose Slack post failed is picked up again on the next run (or by the bot run for its own ids). reviewRequests was dropped from the classify result. setProjectCatalogReviewMessageTs is now also guarded on "reviewMessageTs" IS NULL and the activity warns when 0 rows were updated.
Temporal start failure after the row is already onboard (reviewDecisionCommand.ts): startApprovedOnboarding is best-effort, logs the error and returns whether the workflow started; the row waits for the 08:00 schedule.
"onboarding started" wording (reviewRequest.ts): buildReviewDecisionMessage takes { onboardingStarted } and says "queued for the next onboarding run" when Temporal is missing or the start failed.
Using a permanent workflow ID with REJECT_DUPLICATE prevents immediate onboarding on a later review cycle for the same catalog row. Rows that fail onboarding can be re-queued and classified for review again, but the completed workflow from the first approval makes this start throw; the catch then reports onboarding started even though no new run exists. Allow reuse after a closed execution (duplicate clicks are already rejected by the guarded review update), or include the review-cycle identity in the workflow ID.
Prevent duplicate Slack cards with atomic notification claiming
This activity can create multiple actionable cards for one catalog row. Posting to Slack happens before the guarded timestamp write, so a process failure after the post leaves the row eligible for the next run; overlapping scheduled and ad-hoc evaluation workflows can also both read the null timestamp and post before either write wins. The maximumAttempts: 1 setting only prevents an activity retry and does not prevent these later fetches. Claim the notification atomically before the external side effect (with a recoverable lease/outbox state), then finalize it with the Slack timestamp.
Escape untrusted project names before rendering Slack mrkdwn
services/libs/slack/src/reviewRequest.ts:51
projectName is accepted from API callers and is inserted directly into a mrkdwn block. A value such as <!channel> or <@U123> will be interpreted by Slack and can trigger unwanted mentions in the review channel. Escape &, <, and > before rendering this and other untrusted card text; the existing classification-alert path already applies this escaping in services/libs/project-onboarding/src/requestClassificationAlert.ts:29-30.
Duplicate review cards (projectCatalog.ts, activities.ts): notifyReviewRequest now runs in a writer transaction that takes SELECT … FOR UPDATE on the row (lockProjectCatalogReviewRequest, only when action = 'review' AND reviewMessageTs IS NULL) before posting; a concurrent run blocks, re-reads the stamped ts and skips. A failed post leaves the row unclaimed for the next run.
Unescaped mrkdwn (reviewRequest.ts): escapeSlackText moved to @crowd/slack and applied to repo name, repo url, project name, the non-Slack origin label and the confirm dialogs; regression test with <!channel>.
Workflow id reuse (reviewDecisionCommand.ts): the id is now onboard-review-<catalogId>-<reviewedAt ms>, so a row re-queued after an error and approved again starts a new workflow; the already-started branch only covers a same-cycle retry.
The row lock cannot make the Slack side effect and the database write atomic. If Slack accepts the post but the request times out, the activity is cancelled, or the transaction later fails, reviewMessageTs remains null and the next run posts a duplicate card. Use a Slack-side idempotency key or a durable posting/outbox state with reconciliation rather than relying on a transaction around the network call.
Addressed the transient-failure comment: an unexpected error while recording the decision now posts an ephemeral "please try again" message via the response url without replace_original, so the card with Approve/Reject stays in place (postReviewDecisionFailure). Test added.
A concurrent second click reaches this branch after the first reviewer changes the row, then replaces the same original card with the generic message. If that response arrives last, it erases the winning Approved/Rejected outcome. Return a non-replacing/ephemeral response for the losing click, or rebuild the recorded decision instead of replacing the card.
Make Slack side effects idempotent across transaction failures
The external Slack side effect happens before the database transaction commits. If the timestamp update or commit fails after Slack accepts the message, reviewMessageTs remains null and the next run posts a duplicate, so the activity is not idempotent despite the once-only routing goal. Use a durable outbox/reconciliation mechanism or an externally idempotent request key rather than relying only on the row lock.
Encode sourceUrl before constructing Slack links
services/libs/slack/src/reviewRequest.ts:36
sourceUrl is interpolated into Slack's <url|label> grammar without encoding the destination. A URL containing <, >, or | can terminate or corrupt the link (the new test even supplies angle brackets but expects them raw). Validate and encode the destination before constructing the mrkdwn link, and update the escaping test accordingly.
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
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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Summary
PR 6 of the onboarding-core series (after #4908, #4909, #4910, #4913, #4914, #4916). Rows routed to
reviewby the evaluation worker now land in a Slack channel as a card with Approve / Reject buttons, and the decision is recorded on theprojectCatalogrow.@crowd/slackreviewRequest.ts:buildReviewRequestBlocks(card)(Block Kit: repo, project name, origin link, classification kind, proposed action, two buttons with a confirm dialog,value= catalog id),buildReviewDecisionMessage(card, decision, reviewedBy)(same summary, buttons replaced by the outcome) andpostReviewRequest(card)(bot token, posts to the review channel).getSlackBotConfig()also readsCROWD_SLACK_REVIEW_CHANNEL_ID.approve_review/reject_reviewlive here so worker and backend share them.Evaluation worker
classifyAndRouteProjectscollects rows routed toreviewthat have noreviewMessageTsyet (reviewRequests), so a retried batch or a row routed by a previous attempt is requested once.notifyReviewRequest(project): posts the card and stores the messagetswithsetProjectCatalogReviewMessageTs. Runs with thenotifyproxy (1 attempt, no retry, same reasoning as CM-1791); a failure is logged and the row stays inreview, so the next run requests it again.Backend
interactivity.tsnow handlesapprove_review/reject_reviewalongsideforce_onboarding; theTODO(CM-1791)warning branch is gone. The handler is a factory (createInteractivityHandler(temporal)), mounted with the Temporal client like the events route.reviewDecisionCommand.ts: validates the catalog id,resolveProjectCatalogReview(review → onboard/review → skip, guarded onaction = 'review'), then replaces the card throughresponse_urlwith the decision. On approve it startsonboardProjectsfor that id right away (onboard-review-<catalogId>,REJECT_DUPLICATE, already-started tolerated); if Temporal is not configured the row waits for the 08:00 schedule.postToResponseUrlaccepts the decision message type next to the block-builder DTO.Not in scope
/crowd onboard-project, force onboarding,runPrecheck.ts.Deployment notes
CROWD_SLACK_REVIEW_CHANNEL_ID(channel id, the bot must be a member) to the projects_evaluation_worker deployment. Without it the activity logs a warning and nothing is posted.onboardProjects, which runs the proposed action only with feat: run the proposed action in the automatic onboarding worker (CM-1841) #4918 (PR 7). Merge feat: run the proposed action in the automatic onboarding worker (CM-1841) #4918 before enabling the channel in prod, otherwise LF rows (create_integration/update_integration) would go through the non-LF path.Test plan
pnpm tsc-check,pnpm lint --deny-warnings --format=agentservices/libs/slack: block builder, decision message,postReviewRequestwithout channel / with channelprojects_evaluation_worker:reviewRequestspopulated for new review rows, not for rows with a message, populated for previous-attempt rows without a messagebackend: interactivity dispatch for both buttons, failure replacement, missing user, unknown action; review decision command (approve starts workflow, reject does not, already-started tolerated, no Temporal, row no longer in review, invalid id)onboardProjectsstarted), click Reject on another (expectskip,skipReason = rejected in review by <user>)🤖 Generated with Claude Code