fix(slack): respect verbose setting and preserve thread context for tool notifications
Fixes two bugs in Slack tool notification delivery: 1. Tool notifications ignored verbose=false - normalized verbose values so boolean false/'false' are properly treated as 'off' 2. Thread context lost - Slack outbound adapter now falls back to threadId when replyToId is missing, and MessageThreadId is set for thread replies Closes #1333
This commit is contained in:
committed by
Peter Steinberger
parent
46ab4cb19e
commit
0d6e78b718
@@ -16,19 +16,21 @@ export const createShouldEmitToolResult = (params: {
|
||||
storePath?: string;
|
||||
resolvedVerboseLevel: VerboseLevel;
|
||||
}): (() => boolean) => {
|
||||
// Normalize verbose values from session store/config so false/"false" still means off.
|
||||
const fallbackVerbose = normalizeVerboseLevel(String(params.resolvedVerboseLevel ?? "")) ?? "off";
|
||||
return () => {
|
||||
if (!params.sessionKey || !params.storePath) {
|
||||
return params.resolvedVerboseLevel !== "off";
|
||||
return fallbackVerbose !== "off";
|
||||
}
|
||||
try {
|
||||
const store = loadSessionStore(params.storePath);
|
||||
const entry = store[params.sessionKey];
|
||||
const current = normalizeVerboseLevel(entry?.verboseLevel);
|
||||
const current = normalizeVerboseLevel(String(entry?.verboseLevel ?? ""));
|
||||
if (current) return current !== "off";
|
||||
} catch {
|
||||
// ignore store read failures
|
||||
}
|
||||
return params.resolvedVerboseLevel !== "off";
|
||||
return fallbackVerbose !== "off";
|
||||
};
|
||||
};
|
||||
|
||||
@@ -37,19 +39,21 @@ export const createShouldEmitToolOutput = (params: {
|
||||
storePath?: string;
|
||||
resolvedVerboseLevel: VerboseLevel;
|
||||
}): (() => boolean) => {
|
||||
// Normalize verbose values from session store/config so false/"false" still means off.
|
||||
const fallbackVerbose = normalizeVerboseLevel(String(params.resolvedVerboseLevel ?? "")) ?? "off";
|
||||
return () => {
|
||||
if (!params.sessionKey || !params.storePath) {
|
||||
return params.resolvedVerboseLevel === "full";
|
||||
return fallbackVerbose === "full";
|
||||
}
|
||||
try {
|
||||
const store = loadSessionStore(params.storePath);
|
||||
const entry = store[params.sessionKey];
|
||||
const current = normalizeVerboseLevel(entry?.verboseLevel);
|
||||
const current = normalizeVerboseLevel(String(entry?.verboseLevel ?? ""));
|
||||
if (current) return current === "full";
|
||||
} catch {
|
||||
// ignore store read failures
|
||||
}
|
||||
return params.resolvedVerboseLevel === "full";
|
||||
return fallbackVerbose === "full";
|
||||
};
|
||||
};
|
||||
|
||||
|
||||
@@ -209,6 +209,22 @@ describe("routeReply", () => {
|
||||
expect(mocks.sendMessageSlack).toHaveBeenCalledWith("channel:C123", "hi", expect.any(Object));
|
||||
});
|
||||
|
||||
it("uses threadId for Slack when replyToId is missing", async () => {
|
||||
mocks.sendMessageSlack.mockClear();
|
||||
await routeReply({
|
||||
payload: { text: "hi" },
|
||||
channel: "slack",
|
||||
to: "channel:C123",
|
||||
threadId: "456.789",
|
||||
cfg: {} as never,
|
||||
});
|
||||
expect(mocks.sendMessageSlack).toHaveBeenCalledWith(
|
||||
"channel:C123",
|
||||
"hi",
|
||||
expect.objectContaining({ threadTs: "456.789" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("passes thread id to Telegram sends", async () => {
|
||||
mocks.sendMessageTelegram.mockClear();
|
||||
await routeReply({
|
||||
|
||||
@@ -5,19 +5,23 @@ export const slackOutbound: ChannelOutboundAdapter = {
|
||||
deliveryMode: "direct",
|
||||
chunker: null,
|
||||
textChunkLimit: 4000,
|
||||
sendText: async ({ to, text, accountId, deps, replyToId }) => {
|
||||
sendText: async ({ to, text, accountId, deps, replyToId, threadId }) => {
|
||||
const send = deps?.sendSlack ?? sendMessageSlack;
|
||||
// Use threadId fallback so routed tool notifications stay in the Slack thread.
|
||||
const threadTs = replyToId ?? (threadId != null ? String(threadId) : undefined);
|
||||
const result = await send(to, text, {
|
||||
threadTs: replyToId ?? undefined,
|
||||
threadTs,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
return { channel: "slack", ...result };
|
||||
},
|
||||
sendMedia: async ({ to, text, mediaUrl, accountId, deps, replyToId }) => {
|
||||
sendMedia: async ({ to, text, mediaUrl, accountId, deps, replyToId, threadId }) => {
|
||||
const send = deps?.sendSlack ?? sendMessageSlack;
|
||||
// Use threadId fallback so routed tool notifications stay in the Slack thread.
|
||||
const threadTs = replyToId ?? (threadId != null ? String(threadId) : undefined);
|
||||
const result = await send(to, text, {
|
||||
mediaUrl,
|
||||
threadTs: replyToId ?? undefined,
|
||||
threadTs,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
return { channel: "slack", ...result };
|
||||
|
||||
@@ -476,6 +476,8 @@ export async function prepareSlackMessage(params: {
|
||||
Surface: "slack" as const,
|
||||
MessageSid: message.ts,
|
||||
ReplyToId: message.thread_ts ?? message.ts,
|
||||
// Preserve thread context for routed tool notifications (thread replies only).
|
||||
MessageThreadId: isThreadReply ? threadTs : undefined,
|
||||
ParentSessionKey: threadKeys.parentSessionKey,
|
||||
ThreadStarterBody: threadStarterBody,
|
||||
ThreadLabel: threadLabel,
|
||||
|
||||
Reference in New Issue
Block a user