fix(slack): narrow first turn context seeding to remove redundant thread-starter content (#68402)

Fix Slack thread bootstrap replaying the bot's own prior turns into new sessions and duplicating the thread-starter prompt block.

Narrows first-turn context seeding to exclude only the current Slack bot's own starter/history entries, so self-authored turns no longer pollute new session prompts while preserving human and third-party bot context

Removes the redundant plain-text starter prelude in runPreparedReply() that doubled thread-starter content when no ThreadHistoryBody was present
This commit is contained in:
Bek
2026-04-21 18:28:34 -04:00
committed by GitHub
parent acf67c1a42
commit 70683179a0
11 changed files with 245 additions and 27 deletions

View File

@@ -15,6 +15,7 @@ function createTestContext() {
app: { client: {} } as App,
runtime: {} as RuntimeEnv,
botUserId: "U_BOT",
botId: "B_BOT",
teamId: "T_EXPECTED",
apiAppId: "A_EXPECTED",
historyLimit: 0,

View File

@@ -35,6 +35,7 @@ export type SlackMonitorContext = {
runtime: RuntimeEnv;
botUserId: string;
botId?: string;
teamId: string;
apiAppId: string;
@@ -103,6 +104,7 @@ export function createSlackMonitorContext(params: {
runtime: RuntimeEnv;
botUserId: string;
botId?: string;
teamId: string;
apiAppId: string;
@@ -398,6 +400,7 @@ export function createSlackMonitorContext(params: {
app: params.app,
runtime: params.runtime,
botUserId: params.botUserId,
botId: params.botId,
teamId: params.teamId,
apiAppId: params.apiAppId,
historyLimit: params.historyLimit,

View File

@@ -46,7 +46,7 @@ describe("resolveSlackThreadContextData", () => {
async function resolveAllowlistedThreadContext(params: {
repliesMessages: Array<Record<string, string>>;
threadStarter: { text: string; userId: string; ts: string };
threadStarter: { text: string; userId?: string; ts: string; botId?: string };
allowFromLower: string[];
allowNameMatching: boolean;
}) {
@@ -56,6 +56,8 @@ describe("resolveSlackThreadContextData", () => {
response_metadata: { next_cursor: "" },
});
const ctx = createThreadContext({ replies });
ctx.botUserId = "U_BOT";
ctx.botId = "B1";
ctx.resolveUserName = async (id: string) => ({
name: id === "U1" ? "Alice" : "Mallory",
});
@@ -100,8 +102,8 @@ describe("resolveSlackThreadContextData", () => {
expect(result.threadStarterBody).toBeUndefined();
expect(result.threadLabel).toBe("Slack thread #general");
expect(result.threadHistoryBody).toContain("assistant reply");
expect(result.threadHistoryBody).toContain("allowed follow-up");
expect(result.threadHistoryBody).not.toContain("assistant reply");
expect(result.threadHistoryBody).not.toContain("starter secret");
expect(result.threadHistoryBody).not.toContain("blocked follow-up");
expect(result.threadHistoryBody).not.toContain("current message");
@@ -129,4 +131,72 @@ describe("resolveSlackThreadContextData", () => {
expect(result.threadHistoryBody).toContain("starter from Alice");
expect(result.threadHistoryBody).not.toContain("blocked follow-up");
});
it("omits bot-authored starter text and history from a new thread session", async () => {
const { result } = await resolveAllowlistedThreadContext({
repliesMessages: [
{ text: "bot starter", bot_id: "B1", ts: "100.000" },
{ text: "allowed follow-up", user: "U1", ts: "100.800" },
{ text: "current message", user: "U1", ts: "101.000" },
],
threadStarter: {
text: "bot starter",
botId: "B1",
ts: "100.000",
},
allowFromLower: ["u1"],
allowNameMatching: false,
});
expect(result.threadStarterBody).toBeUndefined();
expect(result.threadLabel).toBe("Slack thread #general");
expect(result.threadHistoryBody).toContain("allowed follow-up");
expect(result.threadHistoryBody).not.toContain("bot starter");
expect(result.threadHistoryBody).not.toContain("current message");
});
it("keeps third-party bot starter text in a new thread session", async () => {
const { result } = await resolveAllowlistedThreadContext({
repliesMessages: [
{ text: "other bot starter", bot_id: "B2", ts: "100.000" },
{ text: "allowed follow-up", user: "U1", ts: "100.800" },
{ text: "current message", user: "U1", ts: "101.000" },
],
threadStarter: {
text: "other bot starter",
botId: "B2",
ts: "100.000",
},
allowFromLower: ["u1"],
allowNameMatching: false,
});
expect(result.threadStarterBody).toBe("other bot starter");
expect(result.threadLabel).toContain("other bot starter");
expect(result.threadHistoryBody).toContain("other bot starter");
expect(result.threadHistoryBody).toContain("Bot (B2) (assistant)");
expect(result.threadHistoryBody).toContain("allowed follow-up");
expect(result.threadHistoryBody).not.toContain("Unknown (user)");
});
it("omits self-authored starter text when identified by bot user id", async () => {
const { result } = await resolveAllowlistedThreadContext({
repliesMessages: [
{ text: "self starter", user: "U_BOT", ts: "100.000" },
{ text: "allowed follow-up", user: "U1", ts: "100.800" },
{ text: "current message", user: "U1", ts: "101.000" },
],
threadStarter: {
text: "self starter",
userId: "U_BOT",
ts: "100.000",
},
allowFromLower: ["u1"],
allowNameMatching: false,
});
expect(result.threadStarterBody).toBeUndefined();
expect(result.threadHistoryBody).toContain("allowed follow-up");
expect(result.threadHistoryBody).not.toContain("self starter");
});
});

View File

@@ -64,6 +64,12 @@ export async function resolveSlackThreadContextData(params: {
>;
effectiveDirectMedia: SlackMediaResult[] | null;
}): Promise<SlackThreadContextData> {
const isCurrentBotAuthor = (author: { userId?: string; botId?: string }): boolean =>
Boolean(
(params.ctx.botUserId && author.userId && author.userId === params.ctx.botUserId) ||
(params.ctx.botId && author.botId && author.botId === params.ctx.botId),
);
let threadStarterBody: string | undefined;
let threadHistoryBody: string | undefined;
let threadSessionPreviousTimestamp: number | undefined;
@@ -85,22 +91,31 @@ export async function resolveSlackThreadContextData(params: {
params.allowNameMatching && starter?.userId
? (await params.ctx.resolveUserName(starter.userId))?.name
: undefined;
const starterIsCurrentBot = Boolean(
starter &&
isCurrentBotAuthor({
userId: starter.userId,
botId: starter.botId,
}),
);
const starterAllowed =
!starter ||
isSlackThreadContextSenderAllowed({
allowFromLower: params.allowFromLower,
allowNameMatching: params.allowNameMatching,
userId: starter.userId,
userName: starterSenderName,
botId: starter.botId,
});
(!starterIsCurrentBot &&
isSlackThreadContextSenderAllowed({
allowFromLower: params.allowFromLower,
allowNameMatching: params.allowNameMatching,
userId: starter.userId,
userName: starterSenderName,
botId: starter.botId,
}));
const includeStarterContext =
!starter ||
shouldIncludeSupplementalContext({
mode: params.contextVisibilityMode,
kind: "thread",
senderAllowed: starterAllowed,
});
(!starterIsCurrentBot &&
shouldIncludeSupplementalContext({
mode: params.contextVisibilityMode,
kind: "thread",
senderAllowed: starterAllowed,
}));
if (starter?.text && includeStarterContext) {
threadStarterBody = starter.text;
@@ -120,7 +135,9 @@ export async function resolveSlackThreadContextData(params: {
} else {
threadLabel = `Slack thread ${params.roomLabel}`;
}
if (starter?.text && !includeStarterContext) {
if (starter?.text && starterIsCurrentBot) {
logVerbose("slack: omitted current-bot thread starter from context");
} else if (starter?.text && !includeStarterContext) {
logVerbose(
`slack: omitted thread starter from context (mode=${params.contextVisibilityMode}, sender_allowed=${starterAllowed ? "yes" : "no"})`,
);
@@ -142,9 +159,21 @@ export async function resolveSlackThreadContextData(params: {
});
if (threadHistory.length > 0) {
const threadHistoryWithoutCurrentBot = threadHistory.filter(
(historyMsg) =>
!isCurrentBotAuthor({
userId: historyMsg.userId,
botId: historyMsg.botId,
}),
);
const omittedCurrentBotHistoryCount =
threadHistory.length - threadHistoryWithoutCurrentBot.length;
const uniqueUserIds = [
...new Set(
threadHistory.map((item) => item.userId).filter((id): id is string => Boolean(id)),
threadHistoryWithoutCurrentBot
.map((item) => item.userId)
.filter((id): id is string => Boolean(id)),
),
];
const userMap = new Map<string, { name?: string }>();
@@ -159,7 +188,7 @@ export async function resolveSlackThreadContextData(params: {
const { items: filteredThreadHistory, omitted: omittedHistoryCount } =
filterSupplementalContextItems({
items: threadHistory,
items: threadHistoryWithoutCurrentBot,
mode: params.contextVisibilityMode,
kind: "thread",
isSenderAllowed: (historyMsg) => {
@@ -173,18 +202,17 @@ export async function resolveSlackThreadContextData(params: {
});
},
});
if (omittedHistoryCount > 0) {
if (omittedHistoryCount > 0 || omittedCurrentBotHistoryCount > 0) {
logVerbose(
`slack: omitted ${omittedHistoryCount} thread message(s) from context (mode=${params.contextVisibilityMode})`,
`slack: omitted ${omittedHistoryCount + omittedCurrentBotHistoryCount} thread message(s) from context (mode=${params.contextVisibilityMode})`,
);
}
const historyParts: string[] = [];
for (const historyMsg of filteredThreadHistory) {
const msgUser = historyMsg.userId ? userMap.get(historyMsg.userId) : null;
const msgSenderName =
msgUser?.name ?? (historyMsg.botId ? `Bot (${historyMsg.botId})` : "Unknown");
const isBot = Boolean(historyMsg.botId);
const msgSenderName = msgUser?.name ?? (isBot ? `Bot (${historyMsg.botId})` : "Unknown");
const role = isBot ? "assistant" : "user";
const msgWithId = `${historyMsg.text}\n[slack message id: ${historyMsg.ts ?? "unknown"} channel: ${params.message.channel}]`;
historyParts.push(

View File

@@ -23,6 +23,7 @@ export function createInboundSlackTestContext(params: {
app: { client: params.appClient ?? {} } as App,
runtime: {} as RuntimeEnv,
botUserId: "B1",
botId: "B1",
teamId: "T1",
apiAppId: "A1",
historyLimit: 0,

View File

@@ -473,8 +473,8 @@ describe("slack prepareSlackMessage inbound contract", () => {
expect(prepared).toBeTruthy();
expect(prepared!.ctxPayload.IsFirstThreadTurn).toBe(true);
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("follow-up question");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message");
expect(replies).toHaveBeenCalledTimes(2);
});

View File

@@ -140,8 +140,8 @@ describe("prepareSlackMessage thread context allowlists", () => {
expect(prepared).toBeTruthy();
expect(prepared!.ctxPayload.ThreadStarterBody).toBe("starter from room user");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("starter from room user");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("allowed follow-up");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message");
expect(replies).toHaveBeenCalledTimes(2);
});
@@ -169,8 +169,8 @@ describe("prepareSlackMessage thread context allowlists", () => {
expect(prepared).toBeTruthy();
expect(prepared!.ctxPayload.ThreadStarterBody).toBe("starter from open room");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("starter from open room");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("open-room follow-up");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message");
expect(replies).toHaveBeenCalledTimes(2);
});
@@ -192,8 +192,8 @@ describe("prepareSlackMessage thread context allowlists", () => {
expect(prepared).toBeTruthy();
expect(prepared!.ctxPayload.ThreadStarterBody).toBe("starter from open dm");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("starter from open dm");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("dm follow-up");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message");
expect(replies).toHaveBeenCalledTimes(2);
});
@@ -215,8 +215,8 @@ describe("prepareSlackMessage thread context allowlists", () => {
expect(prepared).toBeTruthy();
expect(prepared!.ctxPayload.ThreadStarterBody).toBe("starter from mpim");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("starter from mpim");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("mpim follow-up");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply");
expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message");
expect(replies).toHaveBeenCalledTimes(2);
});

View File

@@ -113,6 +113,7 @@ const baseParams = () => ({
app: { client: {} } as App,
runtime: {} as RuntimeEnv,
botUserId: "B1",
botId: "B1",
teamId: "T1",
apiAppId: "A1",
historyLimit: 0,

View File

@@ -383,12 +383,14 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
let unregisterHttpHandler: (() => void) | null = null;
let botUserId = "";
let botId = "";
let teamId = "";
let apiAppId = "";
const expectedApiAppIdFromAppToken = parseApiAppIdFromAppToken(appToken);
try {
const auth = await app.client.auth.test({ token: botToken });
botUserId = auth.user_id ?? "";
botId = (auth as { bot_id?: string }).bot_id ?? "";
teamId = auth.team_id ?? "";
apiAppId = (auth as { api_app_id?: string }).api_app_id ?? "";
} catch {
@@ -408,6 +410,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
app,
runtime,
botUserId,
botId,
teamId,
apiAppId,
historyLimit,

View File

@@ -290,6 +290,117 @@ describe("runPreparedReply media-only handling", () => {
expect(call?.followupRun.prompt).toContain("Earlier message in this thread");
});
it("falls back to thread starter context on follow-up turns when history is absent", async () => {
const result = await runPreparedReply(
baseParams({
isNewSession: false,
ctx: {
Body: "",
RawBody: "",
CommandBody: "",
ThreadStarterBody: "starter message",
ThreadHistoryBody: undefined,
OriginatingChannel: "slack",
OriginatingTo: "C123",
ChatType: "group",
},
sessionCtx: {
Body: "",
BodyStripped: "",
ThreadStarterBody: "starter message",
ThreadHistoryBody: undefined,
MediaPath: "/tmp/input.png",
Provider: "slack",
ChatType: "group",
OriginatingChannel: "slack",
OriginatingTo: "C123",
},
}),
);
expect(result).toEqual({ text: "ok" });
const call = vi.mocked(runReplyAgent).mock.calls[0]?.[0];
expect(call).toBeTruthy();
expect(call?.followupRun.prompt).toContain("[Thread starter - for context]");
expect(call?.followupRun.prompt).toContain("starter message");
});
it("prefers thread history over thread starter on follow-up turns", async () => {
const result = await runPreparedReply(
baseParams({
isNewSession: false,
ctx: {
Body: "",
RawBody: "",
CommandBody: "",
ThreadStarterBody: "starter message",
ThreadHistoryBody: "Earlier message in this thread",
OriginatingChannel: "slack",
OriginatingTo: "C123",
ChatType: "group",
},
sessionCtx: {
Body: "",
BodyStripped: "",
ThreadStarterBody: "starter message",
ThreadHistoryBody: "Earlier message in this thread",
MediaPath: "/tmp/input.png",
Provider: "slack",
ChatType: "group",
OriginatingChannel: "slack",
OriginatingTo: "C123",
},
}),
);
expect(result).toEqual({ text: "ok" });
const call = vi.mocked(runReplyAgent).mock.calls[0]?.[0];
expect(call).toBeTruthy();
expect(call?.followupRun.prompt).toContain("[Thread history - for context]");
expect(call?.followupRun.prompt).not.toContain("[Thread starter - for context]");
});
it("does not duplicate thread starter text with a plain-text prelude", async () => {
vi.mocked(buildInboundUserContextPrefix).mockReturnValueOnce(
[
"Thread starter (untrusted, for context):",
"```json",
'{"body":"starter message"}',
"```",
].join("\n"),
);
const result = await runPreparedReply(
baseParams({
ctx: {
Body: "",
RawBody: "",
CommandBody: "",
ThreadStarterBody: "starter message",
OriginatingChannel: "slack",
OriginatingTo: "C123",
ChatType: "group",
},
sessionCtx: {
Body: "",
BodyStripped: "",
ThreadStarterBody: "starter message",
MediaPath: "/tmp/input.png",
Provider: "slack",
ChatType: "group",
OriginatingChannel: "slack",
OriginatingTo: "C123",
},
}),
);
expect(result).toEqual({ text: "ok" });
const call = vi.mocked(runReplyAgent).mock.calls[0]?.[0];
expect(call).toBeTruthy();
expect(call?.followupRun.prompt).toContain("Thread starter (untrusted, for context):");
expect(call?.followupRun.prompt).not.toContain("[Thread starter - for context]");
});
it("returns the empty-body reply when there is no text and no media", async () => {
const result = await runPreparedReply(
baseParams({

View File

@@ -437,7 +437,7 @@ export async function runPreparedReply(
const threadHistoryBody = normalizeOptionalString(ctx.ThreadHistoryBody);
const threadContextNote = threadHistoryBody
? `[Thread history - for context]\n${threadHistoryBody}`
: threadStarterBody
: !isNewSession && threadStarterBody
? `[Thread starter - for context]\n${threadStarterBody}`
: undefined;
const drainedSystemEventBlocks: string[] = [];