diff --git a/apps/slack-agent/.env.local.example b/apps/slack-agent/.env.local.example index ef0bd4e0c..bc25e8cdf 100644 --- a/apps/slack-agent/.env.local.example +++ b/apps/slack-agent/.env.local.example @@ -7,6 +7,10 @@ OPENROUTER_API_KEY= # Optional overrides (defaults live in agent/agent.ts): # OPENROUTER_MODEL=openai/gpt-5.6-luna # must stream structured tool calls — see README Notes # OPENROUTER_CONTEXT_WINDOW=400000 +# Model for the thread follow-up relevance gate (one tiny RESPOND/PASS call per +# un-mentioned thread reply — agent/lib/follow-up-relevance.ts). A small, fast +# model is ideal here; unset, it follows OPENROUTER_MODEL. +# OPENROUTER_GATE_MODEL= # ── Slack (self-managed, multi-workspace app — no Vercel Connect) ─────────── # Signing secret is per-app/static: it HMAC-verifies every inbound webhook. diff --git a/apps/slack-agent/README.md b/apps/slack-agent/README.md index 190a19f7d..e7f93283e 100644 --- a/apps/slack-agent/README.md +++ b/apps/slack-agent/README.md @@ -301,6 +301,14 @@ before eve's 200, so anything it fetches is spent out of Slack's ~3s delivery bu is parse-only and optimistic; the mention handler confirms it afterwards, against the thread it loads for turn context anyway. +Being engaged in a thread is necessary but not sufficient: a confirmed follow-up still passes a +**relevance gate** — one tiny RESPOND/PASS classifier call (`agent/lib/follow-up-relevance.ts`, +model via `OPENROUTER_GATE_MODEL`) asking whether the reply is actually directed at the bot. Humans +talking to each other in a thread the bot answered once no longer trigger a turn for every message; +the pass is silent (no reaction, no reply), and the next message that _is_ for the bot gets +answered. Real `@mentions` and DMs skip the gate — an explicit address answers the question itself — +and the gate fails open, so a classifier outage means an extra answer, never a silent drop. + Changing a request URL does **not** require reinstalling the app; only changing _scopes_ does. (If you did edit scopes, the sidebar shows a yellow reinstall banner — follow it, and note that reinstalling issues a **new** `SLACK_BOT_TOKEN` that you must copy back into Railway.) diff --git a/apps/slack-agent/agent/channels/slack.ts b/apps/slack-agent/agent/channels/slack.ts index 190683c4f..a99b6a390 100644 --- a/apps/slack-agent/agent/channels/slack.ts +++ b/apps/slack-agent/agent/channels/slack.ts @@ -5,6 +5,7 @@ import { describeActions, truncateTypingStatus } from "#lib/action-status.js" import { botUserIdForTeam, rememberBotUserId } from "#lib/bot-identity.js" import { loadChannelContext } from "#lib/channel-context.js" import { notifyThreadDisengagement } from "#lib/disengage-notice.js" +import { judgeFollowUpRelevance } from "#lib/follow-up-relevance.js" import { resolveBotToken, verifySlackV0Signature, type SlackTokenContext } from "#lib/maple.js" import { emitAgentLog } from "#lib/telemetry-log.js" import { formatThreadContext, loadThreadMessages } from "#lib/thread-context.js" @@ -131,8 +132,9 @@ const webhookVerifier: SlackWebhookVerifier = async (request, body) => { * follow-up decision lives here rather than in the verifier: this handler * already loads the thread, and here it can take as long as it needs. It must * not otherwise throw: eve drops the whole mention when this handler does, so - * both loads degrade to no context instead — the one deliberate throw is the - * disengagement drop below, which is exactly what that escape hatch is for. + * both loads degrade to no context instead — the deliberate throws are the + * two follow-up drops below (disengaged / not addressed to the bot), which is + * exactly what that escape hatch is for. */ async function dispatchWithConversationContext( ctx: SlackContext, @@ -202,7 +204,42 @@ async function dispatchWithConversationContext( `Thread follow-up not dispatched (${decision.reason}): the bot is no longer engaged in ${pending.channelId}:${pending.threadTs}.`, ) } - // Confirmed, so the :eyes: is now a promise we keep. + // Engaged is necessary, not sufficient: the bot being part of the thread + // says nothing about whether THIS reply is for it. Without this gate every + // human reply in an engaged thread — including two people talking to each + // other — dispatched a full turn. Runs before the ack and the typing + // indicator so a message the bot stays out of gets no reaction at all; + // the pass is silent on purpose (the bot is still engaged, and the next + // reply that IS for it will be answered). Fails open — see the module. + const relevance = await judgeFollowUpRelevance({ + reply: { + text: message.text, + markdown: message.markdown, + user: message.author?.userId, + ts: message.ts, + threadTs: message.threadTs, + raw: message.raw, + }, + threadMessages, + botUserId: pending.botUserId, + }) + if (!relevance.respond) { + // Ids only, never text — same rule as the disengagement log above. This + // line is the only trace the drop leaves. + emitAgentLog("info", "follow_up_not_relevant", { + "maple.agent.event": "follow_up_not_relevant", + "maple.slack.team_id": pending.teamId, + "maple.slack.channel_id": pending.channelId, + "maple.slack.thread_ts": pending.threadTs, + "maple.slack.message_ts": pending.messageTs, + }) + // Same escape hatch as the disengagement drop: throwing is the only way + // to un-dispatch an event eve has already accepted. + throw new Error( + `Thread follow-up not dispatched (not addressed to the bot): ${pending.channelId}:${pending.threadTs}.`, + ) + } + // Confirmed and relevant, so the :eyes: is now a promise we keep. if (pending.ackable) { void acknowledgeMessage({ teamId: pending.teamId, diff --git a/apps/slack-agent/agent/lib/follow-up-relevance.test.ts b/apps/slack-agent/agent/lib/follow-up-relevance.test.ts new file mode 100644 index 000000000..19fa0e74d --- /dev/null +++ b/apps/slack-agent/agent/lib/follow-up-relevance.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, test } from "bun:test" +import type { SlackThreadMessage } from "eve/channels/slack" +import { + judgeFollowUpRelevance, + parseRelevanceVerdict, + relevancePrompt, + relevanceSystemPrompt, + type FollowUpRelevanceInput, +} from "./follow-up-relevance.js" + +const BOT_USER_ID = "U0BOT" + +const threadMessage = (overrides: Partial = {}): SlackThreadMessage => ({ + text: "", + markdown: "", + user: undefined, + botId: undefined, + ts: "1700000000.000100", + threadTs: "1700000000.000100", + isMe: false, + raw: {}, + ...overrides, +}) + +const input = (overrides: Partial = {}): FollowUpRelevanceInput => ({ + reply: { + text: "can you check the api service too?", + markdown: "can you check the api service too?", + user: "U456", + ts: "1700000002.000200", + threadTs: "1700000000.000100", + raw: {}, + }, + threadMessages: [ + threadMessage({ text: "why did this alert fire?", markdown: "why did this alert fire?", user: "U456" }), + threadMessage({ + text: "The error rate on checkout spiked at 14:10.", + markdown: "The error rate on checkout spiked at 14:10.", + user: BOT_USER_ID, + ts: "1700000001.000100", + }), + ], + botUserId: BOT_USER_ID, + ...overrides, +}) + +describe("parseRelevanceVerdict", () => { + test("plain verdicts, any case", () => { + expect(parseRelevanceVerdict("RESPOND")).toBe("respond") + expect(parseRelevanceVerdict("pass")).toBe("pass") + expect(parseRelevanceVerdict(" Pass.\n")).toBe("pass") + }) + + test("verdict wrapped in prose still parses", () => { + expect(parseRelevanceVerdict("The user is talking to a teammate, so: PASS")).toBe("pass") + }) + + test("no verdict, or both words, is no answer", () => { + expect(parseRelevanceVerdict("")).toBeNull() + expect(parseRelevanceVerdict("maybe?")).toBeNull() + expect(parseRelevanceVerdict("RESPOND or PASS, hard to say")).toBeNull() + }) + + test("substrings do not count as verdicts", () => { + expect(parseRelevanceVerdict("the password expired")).toBeNull() + expect(parseRelevanceVerdict("correspondence")).toBeNull() + }) +}) + +describe("relevancePrompt", () => { + test("carries the thread tail and the reply", () => { + const prompt = relevancePrompt(input()) + expect(prompt).toContain("why did this alert fire?") + expect(prompt).toContain("can you check the api service too?") + expect(prompt).toContain("Newest message:") + // The bot's own post is attributed as the agent, other bots are not. + expect(prompt).toContain("sender_type: agent") + }) + + test("says so when the thread was unreadable vs merely empty", () => { + expect(relevancePrompt(input({ threadMessages: null }))).toContain("could not be loaded") + expect(relevancePrompt(input({ threadMessages: [] }))).toContain("No earlier thread messages") + }) + + test("system prompt names the bot's user id", () => { + expect(relevanceSystemPrompt(BOT_USER_ID)).toContain(`<@${BOT_USER_ID}>`) + }) +}) + +describe("judgeFollowUpRelevance", () => { + test("model RESPOND answers the follow-up", async () => { + const decision = await judgeFollowUpRelevance(input(), { complete: async () => "RESPOND" }) + expect(decision).toEqual({ respond: true, reason: "model-respond" }) + }) + + test("model PASS drops it", async () => { + const decision = await judgeFollowUpRelevance(input(), { complete: async () => "PASS" }) + expect(decision).toEqual({ respond: false, reason: "model-pass" }) + }) + + test("a throwing classifier fails open", async () => { + const decision = await judgeFollowUpRelevance(input(), { + complete: async () => { + throw new Error("openrouter down") + }, + }) + expect(decision).toEqual({ respond: true, reason: "classifier-error" }) + }) + + test("an unparseable verdict fails open", async () => { + const decision = await judgeFollowUpRelevance(input(), { complete: async () => "hmm, unclear" }) + expect(decision).toEqual({ respond: true, reason: "unparseable-verdict" }) + }) + + test("the classifier sees the reply it is judging", async () => { + let seen: { system: string; prompt: string } | undefined + await judgeFollowUpRelevance(input(), { + complete: async ({ system, prompt }) => { + seen = { system, prompt } + return "PASS" + }, + }) + expect(seen?.prompt).toContain("can you check the api service too?") + expect(seen?.system).toContain("RESPOND or PASS") + }) +}) diff --git a/apps/slack-agent/agent/lib/follow-up-relevance.ts b/apps/slack-agent/agent/lib/follow-up-relevance.ts new file mode 100644 index 000000000..db50498de --- /dev/null +++ b/apps/slack-agent/agent/lib/follow-up-relevance.ts @@ -0,0 +1,198 @@ +import { generateText } from "ai" +import { createOpenRouter } from "@openrouter/ai-sdk-provider" +import type { SlackThreadMessage } from "eve/channels/slack" +import { formatContextBlock, formatContextMessage, type RenderableSlackMessage } from "./slack-context-format.js" + +/** + * Relevance gate for promoted thread follow-ups: should the bot answer this + * reply at all? + * + * `#lib/thread-follow-up.js` answers a *mechanical* question — is the bot part + * of this thread — and once it is, every human reply in the thread dispatched a + * full agent turn. In an incident thread where two engineers are talking to + * each other, that meant the bot answered every one of their messages: a cost + * amplifier and, worse, a bot that butts into conversations that were never + * addressed to it. + * + * So an engaged follow-up now has to pass a second, *semantic* question before + * it becomes a turn: given the thread, is this reply directed at the bot or + * does it expect the bot to act? A small classifier call decides — one model + * round-trip with no tools, orders of magnitude cheaper than the agent turn it + * gates. Real @mentions and DMs never come here: an explicit address is the + * user answering this question themselves. + * + * The gate runs on the dispatch path (post-200, inside eve's `waitUntil`), so + * it spends nothing from Slack's webhook budget, and it runs *before* the + * `:eyes:` ack and the typing indicator — a message the bot decides not to + * answer gets no reaction at all, exactly like one it was never going to + * answer. A pass is silent by design: the bot staying out of a conversation + * between humans must not announce itself. + * + * **It fails open**, same asymmetry as `confirmThreadFollowUp`: when the + * classifier errors, times out, or answers gibberish, the follow-up is + * answered. A wrong drop loses a user's message with nothing on screen to + * explain it; a wrong answer costs one turn in a thread the bot was already + * part of. The tiebreak inside the prompt leans the same way. + */ + +/** + * Bound on the classifier round-trip. Past it the reply is answered without a + * verdict (fail open) — a stalled gate must not turn into a silent drop, and + * the turn behind it is slower than this anyway. + */ +const RELEVANCE_TIMEOUT_MS = 10_000 + +/** + * How much thread the classifier sees. The addressing question is local — who + * is this reply talking to — so the recent tail decides it; the full + * transcript belongs to the turn, not the gate. + */ +const RELEVANCE_THREAD_TAIL = 12 +const RELEVANCE_MAX_CONTENT_CHARS = 600 + +export type FollowUpRelevanceDecision = + | { + readonly respond: true + readonly reason: "model-respond" | "classifier-error" | "unparseable-verdict" + } + | { readonly respond: false; readonly reason: "model-pass" } + +export interface FollowUpRelevanceInput { + /** The reply being judged, as the handler received it. */ + readonly reply: RenderableSlackMessage + /** + * The thread the handler already loaded for turn context (oldest-first, + * without the reply itself), or `null` when Slack could not be read. + */ + readonly threadMessages: readonly SlackThreadMessage[] | null + /** The workspace's bot user id. */ + readonly botUserId: string +} + +export interface FollowUpRelevanceDeps { + /** One classifier completion. The default calls OpenRouter via the AI SDK. */ + readonly complete: (input: { + readonly system: string + readonly prompt: string + readonly signal: AbortSignal + }) => Promise +} + +/** + * The gate model is separately configurable because the job wants a small, + * fast model, not the agent's; unset, it follows the agent's model rather + * than silently using a third one. The literal fallback matches + * `agent/agent.ts` — keep them in sync. + */ +function gateModelId(): string { + return process.env.OPENROUTER_GATE_MODEL ?? process.env.OPENROUTER_MODEL ?? "openai/gpt-5.6-luna" +} + +/** + * Lazily built so importing this module never requires the key; same + * referer/title identity as `agent/agent.ts` on purpose — a different one + * would mint a second OpenRouter app entry and split the rankings. + */ +let defaultDeps: FollowUpRelevanceDeps | undefined + +function buildDefaultDeps(): FollowUpRelevanceDeps { + const openrouter = createOpenRouter({ + apiKey: process.env.OPENROUTER_API_KEY ?? "", + appUrl: "https://maple.dev", + appName: "Maple", + extraBody: { trace: { trace_name: "slack" } }, + }) + const model = openrouter(gateModelId()) + return { + complete: async ({ system, prompt, signal }) => { + const result = await generateText({ model, system, prompt, abortSignal: signal }) + return result.text + }, + } +} + +export function relevanceSystemPrompt(botUserId: string): string { + return [ + `You decide whether Maple AI, an observability assistant in a Slack thread, should reply to the newest message. The assistant's Slack user id is <@${botUserId}>. Users in a thread the assistant is active in can talk to it without @-mentioning it, so the absence of a mention means nothing.`, + "", + "Answer RESPOND when the message is directed at the assistant or expects it to act: a question or request it can serve, a follow-up, correction, or new instruction about work it has been doing in this thread, or an answer to something the assistant asked.", + "", + "Answer PASS when the message expects nothing from the assistant: people talking to each other, a message explicitly addressed to another person, status updates and side conversation, or a bare acknowledgment (\"thanks\", \"ok\", an emoji) that needs no reply.", + "", + "When genuinely torn, answer RESPOND — silently dropping a message meant for the assistant is worse than replying once too often.", + "", + "Your entire answer must be exactly one word: RESPOND or PASS.", + ].join("\n") +} + +/** The user-message half of the classifier prompt. Pure; exported for tests. */ +export function relevancePrompt(input: FollowUpRelevanceInput): string { + const tail = (input.threadMessages ?? []).slice(-RELEVANCE_THREAD_TAIL) + const transcript = formatContextBlock("slack_thread_context", tail, { + botUserId: input.botUserId, + maxContentChars: RELEVANCE_MAX_CONTENT_CHARS, + }) + return [ + transcript ?? + (input.threadMessages === null + ? "(The thread could not be loaded; judge from the reply alone.)" + : "(No earlier thread messages.)"), + "", + "Newest message:", + formatContextMessage(input.reply, { + botUserId: input.botUserId, + maxContentChars: RELEVANCE_MAX_CONTENT_CHARS, + }), + ].join("\n") +} + +/** + * First RESPOND/PASS token in the model's answer, or `null` when there is no + * unambiguous verdict — reasoning models sometimes wrap the word, so this + * scans rather than string-compares, but an answer containing both is no + * answer. + */ +export function parseRelevanceVerdict(text: string): "respond" | "pass" | null { + const respond = /\bRESPOND\b/i.test(text) + const pass = /\bPASS\b/i.test(text) + if (respond === pass) return null + return respond ? "respond" : "pass" +} + +/** + * Judges one promoted follow-up. Never throws — every failure mode is a + * fail-open `respond: true` with a reason the caller can log. + */ +export async function judgeFollowUpRelevance( + input: FollowUpRelevanceInput, + deps?: FollowUpRelevanceDeps, +): Promise { + const { complete } = deps ?? (defaultDeps ??= buildDefaultDeps()) + let answer: string + try { + answer = await complete({ + system: relevanceSystemPrompt(input.botUserId), + prompt: relevancePrompt(input), + signal: AbortSignal.timeout(RELEVANCE_TIMEOUT_MS), + }) + } catch (error) { + console.warn("[follow-up-relevance] Classifier call failed; answering the follow-up.", error) + return { respond: true, reason: "classifier-error" } + } + + const verdict = parseRelevanceVerdict(answer) + if (verdict === null) { + console.warn( + `[follow-up-relevance] Unparseable verdict ${JSON.stringify(answer.slice(0, 200))}; answering the follow-up.`, + ) + return { respond: true, reason: "unparseable-verdict" } + } + return verdict === "respond" + ? { respond: true, reason: "model-respond" } + : { respond: false, reason: "model-pass" } +} + +/** Test-only: resets the memoized default deps. */ +export function resetFollowUpRelevanceStateForTests(): void { + defaultDeps = undefined +}