Skip to content

Commit 5c29a08

Browse files
authored
Discord: settle completed button interactions cleanly (#28)
* Discord: clear sandbox approval buttons * Discord: finish plan questionnaires cleanly
1 parent d4ecc08 commit 5c29a08

File tree

2 files changed

+293
-13
lines changed

2 files changed

+293
-13
lines changed

src/controller.test.ts

Lines changed: 227 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,229 @@ describe("Discord controller flows", () => {
537537
expect(sendComponentMessage).not.toHaveBeenCalled();
538538
});
539539

540+
it("acknowledges and clears Discord pending-input buttons by message id", async () => {
541+
const { controller } = await createControllerHarness();
542+
await (controller as any).store.upsertPendingRequest({
543+
requestId: "pending-1",
544+
conversation: {
545+
channel: "discord",
546+
accountId: "default",
547+
conversationId: "channel:chan-1",
548+
},
549+
threadId: "thread-1",
550+
workspaceDir: "/repo/openclaw",
551+
state: {
552+
requestId: "pending-1",
553+
options: ["Approve Once", "Cancel"],
554+
expiresAt: Date.now() + 60_000,
555+
},
556+
updatedAt: Date.now(),
557+
});
558+
const callback = await (controller as any).store.putCallback({
559+
kind: "pending-input",
560+
conversation: {
561+
channel: "discord",
562+
accountId: "default",
563+
conversationId: "channel:chan-1",
564+
},
565+
requestId: "pending-1",
566+
actionIndex: 0,
567+
});
568+
const acknowledge = vi.fn(async () => {});
569+
const clearComponents = vi.fn(async () => {});
570+
const reply = vi.fn(async () => {});
571+
const followUp = vi.fn(async () => {});
572+
const submitPendingInput = vi.fn(async () => true);
573+
(controller as any).activeRuns.set("discord::default::channel:chan-1::", {
574+
conversation: {
575+
channel: "discord",
576+
accountId: "default",
577+
conversationId: "channel:chan-1",
578+
},
579+
workspaceDir: "/repo/openclaw",
580+
mode: "default",
581+
handle: {
582+
result: Promise.resolve({ threadId: "thread-1", text: "done" }),
583+
queueMessage: vi.fn(async () => false),
584+
submitPendingInput,
585+
submitPendingInputPayload: vi.fn(async () => false),
586+
interrupt: vi.fn(async () => {}),
587+
isAwaitingInput: vi.fn(() => true),
588+
getThreadId: vi.fn(() => "thread-1"),
589+
},
590+
});
591+
592+
await controller.handleDiscordInteractive({
593+
channel: "discord",
594+
accountId: "default",
595+
interactionId: "interaction-1",
596+
conversationId: "channel:chan-1",
597+
auth: { isAuthorizedSender: true },
598+
interaction: {
599+
kind: "button",
600+
data: `codexapp:${callback.token}`,
601+
namespace: "codexapp",
602+
payload: callback.token,
603+
messageId: "message-1",
604+
},
605+
senderId: "user-1",
606+
senderUsername: "Ada",
607+
respond: {
608+
acknowledge,
609+
reply,
610+
followUp,
611+
editMessage: vi.fn(async () => {}),
612+
clearComponents,
613+
},
614+
} as any);
615+
616+
expect(submitPendingInput).toHaveBeenCalledWith(0);
617+
expect(acknowledge).toHaveBeenCalledTimes(1);
618+
expect(clearComponents).not.toHaveBeenCalled();
619+
expect(discordSdkState.editDiscordComponentMessage).toHaveBeenCalledWith(
620+
"channel:chan-1",
621+
"message-1",
622+
{
623+
text: "Sent to Codex.",
624+
},
625+
expect.objectContaining({ accountId: "default" }),
626+
);
627+
expect(reply).not.toHaveBeenCalled();
628+
expect(followUp).not.toHaveBeenCalled();
629+
});
630+
631+
it("does not send a second Discord response after completing a questionnaire", async () => {
632+
const { controller } = await createControllerHarness();
633+
await (controller as any).store.upsertPendingRequest({
634+
requestId: "questionnaire-1",
635+
conversation: {
636+
channel: "discord",
637+
accountId: "default",
638+
conversationId: "channel:chan-1",
639+
},
640+
threadId: "thread-1",
641+
workspaceDir: "/repo/openclaw",
642+
state: {
643+
requestId: "questionnaire-1",
644+
options: [],
645+
expiresAt: Date.now() + 60_000,
646+
questionnaire: {
647+
currentIndex: 1,
648+
awaitingFreeform: false,
649+
questions: [
650+
{
651+
index: 0,
652+
id: "milk",
653+
header: "Milk",
654+
prompt: "Do you like milk on cereal?",
655+
options: [
656+
{ key: "A", label: "Yes", description: "Sure." },
657+
{ key: "B", label: "No", description: "Nope." },
658+
],
659+
},
660+
{
661+
index: 1,
662+
id: "type",
663+
header: "Type",
664+
prompt: "What kind of milk?",
665+
options: [
666+
{ key: "A", label: "Whole", description: "Richer." },
667+
{ key: "B", label: "2%", description: "Lighter." },
668+
],
669+
},
670+
],
671+
answers: [
672+
{
673+
kind: "option",
674+
optionKey: "A",
675+
optionLabel: "Yes",
676+
},
677+
null,
678+
],
679+
},
680+
},
681+
updatedAt: Date.now(),
682+
});
683+
const callback = await (controller as any).store.putCallback({
684+
kind: "pending-questionnaire",
685+
conversation: {
686+
channel: "discord",
687+
accountId: "default",
688+
conversationId: "channel:chan-1",
689+
},
690+
requestId: "questionnaire-1",
691+
questionIndex: 1,
692+
action: "select",
693+
optionIndex: 0,
694+
});
695+
const acknowledge = vi.fn(async () => {});
696+
const clearComponents = vi.fn(async () => {});
697+
const reply = vi.fn(async () => {});
698+
const followUp = vi.fn(async () => {});
699+
const submitPendingInputPayload = vi.fn(async () => true);
700+
(controller as any).activeRuns.set("discord::default::channel:chan-1::", {
701+
conversation: {
702+
channel: "discord",
703+
accountId: "default",
704+
conversationId: "channel:chan-1",
705+
},
706+
workspaceDir: "/repo/openclaw",
707+
mode: "plan",
708+
handle: {
709+
result: Promise.resolve({ threadId: "thread-1", text: "done" }),
710+
queueMessage: vi.fn(async () => false),
711+
submitPendingInput: vi.fn(async () => false),
712+
submitPendingInputPayload,
713+
interrupt: vi.fn(async () => {}),
714+
isAwaitingInput: vi.fn(() => true),
715+
getThreadId: vi.fn(() => "thread-1"),
716+
},
717+
});
718+
719+
await controller.handleDiscordInteractive({
720+
channel: "discord",
721+
accountId: "default",
722+
interactionId: "interaction-1",
723+
conversationId: "channel:chan-1",
724+
auth: { isAuthorizedSender: true },
725+
interaction: {
726+
kind: "button",
727+
data: `codexapp:${callback.token}`,
728+
namespace: "codexapp",
729+
payload: callback.token,
730+
messageId: "message-1",
731+
},
732+
senderId: "user-1",
733+
senderUsername: "Ada",
734+
respond: {
735+
acknowledge,
736+
reply,
737+
followUp,
738+
editMessage: vi.fn(async () => {}),
739+
clearComponents,
740+
},
741+
} as any);
742+
743+
expect(submitPendingInputPayload).toHaveBeenCalledWith({
744+
answers: {
745+
milk: { answers: ["Yes"] },
746+
type: { answers: ["Whole"] },
747+
},
748+
});
749+
expect(acknowledge).toHaveBeenCalledTimes(1);
750+
expect(clearComponents).not.toHaveBeenCalled();
751+
expect(discordSdkState.editDiscordComponentMessage).toHaveBeenCalledWith(
752+
"channel:chan-1",
753+
"message-1",
754+
{
755+
text: "Recorded your answers and sent them to Codex.",
756+
},
757+
expect.objectContaining({ accountId: "default" }),
758+
);
759+
expect(reply).not.toHaveBeenCalled();
760+
expect(followUp).not.toHaveBeenCalled();
761+
});
762+
540763
it("normalizes raw Discord callback conversation ids for guild interactions", async () => {
541764
const { controller, sendComponentMessage } = await createControllerHarness();
542765
const callback = await (controller as any).store.putCallback({
@@ -1742,6 +1965,7 @@ describe("Discord controller flows", () => {
17421965
}));
17431966
(controller as any).client.startTurn = startTurn;
17441967
const reply = vi.fn(async () => {});
1968+
const followUp = vi.fn(async () => {});
17451969

17461970
await controller.handleDiscordInteractive({
17471971
channel: "discord",
@@ -1761,13 +1985,14 @@ describe("Discord controller flows", () => {
17611985
respond: {
17621986
acknowledge: vi.fn(async () => {}),
17631987
reply,
1764-
followUp: vi.fn(async () => {}),
1988+
followUp,
17651989
editMessage: vi.fn(async () => {}),
17661990
clearComponents: vi.fn(async () => {}),
17671991
},
17681992
} as any);
17691993

1770-
expect(reply).toHaveBeenCalledWith({ text: "Sent the plan to Codex.", ephemeral: true });
1994+
expect(reply).not.toHaveBeenCalled();
1995+
expect(followUp).toHaveBeenCalledWith({ text: "Sent the plan to Codex.", ephemeral: true });
17711996
expect(startTurn).toHaveBeenCalledWith(
17721997
expect.objectContaining({
17731998
prompt: "Implement the plan.",

src/controller.ts

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -841,21 +841,66 @@ export class CodexPluginController {
841841
conversationId,
842842
parentConversationId: callback.conversation.parentConversationId ?? ctx.parentConversationId,
843843
};
844+
let interactionSettled = false;
844845
try {
845846
if (callback.kind === "resume-thread") {
846-
await ctx.respond.acknowledge().catch(() => undefined);
847+
await ctx.respond
848+
.acknowledge()
849+
.then(() => {
850+
interactionSettled = true;
851+
})
852+
.catch(() => undefined);
847853
}
848854
await this.dispatchCallbackAction(callback, {
849855
conversation,
850856
clear: async () => {
857+
const messageId = ctx.interaction.messageId?.trim();
858+
if ((callback.kind === "pending-input" || callback.kind === "pending-questionnaire") && messageId) {
859+
await ctx.respond
860+
.acknowledge()
861+
.then(() => {
862+
interactionSettled = true;
863+
})
864+
.catch(() => undefined);
865+
const completionText =
866+
callback.kind === "pending-questionnaire"
867+
? "Recorded your answers and sent them to Codex."
868+
: "Sent to Codex.";
869+
await editDiscordComponentMessage(
870+
conversation.conversationId,
871+
messageId,
872+
{
873+
text: completionText,
874+
},
875+
{
876+
accountId: conversation.accountId,
877+
},
878+
).catch((error) => {
879+
this.api.logger.warn(
880+
`codex discord ${callback.kind} clear failed conversation=${conversationId}: ${String(error)}`,
881+
);
882+
});
883+
return;
884+
}
851885
try {
852886
await ctx.respond.clearComponents();
887+
interactionSettled = true;
853888
} catch {
854-
await ctx.respond.acknowledge().catch(() => undefined);
889+
await ctx.respond
890+
.acknowledge()
891+
.then(() => {
892+
interactionSettled = true;
893+
})
894+
.catch(() => undefined);
855895
}
856896
},
857897
reply: async (text) => {
898+
if (interactionSettled) {
899+
await ctx.respond.followUp({ text, ephemeral: true });
900+
return;
901+
}
858902
await ctx.respond.reply({ text, ephemeral: true });
903+
interactionSettled = true;
859904
},
860905
editPicker: async (picker) => {
861906
this.api.logger.debug(
@@ -867,6 +912,7 @@ export class CodexPluginController {
867912
await ctx.respond.editMessage({
868913
components: builtPicker.components,
869914
});
915+
interactionSettled = true;
870916
if (messageId) {
871917
registerBuiltDiscordComponentMessage({
872918
buildResult: builtPicker,
@@ -881,7 +927,12 @@ export class CodexPluginController {
881927
);
882928
if (messageId) {
883929
if (!detail.includes("already been acknowledged")) {
884-
await ctx.respond.acknowledge().catch(() => undefined);
930+
await ctx.respond
931+
.acknowledge()
932+
.then(() => {
933+
interactionSettled = true;
934+
})
935+
.catch(() => undefined);
885936
}
886937
await editDiscordComponentMessage(
887938
conversation.conversationId,
@@ -929,12 +980,12 @@ export class CodexPluginController {
929980
} catch (error) {
930981
const detail = error instanceof Error ? error.stack ?? error.message : String(error);
931982
this.api.logger.warn(`codex discord interactive failed conversation=${conversationId}: ${detail}`);
932-
await ctx.respond
933-
.reply({
934-
text: "Codex hit an error handling that action. Please retry the command.",
935-
ephemeral: true,
936-
})
937-
.catch(() => undefined);
983+
const errorReply = {
984+
text: "Codex hit an error handling that action. Please retry the command.",
985+
ephemeral: true,
986+
} as const;
987+
const sendError = interactionSettled ? ctx.respond.followUp(errorReply) : ctx.respond.reply(errorReply);
988+
await sendError.catch(() => undefined);
938989
}
939990
}
940991

@@ -2858,7 +2909,9 @@ export class CodexPluginController {
28582909
return;
28592910
}
28602911
await this.store.removeCallback(callback.token);
2861-
await responders.reply("Sent to Codex.");
2912+
if (callback.conversation.channel !== "discord") {
2913+
await responders.reply("Sent to Codex.");
2914+
}
28622915
return;
28632916
}
28642917
if (callback.kind === "pending-questionnaire") {
@@ -2952,7 +3005,9 @@ export class CodexPluginController {
29523005
}
29533006
await responders.clear().catch(() => undefined);
29543007
await this.store.removePendingRequest(pending.requestId);
2955-
await responders.reply("Recorded your answers and sent them to Codex.");
3008+
if (callback.conversation.channel !== "discord") {
3009+
await responders.reply("Recorded your answers and sent them to Codex.");
3010+
}
29563011
return;
29573012
}
29583013
await this.sendPendingQuestionnaire(callback.conversation, pending.state, {

0 commit comments

Comments
 (0)