From 17fcda3e8741dc6daef4bac5696e7257a22656b6 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 16 Jul 2026 16:49:20 -0500 Subject: [PATCH 1/5] [Fix] Auto-confirm an unanswered Discord routing card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Discord routing card had no timeout. Ask someone where to run a task, get distracted, and 15 minutes later the pending route expired with its Redis TTL — no launch, no message, the request simply gone. Slack auto-confirms its suggestion after 30s and Telegram after 5s; only Discord dropped the request. Reuse Slack's mechanism rather than inventing a third: the shared constant is renamed SLACK_AUTO_CONFIRM_TIMEOUT_MS -> ROUTING_AUTO_CONFIRM_TIMEOUT_MS, which is what `block-kit.ts` already called it in prose, and Discord imports the same 30s. The timer is in-process like Slack's and Telegram's, so a restart loses it and the route falls back to expiring. Claiming the pending route is atomic, so a click and the timer cannot both launch. Only a suggestion is auto-confirmed. The card previously announced "The best match is X" using the first option even on a fallback, where options are merely alphabetical and the router had no opinion — so it named a best match it had not chosen. The suggested option is now located in the rendered list and stored, the card only claims a match when one exists, and a fallback card auto-confirms nothing. `launchDiscordTask`'s `replaceMessage` now describes a message rather than an interaction, since an auto-confirm has no interaction to answer through: a click is still answered via its token, while the timer edits the card by id. Both go through `replaceOrPostDiscordMessage`, the analog of Slack's `postOrReplaceSlackMessage`, which falls back to posting when the card is gone. Co-Authored-By: Claude Opus 4.8 --- .../__tests__/routing-confirmation.test.ts | 194 ++++++++++++++++++ .../discord/__tests__/task-launch.test.ts | 36 ++-- apps/api/src/handlers/discord/replies.ts | 66 ++++++ .../handlers/discord/routing-confirmation.ts | 154 +++++++++++++- apps/api/src/handlers/discord/task-launch.ts | 26 +-- .../channel-auto-start-unlinked.test.ts | 2 +- .../message-entry-unmentioned-routing.test.ts | 2 +- .../handlers/slack/events/message-entry.ts | 4 +- .../cloud-agents/src/server/router/index.ts | 2 +- .../cloud-agents/src/server/router/types.ts | 2 +- 10 files changed, 437 insertions(+), 51 deletions(-) diff --git a/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts b/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts index 761e78987..c5ee67fdd 100644 --- a/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts +++ b/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts @@ -15,6 +15,7 @@ const mocks = vi.hoisted(() => ({ vi.mock('@roomote/cloud-agents/server', () => ({ getAvailableEnvironments: mocks.getAvailableEnvironments, getTaskUrl: mocks.getTaskUrl, + ROUTING_AUTO_CONFIRM_TIMEOUT_MS: 30_000, })); vi.mock('@roomote/sdk/server', () => ({ @@ -349,6 +350,199 @@ describe('Discord routing confirmation', () => { }); }); + it('auto-confirms the suggestion when the card goes unanswered', async () => { + // Slack and Telegram both auto-confirm; without this a Discord card just + // expires with its TTL and the request is silently lost. + vi.useFakeTimers(); + try { + mocks.reserveAnchoredThread.mockResolvedValue({ + channelId: 'message-1', + parentChannelId: 'channel-1', + name: 'Fix matchmaking', + kind: 'thread', + messageId: 'message-1', + }); + mocks.reply.mockResolvedValue({ messageId: 'card-1' }); + + await requestDiscordRoutingConfirmation({ + provider: {} as never, + applicationId: 'app-1', + requesterDiscordUserId: 'discord-user-1', + launchOwnerUserId: 'user-1', + queuedMessage: { + provider: 'discord', + text: 'Fix matchmaking', + user: 'Matt', + userId: 'user-1', + ts: 'message-1', + }, + metadata: { + communicationProvider: 'discord', + communicationChannelId: 'channel-1', + communicationMessageId: 'message-1', + communicationAnchorMessageId: 'message-1', + }, + channel: { + channelId: 'channel-1', + channelName: 'general', + channelType: 0, + guildId: 'guild-1', + isDirectMessage: false, + isThread: false, + }, + routingDecision: { + status: 'routed', + result: { + workspace: { + type: 'environment', + id: 'env-1', + name: 'Sunny Acres', + }, + reasoning: 'likely', + debug: { + phase: 'direct', + toolsUsed: [], + needsExternalLookup: false, + confidence: 0.7, + }, + }, + }, + }); + + // The card names the workspace it will fall back to, and says when. + expect(mocks.reply.mock.lastCall?.[0]?.text).toBe( + 'Where should I run this? The best match is **Sunny Acres** — starting in ~30s.', + ); + // The card id is only knowable after posting, so the route is re-stored. + const stored = JSON.parse(mocks.redisSet.mock.lastCall?.[1] as string); + expect(stored).toMatchObject({ + suggestedIndex: 0, + cardMessageId: 'card-1', + }); + expect(mocks.launchTask).not.toHaveBeenCalled(); + + mocks.redisGetdel.mockResolvedValue(JSON.stringify(stored)); + await vi.advanceTimersByTimeAsync(30_000); + + // No interaction — nobody clicked — so the card is replaced by id. + expect(mocks.launchTask).toHaveBeenCalledWith( + expect.objectContaining({ + replaceMessage: { + channel: expect.objectContaining({ channelId: 'message-1' }), + messageId: 'card-1', + }, + }), + ); + } finally { + vi.useRealTimers(); + } + }); + + it('never auto-confirms a card the router had no suggestion for', async () => { + // A fallback card is a plain menu. Auto-confirming its first option would + // launch an alphabetical accident nobody chose. + vi.useFakeTimers(); + try { + await requestDiscordRoutingConfirmation({ + provider: {} as never, + applicationId: 'app-1', + requesterDiscordUserId: 'discord-user-1', + launchOwnerUserId: 'user-1', + queuedMessage: { + provider: 'discord', + text: 'Fix matchmaking', + user: 'Matt', + userId: 'user-1', + ts: 'message-1', + }, + metadata: { + communicationProvider: 'discord', + communicationChannelId: 'channel-1', + communicationMessageId: 'message-1', + }, + channel: { + channelId: 'channel-1', + channelName: 'general', + channelType: 0, + guildId: 'guild-1', + isDirectMessage: false, + isThread: false, + }, + routingDecision: { status: 'fallback', reason: 'ambiguous' }, + }); + + expect(mocks.reply.mock.lastCall?.[0]?.text).toBe( + 'Where should I run this?', + ); + expect( + JSON.parse(mocks.redisSet.mock.lastCall?.[1] as string).suggestedIndex, + ).toBeNull(); + + await vi.advanceTimersByTimeAsync(60_000); + expect(mocks.launchTask).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it('does not auto-confirm a route the requester already answered', async () => { + // The click claims the route atomically, so the timer finds nothing. + vi.useFakeTimers(); + try { + mocks.reserveAnchoredThread.mockResolvedValue(null); + await requestDiscordRoutingConfirmation({ + provider: {} as never, + applicationId: 'app-1', + requesterDiscordUserId: 'discord-user-1', + launchOwnerUserId: 'user-1', + queuedMessage: { + provider: 'discord', + text: 'Fix matchmaking', + user: 'Matt', + userId: 'user-1', + ts: 'message-1', + }, + metadata: { + communicationProvider: 'discord', + communicationChannelId: 'channel-1', + communicationMessageId: 'message-1', + }, + channel: { + channelId: 'channel-1', + channelName: 'general', + channelType: 0, + guildId: 'guild-1', + isDirectMessage: false, + isThread: false, + }, + routingDecision: { + status: 'routed', + result: { + workspace: { + type: 'environment', + id: 'env-1', + name: 'Sunny Acres', + }, + reasoning: 'likely', + debug: { + phase: 'direct', + toolsUsed: [], + needsExternalLookup: false, + confidence: 0.7, + }, + }, + }, + }); + + mocks.redisGetdel.mockResolvedValue(null); + await vi.advanceTimersByTimeAsync(30_000); + + expect(mocks.launchTask).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + it('keeps every stored route visible within Discord action-row limits', async () => { mocks.getAvailableEnvironments.mockResolvedValue( Array.from({ length: 6 }, (_, index) => ({ diff --git a/apps/api/src/handlers/discord/__tests__/task-launch.test.ts b/apps/api/src/handlers/discord/__tests__/task-launch.test.ts index 31a014e93..a7f0d8b24 100644 --- a/apps/api/src/handlers/discord/__tests__/task-launch.test.ts +++ b/apps/api/src/handlers/discord/__tests__/task-launch.test.ts @@ -695,15 +695,17 @@ describe('launchDiscordTask', () => { }, workspace: { repoForPayload: 'acme/repo', workspaceDisplayName: 'Acme' }, replaceMessage: { - applicationId: 'app-1', interaction: { - id: 'interaction-1', - application_id: 'app-1', - type: 3, - token: 'token-1', - channel_id: 'message-1', - } as never, - interactionDeferred: true, + applicationId: 'app-1', + interaction: { + id: 'interaction-1', + application_id: 'app-1', + type: 3, + token: 'token-1', + channel_id: 'message-1', + } as never, + interactionDeferred: true, + }, channel: { channelId: 'message-1', channelName: 'Fix tests', @@ -788,15 +790,17 @@ describe('launchDiscordTask', () => { }, workspace: { repoForPayload: 'acme/repo', workspaceDisplayName: 'Acme' }, replaceMessage: { - applicationId: 'app-1', interaction: { - id: 'interaction-1', - application_id: 'app-1', - type: 3, - token: 'token-1', - channel_id: 'message-1', - } as never, - interactionDeferred: true, + applicationId: 'app-1', + interaction: { + id: 'interaction-1', + application_id: 'app-1', + type: 3, + token: 'token-1', + channel_id: 'message-1', + } as never, + interactionDeferred: true, + }, channel: { channelId: 'message-1', channelName: 'Fix tests', diff --git a/apps/api/src/handlers/discord/replies.ts b/apps/api/src/handlers/discord/replies.ts index d096a2c1b..0ecd05ea8 100644 --- a/apps/api/src/handlers/discord/replies.ts +++ b/apps/api/src/handlers/discord/replies.ts @@ -1,7 +1,9 @@ import type { DiscordInteraction } from '@roomote/communication/discord-event'; import { DiscordApiError, + isDiscordUnknownMessageError, type CommunicationMessageButton, + type CommunicationPostMessageResult, type DiscordCommunicationProvider, } from '@roomote/communication'; @@ -13,6 +15,70 @@ type DiscordInteractionReplyContext = { interactionDeferred: boolean; }; +/** + * An already-posted message to rewrite in place, addressed the only way that + * works for its origin: an interaction still owes its clicker a response and + * must be answered through its token, while a message nobody is waiting on is + * edited by id. + */ +export type DiscordMessageToReplace = { + channel: DiscordChannelContext; + interaction?: DiscordInteractionReplyContext & { applicationId: string }; + messageId?: string; +}; + +/** + * Discord's analog of Slack's `postOrReplaceSlackMessage`: turn an existing + * message into this one, or post it fresh when there is nothing to replace or + * the original is gone. + */ +export async function replaceOrPostDiscordMessage(input: { + provider: DiscordCommunicationProvider; + replace: DiscordMessageToReplace; + text: string; + buttons?: CommunicationMessageButton[][]; +}): Promise { + const { channel } = input.replace; + const message = { + text: input.text, + ...(input.buttons ? { buttons: input.buttons } : {}), + }; + if (input.replace.interaction) { + return replyToDiscordEvent({ + provider: input.provider, + applicationId: input.replace.interaction.applicationId, + channel, + interaction: input.replace.interaction, + ...message, + }); + } + const channelId = channel.parentChannelId ?? channel.channelId; + if (input.replace.messageId) { + try { + await input.provider.editMessage({ + channelId, + messageId: input.replace.messageId, + ...message, + }); + return { + provider: 'discord', + channelId, + messageId: input.replace.messageId, + ...(channel.parentChannelId ? { threadId: channel.channelId } : {}), + }; + } catch (error) { + // Someone deleted the card. Posting is the only way left to say the task + // started, and it still needs its cancel control. + if (!isDiscordUnknownMessageError(error)) throw error; + } + } + return input.provider.postMessage({ + channelId, + ...(channel.parentChannelId ? { threadId: channel.channelId } : {}), + ...message, + }); +} + export async function replyToDiscordEvent(input: { provider: DiscordCommunicationProvider; applicationId: string; diff --git a/apps/api/src/handlers/discord/routing-confirmation.ts b/apps/api/src/handlers/discord/routing-confirmation.ts index 4bd0ed232..fc6efffb8 100644 --- a/apps/api/src/handlers/discord/routing-confirmation.ts +++ b/apps/api/src/handlers/discord/routing-confirmation.ts @@ -3,6 +3,7 @@ import { randomBytes } from 'node:crypto'; import { getAvailableEnvironments, getTaskUrl, + ROUTING_AUTO_CONFIRM_TIMEOUT_MS, type RoutingDecision, type RoutingWorkspace, } from '@roomote/cloud-agents/server'; @@ -16,6 +17,7 @@ import { findDiscordMappedUserId } from '@roomote/sdk/server'; import type { QueuedCommunicationMessage } from '@roomote/types'; import type { DiscordEventCommunicationMetadata } from '@roomote/communication/discord-event'; +import { apiLogger } from '../../logging.js'; import { findCommunicationTaskRunBySourceEvent } from '../tasks/communication-task-run-lookup.js'; import { replyToDiscordEvent } from './replies.js'; import { @@ -53,7 +55,14 @@ type PendingDiscordRoute = { * using `channel`, so it still resolves the same thread parent. */ cardChannel?: DiscordChannelContext; + /** The card itself, so an auto-confirm can turn it into the acknowledgement. */ + cardMessageId?: string; options: PendingRouteOption[]; + /** + * Which option the router actually suggested, or null when it had no opinion + * and the card is a plain menu. Only a suggestion may be auto-confirmed. + */ + suggestedIndex: number | null; forceNewThread?: boolean; }; @@ -132,6 +141,31 @@ async function buildOptions( : [...environmentOptions, allRepositories]; } +function sameWorkspace( + left: RoutingWorkspace, + right: RoutingWorkspace, +): boolean { + return left.type === 'environment' && right.type === 'environment' + ? left.id === right.id + : left.type === right.type; +} + +/** + * The router's pick, located in the rendered options. A suggestion the router + * named can vanish before the card is built, so this never assumes it is the + * first option. + */ +function findSuggestedIndex( + options: PendingRouteOption[], + suggested: RoutingWorkspace | null, +): number | null { + if (!suggested) return null; + const index = options.findIndex((option) => + sameWorkspace(option.workspace, suggested), + ); + return index < 0 ? null : index; +} + function routeButtons(id: string, options: PendingRouteOption[]) { return [ ...options.map((option, index) => [ @@ -162,6 +196,81 @@ function taskThreadChannelContext(input: { }; } +/** + * Launches the option the router suggested when nobody answers the card, so a + * request cannot die of inattention. Slack and Telegram both do this; without + * it a Discord card just expires with its TTL and the request is silently lost. + */ +async function autoConfirmDiscordRouting(input: { + provider: DiscordCommunicationProvider; + applicationId: string; + pendingRouteId: string; +}): Promise { + // Claiming is atomic, so a click landing at the same moment wins one of the + // two paths and the other finds nothing. + const pending = await claimPendingRoute(input.pendingRouteId); + if (!pending || pending.suggestedIndex === null) return; + const option = pending.options[pending.suggestedIndex]; + if (!option) return; + const existingRun = await findCommunicationTaskRunBySourceEvent({ + provider: 'discord', + sourceEventId: pending.queuedMessage.ts, + }); + if (existingRun) return; + const workspace = await resolveDiscordWorkspace(option.workspace); + if (!workspace) { + await input.provider + .postMessage({ + channelId: + pending.cardChannel?.parentChannelId ?? pending.channel.channelId, + ...(pending.cardChannel + ? { threadId: pending.cardChannel.channelId } + : {}), + text: 'Could not start the task — the suggested workspace is no longer available. Send the request again.', + }) + .catch(() => undefined); + return; + } + await launchDiscordTask({ + provider: input.provider, + launchOwnerUserId: pending.launchOwnerUserId, + queuedMessage: pending.queuedMessage, + metadata: pending.metadata, + channel: pending.channel, + workspace, + ...(pending.forceNewThread ? { forceNewThread: true } : {}), + // No interaction here — nobody clicked. The card is edited by id. + ...(pending.cardChannel && pending.cardMessageId + ? { + replaceMessage: { + channel: pending.cardChannel, + messageId: pending.cardMessageId, + }, + } + : {}), + }); +} + +function scheduleDiscordRoutingAutoConfirm(input: { + provider: DiscordCommunicationProvider; + applicationId: string; + pendingRouteId: string; +}): void { + const timer = setTimeout(() => { + autoConfirmDiscordRouting(input).catch((error) => { + apiLogger.warn( + `[discord] Routing auto-confirm failed for ${input.pendingRouteId}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + }); + }, ROUTING_AUTO_CONFIRM_TIMEOUT_MS); + + // Like Slack's and Telegram's, the timer is in-process: a restart loses it + // and the pending route simply expires with its Redis TTL. + timer.unref?.(); +} + export function shouldAutoConfirmDiscordRoute( decision: RoutingDecision, ): boolean { @@ -189,11 +298,12 @@ export async function requestDiscordRoutingConfirmation(input: { forceNewThread?: boolean; }): Promise<{ pendingRouteId: string }> { const pendingRouteId = randomBytes(9).toString('base64url'); - const options = await buildOptions( + const suggestedWorkspace = input.routingDecision.status === 'routed' ? input.routingDecision.result.workspace - : null, - ); + : null; + const options = await buildOptions(suggestedWorkspace); + const suggestedIndex = findSuggestedIndex(options, suggestedWorkspace); // Slack asks this question inside the thread on the requesting message, so // the channel only ever shows the request itself. Start that thread now and // put the card in it; the launch reuses this exact thread. An interaction @@ -218,7 +328,7 @@ export async function requestDiscordRoutingConfirmation(input: { const cardChannel = thread ? taskThreadChannelContext({ channel: input.channel, thread }) : null; - await storePendingRoute(pendingRouteId, { + const pending: PendingDiscordRoute = { requesterDiscordUserId: input.requesterDiscordUserId, launchOwnerUserId: input.launchOwnerUserId, queuedMessage: input.queuedMessage, @@ -226,19 +336,39 @@ export async function requestDiscordRoutingConfirmation(input: { channel: input.channel, ...(cardChannel ? { cardChannel } : {}), options, + suggestedIndex, ...(input.forceNewThread ? { forceNewThread: true } : {}), - }); - const suggested = options[0]?.label; - await replyToDiscordEvent({ + }; + await storePendingRoute(pendingRouteId, pending); + // Only name a best match when the router actually picked one. Without a + // suggestion the card is a plain menu, and nothing gets auto-confirmed. + const suggested = + suggestedIndex === null ? undefined : options[suggestedIndex]?.label; + const card = await replyToDiscordEvent({ provider: input.provider, applicationId: input.applicationId, channel: cardChannel ?? input.channel, ...(input.interaction ? { interaction: input.interaction } : {}), text: suggested - ? `Where should I run this? The best match is **${suggested}**.` + ? `Where should I run this? The best match is **${suggested}** — starting in ~${Math.round(ROUTING_AUTO_CONFIRM_TIMEOUT_MS / 1_000)}s.` : 'Where should I run this?', buttons: routeButtons(pendingRouteId, options), }); + if (suggestedIndex === null) { + return { pendingRouteId }; + } + // Re-store so the timer can turn the card itself into the acknowledgement. + // The route is stored before the card is posted so a fast click cannot miss + // it, which leaves the card id as the one thing learned afterwards. + await storePendingRoute(pendingRouteId, { + ...pending, + ...(card.messageId ? { cardMessageId: card.messageId } : {}), + }); + scheduleDiscordRoutingAutoConfirm({ + provider: input.provider, + applicationId: input.applicationId, + pendingRouteId, + }); return { pendingRouteId }; } @@ -411,10 +541,12 @@ export async function handleDiscordRoutingCallback(input: { ...(pending.cardChannel ? { replaceMessage: { - applicationId: input.applicationId, - interaction: input.interaction, - interactionDeferred: input.interactionDeferred, channel: pending.cardChannel, + interaction: { + applicationId: input.applicationId, + interaction: input.interaction, + interactionDeferred: input.interactionDeferred, + }, }, } : {}), diff --git a/apps/api/src/handlers/discord/task-launch.ts b/apps/api/src/handlers/discord/task-launch.ts index a6358f109..e38b52dd4 100644 --- a/apps/api/src/handlers/discord/task-launch.ts +++ b/apps/api/src/handlers/discord/task-launch.ts @@ -15,14 +15,14 @@ import { type DiscordCommunicationProvider, type DiscordTaskThread, } from '@roomote/communication/discord-provider'; -import type { - DiscordEventCommunicationMetadata, - DiscordInteraction, -} from '@roomote/communication/discord-event'; +import type { DiscordEventCommunicationMetadata } from '@roomote/communication/discord-event'; import { getRedis } from '@roomote/redis'; import { buildCommunicationTaskThreadName } from '../tasks/communication-task-thread.js'; -import { replyToDiscordEvent } from './replies.js'; +import { + replaceOrPostDiscordMessage, + type DiscordMessageToReplace, +} from './replies.js'; import { discordTaskAcknowledgementText, discordTaskButtons, @@ -261,12 +261,7 @@ export async function launchDiscordTask(input: { * started message rather than being followed by an identical one. Only pass * a message that lives where the acknowledgement would have gone. */ - replaceMessage?: { - applicationId: string; - interaction: DiscordInteraction; - interactionDeferred: boolean; - channel: DiscordChannelContext; - }; + replaceMessage?: DiscordMessageToReplace; }) { let createdThread: DiscordTaskThread | null = null; const parentId = taskThreadParentId(input); @@ -361,14 +356,9 @@ export async function launchDiscordTask(input: { // Replacing already falls back to posting when the original message cannot // be edited, so the task is acknowledged either way. const acknowledgement = input.replaceMessage - ? await replyToDiscordEvent({ + ? await replaceOrPostDiscordMessage({ provider: input.provider, - applicationId: input.replaceMessage.applicationId, - channel: input.replaceMessage.channel, - interaction: { - interaction: input.replaceMessage.interaction, - interactionDeferred: input.replaceMessage.interactionDeferred, - }, + replace: input.replaceMessage, ...acknowledgementMessage, }) : await input.provider.postMessage({ diff --git a/apps/api/src/handlers/slack/events/channel-auto-start-unlinked.test.ts b/apps/api/src/handlers/slack/events/channel-auto-start-unlinked.test.ts index e1df2591b..1515f4944 100644 --- a/apps/api/src/handlers/slack/events/channel-auto-start-unlinked.test.ts +++ b/apps/api/src/handlers/slack/events/channel-auto-start-unlinked.test.ts @@ -15,7 +15,7 @@ vi.mock('@roomote/env', () => ({ })); vi.mock('@roomote/cloud-agents/server', () => ({ - SLACK_AUTO_CONFIRM_TIMEOUT_MS: 0, + ROUTING_AUTO_CONFIRM_TIMEOUT_MS: 0, })); vi.mock('@roomote/cloud-agents', () => ({ diff --git a/apps/api/src/handlers/slack/events/message-entry-unmentioned-routing.test.ts b/apps/api/src/handlers/slack/events/message-entry-unmentioned-routing.test.ts index d4078e7dd..b16edfa8a 100644 --- a/apps/api/src/handlers/slack/events/message-entry-unmentioned-routing.test.ts +++ b/apps/api/src/handlers/slack/events/message-entry-unmentioned-routing.test.ts @@ -19,7 +19,7 @@ vi.mock('@roomote/env', () => ({ })); vi.mock('@roomote/cloud-agents/server', () => ({ - SLACK_AUTO_CONFIRM_TIMEOUT_MS: 0, + ROUTING_AUTO_CONFIRM_TIMEOUT_MS: 0, })); vi.mock('@roomote/cloud-agents', () => ({ diff --git a/apps/api/src/handlers/slack/events/message-entry.ts b/apps/api/src/handlers/slack/events/message-entry.ts index 71c9d31ca..3def4e731 100644 --- a/apps/api/src/handlers/slack/events/message-entry.ts +++ b/apps/api/src/handlers/slack/events/message-entry.ts @@ -8,7 +8,7 @@ import { } from '@roomote/redis'; import { FeatureFlag } from '@roomote/feature-flags'; import { getFeatureFlagEvaluator } from '@roomote/feature-flags/server'; -import { SLACK_AUTO_CONFIRM_TIMEOUT_MS } from '@roomote/cloud-agents/server'; +import { ROUTING_AUTO_CONFIRM_TIMEOUT_MS } from '@roomote/cloud-agents/server'; import { autoConfirmRouting, collectAndExtractThreadAttachmentTexts, @@ -96,7 +96,7 @@ import { showManualPickerForAutoRouteFallback } from './auto-route-fallback.js'; async function runSlackAutoConfirm({ threadId, confirmNonce, - delayMs = SLACK_AUTO_CONFIRM_TIMEOUT_MS, + delayMs = ROUTING_AUTO_CONFIRM_TIMEOUT_MS, logContext, }: { threadId: string; diff --git a/packages/cloud-agents/src/server/router/index.ts b/packages/cloud-agents/src/server/router/index.ts index 2fc4564cd..d8805c8cb 100644 --- a/packages/cloud-agents/src/server/router/index.ts +++ b/packages/cloud-agents/src/server/router/index.ts @@ -31,7 +31,7 @@ export type { } from './slack-mcp-setup'; export { - SLACK_AUTO_CONFIRM_TIMEOUT_MS, + ROUTING_AUTO_CONFIRM_TIMEOUT_MS, LINEAR_AUTO_CONFIRM_TIMEOUT_MS, R_SMALL_MODEL_LABEL, MAX_TASK_DESCRIPTION_LENGTH, diff --git a/packages/cloud-agents/src/server/router/types.ts b/packages/cloud-agents/src/server/router/types.ts index 81111f880..7222399b9 100644 --- a/packages/cloud-agents/src/server/router/types.ts +++ b/packages/cloud-agents/src/server/router/types.ts @@ -8,7 +8,7 @@ import type { * Time in milliseconds to wait for the user to confirm or correct * the routing suggestion before auto-accepting it (Slack). */ -export const SLACK_AUTO_CONFIRM_TIMEOUT_MS = 30_000; +export const ROUTING_AUTO_CONFIRM_TIMEOUT_MS = 30_000; /** * Time in milliseconds to wait for the user to confirm or correct From ca0f62c794809227031ba0483bd89f4234339fcd Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 16 Jul 2026 17:02:10 -0500 Subject: [PATCH 2/5] [Fix] Address the routing auto-confirm review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real bugs in the auto-confirm path. Editing the card addressed the thread's parent channel. A thread is itself a channel and `editMessage` takes no separate thread id, unlike `postMessage` — so the edit could only ever return Unknown Message, and the fallback would post a second acknowledgement beside a routing card still showing live buttons. The exact duplicate this flow just removed. The card id is only knowable after posting, so the route is written a second time. That write was unconditional, so a Nevermind landing while the card was still posting would have its claim overwritten, bringing the route back from the dead for the timer to launch. Telegram already solved this with `XX` and said so in a comment; Discord now matches. Co-Authored-By: Claude Opus 4.8 --- .../discord/__tests__/replies.test.ts | 81 ++++++++++++++++++- .../__tests__/routing-confirmation.test.ts | 52 ++++++++++++ apps/api/src/handlers/discord/replies.ts | 11 ++- .../handlers/discord/routing-confirmation.ts | 30 ++++--- 4 files changed, 159 insertions(+), 15 deletions(-) diff --git a/apps/api/src/handlers/discord/__tests__/replies.test.ts b/apps/api/src/handlers/discord/__tests__/replies.test.ts index 5f24c4531..f2093e545 100644 --- a/apps/api/src/handlers/discord/__tests__/replies.test.ts +++ b/apps/api/src/handlers/discord/__tests__/replies.test.ts @@ -1,6 +1,9 @@ import { DiscordApiError } from '@roomote/communication'; -import { replyToDiscordEvent } from '../replies.js'; +import { + replaceOrPostDiscordMessage, + replyToDiscordEvent, +} from '../replies.js'; function interactionContext() { return { @@ -107,3 +110,79 @@ describe('replyToDiscordEvent', () => { expect(postMessage).not.toHaveBeenCalled(); }); }); + +describe('replaceOrPostDiscordMessage', () => { + it('edits a card in a task thread through the thread, not its parent', async () => { + // A thread is itself a channel and editMessage takes no separate thread + // id. Addressing the parent finds an unknown message, which silently + // degrades into a second acknowledgement beside a stale card. + const editMessage = vi.fn().mockResolvedValue(undefined); + const postMessage = vi.fn(); + + const result = await replaceOrPostDiscordMessage({ + provider: { editMessage, postMessage } as never, + replace: { channel: channelContext(), messageId: 'card-1' }, + text: 'Started a task in Acme.', + }); + + expect(editMessage).toHaveBeenCalledWith({ + channelId: 'thread-1', + messageId: 'card-1', + text: 'Started a task in Acme.', + }); + expect(postMessage).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + channelId: 'channel-1', + threadId: 'thread-1', + messageId: 'card-1', + }); + }); + + it('posts when the card it should replace was deleted', async () => { + const editMessage = vi.fn().mockRejectedValue( + new DiscordApiError({ + method: 'PATCH', + path: '/channels/thread-1/messages/card-1', + status: 404, + message: 'Unknown Message', + code: 10008, + }), + ); + const postMessage = vi.fn().mockResolvedValue({ + provider: 'discord', + channelId: 'channel-1', + messageId: 'ack-1', + }); + + await replaceOrPostDiscordMessage({ + provider: { editMessage, postMessage } as never, + replace: { channel: channelContext(), messageId: 'card-1' }, + text: 'Started a task in Acme.', + }); + + expect(postMessage).toHaveBeenCalledWith( + expect.objectContaining({ channelId: 'channel-1', threadId: 'thread-1' }), + ); + }); + + it('answers through the interaction when someone is waiting on one', async () => { + const editInteractionResponse = vi + .fn() + .mockResolvedValue({ messageId: 'card-1' }); + const editMessage = vi.fn(); + + await replaceOrPostDiscordMessage({ + provider: { editInteractionResponse, editMessage } as never, + replace: { + channel: channelContext(), + interaction: { applicationId: 'app-1', ...interactionContext() }, + }, + text: 'Started a task in Acme.', + }); + + expect(editInteractionResponse).toHaveBeenCalledWith( + expect.objectContaining({ interactionToken: 'interaction-token' }), + ); + expect(editMessage).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts b/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts index c5ee67fdd..1b3e9c941 100644 --- a/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts +++ b/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts @@ -438,6 +438,58 @@ describe('Discord routing confirmation', () => { } }); + it('does not resurrect a route claimed while the card was still posting', async () => { + // The card id is only knowable after posting, so the route is written a + // second time. A Nevermind landing in that window has already claimed the + // route; an unconditional write would bring it back and the timer would + // then launch the request the user just canceled. + mocks.reserveAnchoredThread.mockResolvedValue(null); + + await requestDiscordRoutingConfirmation({ + provider: {} as never, + applicationId: 'app-1', + requesterDiscordUserId: 'discord-user-1', + launchOwnerUserId: 'user-1', + queuedMessage: { + provider: 'discord', + text: 'Fix matchmaking', + user: 'Matt', + userId: 'user-1', + ts: 'message-1', + }, + metadata: { + communicationProvider: 'discord', + communicationChannelId: 'channel-1', + communicationMessageId: 'message-1', + }, + channel: { + channelId: 'channel-1', + channelName: 'general', + channelType: 0, + guildId: 'guild-1', + isDirectMessage: false, + isThread: false, + }, + routingDecision: { + status: 'routed', + result: { + workspace: { type: 'environment', id: 'env-1', name: 'Sunny Acres' }, + reasoning: 'likely', + debug: { + phase: 'direct', + toolsUsed: [], + needsExternalLookup: false, + confidence: 0.7, + }, + }, + }, + }); + + expect(mocks.redisSet).toHaveBeenCalledTimes(2); + expect(mocks.redisSet.mock.calls[0]).not.toContain('XX'); + expect(mocks.redisSet.mock.calls[1]).toContain('XX'); + }); + it('never auto-confirms a card the router had no suggestion for', async () => { // A fallback card is a plain menu. Auto-confirming its first option would // launch an alphabetical accident nobody chose. diff --git a/apps/api/src/handlers/discord/replies.ts b/apps/api/src/handlers/discord/replies.ts index 0ecd05ea8..f763147e2 100644 --- a/apps/api/src/handlers/discord/replies.ts +++ b/apps/api/src/handlers/discord/replies.ts @@ -52,17 +52,20 @@ export async function replaceOrPostDiscordMessage(input: { ...message, }); } - const channelId = channel.parentChannelId ?? channel.channelId; + const postChannelId = channel.parentChannelId ?? channel.channelId; if (input.replace.messageId) { try { await input.provider.editMessage({ - channelId, + // A thread is itself a channel, and the message lives in it. Unlike + // postMessage, editMessage takes no separate thread id — addressing + // the parent here would only ever find an unknown message. + channelId: channel.channelId, messageId: input.replace.messageId, ...message, }); return { provider: 'discord', - channelId, + channelId: postChannelId, messageId: input.replace.messageId, ...(channel.parentChannelId ? { threadId: channel.channelId } : {}), }; @@ -73,7 +76,7 @@ export async function replaceOrPostDiscordMessage(input: { } } return input.provider.postMessage({ - channelId, + channelId: postChannelId, ...(channel.parentChannelId ? { threadId: channel.channelId } : {}), ...message, }); diff --git a/apps/api/src/handlers/discord/routing-confirmation.ts b/apps/api/src/handlers/discord/routing-confirmation.ts index fc6efffb8..0b5342ca3 100644 --- a/apps/api/src/handlers/discord/routing-confirmation.ts +++ b/apps/api/src/handlers/discord/routing-confirmation.ts @@ -82,13 +82,17 @@ function parsePendingRoute(value: string | null): PendingDiscordRoute | null { async function storePendingRoute( id: string, pending: PendingDiscordRoute, + options?: { onlyIfExists?: boolean }, ): Promise { - await getRedis().set( - pendingRouteKey(id), - JSON.stringify(pending), - 'EX', - PENDING_ROUTE_TTL_SECONDS, - ); + const key = pendingRouteKey(id); + const value = JSON.stringify(pending); + if (options?.onlyIfExists) { + // XX so a click that already claimed the route is not resurrected by a + // later write — otherwise the timer would launch a canceled request. + await getRedis().set(key, value, 'EX', PENDING_ROUTE_TTL_SECONDS, 'XX'); + return; + } + await getRedis().set(key, value, 'EX', PENDING_ROUTE_TTL_SECONDS); } async function claimPendingRoute( @@ -360,10 +364,16 @@ export async function requestDiscordRoutingConfirmation(input: { // Re-store so the timer can turn the card itself into the acknowledgement. // The route is stored before the card is posted so a fast click cannot miss // it, which leaves the card id as the one thing learned afterwards. - await storePendingRoute(pendingRouteId, { - ...pending, - ...(card.messageId ? { cardMessageId: card.messageId } : {}), - }); + await storePendingRoute( + pendingRouteId, + { + ...pending, + ...(card.messageId ? { cardMessageId: card.messageId } : {}), + }, + // A click can land while the card post is still completing and claim the + // route. This write must not bring it back from the dead. + { onlyIfExists: true }, + ); scheduleDiscordRoutingAutoConfirm({ provider: input.provider, applicationId: input.applicationId, From a03f4cf170febc3fb1452294f603eac00debd0c0 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 16 Jul 2026 17:13:01 -0500 Subject: [PATCH 3/5] fix: finalize Discord auto-confirm routes --- .../__tests__/routing-confirmation.test.ts | 149 ++++++++++++++++++ .../handlers/discord/routing-confirmation.ts | 117 +++++++++----- 2 files changed, 230 insertions(+), 36 deletions(-) diff --git a/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts b/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts index 1b3e9c941..4c1a9dc3d 100644 --- a/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts +++ b/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts @@ -438,6 +438,155 @@ describe('Discord routing confirmation', () => { } }); + it('finalizes an auto-confirmed routing card outside the task thread', async () => { + vi.useFakeTimers(); + try { + const editMessage = vi.fn().mockResolvedValue(undefined); + const provider = { editMessage } as never; + mocks.reserveAnchoredThread.mockResolvedValue(null); + mocks.reply.mockResolvedValue({ messageId: 'card-1' }); + mocks.launchTask.mockResolvedValue({ + createdThread: null, + taskUrl: 'https://roomote.example/task/task-1', + launchResult: { id: 42, taskId: 'task-1' }, + }); + + await requestDiscordRoutingConfirmation({ + provider, + applicationId: 'app-1', + requesterDiscordUserId: 'discord-user-1', + launchOwnerUserId: 'user-1', + queuedMessage: { + provider: 'discord', + text: 'Fix matchmaking', + user: 'Matt', + userId: 'user-1', + ts: 'message-1', + }, + metadata: { + communicationProvider: 'discord', + communicationChannelId: 'dm-1', + communicationMessageId: 'message-1', + }, + channel: { + channelId: 'dm-1', + channelName: 'Direct message', + channelType: 1, + isDirectMessage: true, + isThread: false, + }, + routingDecision: { + status: 'routed', + result: { + workspace: { + type: 'environment', + id: 'env-1', + name: 'Sunny Acres', + }, + reasoning: 'likely', + debug: { + phase: 'direct', + toolsUsed: [], + needsExternalLookup: false, + confidence: 0.7, + }, + }, + }, + }); + + const stored = JSON.parse(mocks.redisSet.mock.lastCall?.[1] as string); + mocks.redisGetdel.mockResolvedValue(JSON.stringify(stored)); + await vi.advanceTimersByTimeAsync(30_000); + + expect(mocks.launchTask).toHaveBeenCalledWith( + expect.not.objectContaining({ replaceMessage: expect.anything() }), + ); + expect(editMessage).toHaveBeenCalledWith({ + channelId: 'dm-1', + messageId: 'card-1', + text: 'Started in **Sunny Acres**.', + buttons: [ + [ + { + text: 'Follow Task', + url: 'https://roomote.example/task/task-1', + }, + ], + ], + }); + } finally { + vi.useRealTimers(); + } + }); + + it('restores the route when an auto-confirm launch fails', async () => { + vi.useFakeTimers(); + try { + mocks.reserveAnchoredThread.mockResolvedValue(null); + mocks.reply.mockResolvedValue({ messageId: 'card-1' }); + + await requestDiscordRoutingConfirmation({ + provider: {} as never, + applicationId: 'app-1', + requesterDiscordUserId: 'discord-user-1', + launchOwnerUserId: 'user-1', + queuedMessage: { + provider: 'discord', + text: 'Fix matchmaking', + user: 'Matt', + userId: 'user-1', + ts: 'message-1', + }, + metadata: { + communicationProvider: 'discord', + communicationChannelId: 'channel-1', + communicationMessageId: 'message-1', + }, + channel: { + channelId: 'channel-1', + channelName: 'general', + channelType: 0, + guildId: 'guild-1', + isDirectMessage: false, + isThread: false, + }, + routingDecision: { + status: 'routed', + result: { + workspace: { + type: 'environment', + id: 'env-1', + name: 'Sunny Acres', + }, + reasoning: 'likely', + debug: { + phase: 'direct', + toolsUsed: [], + needsExternalLookup: false, + confidence: 0.7, + }, + }, + }, + }); + + const routeKey = mocks.redisSet.mock.lastCall?.[0] as string; + const stored = JSON.parse(mocks.redisSet.mock.lastCall?.[1] as string); + mocks.redisGetdel.mockResolvedValue(JSON.stringify(stored)); + mocks.launchTask.mockRejectedValueOnce(new Error('queue unavailable')); + + await vi.advanceTimersByTimeAsync(30_000); + + expect(mocks.redisSet.mock.lastCall).toEqual([ + routeKey, + JSON.stringify(stored), + 'EX', + 900, + ]); + } finally { + vi.useRealTimers(); + } + }); + it('does not resurrect a route claimed while the card was still posting', async () => { // The card id is only knowable after posting, so the route is written a // second time. A Nevermind landing in that window has already claimed the diff --git a/apps/api/src/handlers/discord/routing-confirmation.ts b/apps/api/src/handlers/discord/routing-confirmation.ts index 0b5342ca3..4d073de8b 100644 --- a/apps/api/src/handlers/discord/routing-confirmation.ts +++ b/apps/api/src/handlers/discord/routing-confirmation.ts @@ -216,43 +216,88 @@ async function autoConfirmDiscordRouting(input: { if (!pending || pending.suggestedIndex === null) return; const option = pending.options[pending.suggestedIndex]; if (!option) return; - const existingRun = await findCommunicationTaskRunBySourceEvent({ - provider: 'discord', - sourceEventId: pending.queuedMessage.ts, - }); - if (existingRun) return; - const workspace = await resolveDiscordWorkspace(option.workspace); - if (!workspace) { - await input.provider - .postMessage({ - channelId: - pending.cardChannel?.parentChannelId ?? pending.channel.channelId, - ...(pending.cardChannel - ? { threadId: pending.cardChannel.channelId } - : {}), - text: 'Could not start the task — the suggested workspace is no longer available. Send the request again.', - }) - .catch(() => undefined); - return; + try { + const existingRun = await findCommunicationTaskRunBySourceEvent({ + provider: 'discord', + sourceEventId: pending.queuedMessage.ts, + }); + if (existingRun) return; + const workspace = await resolveDiscordWorkspace(option.workspace); + if (!workspace) { + await input.provider + .postMessage({ + channelId: + pending.cardChannel?.parentChannelId ?? pending.channel.channelId, + ...(pending.cardChannel + ? { threadId: pending.cardChannel.channelId } + : {}), + text: 'Could not start the task — the suggested workspace is no longer available. Send the request again.', + }) + .catch(() => undefined); + return; + } + const launched = await launchDiscordTask({ + provider: input.provider, + launchOwnerUserId: pending.launchOwnerUserId, + queuedMessage: pending.queuedMessage, + metadata: pending.metadata, + channel: pending.channel, + workspace, + ...(pending.forceNewThread ? { forceNewThread: true } : {}), + // No interaction here — nobody clicked. A card that already lives in + // the task thread becomes the launch acknowledgement itself. + ...(pending.cardChannel && pending.cardMessageId + ? { + replaceMessage: { + channel: pending.cardChannel, + messageId: pending.cardMessageId, + }, + } + : {}), + }); + + if (!pending.cardChannel && pending.cardMessageId) { + // DMs, interaction cards, and thread-reservation fallbacks keep their + // routing card outside the task conversation. The launch has already + // posted its acknowledgement and controls in the right destination; + // turn the original card into a terminal receipt so stale route buttons + // do not remain visible. + await input.provider + .editMessage({ + channelId: pending.channel.channelId, + messageId: pending.cardMessageId, + text: launched.createdThread + ? `Started in **${workspace.workspaceDisplayName}**. Continue in the new task thread.` + : `Started in **${workspace.workspaceDisplayName}**.`, + buttons: launched.taskUrl + ? [[{ text: 'Follow Task', url: launched.taskUrl }]] + : [], + }) + .catch((error) => { + apiLogger.warn( + `[discord] Could not finalize routing card ${pending.cardMessageId}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + }); + } + } catch (error) { + // The click path restores its claim when lookup, workspace resolution, or + // launch fails. Auto-confirm must do the same so a transient failure does + // not consume the route and leave its buttons permanently dead. + await restorePendingRoute(input.pendingRouteId, pending).catch( + (restoreError) => { + apiLogger.warn( + `[discord] Could not restore routing choice ${input.pendingRouteId}: ${ + restoreError instanceof Error + ? restoreError.message + : String(restoreError) + }`, + ); + }, + ); + throw error; } - await launchDiscordTask({ - provider: input.provider, - launchOwnerUserId: pending.launchOwnerUserId, - queuedMessage: pending.queuedMessage, - metadata: pending.metadata, - channel: pending.channel, - workspace, - ...(pending.forceNewThread ? { forceNewThread: true } : {}), - // No interaction here — nobody clicked. The card is edited by id. - ...(pending.cardChannel && pending.cardMessageId - ? { - replaceMessage: { - channel: pending.cardChannel, - messageId: pending.cardMessageId, - }, - } - : {}), - }); } function scheduleDiscordRoutingAutoConfirm(input: { From 679f5565329eb7f14e705c0d61e9022d4f819e83 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Fri, 17 Jul 2026 09:50:09 -0500 Subject: [PATCH 4/5] refactor: share routing auto-confirm policy --- .../__tests__/routing-confirmation.test.ts | 28 ++++++++++ .../handlers/discord/routing-confirmation.ts | 9 ++-- .../handlers/telegram/__tests__/index.test.ts | 5 ++ .../handlers/telegram/routing-confirmation.ts | 13 +---- .../__tests__/routing-auto-confirm.test.ts | 39 ++++++++++++++ .../cloud-agents/src/server/router/index.ts | 2 + .../cloud-agents/src/server/router/types.ts | 27 ++++++++-- .../slack/src/__tests__/block-kit.test.ts | 51 ------------------- packages/slack/src/block-kit.ts | 19 ++----- 9 files changed, 107 insertions(+), 86 deletions(-) create mode 100644 packages/cloud-agents/src/server/router/__tests__/routing-auto-confirm.test.ts diff --git a/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts b/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts index 3d35cc8ae..259ae86c0 100644 --- a/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts +++ b/apps/api/src/handlers/discord/__tests__/routing-confirmation.test.ts @@ -1,5 +1,6 @@ const mocks = vi.hoisted(() => ({ getAvailableEnvironments: vi.fn(), + getRoutingAutoConfirmDelayMs: vi.fn(), getTaskUrl: vi.fn(), findMappedUser: vi.fn(), findSourceRun: vi.fn(), @@ -14,6 +15,7 @@ const mocks = vi.hoisted(() => ({ vi.mock('@roomote/cloud-agents/server', () => ({ getAvailableEnvironments: mocks.getAvailableEnvironments, + getRoutingAutoConfirmDelayMs: mocks.getRoutingAutoConfirmDelayMs, getTaskUrl: mocks.getTaskUrl, ROUTING_AUTO_CONFIRM_TIMEOUT_MS: 30_000, })); @@ -52,6 +54,7 @@ describe('Discord routing confirmation', () => { { id: 'env-1', name: 'Sunny Acres', repositoryNames: ['acme/game'] }, { id: 'env-2', name: 'API', repositoryNames: ['acme/api'] }, ]); + mocks.getRoutingAutoConfirmDelayMs.mockReturnValue(30_000); mocks.redisSet.mockResolvedValue('OK'); mocks.reserveAnchoredThread.mockResolvedValue(null); mocks.forgetPendingTaskThread.mockResolvedValue(undefined); @@ -72,6 +75,11 @@ describe('Discord routing confirmation', () => { }); it('auto-confirms only high-confidence, unremapped routes', () => { + mocks.getRoutingAutoConfirmDelayMs + .mockReturnValueOnce(0) + .mockReturnValueOnce(30_000) + .mockReturnValueOnce(30_000); + expect( shouldAutoConfirmDiscordRoute({ status: 'routed', @@ -102,6 +110,26 @@ describe('Discord routing confirmation', () => { }, }), ).toBe(false); + expect( + shouldAutoConfirmDiscordRoute({ + status: 'routed', + result: { + workspace: { type: 'all_repositories' }, + reasoning: 'broad match', + debug: { + phase: 'direct', + toolsUsed: [], + needsExternalLookup: false, + confidence: 0.99, + }, + }, + }), + ).toBe(false); + expect(mocks.getRoutingAutoConfirmDelayMs).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ confidence: 0.99 }), + 'all_repositories', + ); }); it('persists a pending route and posts workspace buttons', async () => { diff --git a/apps/api/src/handlers/discord/routing-confirmation.ts b/apps/api/src/handlers/discord/routing-confirmation.ts index 2e3de59f6..83b076445 100644 --- a/apps/api/src/handlers/discord/routing-confirmation.ts +++ b/apps/api/src/handlers/discord/routing-confirmation.ts @@ -2,6 +2,7 @@ import { randomBytes } from 'node:crypto'; import { getAvailableEnvironments, + getRoutingAutoConfirmDelayMs, getTaskUrl, ROUTING_AUTO_CONFIRM_TIMEOUT_MS, type RoutingDecision, @@ -28,7 +29,6 @@ import { type DiscordChannelContext, } from './task-launch.js'; -const DISCORD_IMMEDIATE_CONFIRM_CONFIDENCE = 0.95; const DISCORD_CHANNEL_TYPE_ANNOUNCEMENT = 5; const DISCORD_CHANNEL_TYPE_ANNOUNCEMENT_THREAD = 10; const DISCORD_CHANNEL_TYPE_PUBLIC_THREAD = 11; @@ -325,9 +325,10 @@ export function shouldAutoConfirmDiscordRoute( ): boolean { return ( decision.status === 'routed' && - decision.result.debug?.workspaceRemapped !== true && - (decision.result.debug?.confidence ?? 0) >= - DISCORD_IMMEDIATE_CONFIRM_CONFIDENCE + getRoutingAutoConfirmDelayMs( + decision.result.debug, + decision.result.workspace.type, + ) === 0 ); } diff --git a/apps/api/src/handlers/telegram/__tests__/index.test.ts b/apps/api/src/handlers/telegram/__tests__/index.test.ts index ca163b9f7..4327f9c6a 100644 --- a/apps/api/src/handlers/telegram/__tests__/index.test.ts +++ b/apps/api/src/handlers/telegram/__tests__/index.test.ts @@ -18,6 +18,7 @@ const { envMock, getAvailableEnvironmentsMock, getBotInfoMock, + getRoutingAutoConfirmDelayMsMock, getTaskUrlMock, insertMock, insertOnConflictDoNothingMock, @@ -52,6 +53,7 @@ const { environmentsFindFirstMock: vi.fn(), getAvailableEnvironmentsMock: vi.fn(), getBotInfoMock: vi.fn(), + getRoutingAutoConfirmDelayMsMock: vi.fn(), envMock: { R_APP_URL: 'https://app.example.com', R_TELEGRAM_BOT_TOKEN: 'bot-token' as string | undefined, @@ -280,6 +282,7 @@ vi.mock('@roomote/cloud-agents/server', () => ({ buildTelegramRoutingContext: buildTelegramRoutingContextMock, enqueueTask: enqueueTaskMock, getAvailableEnvironments: getAvailableEnvironmentsMock, + getRoutingAutoConfirmDelayMs: getRoutingAutoConfirmDelayMsMock, getTaskUrl: getTaskUrlMock, routeTask: routeTaskMock, })); @@ -369,6 +372,7 @@ describe('Telegram webhook handler', () => { redisGetdelMock.mockResolvedValue(null); environmentsFindFirstMock.mockResolvedValue(undefined); getAvailableEnvironmentsMock.mockResolvedValue([]); + getRoutingAutoConfirmDelayMsMock.mockReturnValue(30_000); editMessageTextMock.mockResolvedValue(undefined); setLatestInboundMessageIdMock.mockResolvedValue(undefined); authUsersFindFirstMock.mockResolvedValue(null); @@ -1766,6 +1770,7 @@ describe('Telegram webhook handler', () => { it('launches immediately from a captioned photo when routing confidence is high', async () => { mockTelegramLinkedSender('launch-owner-22'); + getRoutingAutoConfirmDelayMsMock.mockReturnValueOnce(0); getAvailableEnvironmentsMock.mockResolvedValue([ { id: 'env-1', name: 'Web App', repositoryNames: ['org/web'] }, { id: 'env-2', name: 'API', repositoryNames: ['org/api'] }, diff --git a/apps/api/src/handlers/telegram/routing-confirmation.ts b/apps/api/src/handlers/telegram/routing-confirmation.ts index 12366e267..1b95c830b 100644 --- a/apps/api/src/handlers/telegram/routing-confirmation.ts +++ b/apps/api/src/handlers/telegram/routing-confirmation.ts @@ -8,6 +8,7 @@ import type { CommunicationMessageButton } from '@roomote/communication'; import { getRedis } from '@roomote/redis'; import { getAvailableEnvironments, + getRoutingAutoConfirmDelayMs, type RoutingDecision, type RoutingWorkspace, } from '@roomote/cloud-agents/server'; @@ -30,13 +31,6 @@ import { import { launchTelegramTask, resolveTelegramWorkspace } from './task-launch.js'; import type { QueuedTelegramCommunicationMessage } from './types.js'; -/** - * Mirrors Slack's routing auto-confirm gate - * (`SLACK_IMMEDIATE_AUTO_CONFIRM_CONFIDENCE` in `packages/slack/block-kit.ts`): - * only a high-confidence, un-remapped environment route starts without asking. - */ -const TELEGRAM_IMMEDIATE_CONFIRM_CONFIDENCE = 0.95; - /** * How long the Yes/Nope card waits before auto-starting the suggestion. Kept * short so the normal flow feels immediate — the card is a brief veto @@ -374,10 +368,7 @@ export async function maybeRequestTelegramRoutingConfirmation(input: { if ( routed && - routed.workspace.type === 'environment' && - typeof routed.debug?.confidence === 'number' && - routed.debug.confidence >= TELEGRAM_IMMEDIATE_CONFIRM_CONFIDENCE && - routed.debug.workspaceRemapped !== true + getRoutingAutoConfirmDelayMs(routed.debug, routed.workspace.type) === 0 ) { return null; } diff --git a/packages/cloud-agents/src/server/router/__tests__/routing-auto-confirm.test.ts b/packages/cloud-agents/src/server/router/__tests__/routing-auto-confirm.test.ts new file mode 100644 index 000000000..a81133bec --- /dev/null +++ b/packages/cloud-agents/src/server/router/__tests__/routing-auto-confirm.test.ts @@ -0,0 +1,39 @@ +import { + getRoutingAutoConfirmDelayMs, + ROUTING_AUTO_CONFIRM_TIMEOUT_MS, +} from '../types'; + +describe('routing auto-confirm policy', () => { + it('starts high-confidence environment routes immediately', () => { + expect( + getRoutingAutoConfirmDelayMs( + { confidence: 0.95, workspaceRemapped: false }, + 'environment', + ), + ).toBe(0); + }); + + it('keeps the correction window for all-repositories routes', () => { + expect( + getRoutingAutoConfirmDelayMs( + { confidence: 0.99, workspaceRemapped: false }, + 'all_repositories', + ), + ).toBe(ROUTING_AUTO_CONFIRM_TIMEOUT_MS); + }); + + it('keeps the correction window below the confidence threshold', () => { + expect( + getRoutingAutoConfirmDelayMs({ confidence: 0.949 }, 'environment'), + ).toBe(ROUTING_AUTO_CONFIRM_TIMEOUT_MS); + }); + + it('keeps the correction window when the workspace was remapped', () => { + expect( + getRoutingAutoConfirmDelayMs( + { confidence: 0.99, workspaceRemapped: true }, + 'environment', + ), + ).toBe(ROUTING_AUTO_CONFIRM_TIMEOUT_MS); + }); +}); diff --git a/packages/cloud-agents/src/server/router/index.ts b/packages/cloud-agents/src/server/router/index.ts index d8805c8cb..8bfa8892c 100644 --- a/packages/cloud-agents/src/server/router/index.ts +++ b/packages/cloud-agents/src/server/router/index.ts @@ -31,6 +31,8 @@ export type { } from './slack-mcp-setup'; export { + getRoutingAutoConfirmDelayMs, + ROUTING_IMMEDIATE_AUTO_CONFIRM_CONFIDENCE, ROUTING_AUTO_CONFIRM_TIMEOUT_MS, LINEAR_AUTO_CONFIRM_TIMEOUT_MS, R_SMALL_MODEL_LABEL, diff --git a/packages/cloud-agents/src/server/router/types.ts b/packages/cloud-agents/src/server/router/types.ts index 7222399b9..e1eb58df6 100644 --- a/packages/cloud-agents/src/server/router/types.ts +++ b/packages/cloud-agents/src/server/router/types.ts @@ -4,12 +4,12 @@ import type { TaskModelSettings, } from '@roomote/types'; -/** - * Time in milliseconds to wait for the user to confirm or correct - * the routing suggestion before auto-accepting it (Slack). - */ +/** Time to wait before auto-accepting a routed workspace suggestion. */ export const ROUTING_AUTO_CONFIRM_TIMEOUT_MS = 30_000; +/** Confidence required to skip the correction window entirely. */ +export const ROUTING_IMMEDIATE_AUTO_CONFIRM_CONFIDENCE = 0.95; + /** * Time in milliseconds to wait for the user to confirm or correct * the routing suggestion before auto-accepting it (Linear). @@ -176,6 +176,25 @@ export interface RoutingDebugInfo { selectedTaskModel?: RoutingTaskModelSelection; } +/** + * Shared integration policy for routed workspace suggestions. + * + * High-confidence environment matches may start immediately. Broad + * all-repository routes, remapped workspaces, and lower-confidence matches + * retain the normal correction window. + */ +export function getRoutingAutoConfirmDelayMs( + routingDebug?: Pick, + workspaceType?: RoutingWorkspace['type'], +): number { + return typeof routingDebug?.confidence === 'number' && + routingDebug.confidence >= ROUTING_IMMEDIATE_AUTO_CONFIRM_CONFIDENCE && + routingDebug.workspaceRemapped !== true && + workspaceType === 'environment' + ? 0 + : ROUTING_AUTO_CONFIRM_TIMEOUT_MS; +} + export interface RoutingResult { workspace: RoutingWorkspace; model?: RoutingTaskModelSelection; diff --git a/packages/slack/src/__tests__/block-kit.test.ts b/packages/slack/src/__tests__/block-kit.test.ts index ef70f7a65..2ce09f011 100644 --- a/packages/slack/src/__tests__/block-kit.test.ts +++ b/packages/slack/src/__tests__/block-kit.test.ts @@ -3,7 +3,6 @@ import { buildRoutingConfirmBlocks, buildStartedBlocks, buildTaskFailedBlocks, - getSlackRoutingAutoConfirmDelayMs, postSlackAccountLinkThreadReply, } from '../block-kit'; import type { SlackNotifier } from '../slack-notifier'; @@ -310,56 +309,6 @@ describe('Slack routing blocks', () => { }), ]); }); - - it('skips the Slack auto-confirm wait for very high-confidence routes', () => { - expect( - getSlackRoutingAutoConfirmDelayMs({ - phase: 'direct', - toolsUsed: [], - needsExternalLookup: false, - confidence: 0.95, - workspaceRemapped: false, - }), - ).toBe(0); - }); - - it('keeps the Slack auto-confirm wait for all-repositories routes', () => { - expect( - getSlackRoutingAutoConfirmDelayMs( - { - phase: 'direct', - toolsUsed: [], - needsExternalLookup: false, - confidence: 0.99, - workspaceRemapped: false, - }, - 'all_repositories', - ), - ).toBeUndefined(); - }); - - it('keeps the Slack auto-confirm wait below the high-confidence threshold', () => { - expect( - getSlackRoutingAutoConfirmDelayMs({ - phase: 'direct', - toolsUsed: [], - needsExternalLookup: false, - confidence: 0.949, - }), - ).toBeUndefined(); - }); - - it('keeps the Slack auto-confirm wait when the final workspace was remapped', () => { - expect( - getSlackRoutingAutoConfirmDelayMs({ - phase: 'direct', - toolsUsed: [], - needsExternalLookup: false, - confidence: 0.99, - workspaceRemapped: true, - }), - ).toBeUndefined(); - }); }); describe('postSlackAccountLinkThreadReply', () => { diff --git a/packages/slack/src/block-kit.ts b/packages/slack/src/block-kit.ts index c7e3d843e..829382f46 100644 --- a/packages/slack/src/block-kit.ts +++ b/packages/slack/src/block-kit.ts @@ -51,6 +51,7 @@ import { routeTask, classifyFollowUp, buildSlackRoutingContext, + getRoutingAutoConfirmDelayMs, } from '@roomote/cloud-agents/server'; import type { @@ -292,8 +293,6 @@ export async function postSlackAccountLinkThreadReply({ thread_ts: threadTs, }); } -const SLACK_IMMEDIATE_AUTO_CONFIRM_CONFIDENCE = 0.95; - /** * Lua script to atomically HGET + HDEL a hash field. * Returns the value if it existed and, when provided, the stored initiating @@ -377,18 +376,6 @@ interface RetryFailedTaskActionValue { runId: number; } -export function getSlackRoutingAutoConfirmDelayMs( - routingDebug?: RoutingDebugInfo, - workspaceType?: 'environment' | 'all_repositories', -): number | undefined { - return typeof routingDebug?.confidence === 'number' && - routingDebug.confidence >= SLACK_IMMEDIATE_AUTO_CONFIRM_CONFIDENCE && - routingDebug.workspaceRemapped !== true && - workspaceType !== 'all_repositories' - ? 0 - : undefined; -} - /** * Atomically claims a pending workspace selection from Redis. * Returns the stored event JSON if the selection existed, null otherwise. @@ -1059,7 +1046,7 @@ export async function showTaskConfiguration({ const agentName = AGENT_DISPLAY_NAME; const workspaceOnly = routingResult.workspaceOnly === true; - const autoConfirmDelayMs = getSlackRoutingAutoConfirmDelayMs( + const autoConfirmDelayMs = getRoutingAutoConfirmDelayMs( routingResult.debug, routingResult.workspace.type, ); @@ -2630,7 +2617,7 @@ export async function handleSlackRoutingCorrection({ autoConfirmData: { threadId, confirmNonce: correctionNonce, - autoConfirmDelayMs: getSlackRoutingAutoConfirmDelayMs( + autoConfirmDelayMs: getRoutingAutoConfirmDelayMs( result.debug, result.workspace.type, ), From 88032d214fab223be4734aee4bfe13c499322d74 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Fri, 17 Jul 2026 09:59:04 -0500 Subject: [PATCH 5/5] test: update Slack routing policy mock --- packages/slack/src/__tests__/show-task-configuration.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/slack/src/__tests__/show-task-configuration.test.ts b/packages/slack/src/__tests__/show-task-configuration.test.ts index f94172a7b..376a7a711 100644 --- a/packages/slack/src/__tests__/show-task-configuration.test.ts +++ b/packages/slack/src/__tests__/show-task-configuration.test.ts @@ -64,6 +64,7 @@ vi.mock('@roomote/cloud-agents/server', () => ({ enqueueTask: enqueueTaskMock, classifyFollowUp: classifyFollowUpMock, detectSlackMcpSetupRequirement: vi.fn().mockResolvedValue(null), + getRoutingAutoConfirmDelayMs: vi.fn(() => 0), getTaskUrl: getTaskUrlMock, }));