diff --git a/packages/ims/shared/synthetic-owner.test.ts b/packages/ims/shared/synthetic-owner.test.ts index 0f8732d..ecd317d 100644 --- a/packages/ims/shared/synthetic-owner.test.ts +++ b/packages/ims/shared/synthetic-owner.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { isSyntheticOwner } from "./synthetic-owner"; +import { isSyntheticOwner, threadTsField } from "./synthetic-owner"; describe("isSyntheticOwner", () => { it("returns true for task: prefix", () => { @@ -30,3 +30,35 @@ describe("isSyntheticOwner", () => { expect(isSyntheticOwner("user-cron-job:x")).toBe(false); }); }); + +describe("threadTsField", () => { + it("preserves a real Slack message timestamp", () => { + expect(threadTsField("1717000000.000200")).toEqual({ + thread_ts: "1717000000.000200", + }); + }); + + it("omits thread_ts for a cron-job placeholder (ODE-DEAMON-7)", () => { + const field = threadTsField( + "cron-job:a86fbdc5-01df-4caf-9e0c-c0c199f00379:1781391600000" + ); + expect(field).toEqual({}); + expect(field).not.toHaveProperty("thread_ts"); + }); + + it("omits thread_ts for task: and legacy cron: placeholders", () => { + expect(threadTsField("task:abc123")).toEqual({}); + expect(threadTsField("cron:daily")).toEqual({}); + }); + + it("omits thread_ts for missing/empty thread ids", () => { + expect(threadTsField(undefined)).toEqual({}); + expect(threadTsField(null)).toEqual({}); + expect(threadTsField("")).toEqual({}); + }); + + it("spreads into a payload without leaving an undefined key", () => { + const payload = { channel: "C1", ...threadTsField("task:abc") }; + expect(Object.keys(payload)).toEqual(["channel"]); + }); +}); diff --git a/packages/ims/shared/synthetic-owner.ts b/packages/ims/shared/synthetic-owner.ts index 616b414..5766900 100644 --- a/packages/ims/shared/synthetic-owner.ts +++ b/packages/ims/shared/synthetic-owner.ts @@ -17,3 +17,23 @@ export function isSyntheticOwner(userId: string | null | undefined): boolean { if (!userId) return false; return SYNTHETIC_OWNER_PREFIXES.some((prefix) => userId.startsWith(prefix)); } + +/** + * Build the `thread_ts` field for a Slack API payload, omitting it entirely + * when `threadId` is a synthetic placeholder. + * + * The task/cron schedulers address a run by a synthetic thread id + * (`task:{id}` / `cron-job:{id}:{run}`) before Slack has assigned a real + * `thread_ts`. Slack rejects those strings with `invalid_thread_ts` because + * they are not message timestamps, so the message is dropped and reported as + * a delivery failure. Omitting the field degrades to a top-level channel + * post, which keeps the message visible. + * + * Spread the result into the payload: `{ channel, ...threadTsField(id) }`. + */ +export function threadTsField( + threadId: string | null | undefined +): { thread_ts?: string } { + if (!threadId || isSyntheticOwner(threadId)) return {}; + return { thread_ts: threadId }; +} diff --git a/packages/ims/slack/api.ts b/packages/ims/slack/api.ts index d61f34d..bad1012 100644 --- a/packages/ims/slack/api.ts +++ b/packages/ims/slack/api.ts @@ -5,6 +5,7 @@ import { type ThreadMessage, } from "@/ims/shared/thread-messages"; import type { AttachmentSource } from "@/ims/shared/attachment-store"; +import { threadTsField } from "@/ims/shared/synthetic-owner"; import { getApp, getSlackBotToken } from "./client"; // --------------------------------------------------------------------------- @@ -123,7 +124,11 @@ async function slackFileUpload( return slackApiCall("files.completeUploadExternal", { files: [{ id: uploadInfo.file_id, title: args.title || args.filename }], channel_id: args.channelId, - thread_ts: args.threadId, + // Synthetic placeholder thread ids (`task:` / `cron-job:` / `cron:`) are + // not valid Slack timestamps. `ode send file` invoked from a scheduled + // task/cron run before a real thread exists would otherwise fail with + // `invalid_thread_ts`; upload to the channel top level instead. + ...(threadTsField(args.threadId)), initial_comment: args.initialComment, }, args.token); } diff --git a/packages/ims/slack/client.ts b/packages/ims/slack/client.ts index ffc85bd..52f4d65 100644 --- a/packages/ims/slack/client.ts +++ b/packages/ims/slack/client.ts @@ -29,7 +29,7 @@ import { createProcessorManager } from "@/ims/shared/processor-manager"; import { SlackAuthRegistry, type WorkspaceAuth } from "@/ims/slack/state/auth-registry"; import { SlackMessageUpdateManager } from "@/ims/slack/message-update-manager"; import { deliveryStats, isRateLimitError } from "@/ims/shared/delivery-stats"; -import { isSyntheticOwner } from "@/ims/shared/synthetic-owner"; +import { isSyntheticOwner, threadTsField } from "@/ims/shared/synthetic-owner"; export interface MessageContext { channelId: string; @@ -309,7 +309,28 @@ export async function sendMessage( const formattedText = markdownToSlack(text); const chunks = splitForSlack(formattedText); const workspace = slackAuthRegistry.getChannelWorkspaceName(rawChannelId) || "unknown"; - const botToken = getSlackBotTokenForProcessor(processorId) ?? getSlackBotToken(channelId, threadId); + + // A "synthetic" thread id (`task:{id}` / `cron-job:{id}:{run}` / `cron:{id}`) + // is an internal placeholder the task/cron schedulers use before a real + // Slack `thread_ts` exists. Slack rejects these with `invalid_thread_ts` + // because they are not message timestamps, so any intermediate output the + // agent runtime emits during a scheduled run is lost and captured as a + // Sentry delivery failure (ODE-DEAMON-7). Degenerate to a top-level channel + // post instead so the message still lands in the channel. + const threadIsSynthetic = isSyntheticOwner(threadId); + + // Token resolution. For real threads prefer the registry-bound token for + // this (channel, thread) — that's the token the inbound router observed + // delivering the parent message. For synthetic placeholders the call has + // degenerated to a top-level post and the registry has no entry for a fake + // `thread_ts`, so resolve via the channel's workspace first (mirroring + // `sendChannelMessage`) to avoid the multi-workspace case where + // `getSlackBotToken` returns the first registered token instead of the one + // for `rawChannelId`. + const botToken = getSlackBotTokenForProcessor(processorId) + ?? (threadIsSynthetic + ? (getWorkspaceBotTokenForChannel(channelId) ?? getSlackBotToken(channelId)) + : getSlackBotToken(channelId, threadId)); if (!botToken) { log.warn("No Slack bot token available for channel", { channelId }); @@ -320,6 +341,7 @@ export async function sendMessage( workspace, channel: channelId, thread: threadId, + threadIsSynthetic, botTokenLast6: tokenLast6(botToken), text, chunks: chunks.length, @@ -329,6 +351,7 @@ export async function sendMessage( workspace, channel: channelId, thread: threadId, + threadIsSynthetic, botTokenLast6: tokenLast6(botToken), text, chunks: chunks.length, @@ -346,7 +369,7 @@ export async function sendMessage( try { const result = await slackApp.client.chat.postMessage({ channel: rawChannelId, - thread_ts: threadId, + ...(threadTsField(threadId)), text: chunk, token: botToken, }); @@ -358,7 +381,12 @@ export async function sendMessage( }); lastTs = result.ts; if (botToken && result.ts) { - slackAuthRegistry.setThreadBotToken(rawChannelId, threadId, botToken); + // Never bind a real bot token to a synthetic placeholder thread id — + // the real platform-assigned thread is `result.ts` for the first + // message of the run, which `setMessageBotToken` covers below. + if (!threadIsSynthetic) { + slackAuthRegistry.setThreadBotToken(rawChannelId, threadId, botToken); + } slackAuthRegistry.setMessageBotToken(rawChannelId, result.ts, botToken); } } catch (err) {