Handle Slack file-only thread starters for media inheritance

This commit is contained in:
Chane 2026-03-20 19:48:49 -07:00
parent b36e456b09
commit 8ddd238d1c
5 changed files with 253 additions and 6 deletions

View File

@ -13,6 +13,7 @@ import {
resolveSlackAttachmentContent,
resolveSlackMedia,
resolveSlackThreadHistory,
resolveSlackThreadStarter,
} from "./media.js";
// Store original fetch
@ -657,6 +658,37 @@ describe("resolveSlackAttachmentContent", () => {
});
});
describe("resolveSlackThreadStarter", () => {
it("returns file-only root messages so thread replies can hydrate inherited media", async () => {
const replies = vi.fn().mockResolvedValueOnce({
messages: [
{
text: " ",
user: "U1",
ts: "1.000",
files: [{ id: "F1", name: "thread-image.png" }],
},
],
});
const client = {
conversations: { replies },
} as unknown as Parameters<typeof resolveSlackThreadStarter>[0]["client"];
const result = await resolveSlackThreadStarter({
channelId: "C1",
threadTs: "1.000",
client,
});
expect(result).toEqual({
text: "",
userId: "U1",
ts: "1.000",
files: [{ id: "F1", name: "thread-image.png" }],
});
});
});
describe("resolveSlackThreadHistory", () => {
afterEach(() => {
vi.restoreAllMocks();

View File

@ -394,7 +394,8 @@ export async function resolveSlackThreadStarter(params: {
})) as { messages?: Array<{ text?: string; user?: string; ts?: string; files?: SlackFile[] }> };
const message = response?.messages?.[0];
const text = (message?.text ?? "").trim();
if (!message || !text) {
const hasFiles = Boolean(message?.files?.length);
if (!message || (!text && !hasFiles)) {
return null;
}
const starter: SlackThreadStarter = {

View File

@ -51,9 +51,12 @@ export async function resolveSlackThreadContextData(params: {
}
const starter = params.threadStarter;
if (starter?.text) {
threadStarterBody = starter.text;
const snippet = starter.text.replace(/\s+/g, " ").slice(0, 80);
if (starter) {
const starterText = starter.text.trim();
if (starterText) {
threadStarterBody = starterText;
}
const snippet = starterText.replace(/\s+/g, " ").slice(0, 80);
threadLabel = `Slack thread ${params.roomLabel}${snippet ? `: ${snippet}` : ""}`;
if (!params.effectiveDirectMedia && starter.files && starter.files.length > 0) {
threadStarterMedia = await resolveSlackMedia({

View File

@ -10,6 +10,7 @@ import { resolveThreadSessionKeys } from "../../../../../src/routing/session-key
import type { ResolvedSlackAccount } from "../../accounts.js";
import type { SlackMessageEvent } from "../../types.js";
import type { SlackMonitorContext } from "../context.js";
import * as slackMedia from "../media.js";
import { prepareSlackMessage } from "./prepare.js";
import { createInboundSlackTestContext, createSlackTestAccount } from "./prepare.test-helpers.js";
@ -226,6 +227,49 @@ describe("slack prepareSlackMessage inbound contract", () => {
expect(prepared!.ctxPayload.RawBody).toContain("[Forwarded message from Bob]\nForwarded hello");
});
it("populates finalized media fields for forwarded shared attachment media", async () => {
const attachmentSpy = vi
.spyOn(slackMedia, "resolveSlackAttachmentContent")
.mockResolvedValueOnce({
text: "[Forwarded message from Bob]\nForwarded hello",
media: [
{
path: "/tmp/forwarded.jpg",
contentType: "image/jpeg",
placeholder: "[Forwarded image: forwarded.jpg]",
},
{
path: "/tmp/forwarded-file.png",
contentType: "image/png",
placeholder: "[Slack file: forwarded-file.png]",
},
],
});
try {
const prepared = await prepareWithDefaultCtx(
createSlackMessage({
text: "",
attachments: [{ is_share: true, author_name: "Bob", text: "Forwarded hello" }],
}),
);
expect(prepared).toBeTruthy();
expect(prepared!.ctxPayload.RawBody).toContain(
"[Forwarded message from Bob]\nForwarded hello",
);
expect(prepared!.ctxPayload.MediaPath).toBe("/tmp/forwarded.jpg");
expect(prepared!.ctxPayload.MediaType).toBe("image/jpeg");
expect(prepared!.ctxPayload.MediaPaths).toEqual([
"/tmp/forwarded.jpg",
"/tmp/forwarded-file.png",
]);
expect(prepared!.ctxPayload.MediaTypes).toEqual(["image/jpeg", "image/png"]);
} finally {
attachmentSpy.mockRestore();
}
});
it("ignores non-forward attachments when no direct text/files are present", async () => {
const prepared = await prepareWithDefaultCtx(
createSlackMessage({
@ -457,6 +501,134 @@ describe("slack prepareSlackMessage inbound contract", () => {
expect(replies).toHaveBeenCalledTimes(2);
});
it("populates finalized media fields for direct Slack media", async () => {
const mediaSpy = vi.spyOn(slackMedia, "resolveSlackMedia").mockResolvedValueOnce([
{
path: "/tmp/direct-a.png",
contentType: "image/png",
placeholder: "[Slack file: direct-a.png]",
},
{
path: "/tmp/direct-b.jpg",
contentType: "image/jpeg",
placeholder: "[Slack file: direct-b.jpg]",
},
]);
try {
const prepared = await prepareWithDefaultCtx(
createSlackMessage({
text: "please inspect",
files: [
{ id: "F1", name: "direct-a.png", url_private: "https://files.slack.com/a" },
{ id: "F2", name: "direct-b.jpg", url_private: "https://files.slack.com/b" },
],
}),
);
expect(prepared).toBeTruthy();
expect(prepared!.ctxPayload.MediaPath).toBe("/tmp/direct-a.png");
expect(prepared!.ctxPayload.MediaType).toBe("image/png");
expect(prepared!.ctxPayload.MediaPaths).toEqual(["/tmp/direct-a.png", "/tmp/direct-b.jpg"]);
expect(prepared!.ctxPayload.MediaTypes).toEqual(["image/png", "image/jpeg"]);
} finally {
mediaSpy.mockRestore();
}
});
it("inherits thread-root media into finalized media fields when reply has no direct media", async () => {
const { storePath } = makeTmpStorePath();
const replies = vi.fn();
const slackCtx = createThreadSlackCtx({
cfg: {
session: { store: storePath },
channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } },
} as OpenClawConfig,
replies,
});
slackCtx.resolveUserName = async (id: string) => ({ name: id === "U1" ? "Alice" : "Bob" });
slackCtx.resolveChannelName = async () => ({ name: "general", type: "channel" });
const starterSpy = vi.spyOn(slackMedia, "resolveSlackThreadStarter").mockResolvedValue({
text: "",
userId: "U2",
ts: "100.000",
files: [{ id: "F1", name: "root.png", url_private: "https://files.slack.com/root" }],
});
const mediaSpy = vi
.spyOn(slackMedia, "resolveSlackMedia")
.mockResolvedValueOnce(null)
.mockResolvedValueOnce([
{
path: "/tmp/root.png",
contentType: "image/png",
placeholder: "[Slack file: root.png]",
},
]);
try {
const prepared = await prepareThreadMessage(slackCtx, {
text: "what is in the image above?",
ts: "101.000",
});
expect(prepared).toBeTruthy();
expect(prepared!.ctxPayload.MediaPath).toBe("/tmp/root.png");
expect(prepared!.ctxPayload.MediaType).toBe("image/png");
expect(prepared!.ctxPayload.MediaPaths).toEqual(["/tmp/root.png"]);
expect(prepared!.ctxPayload.MediaTypes).toEqual(["image/png"]);
expect(prepared!.ctxPayload.ThreadStarterBody).toBeUndefined();
expect(starterSpy).toHaveBeenCalledTimes(1);
} finally {
starterSpy.mockRestore();
mediaSpy.mockRestore();
}
});
it("preserves ordered multi-image arrays in finalized media fields", async () => {
const mediaSpy = vi.spyOn(slackMedia, "resolveSlackMedia").mockResolvedValueOnce([
{
path: "/tmp/01.png",
contentType: "image/png",
placeholder: "[Slack file: 01.png]",
},
{
path: "/tmp/02.webp",
contentType: "image/webp",
placeholder: "[Slack file: 02.webp]",
},
{
path: "/tmp/03.jpg",
contentType: "image/jpeg",
placeholder: "[Slack file: 03.jpg]",
},
]);
try {
const prepared = await prepareWithDefaultCtx(
createSlackMessage({
text: "compare these",
files: [
{ id: "F1", name: "01.png", url_private: "https://files.slack.com/1" },
{ id: "F2", name: "02.webp", url_private: "https://files.slack.com/2" },
{ id: "F3", name: "03.jpg", url_private: "https://files.slack.com/3" },
],
}),
);
expect(prepared).toBeTruthy();
expect(prepared!.ctxPayload.MediaPath).toBe("/tmp/01.png");
expect(prepared!.ctxPayload.MediaPaths).toEqual([
"/tmp/01.png",
"/tmp/02.webp",
"/tmp/03.jpg",
]);
expect(prepared!.ctxPayload.MediaTypes).toEqual(["image/png", "image/webp", "image/jpeg"]);
} finally {
mediaSpy.mockRestore();
}
});
it("skips loading thread history when thread session already exists in store (bloat fix)", async () => {
const { storePath } = makeTmpStorePath();
const cfg = {

View File

@ -149,9 +149,15 @@ const baseParams = () => ({
});
type ThreadStarterClient = Parameters<typeof resolveSlackThreadStarter>[0]["client"];
type ThreadStarterReplyMessage = {
text?: string;
user?: string;
ts?: string;
files?: SlackMessageEvent["files"];
};
function createThreadStarterRepliesClient(
response: { messages?: Array<{ text?: string; user?: string; ts?: string }> } = {
response: { messages?: ThreadStarterReplyMessage[] } = {
messages: [{ text: "root message", user: "U1", ts: "1000.1" }],
},
): { replies: ReturnType<typeof vi.fn>; client: ThreadStarterClient } {
@ -351,7 +357,7 @@ describe("resolveSlackThreadStarter cache", () => {
expect(replies).toHaveBeenCalledTimes(2);
});
it("does not cache empty starter text", async () => {
it("does not cache empty starter messages without files", async () => {
const { replies, client } = createThreadStarterRepliesClient({
messages: [{ text: " ", user: "U1", ts: "1000.1" }],
});
@ -372,6 +378,39 @@ describe("resolveSlackThreadStarter cache", () => {
expect(replies).toHaveBeenCalledTimes(2);
});
it("caches file-only thread starters so replies can inherit parent media", async () => {
const { replies, client } = createThreadStarterRepliesClient({
messages: [
{
text: " ",
user: "U1",
ts: "1000.1",
files: [{ id: "F1", name: "thread-image.png" }],
},
],
});
const first = await resolveSlackThreadStarter({
channelId: "C1",
threadTs: "1000.1",
client,
});
const second = await resolveSlackThreadStarter({
channelId: "C1",
threadTs: "1000.1",
client,
});
expect(first).toEqual({
text: "",
userId: "U1",
ts: "1000.1",
files: [{ id: "F1", name: "thread-image.png" }],
});
expect(second).toEqual(first);
expect(replies).toHaveBeenCalledTimes(1);
});
it("evicts oldest entries once cache exceeds bounded size", async () => {
const { replies, client } = createThreadStarterRepliesClient();