Skip to content

Commit 555b261

Browse files
传妈传妈
传妈
authored and
传妈
committed
fix(message): keep automatic replies out of implicit sends
1 parent f613f32 commit 555b261

18 files changed

Lines changed: 960 additions & 69 deletions

‎extensions/codex/src/app-server/dynamic-tool-build.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -837,6 +837,30 @@ describe("Codex app-server dynamic tool build", () => {
837837
expect(shouldForceMessageTool(params)).toBe(false);
838838
});
839839

840+
it.each(["automatic", "message_tool_only"] as const)(
841+
"allows %s source replies to use the internal sink in Codex app-server tools",
842+
async (sourceReplyDeliveryMode) => {
843+
const workspaceDir = path.join(tempDir, "workspace");
844+
const params = createParams(path.join(tempDir, "session.jsonl"), workspaceDir);
845+
params.disableTools = false;
846+
params.runtimePlan = createCodexRuntimePlanFixture();
847+
params.sourceReplyDeliveryMode = sourceReplyDeliveryMode;
848+
const factoryOptions: unknown[] = [];
849+
setOpenClawCodingToolsFactoryForTests((options) => {
850+
factoryOptions.push(options);
851+
return [];
852+
});
853+
854+
await buildDynamicToolsForTest(params, workspaceDir, { sandbox: null as never });
855+
856+
expect(factoryOptions).toHaveLength(1);
857+
expect(factoryOptions[0]).toMatchObject({
858+
sourceReplyDeliveryMode,
859+
allowInternalSourceReplySink: true,
860+
});
861+
},
862+
);
863+
840864
it("passes the live run session key to Codex dynamic tools when sandbox policy uses another key", () => {
841865
const workspaceDir = path.join(tempDir, "workspace");
842866
const params = createParams(path.join(tempDir, "session.jsonl"), workspaceDir);

‎extensions/codex/src/app-server/dynamic-tool-build.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ export async function buildDynamicTools(input: DynamicToolBuildParams) {
249249
requireExplicitMessageTarget:
250250
params.requireExplicitMessageTarget ?? isSubagentSessionKey(params.sessionKey),
251251
sourceReplyDeliveryMode: params.sourceReplyDeliveryMode,
252+
allowInternalSourceReplySink: true,
252253
disableMessageTool: params.disableMessageTool,
253254
forceMessageTool: shouldForceMessageTool(params),
254255
enableHeartbeatTool: params.trigger === "heartbeat" || input.forceHeartbeatTool === true,

‎src/agents/agent-tools.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,8 @@ export function createOpenClawCodingTools(options?: {
479479
requireExplicitMessageTarget?: boolean;
480480
/** Visible source replies must be sent through the message tool when set to message_tool_only. */
481481
sourceReplyDeliveryMode?: SourceReplyDeliveryMode;
482+
/** If true, message-tool source replies can be consumed by the embedded run subscriber. */
483+
allowInternalSourceReplySink?: boolean;
482484
inboundEventKind?: InboundEventKind;
483485
/** If true, omit the message tool from the tool list. */
484486
disableMessageTool?: boolean;
@@ -1000,6 +1002,7 @@ export function createOpenClawCodingTools(options?: {
10001002
modelHasVision: options?.modelHasVision,
10011003
requireExplicitMessageTarget: options?.requireExplicitMessageTarget,
10021004
sourceReplyDeliveryMode: options?.sourceReplyDeliveryMode,
1005+
allowInternalSourceReplySink: options?.allowInternalSourceReplySink,
10031006
inboundEventKind: options?.inboundEventKind,
10041007
disableMessageTool: options?.disableMessageTool,
10051008
enableHeartbeatTool,

‎src/agents/embedded-agent-runner/run/attempt.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,6 +1166,7 @@ export async function runEmbeddedAttempt(
11661166
requireExplicitMessageTarget:
11671167
params.requireExplicitMessageTarget ?? isSubagentSessionKey(params.sessionKey),
11681168
sourceReplyDeliveryMode: params.sourceReplyDeliveryMode,
1169+
allowInternalSourceReplySink: true,
11691170
inboundEventKind: params.currentInboundEventKind,
11701171
disableMessageTool: params.disableMessageTool,
11711172
forceMessageTool: params.forceMessageTool,

‎src/agents/embedded-agent-runner/run/payloads.test.ts‎

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -221,39 +221,42 @@ describe("buildEmbeddedRunPayloads tool-error warnings", () => {
221221
expectSinglePayloadText(payloads, "The schema export is fixed.");
222222
});
223223

224-
it("turns internal message-tool source replies into suppression-safe final payloads", () => {
225-
const payloads = buildPayloads({
226-
assistantTexts: ["ordinary final should stay private"],
227-
didSendViaMessagingTool: true,
228-
messagingToolSourceReplyPayloads: [
229-
{
230-
text: "sent through message tool",
231-
mediaUrls: ["/tmp/reply.png"],
232-
},
233-
],
234-
sourceReplyDeliveryMode: "message_tool_only",
235-
sessionKey: "agent:main",
236-
agentId: "main",
237-
runId: "run-1",
238-
});
239-
240-
expect(payloads).toHaveLength(1);
241-
expect(payloads[0]).toMatchObject({
242-
text: "sent through message tool",
243-
mediaUrl: "/tmp/reply.png",
244-
mediaUrls: ["/tmp/reply.png"],
245-
});
246-
expect(getReplyPayloadMetadata(payloads[0] as object)).toMatchObject({
247-
deliverDespiteSourceReplySuppression: true,
248-
sourceReplyTranscriptMirror: {
224+
it.each(["message_tool_only", "automatic"] as const)(
225+
"turns %s internal message-tool source replies into suppression-safe final payloads",
226+
(sourceReplyDeliveryMode) => {
227+
const payloads = buildPayloads({
228+
assistantTexts: ["ordinary final should stay private"],
229+
didSendViaMessagingTool: true,
230+
messagingToolSourceReplyPayloads: [
231+
{
232+
text: "sent through message tool",
233+
mediaUrls: ["/tmp/reply.png"],
234+
},
235+
],
236+
sourceReplyDeliveryMode,
249237
sessionKey: "agent:main",
250238
agentId: "main",
239+
runId: "run-1",
240+
});
241+
242+
expect(payloads).toHaveLength(1);
243+
expect(payloads[0]).toMatchObject({
251244
text: "sent through message tool",
245+
mediaUrl: "/tmp/reply.png",
252246
mediaUrls: ["/tmp/reply.png"],
253-
idempotencyKey: "run-1:internal-source-reply:0",
254-
},
255-
});
256-
});
247+
});
248+
expect(getReplyPayloadMetadata(payloads[0] as object)).toMatchObject({
249+
deliverDespiteSourceReplySuppression: true,
250+
sourceReplyTranscriptMirror: {
251+
sessionKey: "agent:main",
252+
agentId: "main",
253+
text: "sent through message tool",
254+
mediaUrls: ["/tmp/reply.png"],
255+
idempotencyKey: "run-1:internal-source-reply:0",
256+
},
257+
});
258+
},
259+
);
257260

258261
it("preserves rich-only internal message-tool source replies", () => {
259262
const presentation = {

‎src/agents/embedded-agent-runner/run/payloads.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,8 @@ export function buildEmbeddedRunPayloads(params: {
247247
}> = [];
248248

249249
const sourceReplyPayloads =
250-
params.sourceReplyDeliveryMode === "message_tool_only"
250+
params.sourceReplyDeliveryMode === "message_tool_only" ||
251+
params.sourceReplyDeliveryMode === "automatic"
251252
? (params.messagingToolSourceReplyPayloads ?? [])
252253
: [];
253254
const sourceReplyStartIndex = replyItems.length;

‎src/agents/embedded-agent-runner/run/tool-media-payloads.test.ts‎

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -90,32 +90,35 @@ describe("mergeAttemptToolMediaPayloads", () => {
9090
]);
9191
});
9292

93-
it("does not attach tool media to message-tool-only source reply mirrors", () => {
94-
const sourceReply = setReplyPayloadMetadata(
95-
{ text: "sent through message tool" },
96-
{
93+
it.each(["message_tool_only", "automatic"] as const)(
94+
"does not attach tool media to %s source reply mirrors",
95+
(sourceReplyDeliveryMode) => {
96+
const sourceReply = setReplyPayloadMetadata(
97+
{ text: "sent through message tool" },
98+
{
99+
deliverDespiteSourceReplySuppression: true,
100+
sourceReplyTranscriptMirror: {
101+
sessionKey: "agent:main",
102+
text: "sent through message tool",
103+
},
104+
},
105+
);
106+
107+
const [mergedReply] =
108+
mergeAttemptToolMediaPayloads({
109+
payloads: [sourceReply],
110+
toolMediaUrls: ["/tmp/generated.png"],
111+
sourceReplyDeliveryMode,
112+
}) ?? [];
113+
114+
expect(mergedReply).toEqual({ text: "sent through message tool" });
115+
expect(getReplyPayloadMetadata(mergedReply ?? {})).toMatchObject({
97116
deliverDespiteSourceReplySuppression: true,
98117
sourceReplyTranscriptMirror: {
99118
sessionKey: "agent:main",
100119
text: "sent through message tool",
101120
},
102-
},
103-
);
104-
105-
const [mergedReply] =
106-
mergeAttemptToolMediaPayloads({
107-
payloads: [sourceReply],
108-
toolMediaUrls: ["/tmp/generated.png"],
109-
sourceReplyDeliveryMode: "message_tool_only",
110-
}) ?? [];
111-
112-
expect(mergedReply).toEqual({ text: "sent through message tool" });
113-
expect(getReplyPayloadMetadata(mergedReply ?? {})).toMatchObject({
114-
deliverDespiteSourceReplySuppression: true,
115-
sourceReplyTranscriptMirror: {
116-
sessionKey: "agent:main",
117-
text: "sent through message tool",
118-
},
119-
});
120-
});
121+
});
122+
},
123+
);
121124
});

‎src/agents/embedded-agent-runner/run/tool-media-payloads.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ export function mergeAttemptToolMediaPayloads(params: {
2626
if (payloadIndex >= 0) {
2727
const payload = payloads[payloadIndex];
2828
if (
29-
params.sourceReplyDeliveryMode === "message_tool_only" &&
29+
(params.sourceReplyDeliveryMode === "message_tool_only" ||
30+
params.sourceReplyDeliveryMode === "automatic") &&
3031
getReplyPayloadMetadata(payload)?.sourceReplyTranscriptMirror
3132
) {
3233
return payloads;

‎src/agents/embedded-agent-subscribe.handlers.messages.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -756,14 +756,15 @@ describe("handleMessageEnd", () => {
756756
expect(metadata?.registeredTool).toBe(true);
757757
});
758758

759-
it("unwraps only source-routed or message-tool-only standalone message-tool JSON", () => {
759+
it("unwraps only source-routed or source-reply-mode standalone message-tool JSON", () => {
760760
const visibleReply = "No specific tasks planned, but I'll keep watching for updates.";
761761
const unroutedEnvelope = createMessageToolEnvelope(visibleReply);
762762
const routedEnvelope = createMessageToolEnvelope(visibleReply, { target: "user:redacted" });
763763
const toRoutedEnvelope = createMessageToolEnvelope(visibleReply, { to: "user:redacted" });
764764

765765
for (const [text, api, builtinToolNames, sourceReplyDeliveryMode, expected] of [
766766
[unroutedEnvelope, undefined, new Set(["message"]), "message_tool_only", visibleReply],
767+
[unroutedEnvelope, undefined, new Set(["message"]), "automatic", visibleReply],
767768
[routedEnvelope, "openai-completions", new Set<string>(), undefined, visibleReply],
768769
[toRoutedEnvelope, "openai-completions", new Set<string>(), undefined, visibleReply],
769770
[routedEnvelope, undefined, new Set<string>(), undefined, routedEnvelope],

‎src/agents/embedded-agent-subscribe.handlers.messages.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,8 @@ export function handleMessageEnd(
797797
extractStandaloneMessageToolText(rawVisibleText, {
798798
allowRoutedReply: isOpenAiCompletionsAssistantMessage(assistantMessage),
799799
allowCurrentSourceReply:
800-
ctx.params.sourceReplyDeliveryMode === "message_tool_only" &&
800+
(ctx.params.sourceReplyDeliveryMode === "message_tool_only" ||
801+
ctx.params.sourceReplyDeliveryMode === "automatic") &&
801802
ctx.builtinToolNames?.has("message") === true,
802803
}) ?? rawVisibleText;
803804

0 commit comments

Comments
 (0)