Skip to content

Commit 51fd88e

Browse files
authored
fix(agent): dedupe redelivered user_message commands by messageId (#3095)
1 parent 84bfe68 commit 51fd88e

2 files changed

Lines changed: 117 additions & 0 deletions

File tree

packages/agent/src/server/agent-server.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1184,6 +1184,99 @@ describe("AgentServer HTTP Mode", () => {
11841184
expect(sentMeta?.localSkillContext).toContain("with context");
11851185
expect(sentMeta?.localSkillName).toBe("local-test-skill");
11861186
}, 20000);
1187+
1188+
it("ignores a redelivered user_message whose messageId was already accepted", async () => {
1189+
const s = createServer();
1190+
await s.start();
1191+
const prompt = vi.fn(async () => ({ stopReason: "end_turn" }));
1192+
const serverInternals = s as unknown as {
1193+
session: { clientConnection: { prompt: typeof prompt } };
1194+
};
1195+
serverInternals.session.clientConnection.prompt = prompt;
1196+
1197+
const token = createToken();
1198+
const send = async (messageId: string | undefined) => {
1199+
const response = await fetch(`http://localhost:${port}/command`, {
1200+
method: "POST",
1201+
headers: {
1202+
Authorization: `Bearer ${token}`,
1203+
"Content-Type": "application/json",
1204+
},
1205+
body: JSON.stringify({
1206+
jsonrpc: "2.0",
1207+
id: messageId ?? "no-id",
1208+
method: "user_message",
1209+
params: {
1210+
content: "do the thing",
1211+
...(messageId ? { messageId } : {}),
1212+
},
1213+
}),
1214+
});
1215+
expect(response.status).toBe(200);
1216+
return (await response.json()) as {
1217+
result?: { stopReason?: string; duplicate?: boolean };
1218+
};
1219+
};
1220+
1221+
const first = await send("m-1");
1222+
expect(first.result?.stopReason).toBe("end_turn");
1223+
expect(prompt).toHaveBeenCalledTimes(1);
1224+
1225+
const redelivery = await send("m-1");
1226+
expect(redelivery.result?.duplicate).toBe(true);
1227+
expect(redelivery.result?.stopReason).toBe("duplicate_delivery");
1228+
expect(prompt).toHaveBeenCalledTimes(1);
1229+
1230+
const distinct = await send("m-2");
1231+
expect(distinct.result?.stopReason).toBe("end_turn");
1232+
expect(prompt).toHaveBeenCalledTimes(2);
1233+
1234+
const anonymousFirst = await send(undefined);
1235+
const anonymousSecond = await send(undefined);
1236+
expect(anonymousFirst.result?.stopReason).toBe("end_turn");
1237+
expect(anonymousSecond.result?.stopReason).toBe("end_turn");
1238+
expect(prompt).toHaveBeenCalledTimes(4);
1239+
}, 20000);
1240+
1241+
it("redelivers a messageId whose first delivery failed before producing a turn", async () => {
1242+
const s = createServer();
1243+
await s.start();
1244+
const prompt = vi
1245+
.fn(async () => ({ stopReason: "end_turn" }))
1246+
.mockRejectedValueOnce(new Error("sdk connection lost"));
1247+
const serverInternals = s as unknown as {
1248+
session: { clientConnection: { prompt: typeof prompt } };
1249+
};
1250+
serverInternals.session.clientConnection.prompt = prompt;
1251+
1252+
const token = createToken();
1253+
const send = async () =>
1254+
fetch(`http://localhost:${port}/command`, {
1255+
method: "POST",
1256+
headers: {
1257+
Authorization: `Bearer ${token}`,
1258+
"Content-Type": "application/json",
1259+
},
1260+
body: JSON.stringify({
1261+
jsonrpc: "2.0",
1262+
id: "m-err",
1263+
method: "user_message",
1264+
params: { content: "do the thing", messageId: "m-err" },
1265+
}),
1266+
});
1267+
1268+
await send();
1269+
expect(prompt).toHaveBeenCalledTimes(1);
1270+
1271+
const retry = await send();
1272+
expect(retry.status).toBe(200);
1273+
const body = (await retry.json()) as {
1274+
result?: { stopReason?: string; duplicate?: boolean };
1275+
};
1276+
expect(body.result?.duplicate).toBeUndefined();
1277+
expect(body.result?.stopReason).toBe("end_turn");
1278+
expect(prompt).toHaveBeenCalledTimes(2);
1279+
}, 20000);
11871280
});
11881281

11891282
describe("404 handling", () => {

packages/agent/src/server/agent-server.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ export class AgentServer {
307307
// causing a second session to be created and duplicate Slack messages to be sent.
308308
private initializationPromise: Promise<void> | null = null;
309309
private pendingEvents: Record<string, unknown>[] = [];
310+
private deliveredMessageIds = new Set<string>();
310311
private pendingPermissions = new Map<
311312
string,
312313
{
@@ -822,6 +823,26 @@ export class AgentServer {
822823
if (prompt.length === 0) {
823824
throw new Error("User message cannot be empty");
824825
}
826+
827+
const messageId =
828+
typeof params.messageId === "string" && params.messageId
829+
? params.messageId
830+
: undefined;
831+
if (messageId) {
832+
if (this.deliveredMessageIds.has(messageId)) {
833+
this.logger.info("Duplicate user_message delivery ignored", {
834+
messageId,
835+
});
836+
return { stopReason: "duplicate_delivery", duplicate: true };
837+
}
838+
this.deliveredMessageIds.add(messageId);
839+
if (this.deliveredMessageIds.size > 500) {
840+
const oldest = this.deliveredMessageIds.values().next().value;
841+
if (oldest !== undefined) {
842+
this.deliveredMessageIds.delete(oldest);
843+
}
844+
}
845+
}
825846
this.logger.debug("Built user_message prompt", {
826847
blockTypes: prompt.map((block) => block.type),
827848
});
@@ -852,6 +873,9 @@ export class AgentServer {
852873
: {}),
853874
});
854875
} catch (error) {
876+
if (messageId) {
877+
this.deliveredMessageIds.delete(messageId);
878+
}
855879
await this.session.logWriter.flushAll();
856880
const { recoverable } = await this.handleTurnFailure(
857881
this.session.payload,

0 commit comments

Comments
 (0)