From 7b62c8b255b2dde916a731598992bd2e55cbd94c Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 30 Jul 2026 00:53:28 +0000 Subject: [PATCH] fix(slack): wake passive router on outstanding-request answers The subscribed classifier treated bot/human answers to Junior's own asks as side conversation, so threads stayed silent until an explicit re-tag. Teach routing that fulfilling an outstanding assistant request is a turn-back, and stop bare @botUserId mentions from preflight-skipping as other-party. Co-Authored-By: David Cramer --- packages/junior/src/chat/app/factory.ts | 11 ++- packages/junior/src/chat/app/services.ts | 1 + .../junior/src/chat/runtime/slack-runtime.ts | 2 + .../src/chat/services/subscribed-decision.ts | 69 ++++++++++++++++--- .../chat/services/subscribed-reply-policy.ts | 2 + .../unit/routing/subscribed-decision.test.ts | 25 +++++++ 6 files changed, 101 insertions(+), 9 deletions(-) diff --git a/packages/junior/src/chat/app/factory.ts b/packages/junior/src/chat/app/factory.ts index 59c00d387..455f0ddc7 100644 --- a/packages/junior/src/chat/app/factory.ts +++ b/packages/junior/src/chat/app/factory.ts @@ -101,7 +101,15 @@ function upsertSkippedConversationMessage( } export function createSlackRuntime(options: CreateSlackRuntimeOptions) { - const services = createJuniorRuntimeServices(options.services); + const services = createJuniorRuntimeServices({ + ...options.services, + subscribedReplyPolicy: { + ...options.services?.subscribedReplyPolicy, + getBotUserId: + options.services?.subscribedReplyPolicy?.getBotUserId ?? + (() => options.getSlackAdapter().botUserId), + }, + }); const prepareTurnState = createPrepareTurnState({ compactConversationIfNeeded: services.conversationMemory.compactConversationIfNeeded, @@ -120,6 +128,7 @@ export function createSlackRuntime(options: CreateSlackRuntimeOptions) { AssistantLifecycleEvent >({ assistantUserName: botConfig.userName, + getAssistantUserId: () => options.getSlackAdapter().botUserId, cancelEventSubscriptions, modelId: standardModelId(botConfig), now: options.now ?? (() => Date.now()), diff --git a/packages/junior/src/chat/app/services.ts b/packages/junior/src/chat/app/services.ts index 651120d86..a7d2c4b15 100644 --- a/packages/junior/src/chat/app/services.ts +++ b/packages/junior/src/chat/app/services.ts @@ -103,6 +103,7 @@ export function createJuniorRuntimeServices( subscribedReplyPolicy: createSubscribedReplyPolicy({ completeObject: overrides.subscribedReplyPolicy?.completeObject ?? completeObject, + getBotUserId: overrides.subscribedReplyPolicy?.getBotUserId, }), visionContext, }; diff --git a/packages/junior/src/chat/runtime/slack-runtime.ts b/packages/junior/src/chat/runtime/slack-runtime.ts index cc183feba..a5fb6bef5 100644 --- a/packages/junior/src/chat/runtime/slack-runtime.ts +++ b/packages/junior/src/chat/runtime/slack-runtime.ts @@ -111,6 +111,7 @@ type RuntimeLogContext = Record & { export interface SlackTurnRuntimeDependencies { assistantUserName: string; + getAssistantUserId?: () => string | undefined; cancelEventSubscriptions: (input: { conversationId: string; }) => Promise; @@ -949,6 +950,7 @@ export function createSlackTurnRuntime< ? undefined : getSubscribedReplyPreflightDecision({ botUserName: deps.assistantUserName, + botUserId: deps.getAssistantUserId?.(), rawText: combinedText.rawText, text: combinedText.userText, isExplicitMention: turnIsExplicitMention, diff --git a/packages/junior/src/chat/services/subscribed-decision.ts b/packages/junior/src/chat/services/subscribed-decision.ts index cd28978ae..d06e7238e 100644 --- a/packages/junior/src/chat/services/subscribed-decision.ts +++ b/packages/junior/src/chat/services/subscribed-decision.ts @@ -43,6 +43,7 @@ interface TranscriptMessage { interface RouterSignals { assistantWasLastSpeaker: boolean; + assistantLatestMessageIsOutstandingRequest: boolean; currentMessageHasDirectedFollowUpCue: boolean; currentMessageHasAttachments: boolean; currentMessageIsTerseClarification: boolean; @@ -103,6 +104,7 @@ function escapeRegExp(value: string): string { function containsAssistantInvocation( text: string, botUserName: string, + botUserId?: string, ): boolean { const escapedUserName = escapeRegExp(botUserName); const plainNameMentionRe = new RegExp(`(^|\\s)@${escapedUserName}\\b`, "i"); @@ -111,23 +113,43 @@ function containsAssistantInvocation( "i", ); - return plainNameMentionRe.test(text) || labeledEntityMentionRe.test(text); + if (plainNameMentionRe.test(text) || labeledEntityMentionRe.test(text)) { + return true; + } + + if (!botUserId) { + return false; + } + + const escapedUserId = escapeRegExp(botUserId); + const plainIdMentionRe = new RegExp(`(^|\\s)@${escapedUserId}\\b`, "i"); + const slackIdMentionRe = new RegExp(`<@${escapedUserId}(?:\\|[^>]+)?>`, "i"); + return plainIdMentionRe.test(text) || slackIdMentionRe.test(text); } function detectLeadingOtherPartyAddress( rawText: string, text: string, botUserName: string, + botUserId?: string, ): string | undefined { if ( - containsAssistantInvocation(rawText, botUserName) || - containsAssistantInvocation(text, botUserName) + containsAssistantInvocation(rawText, botUserName, botUserId) || + containsAssistantInvocation(text, botUserName, botUserId) ) { return undefined; } const leadingSlackMention = rawText.match(LEADING_SLACK_MENTION_RE); if (leadingSlackMention) { + const mentionedUserId = leadingSlackMention[1]?.trim(); + if ( + botUserId && + mentionedUserId && + mentionedUserId.toUpperCase() === botUserId.toUpperCase() + ) { + return undefined; + } const label = leadingSlackMention[2]?.trim(); return label ? `slack_mention:${label}` : "slack_mention"; } @@ -140,7 +162,8 @@ function detectLeadingOtherPartyAddress( const directedName = leadingNamedMention[1]?.trim(); if ( !directedName || - directedName.toLowerCase() === botUserName.toLowerCase() + directedName.toLowerCase() === botUserName.toLowerCase() || + (botUserId && directedName.toUpperCase() === botUserId.toUpperCase()) ) { return undefined; } @@ -148,6 +171,19 @@ function detectLeadingOtherPartyAddress( return `named_mention:${directedName}`; } +function looksLikeOutstandingRequest(text: string): boolean { + const trimmed = text.trim(); + if (!trimmed) { + return false; + } + if (trimmed.includes("?")) { + return true; + } + return /\b(?:can you|could you|would you|please|paste|send|share|provide|need you to|let me know|tell me|give me)\b/i.test( + trimmed, + ); +} + function isThreadOptOutInstruction(rawText: string, text: string): boolean { return THREAD_OPTOUT_PATTERNS.some( (pattern) => pattern.test(rawText) || pattern.test(text), @@ -241,6 +277,9 @@ function buildRouterSignals(input: SubscribedDecisionInput): RouterSignals { return { assistantWasLastSpeaker: latestPriorMessage?.role === "assistant", + assistantLatestMessageIsOutstandingRequest: looksLikeOutstandingRequest( + latestPriorAssistantMessage?.text || "", + ), currentMessageHasDirectedFollowUpCue: hasDirectedFollowUpCue(input.text), currentMessageHasAttachments: Boolean(input.hasAttachments), currentMessageIsTerseClarification: isTerseClarification(input.text), @@ -265,6 +304,9 @@ function buildRouterPrompt(rawText: string, signals: RouterSignals): string { `${escapeXml(rawText.trim() || "[attachment-only message]")}`, "", `assistant_was_last_speaker=${signals.assistantWasLastSpeaker ? "true" : "false"}`, + `assistant_latest_message_is_outstanding_request=${ + signals.assistantLatestMessageIsOutstandingRequest ? "true" : "false" + }`, `human_messages_since_last_assistant=${ signals.humanMessagesSinceLastAssistant ?? "none" }`, @@ -297,14 +339,19 @@ function getReplyConfidenceThreshold(signals: RouterSignals): number { ) { if ( signals.currentMessageHasDirectedFollowUpCue || - signals.currentMessageIsTerseClarification + signals.currentMessageIsTerseClarification || + signals.assistantLatestMessageIsOutstandingRequest ) { threshold = 0.65; } else { threshold = 0.9; } } else if (signals.humanMessagesSinceLastAssistant === 1) { - threshold = signals.currentMessageHasDirectedFollowUpCue ? 0.8 : 0.9; + threshold = signals.currentMessageHasDirectedFollowUpCue + ? 0.8 + : signals.assistantLatestMessageIsOutstandingRequest + ? 0.75 + : 0.9; } else if (signals.humanMessagesSinceLastAssistant === undefined) { threshold = 0.85; } else if (signals.humanMessagesSinceLastAssistant >= 2) { @@ -317,6 +364,7 @@ function getReplyConfidenceThreshold(signals: RouterSignals): number { /** Fast heuristic check before the LLM classifier — skips messages directed at another party. */ export function getSubscribedReplyPreflightDecision(args: { botUserName: string; + botUserId?: string; rawText: string; text: string; isExplicitMention?: boolean; @@ -332,6 +380,7 @@ export function getSubscribedReplyPreflightDecision(args: { rawText, text, args.botUserName, + args.botUserId, ); if (!leadingOtherPartyAddress) { return undefined; @@ -350,12 +399,14 @@ function buildRouterSystemPrompt(botUserName: string): string { `You are a message router for a Slack assistant named ${assistantName} in a subscribed Slack thread.`, `Decide whether ${assistantName} should reply to the latest message.`, "Subscribed threads are passive by default.", - `Reply true only when the latest message is aimed at ${assistantName}.`, + `Reply true only when the latest message is aimed at ${assistantName} or directly continues ${assistantName}'s outstanding work.`, "Use who currently has the conversation floor, not just topic overlap.", `If ${assistantName} was the last speaker, only a clear turn back to ${assistantName} should count as an implicit follow-up.`, + `If ${assistantName}'s latest prior message asked a person or bot for information, evidence, logs, diffs, or other details, and the latest message supplies that material, set should_reply=true even without an explicit mention of ${assistantName}.`, + `That includes answers from other bots when ${assistantName} asked them a question. Delivering the requested answer is a turn back to ${assistantName}.`, `Terse clarifications like 'which one?' or 'why?' right after ${assistantName} answers can be should_reply=true.`, `Direct self-reference to ${assistantName}'s prior answer like 'what did you just say?' or 'explain that more' can be should_reply=true.`, - `If one or more humans spoke after ${assistantName}, require a clear turn back to ${assistantName}. Shared domain vocabulary alone is not enough.`, + `If one or more humans spoke after ${assistantName}, require a clear turn back to ${assistantName} unless the latest message is clearly fulfilling ${assistantName}'s outstanding request. Shared domain vocabulary alone is not enough.`, `Questions like 'what about auth?' or 'can you check on this?' are usually human-to-human unless the thread clearly turns back to ${assistantName}.`, `A vague question like 'is that the right approach?' is still should_reply=false unless it clearly turns back to ${assistantName}.`, "Acknowledgments, reactions, status chatter, and team coordination should be should_reply=false.", @@ -372,6 +423,7 @@ function buildRouterSystemPrompt(botUserName: string): string { /** Decide whether to reply to a message in a subscribed thread using an LLM classifier. */ export async function decideSubscribedThreadReply(args: { botUserName: string; + botUserId?: string; modelId: string; input: SubscribedDecisionInput; completeObject: (args: { @@ -392,6 +444,7 @@ export async function decideSubscribedThreadReply(args: { const rawText = args.input.rawText.trim(); const preflightDecision = getSubscribedReplyPreflightDecision({ botUserName: args.botUserName, + botUserId: args.botUserId, rawText, text, isExplicitMention: args.input.isExplicitMention, diff --git a/packages/junior/src/chat/services/subscribed-reply-policy.ts b/packages/junior/src/chat/services/subscribed-reply-policy.ts index bde5ab57a..a943c9871 100644 --- a/packages/junior/src/chat/services/subscribed-reply-policy.ts +++ b/packages/junior/src/chat/services/subscribed-reply-policy.ts @@ -8,6 +8,7 @@ import type { completeObject } from "@/chat/pi/client"; export interface SubscribedReplyPolicyDeps { completeObject: typeof completeObject; + getBotUserId?: () => string | undefined; } export interface SubscribedReplyDecision { @@ -26,6 +27,7 @@ export function createSubscribedReplyPolicy( return async (args) => { const decision = await decideSubscribedThreadReply({ botUserName: botConfig.userName, + botUserId: deps.getBotUserId?.(), modelId: botConfig.fastModelId, input: args, completeObject: deps.completeObject, diff --git a/packages/junior/tests/unit/routing/subscribed-decision.test.ts b/packages/junior/tests/unit/routing/subscribed-decision.test.ts index e45ea168e..517bf6d43 100644 --- a/packages/junior/tests/unit/routing/subscribed-decision.test.ts +++ b/packages/junior/tests/unit/routing/subscribed-decision.test.ts @@ -55,6 +55,7 @@ describe("subscribed reply decision", () => { expect( getSubscribedReplyPreflightDecision({ botUserName: "junior", + botUserId: "U0JUNIOR", rawText: fixture.rawText, text: fixture.text, isExplicitMention: false, @@ -73,6 +74,7 @@ describe("subscribed reply decision", () => { expect( getSubscribedReplyPreflightDecision({ botUserName: "junior", + botUserId: "U0JUNIOR", rawText: text, text, isExplicitMention: false, @@ -80,6 +82,29 @@ describe("subscribed reply decision", () => { ).toBeUndefined(); }); + it.each([ + { + name: "normalized bot user id mention", + rawText: "@U0JUNIOR continue", + text: "@U0JUNIOR continue", + }, + { + name: "slack bot user id mention", + rawText: "<@U0JUNIOR> continue", + text: "continue", + }, + ])("does not preflight-skip $name", (fixture) => { + expect( + getSubscribedReplyPreflightDecision({ + botUserName: "junior", + botUserId: "U0JUNIOR", + rawText: fixture.rawText, + text: fixture.text, + isExplicitMention: false, + }), + ).toBeUndefined(); + }); + it.each([ { name: "a negative decision",