fix(slack-agent): decide thread follow-ups after the ack, not inside it

apps/slack-agent/README.md

@@ -153,8 +153,9 @@ settings:
         bot_events:
             - app_mention
             - message.im
-            # Thread follow-ups without re-mentioning the bot: replies in threads the
-            # bot is engaged in are promoted to app_mention by the webhookVerifier
+            # Thread follow-ups without re-mentioning the bot: candidate thread
+            # replies are promoted to app_mention by the webhookVerifier, and
+            # confirmed (or dropped) after the 200 by the mention handler
             # (agent/lib/thread-follow-up.ts). Only channels the bot is a member of
             # deliver these events.
             - message.channels
@@ -288,11 +289,17 @@ Both should show a green **Verified ✓** next to the field once saved. Event Su
 `app_mention`, `message.im`, `message.channels`, and `message.groups` listed under _Subscribe to bot
 events_ — the manifest from step 1 sets these, so they should already be there. The two channel
 message events power thread follow-ups: once the bot has been mentioned (or replied) in a thread,
-further replies in that thread reach it without a new `@mention` — but only while the engagement is
-**recent**: within 30 minutes and within the last 15 messages of the thread (see
-`agent/lib/thread-follow-up.ts`). Past either bound, replies pass through untouched and the user
-@-mentions the bot again. Unbounded, one mention would turn every later reply by anyone into a full
-agent turn, forever.
+further replies in that thread reach it without a new `@mention` — but only while the thread is
+still **alive** (nobody has touched it for less than 24 hours) and the bot is still within its
+**last 15 messages**. Past either bound the reply is dropped, everyone who spoke in the thread gets
+a DM saying so, and one `@mention` brings the bot back (`agent/lib/thread-follow-up.ts`,
+`agent/lib/disengage-notice.ts`). Unbounded, one mention would turn every later reply by anyone into
+a full agent turn, forever.
+
+The decision deliberately does **not** happen in the webhook verifier — that is the only awaited work
+before eve's 200, so anything it fetches is spent out of Slack's ~3s delivery budget. Promotion there
+is parse-only and optimistic; the mention handler confirms it afterwards, against the thread it loads
+for turn context anyway.
 
 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

apps/slack-agent/agent/channels/slack.ts

@@ -1,12 +1,19 @@
 import { defaultSlackAuth, slackChannel } from "eve/channels/slack"
 import type { SlackContext, SlackMentionResult, SlackMessage, SlackWebhookVerifier } from "eve/channels/slack"
-import { acknowledgeIncomingMessage } from "#lib/ack-reaction.js"
+import { acknowledgeIncomingMessage, acknowledgeMessage } from "#lib/ack-reaction.js"
 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 { resolveBotToken, verifySlackV0Signature, type SlackTokenContext } from "#lib/maple.js"
-import { loadThreadContext } from "#lib/thread-context.js"
-import { promoteThreadFollowUp, recordThreadEngagement } from "#lib/thread-follow-up.js"
+import { emitAgentLog } from "#lib/telemetry-log.js"
+import { formatThreadContext, loadThreadMessages } from "#lib/thread-context.js"
+import {
+	confirmThreadFollowUp,
+	pendingFollowUp,
+	promoteThreadFollowUp,
+	recordThreadEngagement,
+} from "#lib/thread-follow-up.js"
 import { formatTurnTime } from "#lib/turn-time.js"
 import { forwardUninstallEvent } from "#lib/uninstall-detection.js"
 
@@ -48,10 +55,9 @@ const webhookVerifier: SlackWebhookVerifier = async (request, body) => {
 	rememberBotUserId(body)
 
 	// Learn engagement from events that already prove it — the bot's own posts
-	// echoing back, and @mentions of it — so the promotion below can answer from
-	// cache instead of spending Slack's webhook budget on `conversations.replies`
-	// in a thread the bot is demonstrably active in (#lib/thread-follow-up.js).
-	// Synchronous and network-free; must run before the promotion.
+	// echoing back, and @mentions of it — so the confirmation step downstream can
+	// answer from memory in a thread the bot is demonstrably active in
+	// (#lib/thread-follow-up.js). Synchronous and network-free.
 	recordThreadEngagement(body)
 
 	// app_uninstalled / tokens_revoked: eve only dispatches app_mention + DM
@@ -61,34 +67,39 @@ const webhookVerifier: SlackWebhookVerifier = async (request, body) => {
 	void forwardUninstallEvent(body)
 
 	// Instant "received" ack: react with :eyes: on any message eve will
-	// dispatch as an agent turn (mentions + DMs), before the turn is even
+	// dispatch as an agent turn (real mentions + DMs), before the turn is even
 	// scheduled. Fired without awaiting — never delays the webhook ack.
 	// Slack redelivery retries skip it (`already_reacted` is also tolerated
 	// downstream, this just avoids the pointless call).
+	//
+	// Promoted follow-ups are deliberately NOT ack'd here. Their promotion is
+	// optimistic and `dispatchWithConversationContext` may still drop them, and
+	// acking a message we then never answer is worse than not acking at all: the
+	// :eyes: is a promise. They get their ack there, once the promotion is
+	// confirmed.
 	const isSlackRetry = request.headers.get("x-slack-retry-num") !== null
 	if (!isSlackRetry) void acknowledgeIncomingMessage(body)
 
 	// eve parses whatever body we return, which is also our hook for thread
 	// follow-ups: eve only dispatches app_mention + DM events, so an un-mentioned
-	// reply in a thread the bot is engaged in gets its `event.type` promoted to
-	// "app_mention" here (see #lib/thread-follow-up.js). Everything else passes
-	// through verified-but-unchanged.
+	// reply in a thread that could be a follow-up gets its `event.type` promoted
+	// to "app_mention" here (see #lib/thread-follow-up.js). Everything else
+	// passes through verified-but-unchanged.
+	//
+	// This is the LAST awaited work before eve returns 200, i.e. the only thing
+	// still spending Slack's ~3s delivery budget — which is why the promotion is
+	// parse-only and synchronous, and why the engagement decision itself now
+	// happens after the 200. It cannot throw, but a throw here would fail every
+	// inbound event, so the guard stays.
 	try {
-		const promoted = await promoteThreadFollowUp(body)
-		if (promoted !== null) {
-			// A promoted follow-up is agent work too, but its raw body (a plain
-			// channel `message`) doesn't qualify above — ack it now that we know
-			// the bot is engaged.
-			if (!isSlackRetry) void acknowledgeIncomingMessage(promoted)
-			return promoted
-		}
+		return promoteThreadFollowUp(body, { isSlackRetry }) ?? body
 	} catch (error) {
 		console.warn(
 			"[slack-webhook] Thread follow-up promotion failed; passing the event through unchanged.",
 			error,
 		)
+		return body
 	}
-	return body
 }
 
 /**
@@ -116,25 +127,104 @@ const webhookVerifier: SlackWebhookVerifier = async (request, body) => {
  * the alert's window instead of the current one.
  *
  * Runs after eve has already returned 200 to Slack (`waitUntil`), so the Slack
- * fetches are off the webhook's delivery budget. It must not throw: eve drops
- * the whole mention when this handler does, so both loads degrade to no context
- * instead.
+ * fetches are off the webhook's delivery budget. That is also why the thread
+ * 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.
  */
 async function dispatchWithConversationContext(
 	ctx: SlackContext,
 	message: SlackMessage,
 ): Promise<SlackMentionResult> {
-	await ctx.thread.startTyping("Thinking...")
-	const [threadContext, channelContext] = await Promise.all([
-		loadThreadContext(ctx.thread, message, { botUserId: botUserIdForTeam(message.teamId) }),
+	// Non-null only for a follow-up the verifier promoted optimistically — the
+	// one kind of dispatch that still has to earn its turn. Registry lookup, not
+	// a text sniff: eve re-renders inbound mrkdwn, so `message.text` is no longer
+	// proof of what Slack actually sent (#lib/thread-follow-up.js).
+	const pending = pendingFollowUp({
+		teamId: message.teamId,
+		channelId: message.channelId,
+		messageTs: message.ts,
+	})
+
+	// A real mention is answered for certain, so it gets the typing indicator
+	// immediately. A follow-up waits until it is confirmed: "Thinking..." on a
+	// message the bot then silently drops reads worse than saying nothing.
+	if (pending === null) await ctx.thread.startTyping("Thinking...")
+
+	const botUserId = botUserIdForTeam(message.teamId)
+	const [threadMessages, channelContext] = await Promise.all([
+		loadThreadMessages(ctx.thread, message),
 		loadChannelContext(message),
 	])
+
+	if (pending !== null) {
+		const decision = confirmThreadFollowUp(pending, threadMessages)
+		if (!decision.engaged) {
+			// The drop is silent by construction — no turn, no reply, no reaction —
+			// so this log is the only trace it leaves. hooks/outcome-log.ts cannot
+			// cover it: there is no turn for it to report on. Ids only, never text.
+			//
+			// Severity splits on the same distinction the DM does: declining a
+			// thread the bot was never in is the ordinary outcome of promoting
+			// optimistically (most channel replies are not follow-ups), while
+			// leaving a conversation the bot WAS in is a user who did not get an
+			// answer, and worth finding later.
+			emitAgentLog(decision.workedInThread ? "warn" : "info", "follow_up_disengaged", {
+				"maple.agent.event": "follow_up_disengaged",
+				"maple.agent.disengage_reason": decision.reason,
+				"maple.agent.worked_in_thread": decision.workedInThread,
+				"maple.slack.team_id": pending.teamId,
+				"maple.slack.channel_id": pending.channelId,
+				"maple.slack.thread_ts": pending.threadTs,
+				"maple.slack.message_ts": pending.messageTs,
+			})
+			// Only for a thread the bot actually worked in: leaving those people
+			// wondering why it stopped answering is the failure this fixes. A thread
+			// it was never in is ordinary channel chatter and must stay silent.
+			if (decision.workedInThread && threadMessages !== null) {
+				void notifyThreadDisengagement({
+					teamId: pending.teamId,
+					channelId: pending.channelId,
+					threadTs: pending.threadTs,
+					messageTs: pending.messageTs,
+					reason: decision.reason,
+					messages: threadMessages,
+					replierUserId: message.author?.userId,
+					botUserId,
+				})
+			}
+			// eve's `dispatchInboundMessage` catches a throwing handler and abandons
+			// the turn before the model sees anything — the only way to un-dispatch
+			// an event it has already accepted.
+			throw new Error(
+				`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.
+		if (pending.ackable) {
+			void acknowledgeMessage({
+				teamId: pending.teamId,
+				channelId: pending.channelId,
+				messageTs: pending.messageTs,
+				threadTs: pending.threadTs,
+			})
+		}
+		await ctx.thread.startTyping("Thinking...")
+	}
+
+	const threadContext = formatThreadContext(threadMessages ?? [], { botUserId })
 	// Channel first: it is the background the thread happens in. The clock last,
 	// so it sits closest to the message being answered — and so that in a thread
 	// with several turns' worth of these, the newest is the one nearest the ask.
 	return {
 		auth: defaultSlackAuth(message, ctx),
-		context: [...channelContext, ...threadContext, formatTurnTime(message)],
+		context: [
+			...channelContext,
+			...(threadContext === undefined ? [] : [threadContext]),
+			formatTurnTime(message),
+		],
 	}
 }
 

apps/slack-agent/agent/lib/ack-reaction.ts

@@ -13,12 +13,16 @@ import { ACK_REACTION_NAME, addReactionViaSlack, registerAckedTriggeringMessage
  * throws — a failed reaction is cosmetic, the turn still runs.
  *
  * It reacts exactly to the bodies eve will dispatch as agent work:
- *   - `app_mention` events (including thread follow-ups promoted to
- *     `app_mention` by `promoteThreadFollowUp` — call this on the PROMOTED
- *     body; the unpromoted twin does not qualify, so no double reaction);
+ *   - real `app_mention` events;
  *   - user-authored DM `message` events (`channel_type: "im"`, no `bot_id`,
  *     no subtype except `file_share` — mirrors eve's own DM dispatch filter).
  *
+ * Thread follow-ups promoted to `app_mention` by `promoteThreadFollowUp` are
+ * NOT ack'd from the webhook. That promotion is optimistic and the handler may
+ * still drop the message, and a `:eyes:` on a message the bot never answers is
+ * a promise broken in public — so `#channels/slack.js` acks them through
+ * `acknowledgeMessage` once the engagement check has confirmed the turn.
+ *
  * Requires the Slack app's `reactions:write` scope.
  *
  * Each qualifying message is also registered in lib/reaction.ts's
@@ -107,10 +111,23 @@ export function parseAckReactionTarget(rawBody: string): AckReactionTarget | nul
 export async function acknowledgeIncomingMessage(
 	rawBody: string,
 	deps: AckReactionDeps = defaultDeps,
+): Promise<void> {
+	const target = parseAckReactionTarget(rawBody)
+	if (!target) return
+	await acknowledgeMessage(target, deps)
+}
+
+/**
+ * Reacts with `:eyes:` on an already-identified message. The entry point for
+ * callers that no longer hold a raw webhook body — the thread-follow-up dispatch
+ * path, which only learns the turn is really happening after eve has parsed the
+ * event away. Never throws.
+ */
+export async function acknowledgeMessage(
+	target: AckReactionTarget,
+	deps: AckReactionDeps = defaultDeps,
 ): Promise<void> {
 	try {
-		const target = parseAckReactionTarget(rawBody)
-		if (!target) return
 		// Registered before the reaction call: the `add_reaction` tool needs the
 		// triggering message's ts even when this ack itself fails (its remove of
 		// a never-added `:eyes:` is tolerated downstream).