Skip to content

Commit 97944d9

Browse files
849261680849261680
committed
fix(agents): keep message send loop blocks sticky
1 parent 9ebe09c commit 97944d9

4 files changed

Lines changed: 121 additions & 43 deletions

File tree

‎src/agents/agent-tools.before-tool-call.ts‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,13 +1140,17 @@ export function wrapToolWithBeforeToolCallHook(
11401140
reason: outcome.reason,
11411141
deniedReason: outcome.deniedReason ?? "plugin-before-tool-call",
11421142
});
1143-
await recordLoopOutcome({
1144-
ctx,
1145-
toolName: normalizedToolName,
1146-
toolParams: outcome.params ?? hookParams,
1147-
toolCallId,
1148-
result: blockedResult,
1149-
});
1143+
// A loop detector veto is not tool progress. Recording it would replace
1144+
// the repeated successful outcome and let the same loop resume.
1145+
if (outcome.deniedReason !== "tool-loop") {
1146+
await recordLoopOutcome({
1147+
ctx,
1148+
toolName: normalizedToolName,
1149+
toolParams: outcome.params ?? hookParams,
1150+
toolCallId,
1151+
result: blockedResult,
1152+
});
1153+
}
11501154
return blockedResult;
11511155
}
11521156
const executeParams = reconcileCodeModeExecBeforeHookParams({

‎src/agents/tool-loop-detection.test.ts‎

Lines changed: 70 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,23 @@ describe("tool-loop-detection", () => {
656656
messageId: `plugin_volatile_${index}`,
657657
}),
658658
},
659+
{
660+
name: "qa-channel plugin message result",
661+
payloadAt: (index: number) => ({
662+
message: {
663+
id: `qa_volatile_${index}`,
664+
accountId: "default",
665+
direction: "outbound",
666+
conversation: {
667+
id: "loop-room",
668+
chatType: "channel",
669+
},
670+
senderId: "openclaw",
671+
text: "hello",
672+
timestamp: 1_800_000_000_000 + index,
673+
},
674+
}),
675+
},
659676
];
660677

661678
for (const { name, payloadAt } of cases) {
@@ -685,37 +702,62 @@ describe("tool-loop-detection", () => {
685702
});
686703

687704
it("does not block repeated message sends when stable delivery facts change", () => {
688-
const state = createState();
689705
const params = { action: "send", to: "feishu:chat-1", content: "hello" };
706+
const cases = [
707+
{
708+
name: "chat id changes",
709+
payloadAt: (index: number) => ({
710+
channel: "feishu",
711+
to: "feishu:chat-1",
712+
via: "direct",
713+
result: {
714+
ok: true,
715+
chatId: `oc_chat_${index}`,
716+
messageId: `om_volatile_${index}`,
717+
},
718+
}),
719+
},
720+
{
721+
name: "qa-channel conversation id changes",
722+
payloadAt: (index: number) => ({
723+
message: {
724+
id: `qa_volatile_${index}`,
725+
accountId: "default",
726+
direction: "outbound",
727+
conversation: {
728+
id: `loop-room-${index}`,
729+
chatType: "channel",
730+
},
731+
senderId: "openclaw",
732+
text: "hello",
733+
timestamp: 1_800_000_000_000 + index,
734+
},
735+
}),
736+
},
737+
];
690738

691-
for (let index = 0; index < CRITICAL_THRESHOLD; index += 1) {
692-
const payload = {
693-
channel: "feishu",
694-
to: "feishu:chat-1",
695-
via: "direct",
696-
result: {
697-
ok: true,
698-
chatId: `oc_chat_${index}`,
699-
messageId: `om_volatile_${index}`,
700-
},
701-
};
702-
recordSuccessfulCall(
703-
state,
704-
"message",
705-
params,
706-
{
707-
content: [{ type: "text", text: JSON.stringify(payload) }],
708-
details: payload,
709-
},
710-
index,
711-
);
712-
}
739+
for (const { name, payloadAt } of cases) {
740+
const state = createState();
741+
for (let index = 0; index < CRITICAL_THRESHOLD; index += 1) {
742+
const payload = payloadAt(index);
743+
recordSuccessfulCall(
744+
state,
745+
"message",
746+
params,
747+
{
748+
content: [{ type: "text", text: JSON.stringify(payload) }],
749+
details: payload,
750+
},
751+
index,
752+
);
753+
}
713754

714-
const loopResult = detectToolCallLoop(state, "message", params, enabledLoopDetectionConfig);
715-
expect(loopResult.stuck).toBe(true);
716-
if (loopResult.stuck) {
717-
expect(loopResult.level).toBe("warning");
718-
expect(loopResult.detector).toBe("generic_repeat");
755+
const loopResult = detectToolCallLoop(state, "message", params, enabledLoopDetectionConfig);
756+
expect(loopResult.stuck, name).toBe(true);
757+
if (loopResult.stuck) {
758+
expect(loopResult.level, name).toBe("warning");
759+
expect(loopResult.detector, name).toBe("generic_repeat");
760+
}
719761
}
720762
});
721763

‎src/agents/tool-loop-detection.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,17 @@ function parseJsonObject(text: string): Record<string, unknown> | undefined {
203203
}
204204
}
205205

206+
function isMessageDeliveryObject(value: Record<string, unknown>): boolean {
207+
return (
208+
typeof value.id === "string" &&
209+
typeof value.text === "string" &&
210+
(typeof value.direction === "string" ||
211+
typeof value.senderId === "string" ||
212+
typeof value.accountId === "string" ||
213+
isPlainObject(value.conversation))
214+
);
215+
}
216+
206217
function normalizeMessageSendOutcome(value: unknown): unknown {
207218
if (Array.isArray(value)) {
208219
return value.map((entry) => normalizeMessageSendOutcome(entry));
@@ -211,13 +222,15 @@ function normalizeMessageSendOutcome(value: unknown): unknown {
211222
return value;
212223
}
213224

225+
const stripMessageObjectId = isMessageDeliveryObject(value);
214226
const normalized: Record<string, unknown> = {};
215227
for (const [key, entry] of Object.entries(value)) {
216228
if (
217229
key === "messageId" ||
218230
key === "messageIds" ||
219231
key === "platformMessageIds" ||
220232
key === "receipt" ||
233+
(key === "id" && stripMessageObjectId) ||
221234
key === "timestamp"
222235
) {
223236
continue;

‎src/agents/tools/message-tool.test.ts‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,11 +1098,17 @@ describe("message tool loop detection action runner proof", () => {
10981098
to,
10991099
handledBy: "plugin",
11001100
payload: {
1101-
channel: "qa-channel",
1102-
to,
1103-
via: "gateway",
1104-
result: {
1105-
messageId: `qa-message-${callIndex}`,
1101+
message: {
1102+
id: `qa-message-${callIndex}`,
1103+
accountId: "default",
1104+
direction: "outbound",
1105+
conversation: {
1106+
id: "loop-room",
1107+
chatType: "channel",
1108+
},
1109+
senderId: "openclaw",
1110+
text: "same visible reply",
1111+
timestamp: 1_800_000_000_000 + callIndex,
11061112
},
11071113
},
11081114
dryRun: false,
@@ -1132,9 +1138,12 @@ describe("message tool loop detection action runner proof", () => {
11321138
for (let i = 0; i < CRITICAL_THRESHOLD; i += 1) {
11331139
const result = await wrappedTool.execute(`message-tool-send-${i}`, params);
11341140
expect(result.details).toMatchObject({
1135-
channel: "qa-channel",
1136-
to: "channel:loop-room",
1137-
via: "gateway",
1141+
message: {
1142+
conversation: {
1143+
id: "loop-room",
1144+
},
1145+
text: "same visible reply",
1146+
},
11381147
});
11391148
}
11401149

@@ -1146,6 +1155,16 @@ describe("message tool loop detection action runner proof", () => {
11461155
});
11471156
const blockedDetails = blocked.details as { reason?: unknown } | undefined;
11481157
expect(String(blockedDetails?.reason)).toContain("CRITICAL");
1158+
1159+
const blockedAgain = await wrappedTool.execute(
1160+
`message-tool-send-${CRITICAL_THRESHOLD + 1}`,
1161+
params,
1162+
);
1163+
expect(mocks.runMessageAction).toHaveBeenCalledTimes(CRITICAL_THRESHOLD);
1164+
expect(blockedAgain.details).toMatchObject({
1165+
status: "blocked",
1166+
deniedReason: "tool-loop",
1167+
});
11491168
});
11501169
});
11511170

0 commit comments

Comments
 (0)