fix(slack): recover MPIM thread context from outbound replies (#116660)

Co-authored-by: scoootscooob <167050519+scoootscooob@users.noreply.github.com>
This commit is contained in:
Harjoth Khara
2026-07-31 08:12:56 -07:00
committed by GitHub
parent 77b3667757
commit 102c2e1a64
16 changed files with 699 additions and 48 deletions
+3 -2
View File
@@ -685,8 +685,9 @@ Slack YAML module scenarios (`qa/scenarios/channels/slack-*.yaml`):
- `slack-canary`
- `slack-mention-gating`
- `slack-mpim-app-mention-dedupe` - opens a real C-prefixed group DM, sends one
mention, verifies exactly one SUT reply in that MPIM, then closes it.
- `slack-mpim-app-mention-dedupe` - opens a real C-prefixed group DM, verifies
exactly one SUT reply after message/app-mention twin delivery, confirms a
native threaded follow-up can recall that bot reply, then closes the MPIM.
- `slack-allowlist-block`
- `slack-channel-disabled-warning` - opt-in real-Slack probe that confirms a
configured disabled channel emits a structured warning without replying.
@@ -166,13 +166,17 @@ describe("Slack live QA runtime helpers", () => {
).toEqual(["slack-mpim-app-mention-dedupe"]);
});
it("enables group DMs for the MPIM app-mention scenario", () => {
it("enables group DMs and threaded replies for the MPIM app-mention scenario", () => {
const scenario = testing.findScenario(["slack-mpim-app-mention-dedupe"])[0];
if (!scenario) {
throw new Error("missing Slack MPIM app-mention scenario");
}
const cfg = testing.buildSlackQaConfig(
{},
{
channelId: "C123456789",
driverBotUserId: "U999999999",
overrides: { groupDmEnabled: true },
overrides: scenario.configOverrides,
sutAccountId: "sut",
sutAppToken: "xapp-sut",
sutBotToken: "xoxb-sut",
@@ -183,6 +187,7 @@ describe("Slack live QA runtime helpers", () => {
enabled: true,
groupEnabled: true,
});
expect(cfg.channels?.slack?.accounts?.sut?.replyToMode).toBe("all");
});
it("surfaces MPIM cleanup failures and retains ownership for a retry", async () => {
@@ -227,6 +232,144 @@ describe("Slack live QA runtime helpers", () => {
expect(close).toHaveBeenNthCalledWith(3, { channel: "C_MPIM" });
});
it("keeps the MPIM recall turn in the native thread", async () => {
const run = testing.findScenario(["slack-mpim-app-mention-dedupe"])[0]?.buildRun("U_SUT");
if (
!run ||
run.kind === "approval" ||
run.kind === "codex-approval" ||
run.kind === "direct-transport" ||
!run.afterReply
) {
throw new Error("expected Slack MPIM message scenario with a recall turn");
}
const seedMarker = /SLACK_QA_MPIM_SEED_[A-Z0-9]+/u.exec(run.input)?.[0];
if (!seedMarker) {
throw new Error("missing Slack MPIM seed marker");
}
expect(run.input).toContain(
`Reply with only a marker in this exact format: ${seedMarker}_BOT_<NONCE>.`,
);
expect(run.input).toContain("Replace <NONCE> with 8 to 32 new uppercase letters or digits.");
const botReplyMarker = `${seedMarker}_BOT_TESTNONCE`;
const recallMarker = seedMarker.replace("SEED", "RECALL");
const expectedRecallMarker = `${recallMarker}_TESTNONCE`;
const postMessage = vi.fn(async (_request: { text?: string }) => ({
channel: "C_MPIM",
ts: "2.000000",
}));
const history = vi.fn(async () => ({ messages: [] }));
const replies = vi.fn(async () => ({
messages: [
{
bot_id: "B_SUT",
text: expectedRecallMarker,
thread_ts: "1.000000",
ts: "3.000000",
user: "U_SUT",
},
],
}));
await expect(
run.afterReply(
{
text: botReplyMarker,
thread_ts: "1.000000",
ts: "1.500000",
user: "U_SUT",
},
{
channelId: "C_MPIM",
driverClient: { chat: { postMessage } },
sentTs: "1.000000",
sutIdentity: { botId: "B_SUT", userId: "U_SUT" },
sutReadClient: { conversations: { history, replies } },
} as never,
),
).resolves.toContain("recovered the prior bot reply");
expect(postMessage).toHaveBeenCalledWith(
expect.objectContaining({
channel: "C_MPIM",
thread_ts: "1.000000",
}),
);
const recallText = postMessage.mock.calls[0]?.[0]?.text;
expect(recallText).toContain(`previous reply beginning with ${seedMarker}_BOT_`);
expect(recallText).toContain(`exact format: ${recallMarker}_<NONCE>`);
expect(recallText).not.toContain(botReplyMarker);
expect(recallText).not.toContain("TESTNONCE");
});
it("rejects an MPIM seed reply without text before sending the recall turn", async () => {
const run = testing.findScenario(["slack-mpim-app-mention-dedupe"])[0]?.buildRun("U_SUT");
if (
!run ||
run.kind === "approval" ||
run.kind === "codex-approval" ||
run.kind === "direct-transport" ||
!run.afterReply
) {
throw new Error("expected Slack MPIM message scenario with a recall turn");
}
const postMessage = vi.fn();
await expect(
run.afterReply(
{
thread_ts: "1.000000",
ts: "1.500000",
user: "U_SUT",
},
{
channelId: "C_MPIM",
driverClient: { chat: { postMessage } },
sentTs: "1.000000",
sutIdentity: { botId: "B_SUT", userId: "U_SUT" },
sutReadClient: { conversations: {} },
} as never,
),
).rejects.toThrow("MPIM seed reply did not contain the provider-generated bot nonce");
expect(postMessage).not.toHaveBeenCalled();
});
it("rejects an MPIM seed reply outside the native thread", async () => {
const run = testing.findScenario(["slack-mpim-app-mention-dedupe"])[0]?.buildRun("U_SUT");
if (
!run ||
run.kind === "approval" ||
run.kind === "codex-approval" ||
run.kind === "direct-transport" ||
!run.afterReply
) {
throw new Error("expected Slack MPIM message scenario with a recall turn");
}
const seedMarker = /SLACK_QA_MPIM_SEED_[A-Z0-9]+/u.exec(run.input)?.[0];
if (!seedMarker) {
throw new Error("missing Slack MPIM seed marker");
}
const postMessage = vi.fn();
await expect(
run.afterReply(
{
text: `${seedMarker}_BOT_TESTNONCE`,
ts: "1.500000",
user: "U_SUT",
},
{
channelId: "C_MPIM",
driverClient: { chat: { postMessage } },
sentTs: "1.000000",
sutIdentity: { botId: "B_SUT", userId: "U_SUT" },
sutReadClient: { conversations: {} },
} as never,
),
).rejects.toThrow("MPIM seed reply escaped the native Slack thread");
expect(postMessage).not.toHaveBeenCalled();
});
it("selects native scenarios by explicit id", () => {
const scenarioIds = [
"slack-chart-presentation-native",
@@ -9,10 +9,12 @@ import {
type SlackQaScenarioImplementation,
type SlackQaScenarioContext,
} from "./slack-live.contracts.js";
import { waitForSlackScenarioReply } from "./slack-live.message-observations.js";
import {
isExpectedSlackNativeChartMessage,
isExpectedSlackNativeTableMessage,
runSlackTableInvalidBlocksFallbackScenario,
sendSlackChannelMessage,
waitForSlackStoredMessage,
} from "./slack-live.observations.js";
import {
@@ -46,9 +48,12 @@ export const slackQaMentionGatingScenario: SlackQaScenarioImplementation = {
};
export const slackQaMpimAppMentionDedupeScenario: SlackQaScenarioImplementation = {
configOverrides: { groupDmEnabled: true },
configOverrides: { groupDmEnabled: true, replyToMode: "all" },
buildRun: (sutUserId) => {
const token = `SLACK_QA_MPIM_${randomUUID().slice(0, 8).toUpperCase()}`;
const suffix = randomUUID().slice(0, 8).toUpperCase();
const seedMarker = `SLACK_QA_MPIM_SEED_${suffix}`;
const recallMarker = `SLACK_QA_MPIM_RECALL_${suffix}`;
const missingMarker = `SLACK_QA_MPIM_MISSING_${suffix}`;
let openedChannelId: string | undefined;
const closeOpenedChannel = async (context: Omit<SlackQaScenarioContext, "sentTs">) => {
if (!openedChannelId) {
@@ -72,8 +77,13 @@ export const slackQaMpimAppMentionDedupeScenario: SlackQaScenarioImplementation
};
return {
expectReply: true,
input: `<@${sutUserId}> reply with only this exact marker: ${token}`,
matchText: token,
input: [
`<@${sutUserId}> Slack MPIM assistant-history seed check.`,
`Reply with only a marker in this exact format: ${seedMarker}_BOT_<NONCE>.`,
"Replace <NONCE> with 8 to 32 new uppercase letters or digits.",
"Do not include angle brackets, spaces, Markdown, or punctuation.",
].join(" "),
matchText: seedMarker,
settleObservedMs: 60_000,
beforeRun: async (context) => {
const driverAuth = await context.driverClient.auth.test();
@@ -115,7 +125,7 @@ export const slackQaMpimAppMentionDedupeScenario: SlackQaScenarioImplementation
verifyObserved: ({ messages }) => {
const uniqueReplies = new Map(messages.map((message) => [message.ts, message]));
const matchingReplies = [...uniqueReplies.values()].filter((message) =>
message.text.includes(token),
message.text.includes(seedMarker),
);
if (uniqueReplies.size !== 1 || matchingReplies.length !== 1) {
throw new Error(
@@ -124,6 +134,55 @@ export const slackQaMpimAppMentionDedupeScenario: SlackQaScenarioImplementation
}
return "one MPIM reply observed after message/app_mention twin delivery";
},
afterReply: async (message, context) => {
if (message.thread_ts !== context.sentTs) {
throw new Error("MPIM seed reply escaped the native Slack thread");
}
const botReplyMarker = message.text?.trim() ?? "";
const botReplyPrefix = `${seedMarker}_BOT_`;
const botNonce = botReplyMarker.startsWith(botReplyPrefix)
? botReplyMarker.slice(botReplyPrefix.length)
: "";
if (!/^[A-Z0-9]{8,32}$/u.test(botNonce)) {
throw new Error("MPIM seed reply did not contain the provider-generated bot nonce");
}
const expectedRecallMarker = `${recallMarker}_${botNonce}`;
const sent = await sendSlackChannelMessage({
channelId: context.channelId,
client: context.driverClient,
text: [
`<@${sutUserId}> Slack MPIM assistant-history recall check.`,
`Recall the nonce from your immediately previous reply beginning with ${botReplyPrefix}.`,
`Reply with only this exact format: ${recallMarker}_<NONCE>, using that same nonce.`,
`Otherwise reply with only: ${missingMarker}`,
].join(" "),
threadTs: context.sentTs,
});
const reply = await waitForSlackScenarioReply({
channelId: context.channelId,
client: context.sutReadClient,
matchText: expectedRecallMarker,
observedMessages: [],
observationScenarioId: "slack-mpim-app-mention-dedupe",
observationScenarioTitle: "Slack MPIM app mention dispatches once with thread context",
sentTs: sent.ts,
sutIdentity: context.sutIdentity,
threadTs: context.sentTs,
timeoutMs: 60_000,
});
if (reply.message.thread_ts !== context.sentTs) {
throw new Error("MPIM assistant-history recall reply escaped the native Slack thread");
}
if (reply.message.text?.trim() !== expectedRecallMarker) {
throw new Error("MPIM assistant-history recall reply did not reproduce the hidden nonce");
}
return [
"threadHistoryHeader=true",
"assistantAttributedSeed=true",
"recalledNonceMatched=true",
"threaded MPIM follow-up recovered the prior bot reply as assistant history",
].join("; ");
},
cleanup: closeOpenedChannel,
};
},
@@ -11,6 +11,9 @@ import {
QA_SUBAGENT_DIRECT_FALLBACK_MARKER,
QA_IMAGE_GENERATION_PROMPT_RE,
QA_SKILL_WORKSHOP_GIF_PROMPT_RE,
QA_SLACK_MPIM_HISTORY_RECALL_PROMPT_RE,
QA_SLACK_MPIM_HISTORY_SEED_PROMPT_RE,
buildSlackMpimHistoryBotReply,
QA_TOOL_SEARCH_PROMPT_RE,
QA_TOOL_SEARCH_FAILURE_PROMPT_RE,
type MockScenarioState,
@@ -33,6 +36,7 @@ import {
extractLastUserText,
extractToolOutput,
extractLatestToolOutput,
extractSlackMpimRetainedBotNonce,
extractAllUserTexts,
extractAllRequestTexts,
extractLatestImageUserTurn,
@@ -137,6 +141,18 @@ export function buildAssistantText(
toolJson,
});
const slackMpimHistoryRecall = QA_SLACK_MPIM_HISTORY_RECALL_PROMPT_RE.exec(prompt);
if (slackMpimHistoryRecall) {
const [, botReplyPrefix, recalledMarker, missingMarker] = slackMpimHistoryRecall;
const nonce = botReplyPrefix
? extractSlackMpimRetainedBotNonce(prompt, botReplyPrefix)
: undefined;
return nonce && recalledMarker ? `${recalledMarker}_${nonce}` : (missingMarker ?? "");
}
const slackMpimHistorySeed = QA_SLACK_MPIM_HISTORY_SEED_PROMPT_RE.exec(prompt)?.[1];
if (slackMpimHistorySeed) {
return buildSlackMpimHistoryBotReply(slackMpimHistorySeed);
}
if (/what was the qa canary code/i.test(prompt) && rememberedFact) {
return `Protocol note: the QA canary code was ${rememberedFact}.`;
}
@@ -1,4 +1,5 @@
// QA Lab mock provider contracts, wire helpers, and scenario constants.
import { randomUUID } from "node:crypto";
import type { IncomingMessage, ServerResponse } from "node:http";
import { setTimeout as sleep } from "node:timers/promises";
import { readRequestBodyWithLimit } from "openclaw/plugin-sdk/webhook-ingress";
@@ -207,6 +208,14 @@ export const QA_TELEGRAM_LONG_FINAL_PROMPT_RE = /telegram long final qa check/i;
export const QA_WHATSAPP_LONG_FINAL_PROMPT_RE = /whatsapp long final qa check/i;
export const QA_SLACK_CHART_PRESENTATION_PROMPT_RE =
/Slack native chart QA check\s+(SLACK_QA_CHART_SUMMARY_[A-Z0-9]+)[\s\S]*?reply with only this exact marker:\s*(SLACK_QA_CHART_DONE_[A-Z0-9]+)/i;
export const QA_SLACK_MPIM_HISTORY_SEED_PROMPT_RE =
/Slack MPIM assistant-history seed check[\s\S]*?exact format:\s*(SLACK_QA_MPIM_SEED_[A-Z0-9]+)_BOT_<NONCE>/i;
export const QA_SLACK_MPIM_HISTORY_RECALL_PROMPT_RE =
/Slack MPIM assistant-history recall check[\s\S]*?previous reply beginning with\s+(SLACK_QA_MPIM_SEED_[A-Z0-9]+_BOT_)[\s\S]*?exact format:\s*(SLACK_QA_MPIM_RECALL_[A-Z0-9]+)_<NONCE>[\s\S]*?otherwise reply with only:\s*(SLACK_QA_MPIM_MISSING_[A-Z0-9]+)/i;
export function buildSlackMpimHistoryBotReply(seedMarker: string) {
return `${seedMarker}_BOT_${randomUUID().replaceAll("-", "").toUpperCase()}`;
}
export const QA_WHATSAPP_AGENT_MESSAGE_ACTION_REACT_PROMPT_RE =
/react to this whatsapp(?: group)? message with thumbs up for qa action check\s+(?:WHATSAPP_QA_AGENT_REACT|WHATSAPP_QA_GROUP_AGENT_REACT)_[A-Z0-9]+/i;
export const QA_WHATSAPP_AGENT_MESSAGE_ACTION_UPLOAD_PROMPT_RE =
@@ -291,6 +291,41 @@ export function extractAllUserTexts(input: ResponsesInputItem[]) {
return texts;
}
export function extractSlackMpimRetainedBotNonce(
prompt: string,
botReplyPrefix: string,
): string | undefined {
const historyHeader = "[Thread history - for context]\n";
const historyStart = prompt.indexOf(historyHeader);
if (historyStart < 0) {
return undefined;
}
const historyBodyStart = historyStart + historyHeader.length;
const currentTurnStart = prompt.lastIndexOf("Slack MPIM assistant-history recall check.");
if (currentTurnStart < historyBodyStart) {
return undefined;
}
for (const line of prompt.slice(historyBodyStart, currentTurnStart).split(/\r?\n/u)) {
const headerEnd = line.indexOf("] ");
if (headerEnd < 0) {
continue;
}
const header = line.slice(0, headerEnd);
if (!header.startsWith("[Slack ") || !header.includes(" (this assistant) (assistant) ")) {
continue;
}
const reply = line.slice(headerEnd + 2);
if (!reply.startsWith(botReplyPrefix)) {
continue;
}
const nonce = reply.slice(botReplyPrefix.length);
if (/^[A-Z0-9]{8,32}$/u.test(nonce)) {
return nonce;
}
}
return undefined;
}
export function extractAllInputTexts(input: ResponsesInputItem[]) {
const texts: string[] = [];
for (const item of input) {
@@ -1433,6 +1433,85 @@ describe("qa mock openai server", () => {
expect(body).not.toContain('"name":"read"');
});
it("requires retained bot history in the Slack MPIM thread-history prelude", async () => {
const server = await startMockServer();
const seedMarker = "SLACK_QA_MPIM_SEED_A1B2C3D4";
const recallMarker = "SLACK_QA_MPIM_RECALL_A1B2C3D4";
const missingMarker = "SLACK_QA_MPIM_MISSING_A1B2C3D4";
const seedPrompt =
`Slack MPIM assistant-history seed check. Reply with only a marker in this exact format: ${seedMarker}_BOT_<NONCE>. ` +
"Replace <NONCE> with 8 to 32 new uppercase letters or digits. " +
"Do not include angle brackets, spaces, Markdown, or punctuation.";
const seedResponse = await expectResponsesJson<unknown>(server, {
stream: false,
model: "gpt-5.6-luna",
input: [makeUserInput(seedPrompt)],
});
const botReplyMarker = outputText(seedResponse);
expect(botReplyMarker).toMatch(new RegExp(`^${seedMarker}_BOT_[A-Z0-9]+$`, "u"));
const botNonce = botReplyMarker.slice(`${seedMarker}_BOT_`.length);
const expectedRecallMarker = `${recallMarker}_${botNonce}`;
const recallPrompt = [
"Slack MPIM assistant-history recall check.",
`Recall the nonce from your immediately previous reply beginning with ${seedMarker}_BOT_.`,
`Reply with only this exact format: ${recallMarker}_<NONCE>, using that same nonce.`,
`Otherwise reply with only: ${missingMarker}`,
].join(" ");
expect(recallPrompt).not.toContain(botReplyMarker);
expect(recallPrompt).not.toContain(botNonce);
const withRetainedBotHistory = await expectResponsesJson<unknown>(server, {
stream: false,
model: "gpt-5.6-luna",
input: [
makeUserInput(
[
"[Thread history - for context]",
`[Slack Driver (user) Fri 2026-07-31 10:00 UTC] ${seedPrompt}`,
"[slack message id: 1.000000 channel: C123]",
"",
`[Slack OpenClaw (this assistant) (assistant) Fri 2026-07-31 10:01 UTC] ${botReplyMarker}`,
"[slack message id: 1.500000 channel: C123]",
"",
`[Slack Driver (user) Fri 2026-07-31 10:02 UTC] ${recallPrompt}`,
].join("\n"),
),
],
});
expect(outputText(withRetainedBotHistory)).toBe(expectedRecallMarker);
const withStructuredAssistantHistoryOnly = await expectResponsesJson<unknown>(server, {
stream: false,
model: "gpt-5.6-luna",
input: [
makeUserInput(seedPrompt),
{
role: "assistant",
content: [{ type: "output_text", text: botReplyMarker }],
},
makeUserInput(recallPrompt),
],
});
expect(outputText(withStructuredAssistantHistoryOnly)).toBe(missingMarker);
const withHumanAttributedSeed = await expectResponsesJson<unknown>(server, {
stream: false,
model: "gpt-5.6-luna",
input: [
makeUserInput(
[
"[Thread history - for context]",
`[Slack Alice (user) Fri 2026-07-31 10:00 UTC] ${botReplyMarker}`,
"[slack message id: 1.000000 channel: C123]",
"",
`[Slack Driver (user) Fri 2026-07-31 10:02 UTC] ${recallPrompt}`,
].join("\n"),
),
],
});
expect(outputText(withHumanAttributedSeed)).toBe(missingMarker);
});
it("drives repo-contract followthrough as read-read-read-write-then-report", async () => {
const server = await startMockServer();
@@ -43,6 +43,9 @@ import {
QA_TELEGRAM_LONG_FINAL_PROMPT_RE,
QA_WHATSAPP_LONG_FINAL_PROMPT_RE,
QA_SLACK_CHART_PRESENTATION_PROMPT_RE,
QA_SLACK_MPIM_HISTORY_RECALL_PROMPT_RE,
QA_SLACK_MPIM_HISTORY_SEED_PROMPT_RE,
buildSlackMpimHistoryBotReply,
QA_WHATSAPP_AGENT_MESSAGE_ACTION_REACT_PROMPT_RE,
QA_WHATSAPP_AGENT_MESSAGE_ACTION_UPLOAD_PROMPT_RE,
QA_SUBAGENT_DIRECT_FALLBACK_PROMPT_RE,
@@ -126,6 +129,7 @@ import {
extractLatestToolOutput,
extractAllToolOutputText,
extractUserTextAfterLatestToolOutput,
extractSlackMpimRetainedBotNonce,
extractAllUserTexts,
extractAllInputTexts,
extractInstructionsText,
@@ -1015,6 +1019,20 @@ async function buildResponsesPayload(
if (whatsAppStickerMarker) {
return buildAssistantEvents(whatsAppStickerMarker);
}
const slackMpimHistoryRecall = QA_SLACK_MPIM_HISTORY_RECALL_PROMPT_RE.exec(prompt);
if (slackMpimHistoryRecall) {
const [, botReplyPrefix, recalledMarker, missingMarker] = slackMpimHistoryRecall;
const nonce = botReplyPrefix
? extractSlackMpimRetainedBotNonce(prompt, botReplyPrefix)
: undefined;
return buildAssistantEvents(
nonce && recalledMarker ? `${recalledMarker}_${nonce}` : (missingMarker ?? ""),
);
}
const slackMpimHistorySeed = QA_SLACK_MPIM_HISTORY_SEED_PROMPT_RE.exec(prompt)?.[1];
if (slackMpimHistorySeed) {
return buildAssistantEvents(buildSlackMpimHistoryBotReply(slackMpimHistorySeed));
}
if (/\bmarker\b/i.test(prompt) && promptExactMarkerDirective) {
return buildAssistantEvents(promptExactMarkerDirective);
}
@@ -72,7 +72,7 @@ describe("resolveSlackThreadHistoryFilterPolicy", () => {
includeBotStarterAsRootContext: true,
starterTs: "1",
}),
).toEqual({ retainCurrentBotRootTs: "1" });
).toEqual({ currentBot: "root-only", rootTs: "1" });
});
it("filters current-bot messages on existing sessions", () => {
@@ -81,7 +81,17 @@ describe("resolveSlackThreadHistoryFilterPolicy", () => {
includeBotStarterAsRootContext: false,
starterTs: "1",
}),
).toEqual({});
).toEqual({ currentBot: "omit" });
});
it("retains current-bot history when the owner requests thread reconstruction", () => {
expect(
resolveSlackThreadHistoryFilterPolicy({
includeBotStarterAsRootContext: false,
starterTs: "1",
retainCurrentBotHistory: true,
}),
).toEqual({ currentBot: "all" });
});
});
@@ -96,7 +106,7 @@ describe("applySlackThreadHistoryFilterPolicy", () => {
];
const result = applySlackThreadHistoryFilterPolicy({
history,
policy: { retainCurrentBotRootTs: "1" },
policy: { currentBot: "root-only", rootTs: "1" },
identity,
});
expect(result.kept.map((entry) => entry.ts)).toEqual(["1", "2"]);
@@ -112,17 +122,32 @@ describe("applySlackThreadHistoryFilterPolicy", () => {
];
const result = applySlackThreadHistoryFilterPolicy({
history,
policy: {},
policy: { currentBot: "omit" },
identity,
});
expect(result.kept.map((entry) => entry.ts)).toEqual(["3", "4"]);
expect(result.omittedCurrentBot).toBe(2);
});
it("keeps all current-bot messages when reconstructing a fresh thread", () => {
const history = [
{ ts: "1", userId: "U1", text: "user root" },
{ ts: "2", botId: "B1", text: "assistant reply" },
{ ts: "3", userId: "U_BOT", text: "assistant follow-up" },
];
const result = applySlackThreadHistoryFilterPolicy({
history,
policy: { currentBot: "all" },
identity,
});
expect(result.kept).toEqual(history);
expect(result.omittedCurrentBot).toBe(0);
});
it("returns an empty result for empty history", () => {
const result = applySlackThreadHistoryFilterPolicy({
history: [] as Array<{ ts: string; userId?: string; botId?: string }>,
policy: {},
policy: { currentBot: "omit" },
identity,
});
expect(result.kept).toEqual([]);
@@ -16,9 +16,9 @@ type SlackThreadRootCandidate = SlackThreadAuthorTuple & {
ts?: string;
};
type SlackThreadHistoryFilterPolicy = {
retainCurrentBotRootTs?: string;
};
type SlackThreadHistoryFilterPolicy =
| { currentBot: "omit" | "all" }
| { currentBot: "root-only"; rootTs: string };
type SlackThreadHistoryFilterResult<T> = {
kept: T[];
@@ -42,12 +42,17 @@ export function isSlackThreadAuthorCurrentBot(params: {
export function resolveSlackThreadHistoryFilterPolicy(params: {
includeBotStarterAsRootContext: boolean;
starterTs?: string;
retainCurrentBotHistory?: boolean;
}): SlackThreadHistoryFilterPolicy {
if (params.retainCurrentBotHistory) {
return { currentBot: "all" };
}
if (!params.includeBotStarterAsRootContext || !params.starterTs) {
return {};
return { currentBot: "omit" };
}
return {
retainCurrentBotRootTs: params.starterTs,
currentBot: "root-only",
rootTs: params.starterTs,
};
}
@@ -67,7 +72,10 @@ export function applySlackThreadHistoryFilterPolicy<T extends SlackThreadRootCan
kept.push(entry);
continue;
}
if (params.policy.retainCurrentBotRootTs && entry.ts === params.policy.retainCurrentBotRootTs) {
if (
params.policy.currentBot === "all" ||
(params.policy.currentBot === "root-only" && entry.ts === params.policy.rootTs)
) {
kept.push(entry);
} else {
omittedCurrentBot += 1;
@@ -62,6 +62,9 @@ describe("resolveSlackThreadContextData", () => {
allowFromLower: string[];
allowNameMatching: boolean;
sessionState?: "missing" | "fresh" | "stale";
sessionLastInteractionAt?: number;
sessionUpdatedAt?: number;
isGroupDm?: boolean;
}) {
const { storePath } = storeFixture.makeTmpStorePath();
const replies = vi.fn().mockResolvedValue({
@@ -73,7 +76,20 @@ describe("resolveSlackThreadContextData", () => {
ctx.channelRuntime = {
...ctx.channelRuntime!,
session: {
resolveEntryResetFreshness: () => ({ state: params.sessionState }),
resolveEntryResetFreshness: () =>
params.sessionState === "missing"
? { state: "missing", entry: undefined }
: {
state: params.sessionState,
entry: {
...(params.sessionLastInteractionAt !== undefined
? { lastInteractionAt: params.sessionLastInteractionAt }
: {}),
...(params.sessionUpdatedAt !== undefined
? { updatedAt: params.sessionUpdatedAt }
: {}),
},
},
},
};
}
@@ -87,6 +103,7 @@ describe("resolveSlackThreadContextData", () => {
ctx,
account: createSlackTestAccount({ thread: { initialHistoryLimit: 20 } }),
message: createThreadMessage(),
isGroupDm: params.isGroupDm ?? false,
isThreadReply: true,
threadTs: "100.000",
threadStarter: params.threadStarter,
@@ -128,14 +145,20 @@ describe("resolveSlackThreadContextData", () => {
{
title: "does not hydrate starter media for an existing thread session",
sessionState: "fresh" as const,
sessionLastInteractionAt: 100,
hydrates: false,
},
{
title: "hydrates starter media for an outbound-only thread session",
sessionState: "fresh" as const,
hydrates: true,
},
{
title: "hydrates starter media after a thread session reset",
sessionState: "stale" as const,
hydrates: true,
},
])("$title", async ({ sessionState, hydrates }) => {
])("$title", async ({ sessionState, sessionLastInteractionAt, hydrates }) => {
const resolveSlackMedia = vi
.spyOn(mediaModule, "resolveSlackMedia")
.mockResolvedValue(starterMedia);
@@ -145,6 +168,7 @@ describe("resolveSlackThreadContextData", () => {
allowFromLower: ["u1"],
allowNameMatching: false,
sessionState,
sessionLastInteractionAt,
});
expect(result.threadStarterMedia).toEqual(hydrates ? starterMedia : null);
@@ -179,28 +203,120 @@ describe("resolveSlackThreadContextData", () => {
expect(replies).toHaveBeenCalledTimes(1);
});
it("filters prior current-bot replies from user-started threads on new sessions", async () => {
it.each([
{
title: "filters them from missing channel threads",
isGroupDm: false,
sessionState: "missing" as const,
retained: false,
},
{
title: "filters them from fresh outbound-only channel threads",
isGroupDm: false,
sessionState: "fresh" as const,
retained: false,
},
{
title: "filters them from stale outbound-only channel threads",
isGroupDm: false,
sessionState: "stale" as const,
retained: false,
},
{
title: "retains them for missing MPIM threads",
isGroupDm: true,
sessionState: "missing" as const,
retained: true,
},
{
title: "retains them for fresh outbound-only MPIM threads",
isGroupDm: true,
sessionState: "fresh" as const,
retained: true,
},
{
title: "retains them for stale outbound-only MPIM threads",
isGroupDm: true,
sessionState: "stale" as const,
retained: true,
},
{
title: "filters them after an inbound MPIM interaction",
isGroupDm: true,
sessionState: "stale" as const,
sessionLastInteractionAt: 100,
retained: false,
},
{
title: "filters them after an explicit MPIM reset",
isGroupDm: true,
sessionState: "stale" as const,
sessionUpdatedAt: 0,
retained: false,
},
])(
"$title",
async ({ isGroupDm, sessionState, sessionLastInteractionAt, sessionUpdatedAt, retained }) => {
const { result } = await resolveAllowlistedThreadContext({
repliesMessages: [
{ text: "starter from Alice", user: "U1", ts: "100.000" },
{ text: "assistant progress update", bot_id: "B1", ts: "100.200" },
{ text: "allowed follow-up", user: "U1", ts: "100.800" },
{ text: "current message", user: "U1", ts: "101.000" },
],
threadStarter: {
text: "starter from Alice",
userId: "U1",
ts: "100.000",
},
allowFromLower: ["u1"],
allowNameMatching: false,
sessionState,
sessionLastInteractionAt,
sessionUpdatedAt,
isGroupDm,
});
expect(result.threadStarterBody).toBe("starter from Alice");
expect(result.threadHistoryBody).toContain("starter from Alice");
expect(result.threadHistoryBody).toContain("allowed follow-up");
if (retained) {
expect(result.threadHistoryBody).toContain("assistant progress update");
expect(result.threadHistoryBody).toContain("Bot (this assistant) (assistant)");
} else {
expect(result.threadHistoryBody).not.toContain("assistant progress update");
}
expect(result.threadHistoryBody).not.toContain("current message");
},
);
it("keeps the 20-message cap and excludes the current MPIM message", async () => {
const priorMessages = Array.from({ length: 22 }, (_, index) => ({
text: index === 20 ? "assistant answer to retain" : `prior user message ${index}`,
...(index === 20 ? { bot_id: "B1" } : { user: "U1" }),
ts: `100.${String(index).padStart(3, "0")}`,
}));
const { result } = await resolveAllowlistedThreadContext({
repliesMessages: [
{ text: "starter from Alice", user: "U1", ts: "100.000" },
{ text: "assistant progress update", bot_id: "B1", ts: "100.200" },
{ text: "allowed follow-up", user: "U1", ts: "100.800" },
{ text: "current message", user: "U1", ts: "101.000" },
],
repliesMessages: [...priorMessages, { text: "current message", user: "U1", ts: "101.000" }],
threadStarter: {
text: "starter from Alice",
text: "prior user message 0",
userId: "U1",
ts: "100.000",
},
allowFromLower: ["u1"],
allowNameMatching: false,
sessionState: "fresh",
isGroupDm: true,
});
expect(result.threadStarterBody).toBe("starter from Alice");
expect(result.threadHistoryBody).toContain("starter from Alice");
expect(result.threadHistoryBody).toContain("allowed follow-up");
expect(result.threadHistoryBody).not.toContain("assistant progress update");
expect(result.threadHistoryBody).not.toContain("current message");
const history = result.threadHistoryBody ?? "";
expect(history.match(/\[slack message id:/g)).toHaveLength(20);
expect(history).not.toContain("[slack message id: 100.000 channel: C123]");
expect(history).not.toContain("[slack message id: 100.001 channel: C123]");
expect(history).toContain("prior user message 21");
expect(history).toContain("assistant answer to retain");
expect(history).toContain("Bot (this assistant) (assistant)");
expect(history).not.toContain("current message");
});
it("keeps starter text and history when allowNameMatching authorizes the sender", async () => {
@@ -285,6 +401,7 @@ describe("resolveSlackThreadContextData", () => {
ctx,
account: createSlackTestAccount({ thread: { initialHistoryLimit: 20 } }),
message: createThreadMessage(),
isGroupDm: false,
isThreadReply: true,
threadTs: "100.000",
threadStarter: {
@@ -331,6 +448,7 @@ describe("resolveSlackThreadContextData", () => {
ctx,
account: createSlackTestAccount({ thread: { initialHistoryLimit: 1 } }),
message: createThreadMessage(),
isGroupDm: false,
isThreadReply: true,
threadTs: "100.000",
threadStarter: {
@@ -455,6 +573,7 @@ describe("resolveSlackThreadContextData", () => {
text: "actually it's Sunday 12:30 pm - apologize and correct",
ts: "101.000",
}),
isGroupDm: false,
isThreadReply: true,
threadTs: "100.000",
threadStarter: {
@@ -43,9 +43,18 @@ type SlackThreadContextData = {
const SLACK_THREAD_CONTEXT_USER_LOOKUP_CONCURRENCY = 4;
type SlackSessionResetFreshness = {
state: "missing" | "fresh" | "stale";
};
type SlackSessionResetFreshness =
| {
state: "missing";
entry: undefined;
}
| {
state: "fresh" | "stale";
entry: {
lastInteractionAt?: number;
updatedAt?: number;
};
};
type SlackSessionFreshnessRuntime = {
session?: {
@@ -146,6 +155,7 @@ export async function resolveSlackThreadContextData(params: {
ctx: SlackMonitorContext;
account: ResolvedSlackAccount;
message: SlackMessageEvent;
isGroupDm: boolean;
isThreadReply: boolean;
threadTs: string | undefined;
threadStarter: SlackThreadStarter | null;
@@ -188,11 +198,21 @@ export async function resolveSlackThreadContextData(params: {
sessionKey: params.sessionKey,
})
: undefined;
const isMissingThreadSession = threadSessionFreshness
? threadSessionFreshness.state === "missing"
: threadSessionPreviousTimestamp === undefined;
// A zero updatedAt is an explicit reset tombstone, not an outbound-created row.
// Rehydrating it would resurrect history that the reset intentionally discarded.
const isOutboundOnlyThreadSession =
threadSessionFreshness !== undefined &&
threadSessionFreshness.state !== "missing" &&
threadSessionFreshness.entry.lastInteractionAt === undefined &&
threadSessionFreshness.entry.updatedAt !== 0;
const shouldSeedInitialThreadContext = Boolean(
params.isThreadReply &&
params.threadTs &&
(threadSessionFreshness
? threadSessionFreshness.state !== "fresh"
? threadSessionFreshness.state !== "fresh" || isOutboundOnlyThreadSession
: threadSessionPreviousTimestamp === undefined),
);
const shouldLoadInitialThreadHistory =
@@ -309,6 +329,11 @@ export async function resolveSlackThreadContextData(params: {
const historyFilterPolicy = resolveSlackThreadHistoryFilterPolicy({
includeBotStarterAsRootContext,
starterTs: currentBotRootTs,
// MPIM roots intentionally stay on the flat group session. Outbound
// delivery may create the reply-thread session before its first inbound
// turn, so recover those assistant replies when hydrating that session.
retainCurrentBotHistory:
params.isGroupDm && (isMissingThreadSession || isOutboundOnlyThreadSession),
});
const {
kept: threadHistoryWithoutCurrentBot,
@@ -1182,6 +1182,7 @@ describe("slack prepareSlackMessage inbound contract", () => {
currentTs: string;
channelsConfig?: Parameters<typeof createInboundSlackCtx>[0]["channelsConfig"];
allowFrom?: string[];
outboundOnlySessionKey?: string;
resolveChannelName?: (channelId: string) => Promise<{
name?: string;
type?: SlackMessageEvent["channel_type"];
@@ -1192,6 +1193,16 @@ describe("slack prepareSlackMessage inbound contract", () => {
async function prepareThreadContextAllowlistCase(params: ThreadContextAllowlistCaseParams) {
const { storePath } = storeFixture.makeTmpStorePath();
if (params.outboundOnlySessionKey) {
const now = Date.now();
await seedSessionEntries(storePath, {
[params.outboundOnlySessionKey]: {
sessionId: "outbound-only-thread-session",
updatedAt: now,
sessionStartedAt: now,
},
});
}
const historyUser = params.historyUser ?? params.user;
const replies = vi
.fn()
@@ -1257,7 +1268,7 @@ describe("slack prepareSlackMessage inbound contract", () => {
replies: ReturnType<typeof vi.fn>,
starterText: string,
followUpText: string,
options?: { expectStarterBody?: boolean },
options?: { expectStarterBody?: boolean; expectAssistantHistory?: boolean },
) {
assertPrepared(prepared);
if (options?.expectStarterBody === false) {
@@ -1267,7 +1278,12 @@ describe("slack prepareSlackMessage inbound contract", () => {
}
expect(prepared.ctxPayload.ThreadHistoryBody).toContain(starterText);
expect(prepared.ctxPayload.ThreadHistoryBody).toContain(followUpText);
expect(prepared.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply");
if (options?.expectAssistantHistory) {
expect(prepared.ctxPayload.ThreadHistoryBody).toContain("assistant reply");
expect(prepared.ctxPayload.ThreadHistoryBody).toContain("Bot (this assistant) (assistant)");
} else {
expect(prepared.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply");
}
expect(prepared.ctxPayload.ThreadHistoryBody).not.toContain("current message");
expect(replies).toHaveBeenCalledTimes(2);
}
@@ -2873,9 +2889,18 @@ Second paragraph should still reach the agent after Slack's preview cutoff.`;
followUpTs: "400.800",
currentTs: "401.000",
allowFrom: ["U4"],
outboundOnlySessionKey: "agent:main:slack:group:g400:thread:400.000",
});
expectThreadContextAllowsHumanHistory(prepared, replies, "starter from mpim", "mpim follow-up");
expectThreadContextAllowsHumanHistory(
prepared,
replies,
"starter from mpim",
"mpim follow-up",
{
expectAssistantHistory: true,
},
);
});
it("skips loading thread history when thread session already exists in store (bloat fix)", async () => {
@@ -365,9 +365,96 @@ describe("thread-level session keys", () => {
expect(routing.threadContext.replyToId).toBeUndefined();
});
it("does not seed top-level group DM mentions into thread sessions", () => {
it.each(
(["off", "first", "all", "batched"] as const).flatMap((replyToMode) =>
[false, true].map((mentioned) => ({ replyToMode, mentioned })),
),
)(
"keeps $replyToMode MPIM roots flat and routes $mentioned mention follow-ups by thread",
({ replyToMode, mentioned }) => {
const ctx = buildCtx({ replyToMode });
const account = buildAccount(replyToMode);
const rootTs = "1777244692.409919";
const root = resolveSlackRoutingContext({
ctx,
account,
message: buildChannelMessage({
channel: "G123",
channel_type: "mpim",
text: mentioned ? "<@B1> send a subagent" : "send a subagent",
ts: rootTs,
}),
isDirectMessage: false,
isGroupDm: true,
isRoom: false,
isRoomish: true,
seedTopLevelRoomThread: mentioned,
});
const followUp = resolveSlackRoutingContext({
ctx,
account,
message: buildChannelMessage({
channel: "G123",
channel_type: "mpim",
text: "what did you find?",
ts: "1777244714.000100",
thread_ts: rootTs,
parent_user_id: "U1",
}),
isDirectMessage: false,
isGroupDm: true,
isRoom: false,
isRoomish: true,
});
expect(root.sessionKey).toBe("agent:main:slack:group:g123");
expect(root.historyKey).toBe("G123");
expect(root.threadContext.replyToId).toBeUndefined();
expect(root.threadContext.messageThreadId).toBe(replyToMode === "all" ? rootTs : undefined);
expect(followUp.sessionKey).toBe(`agent:main:slack:group:g123:thread:${rootTs}`);
expect(followUp.historyKey).toBe(followUp.sessionKey);
expect(followUp.threadContext.replyToId).toBe(rootTs);
expect(followUp.threadContext.messageThreadId).toBe(rootTs);
expect(followUp.sessionKey).not.toContain("1777244714.000100");
},
);
it("keeps configured MPIM bindings flat when Slack starts a reply thread", () => {
const ctx = buildCtx({ replyToMode: "all" });
const account = buildAccount("all");
const targetSessionKey = "agent:codex:acp:binding:slack:default:g123";
resolveConfiguredBindingRouteMock.mockImplementation(({ route, conversation }) => ({
bindingResolution: {
conversation,
record: {
bindingId: "config:acp:slack:default:g123",
targetSessionKey,
targetKind: "session",
conversation: {
channel: "slack",
accountId: "default",
conversationId: "g123",
},
status: "active",
boundAt: 0,
metadata: {
source: "config",
mode: "persistent",
agentId: "codex",
},
},
},
boundSessionKey: targetSessionKey,
boundAgentId: "codex",
route: {
...route,
agentId: "codex",
sessionKey: targetSessionKey,
mainSessionKey: "agent:codex:main",
matchedBy: "binding.channel",
lastRoutePolicy: "session",
},
}));
const routing = resolveSlackRoutingContext({
ctx,
@@ -375,17 +462,18 @@ describe("thread-level session keys", () => {
message: buildChannelMessage({
channel: "G123",
channel_type: "mpim",
text: "<@B1> send a subagent",
ts: "1777244692.409919",
text: "what did you find?",
ts: "1777244714.000100",
thread_ts: "1777244692.409919",
parent_user_id: "U1",
}),
isDirectMessage: false,
isGroupDm: true,
isRoom: false,
isRoomish: true,
seedTopLevelRoomThread: true,
});
expect(routing.sessionKey).toBe("agent:main:slack:group:g123");
expect(routing.sessionKey).toBe(targetSessionKey);
expect(routing.sessionKey).not.toContain(":thread:");
});
@@ -1555,6 +1555,7 @@ export async function prepareSlackMessage(params: {
ctx,
account,
message,
isGroupDm,
isThreadReply,
threadTs,
threadStarter,
@@ -1,4 +1,4 @@
title: Slack MPIM app mention dispatches once
title: Slack MPIM app mention dispatches once with thread context
scenario:
id: slack-mpim-app-mention-dedupe
surface: channels