From d75b594e07136298fb4448154cfdd12c7cd3ad85 Mon Sep 17 00:00:00 2001 From: Vignesh Natarajan Date: Sun, 22 Feb 2026 13:30:16 -0800 Subject: [PATCH] Agents/Replies: scope done fallback to direct sessions --- CHANGELOG.md | 2 +- src/agents/pi-embedded-runner/run.ts | 1 + .../run/payloads.errors.test.ts | 44 +++++++++++++++++++ src/agents/pi-embedded-runner/run/payloads.ts | 9 +++- src/routing/session-key.test.ts | 29 +++++++++++- src/sessions/send-policy.ts | 14 ++---- src/sessions/session-key-utils.ts | 29 ++++++++++++ 7 files changed, 114 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc533a900..2206bf353 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,7 +158,7 @@ Docs: https://docs.openclaw.ai - Agents/Transcripts: validate assistant tool-call names (syntax/length + registered tool allowlist) before transcript persistence and during replay sanitization so malformed failover tool names no longer poison sessions with repeated provider HTTP 400 errors. (#23324) Thanks @johnsantry. - Agents/Mistral: sanitize tool-call IDs in the embedded agent loop and generate strict provider-safe pending tool-call IDs, preventing Mistral strict9 `HTTP 400` failures on tool continuations. (#23698) Thanks @echoVic. - Agents/Compaction: strip stale assistant usage snapshots from pre-compaction turns when replaying history after a compaction summary so context-token estimation no longer reuses pre-compaction totals and immediately re-triggers destructive follow-up compactions. (#19127) Thanks @tedwatson. -- Agents/Replies: emit a default completion acknowledgement (`✅ Done.`) when runs execute tools successfully but return no final assistant text, preventing silent no-reply turns after tool-only completions. (#22834) Thanks @Oldshue. +- Agents/Replies: emit a default completion acknowledgement (`✅ Done.`) only for direct/private tool-only completions with no final assistant text, while suppressing synthetic acknowledgements for channel/group sessions and runs that already delivered output via messaging tools. (#22834) Thanks @Oldshue. - Agents/Subagents: honor `tools.subagents.tools.alsoAllow` and explicit subagent `allow` entries when resolving built-in subagent deny defaults, so explicitly granted tools (for example `sessions_send`) are no longer blocked unless re-denied in `tools.subagents.tools.deny`. (#23359) Thanks @goren-beehero. - Agents/Subagents: make announce call timeouts configurable via `agents.defaults.subagents.announceTimeoutMs` and restore a 60s default to prevent false timeout failures on slower announce paths. (#22719) Thanks @Valadon. - Agents/Diagnostics: include resolved lifecycle error text in `embedded run agent end` warnings so UI/TUI “Connection error” runs expose actionable provider failure reasons in gateway logs. (#23054) Thanks @Raize. diff --git a/src/agents/pi-embedded-runner/run.ts b/src/agents/pi-embedded-runner/run.ts index 1347354e3..d326ec556 100644 --- a/src/agents/pi-embedded-runner/run.ts +++ b/src/agents/pi-embedded-runner/run.ts @@ -1066,6 +1066,7 @@ export async function runEmbeddedPiAgent( toolResultFormat: resolvedToolResultFormat, suppressToolErrorWarnings: params.suppressToolErrorWarnings, inlineToolResultsAllowed: false, + didSendViaMessagingTool: attempt.didSendViaMessagingTool, }); // Timeout aborts can leave the run without any assistant payloads. diff --git a/src/agents/pi-embedded-runner/run/payloads.errors.test.ts b/src/agents/pi-embedded-runner/run/payloads.errors.test.ts index 97e3d188b..b382da02f 100644 --- a/src/agents/pi-embedded-runner/run/payloads.errors.test.ts +++ b/src/agents/pi-embedded-runner/run/payloads.errors.test.ts @@ -131,6 +131,7 @@ describe("buildEmbeddedRunPayloads", () => { it("adds completion fallback when tools run successfully without final assistant text", () => { const payloads = buildPayloads({ + sessionKey: "agent:main:discord:direct:u123", toolMetas: [{ toolName: "write", meta: "/tmp/out.md" }], lastAssistant: makeStoppedAssistant(), }); @@ -139,6 +140,49 @@ describe("buildEmbeddedRunPayloads", () => { expect(payloads[0]?.isError).toBeUndefined(); }); + it("does not add completion fallback for channel sessions", () => { + const payloads = buildPayloads({ + sessionKey: "agent:main:discord:channel:c123", + toolMetas: [{ toolName: "write", meta: "/tmp/out.md" }], + lastAssistant: makeAssistant({ + stopReason: "stop", + errorMessage: undefined, + content: [], + }), + }); + + expect(payloads).toHaveLength(0); + }); + + it("does not add completion fallback for group sessions", () => { + const payloads = buildPayloads({ + sessionKey: "agent:main:telegram:group:g123", + toolMetas: [{ toolName: "write", meta: "/tmp/out.md" }], + lastAssistant: makeAssistant({ + stopReason: "stop", + errorMessage: undefined, + content: [], + }), + }); + + expect(payloads).toHaveLength(0); + }); + + it("does not add completion fallback when messaging tool already delivered output", () => { + const payloads = buildPayloads({ + sessionKey: "agent:main:discord:direct:u123", + toolMetas: [{ toolName: "message_send", meta: "sent to #ops" }], + didSendViaMessagingTool: true, + lastAssistant: makeAssistant({ + stopReason: "stop", + errorMessage: undefined, + content: [], + }), + }); + + expect(payloads).toHaveLength(0); + }); + it("does not add completion fallback when the run still has a tool error", () => { const payloads = buildPayloads({ toolMetas: [{ toolName: "browser", meta: "open https://example.com" }], diff --git a/src/agents/pi-embedded-runner/run/payloads.ts b/src/agents/pi-embedded-runner/run/payloads.ts index 78e70d061..d055a3e20 100644 --- a/src/agents/pi-embedded-runner/run/payloads.ts +++ b/src/agents/pi-embedded-runner/run/payloads.ts @@ -4,6 +4,7 @@ import type { ReasoningLevel, VerboseLevel } from "../../../auto-reply/thinking. import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../../../auto-reply/tokens.js"; import { formatToolAggregate } from "../../../auto-reply/tool-meta.js"; import type { OpenClawConfig } from "../../../config/config.js"; +import { deriveSessionChatType } from "../../../sessions/session-key-utils.js"; import { BILLING_ERROR_USER_MESSAGE, formatAssistantErrorText, @@ -95,6 +96,7 @@ export function buildEmbeddedRunPayloads(params: { toolResultFormat?: ToolResultFormat; suppressToolErrorWarnings?: boolean; inlineToolResultsAllowed: boolean; + didSendViaMessagingTool?: boolean; }): Array<{ text?: string; mediaUrl?: string; @@ -332,8 +334,13 @@ export function buildEmbeddedRunPayloads(params: { payloads.length === 0 && params.toolMetas.length > 0 && !params.lastToolError && - !lastAssistantErrored + !lastAssistantErrored && + !params.didSendViaMessagingTool ) { + const sessionChatType = deriveSessionChatType(params.sessionKey); + if (sessionChatType === "channel" || sessionChatType === "group") { + return []; + } return [{ text: "✅ Done." }]; } return payloads; diff --git a/src/routing/session-key.test.ts b/src/routing/session-key.test.ts index 5caed36e7..41e659a9f 100644 --- a/src/routing/session-key.test.ts +++ b/src/routing/session-key.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { getSubagentDepth, isCronSessionKey } from "../sessions/session-key-utils.js"; +import { + deriveSessionChatType, + getSubagentDepth, + isCronSessionKey, +} from "../sessions/session-key-utils.js"; import { classifySessionKeyShape } from "./session-key.js"; describe("classifySessionKeyShape", () => { @@ -66,3 +70,26 @@ describe("isCronSessionKey", () => { expect(isCronSessionKey(undefined)).toBe(false); }); }); + +describe("deriveSessionChatType", () => { + it("detects canonical direct/group/channel session keys", () => { + expect(deriveSessionChatType("agent:main:discord:direct:user1")).toBe("direct"); + expect(deriveSessionChatType("agent:main:telegram:group:g1")).toBe("group"); + expect(deriveSessionChatType("agent:main:discord:channel:c1")).toBe("channel"); + }); + + it("detects legacy direct markers", () => { + expect(deriveSessionChatType("agent:main:telegram:dm:123456")).toBe("direct"); + expect(deriveSessionChatType("telegram:dm:123456")).toBe("direct"); + }); + + it("detects legacy discord guild channel keys", () => { + expect(deriveSessionChatType("discord:acc-1:guild-123:channel-456")).toBe("channel"); + }); + + it("returns unknown for main or malformed session keys", () => { + expect(deriveSessionChatType("agent:main:main")).toBe("unknown"); + expect(deriveSessionChatType("agent:main")).toBe("unknown"); + expect(deriveSessionChatType("")).toBe("unknown"); + }); +}); diff --git a/src/sessions/send-policy.ts b/src/sessions/send-policy.ts index b67a02f82..e41a93f7c 100644 --- a/src/sessions/send-policy.ts +++ b/src/sessions/send-policy.ts @@ -1,6 +1,7 @@ import { normalizeChatType } from "../channels/chat-type.js"; import type { OpenClawConfig } from "../config/config.js"; import type { SessionChatType, SessionEntry } from "../config/sessions.js"; +import { deriveSessionChatType } from "./session-key-utils.js"; export type SessionSendPolicyDecision = "allow" | "deny"; @@ -45,17 +46,8 @@ function deriveChannelFromKey(key?: string) { } function deriveChatTypeFromKey(key?: string): SessionChatType | undefined { - const normalizedKey = stripAgentSessionKeyPrefix(key); - if (!normalizedKey) { - return undefined; - } - if (normalizedKey.includes(":group:")) { - return "group"; - } - if (normalizedKey.includes(":channel:")) { - return "channel"; - } - return undefined; + const chatType = deriveSessionChatType(key); + return chatType === "unknown" ? undefined : chatType; } export function resolveSendPolicy(params: { diff --git a/src/sessions/session-key-utils.ts b/src/sessions/session-key-utils.ts index 61bd40199..d6061a886 100644 --- a/src/sessions/session-key-utils.ts +++ b/src/sessions/session-key-utils.ts @@ -3,6 +3,8 @@ export type ParsedAgentSessionKey = { rest: string; }; +export type SessionKeyChatType = "direct" | "group" | "channel" | "unknown"; + export function parseAgentSessionKey( sessionKey: string | undefined | null, ): ParsedAgentSessionKey | null { @@ -25,6 +27,33 @@ export function parseAgentSessionKey( return { agentId, rest }; } +/** + * Best-effort chat-type extraction from session keys across canonical and legacy formats. + */ +export function deriveSessionChatType(sessionKey: string | undefined | null): SessionKeyChatType { + const raw = (sessionKey ?? "").trim().toLowerCase(); + if (!raw) { + return "unknown"; + } + const scoped = parseAgentSessionKey(raw)?.rest ?? raw; + const tokens = new Set(scoped.split(":").filter(Boolean)); + if (tokens.has("group")) { + return "group"; + } + if (tokens.has("channel")) { + return "channel"; + } + if (tokens.has("direct") || tokens.has("dm")) { + return "direct"; + } + // Legacy Discord keys can be shaped like: + // discord::guild-:channel- + if (/^discord:(?:[^:]+:)?guild-[^:]+:channel-[^:]+$/.test(scoped)) { + return "channel"; + } + return "unknown"; +} + export function isCronRunSessionKey(sessionKey: string | undefined | null): boolean { const parsed = parseAgentSessionKey(sessionKey); if (!parsed) {