Skip to content

Commit cea7162

Browse files
hsiaoafrankekn
hsiaoa
and
frankekn
authored
feat(slack): status reaction lifecycle for tool/thinking progress indicators (#56430)
Merged via squash. Prepared head SHA: 1ba5df3 Co-authored-by: hsiaoa <[email protected]> Co-authored-by: frankekn <[email protected]> Reviewed-by: @frankekn
1 parent e28fdb0 commit cea7162

7 files changed

Lines changed: 617 additions & 53 deletions

File tree

‎extensions/slack/src/monitor.test-helpers.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ type SlackClient = {
4747
};
4848
reactions: {
4949
add: (...args: unknown[]) => unknown;
50+
remove: (...args: unknown[]) => unknown;
5051
};
5152
};
5253

@@ -87,6 +88,7 @@ function ensureSlackTestRuntime(): {
8788
},
8889
reactions: {
8990
add: (...args: unknown[]) => slackTestState.reactMock(...args),
91+
remove: (...args: unknown[]) => slackTestState.reactMock(...args),
9092
},
9193
};
9294
}

‎extensions/slack/src/monitor.tool-result.test.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,13 +546,161 @@ describe("monitorSlackProvider tool results", () => {
546546
}),
547547
});
548548

549+
expect(reactMock).toHaveBeenCalledWith({
550+
channel: "C1",
551+
timestamp: "456",
552+
name: "eyes",
553+
});
554+
});
555+
556+
it("keeps ack reaction when no reply is delivered and status reactions are disabled", async () => {
557+
replyMock.mockResolvedValue(undefined);
558+
slackTestState.config = {
559+
messages: {
560+
responsePrefix: "PFX",
561+
ackReaction: "👀",
562+
ackReactionScope: "group-mentions",
563+
removeAckAfterReply: true,
564+
statusReactions: { enabled: false },
565+
},
566+
channels: {
567+
slack: {
568+
dm: { enabled: true, policy: "open", allowFrom: ["*"] },
569+
groupPolicy: "open",
570+
},
571+
},
572+
};
573+
const client = getSlackClient();
574+
if (!client) {
575+
throw new Error("Slack client not registered");
576+
}
577+
const conversations = client.conversations as {
578+
info: ReturnType<typeof vi.fn>;
579+
};
580+
conversations.info.mockResolvedValueOnce({
581+
channel: { name: "general", is_channel: true },
582+
});
583+
584+
await runSlackMessageOnce(monitorSlackProvider, {
585+
event: makeSlackMessageEvent({
586+
text: "<@bot-user> hello",
587+
ts: "456",
588+
channel_type: "channel",
589+
}),
590+
});
591+
await new Promise((resolve) => setTimeout(resolve, 0));
592+
await flush();
593+
594+
expect(sendMock).not.toHaveBeenCalled();
595+
expect(reactMock).toHaveBeenCalledTimes(1);
549596
expect(reactMock).toHaveBeenCalledWith({
550597
channel: "C1",
551598
timestamp: "456",
552599
name: "👀",
553600
});
554601
});
555602

603+
it("keeps ack reaction when no reply is delivered and status reactions are enabled", async () => {
604+
replyMock.mockResolvedValue(undefined);
605+
slackTestState.config = {
606+
messages: {
607+
responsePrefix: "PFX",
608+
ackReaction: "👀",
609+
ackReactionScope: "group-mentions",
610+
removeAckAfterReply: true,
611+
statusReactions: {
612+
enabled: true,
613+
timing: { debounceMs: 0, doneHoldMs: 0, errorHoldMs: 0 },
614+
},
615+
},
616+
channels: {
617+
slack: {
618+
dm: { enabled: true, policy: "open", allowFrom: ["*"] },
619+
groupPolicy: "open",
620+
},
621+
},
622+
};
623+
const client = getSlackClient();
624+
if (!client) {
625+
throw new Error("Slack client not registered");
626+
}
627+
const conversations = client.conversations as {
628+
info: ReturnType<typeof vi.fn>;
629+
};
630+
conversations.info.mockResolvedValueOnce({
631+
channel: { name: "general", is_channel: true },
632+
});
633+
634+
await runSlackMessageOnce(monitorSlackProvider, {
635+
event: makeSlackMessageEvent({
636+
text: "<@bot-user> hello",
637+
ts: "456",
638+
channel_type: "channel",
639+
}),
640+
});
641+
await new Promise((resolve) => setTimeout(resolve, 0));
642+
await flush();
643+
644+
expect(sendMock).not.toHaveBeenCalled();
645+
expect(reactMock).toHaveBeenCalledTimes(1);
646+
expect(reactMock).toHaveBeenCalledWith({
647+
channel: "C1",
648+
timestamp: "456",
649+
name: "eyes",
650+
});
651+
});
652+
653+
it("restores ack reaction when dispatch fails before any reply is delivered", async () => {
654+
replyMock.mockRejectedValue(new Error("boom"));
655+
slackTestState.config = {
656+
messages: {
657+
responsePrefix: "PFX",
658+
ackReaction: "👀",
659+
ackReactionScope: "group-mentions",
660+
removeAckAfterReply: true,
661+
statusReactions: {
662+
enabled: true,
663+
timing: { debounceMs: 0, doneHoldMs: 0, errorHoldMs: 0 },
664+
},
665+
},
666+
channels: {
667+
slack: {
668+
dm: { enabled: true, policy: "open", allowFrom: ["*"] },
669+
groupPolicy: "open",
670+
},
671+
},
672+
};
673+
const client = getSlackClient();
674+
if (!client) {
675+
throw new Error("Slack client not registered");
676+
}
677+
const conversations = client.conversations as {
678+
info: ReturnType<typeof vi.fn>;
679+
};
680+
conversations.info.mockResolvedValueOnce({
681+
channel: { name: "general", is_channel: true },
682+
});
683+
684+
await runSlackMessageOnce(monitorSlackProvider, {
685+
event: makeSlackMessageEvent({
686+
text: "<@bot-user> hello",
687+
ts: "456",
688+
channel_type: "channel",
689+
}),
690+
});
691+
await new Promise((resolve) => setTimeout(resolve, 0));
692+
await flush();
693+
694+
expect(sendMock).not.toHaveBeenCalled();
695+
expect(reactMock.mock.calls.map(([args]) => String((args as { name: string }).name))).toEqual([
696+
"eyes",
697+
"scream",
698+
"eyes",
699+
"eyes",
700+
"scream",
701+
]);
702+
});
703+
556704
it("replies with pairing code when dmPolicy is pairing and no allowFrom is set", async () => {
557705
setPairingOnlyDirectMessages();
558706

0 commit comments

Comments
 (0)