Merge 808add5 into c85113e

tangtaizong666 · web-flow · commit 8a88fc43cfb3 · 2026-06-21T12:51:46.000Z
diff --git a/src/auto-reply/heartbeat-reply-payload.test.ts b/src/auto-reply/heartbeat-reply-payload.test.ts
@@ -0,0 +1,73 @@
+// Heartbeat reply payload selector tests.
+import { describe, expect, it } from "vitest";
+import { resolveHeartbeatReplyPayload } from "./heartbeat-reply-payload.js";
+import type { ReplyPayload } from "./types.js";
+
+describe("resolveHeartbeatReplyPayload", () => {
+  it("returns a single non-array payload unchanged", () => {
+    const payload: ReplyPayload = { text: "HEARTBEAT_OK" };
+    expect(resolveHeartbeatReplyPayload(payload)).toBe(payload);
+  });
+
+  it("returns undefined for undefined input", () => {
+    expect(resolveHeartbeatReplyPayload(undefined)).toBeUndefined();
+  });
+
+  it("returns the last outbound payload when none are reasoning", () => {
+    const first: ReplyPayload = { text: "first" };
+    const second: ReplyPayload = { text: "second" };
+    expect(resolveHeartbeatReplyPayload([first, second])).toBe(second);
+  });
+
+  it("skips a trailing reasoning payload and returns the assistant answer", () => {
+    const answer: ReplyPayload = { text: "HEARTBEAT_OK" };
+    const reasoning: ReplyPayload = {
+      text: "The message is an OpenClaw heartbeat poll. I should check recent chat...",
+      isReasoning: true,
+    };
+    expect(resolveHeartbeatReplyPayload([answer, reasoning])).toBe(answer);
+  });
+
+  it("returns undefined when every outbound payload is reasoning", () => {
+    const reasoning: ReplyPayload = {
+      text: "Deliberating about whether to respond...",
+      isReasoning: true,
+    };
+    expect(resolveHeartbeatReplyPayload([reasoning])).toBeUndefined();
+  });
+
+  it("returns undefined for a scalar reasoning payload", () => {
+    const reasoning: ReplyPayload = {
+      text: "Considering whether the heartbeat needs a reply...",
+      isReasoning: true,
+    };
+    expect(resolveHeartbeatReplyPayload(reasoning)).toBeUndefined();
+  });
+
+  it("skips a trailing legacy 'Reasoning:'-prefixed payload and returns the final answer", () => {
+    const answer: ReplyPayload = { text: "All clear" };
+    const legacyReasoning: ReplyPayload = { text: "Reasoning: because nothing changed" };
+    expect(resolveHeartbeatReplyPayload([answer, legacyReasoning])).toBe(answer);
+  });
+
+  it("returns undefined for a scalar legacy 'Reasoning:'-prefixed payload", () => {
+    const legacyReasoning: ReplyPayload = { text: "Reasoning: because nothing changed" };
+    expect(resolveHeartbeatReplyPayload(legacyReasoning)).toBeUndefined();
+  });
+
+  it("returns undefined for a scalar blockquoted 'Thinking' reasoning payload", () => {
+    const blockquoted: ReplyPayload = { text: "Thinking... _weighing the options_" };
+    expect(resolveHeartbeatReplyPayload(blockquoted)).toBeUndefined();
+  });
+
+  it("skips a trailing lowercase 'reasoning:' payload and returns the final answer", () => {
+    const answer: ReplyPayload = { text: "All clear" };
+    const lowercased: ReplyPayload = { text: "reasoning: because nothing changed" };
+    expect(resolveHeartbeatReplyPayload([answer, lowercased])).toBe(answer);
+  });
+
+  it("returns undefined for a Markdown blockquoted thinking payload", () => {
+    const quoted: ReplyPayload = { text: "> thinking... _weighing the options_" };
+    expect(resolveHeartbeatReplyPayload(quoted)).toBeUndefined();
+  });
+});
diff --git a/src/auto-reply/heartbeat-reply-payload.ts b/src/auto-reply/heartbeat-reply-payload.ts
@@ -1,22 +1,38 @@
 // Heartbeat reply payload selector for multi-payload auto-reply results.
-import { hasOutboundReplyContent } from "openclaw/plugin-sdk/reply-payload";
+import { hasOutboundReplyContent, isReasoningReplyPayload } from "openclaw/plugin-sdk/reply-payload";
 import type { ReplyPayload } from "./types.js";
 
-/** Pick the last outbound-capable reply payload for heartbeat delivery. */
+/**
+ * Pick the last outbound-capable reply payload for heartbeat delivery.
+ *
+ * Reasoning payloads are skipped using the shared SDK classifier
+ * `isReasoningReplyPayload`, which recognizes the `isReasoning` flag plus the
+ * common reasoning/thinking text prefixes (including lowercased and Markdown
+ * blockquoted forms). Heartbeat reasoning is delivered separately and only when
+ * `includeReasoning` is enabled; without this guard a trailing reasoning
+ * payload (which reasoning models can emit after the final answer) would be
+ * selected as the user-visible heartbeat reply.
+ */
 export function resolveHeartbeatReplyPayload(
   replyResult: ReplyPayload | ReplyPayload[] | undefined,
 ): ReplyPayload | undefined {
   if (!replyResult) {
     return undefined;
   }
   if (!Array.isArray(replyResult)) {
-    return replyResult;
+    // Scalar results can be reasoning-only too; without this guard a scalar
+    // reasoning payload becomes the user-visible reply while the array path
+    // filters it, so the leak depends on the result shape.
+    return isReasoningReplyPayload(replyResult) ? undefined : replyResult;
   }
   for (let idx = replyResult.length - 1; idx >= 0; idx -= 1) {
     const payload = replyResult[idx];
     if (!payload) {
       continue;
     }
+    if (isReasoningReplyPayload(payload)) {
+      continue;
+    }
     if (hasOutboundReplyContent(payload)) {
       return payload;
     }
diff --git a/src/infra/heartbeat-runner.returns-default-unset.test.ts b/src/infra/heartbeat-runner.returns-default-unset.test.ts
@@ -1286,6 +1286,15 @@ describe("runHeartbeatOnce", () => {
         replies: [{ text: "Because it helps", isReasoning: true }, { text: "HEARTBEAT_OK" }],
         expectedTexts: ["Thinking\n\n_Because it helps_"],
       },
+      {
+        // Reasoning-only result: the selector returns no main reply, but the
+        // documented includeReasoning opt-in must still deliver the Thinking
+        // message instead of going silent (#92242 follow-up / review finding).
+        name: "raw flagged reasoning only (no main reply)",
+        caseDir: "hb-reasoning-only",
+        replies: [{ text: "Because it helps", isReasoning: true }],
+        expectedTexts: ["Thinking\n\n_Because it helps_"],
+      },
       {
         name: "visible final that starts with thinking prose",
         caseDir: "hb-thinking-visible-final",
@@ -1371,6 +1380,67 @@ describe("runHeartbeatOnce", () => {
     },
   );
 
+  it("does not surface a trailing legacy reasoning payload as the reply when includeReasoning is unset", async () => {
+    // With includeReasoning unset, a legacy "Reasoning:"-prefixed payload after
+    // the final answer must not become the visible heartbeat reply, and no
+    // separate Thinking message is sent. (#92242 review follow-up)
+    const replySpy = vi.fn();
+    try {
+      const tmpDir = await createCaseDir("hb-legacy-reasoning-unset");
+      const storePath = path.join(tmpDir, "sessions.json");
+      const cfg: OpenClawConfig = {
+        agents: {
+          defaults: {
+            workspace: tmpDir,
+            heartbeat: { every: "5m", target: "whatsapp" },
+          },
+        },
+        channels: { whatsapp: { allowFrom: ["*"] } },
+        session: { store: storePath },
+      };
+      const sessionKey = resolveMainSessionKey(cfg);
+      await fs.writeFile(
+        storePath,
+        JSON.stringify({
+          [sessionKey]: {
+            sessionId: "sid",
+            updatedAt: Date.now(),
+            lastChannel: "whatsapp",
+            lastProvider: "whatsapp",
+            lastTo: "120363401234567890@g.us",
+          },
+        }),
+      );
+
+      replySpy.mockResolvedValue([
+        { text: "All clear" },
+        { text: "Reasoning: because nothing changed" },
+      ]);
+      const sendWhatsApp = vi
+        .fn<
+          (
+            to: string,
+            text: string,
+            opts?: unknown,
+          ) => Promise<{ messageId: string; toJid: string }>
+        >()
+        .mockResolvedValue({ messageId: "m1", toJid: "jid" });
+
+      await runHeartbeatOnce({
+        cfg,
+        deps: createHeartbeatDeps(sendWhatsApp, { getReplyFromConfig: replySpy }),
+      });
+
+      expect(sendWhatsApp).toHaveBeenCalledTimes(1);
+      expectWhatsAppSendCall(sendWhatsApp, 0, {
+        to: "120363401234567890@g.us",
+        text: "All clear",
+      });
+    } finally {
+      replySpy.mockReset();
+    }
+  });
+
   it("loads the default agent session from templated stores", async () => {
     const tmpDir = await createCaseDir("openclaw-hb");
     const storeTemplate = path.join(tmpDir, "agents", "{agentId}", "sessions.json");
diff --git a/src/infra/heartbeat-runner.ts b/src/infra/heartbeat-runner.ts
@@ -9,6 +9,7 @@ import {
 } from "@openclaw/normalization-core/string-coerce";
 import {
   hasOutboundReplyContent,
+  isReasoningReplyPayload,
   resolveSendableOutboundReplyParts,
 } from "openclaw/plugin-sdk/reply-payload";
 import { normalizeOptionalAgentRuntimeId } from "../agents/agent-runtime-id.js";
@@ -753,21 +754,29 @@ function resolveStaleHeartbeatIsolatedSessionKey(params: {
   return undefined;
 }
 
+// Display-format check (not a classifier): whether reasoning text is already
+// rendered in the heartbeat "Reasoning:" / "Thinking..._" form, so it should be
+// delivered as-is rather than re-wrapped by formatReasoningMessage. Reasoning
+// classification itself is the shared SDK `isReasoningReplyPayload`.
+const HEARTBEAT_REASONING_DISPLAY_PREFIX = /^(?:Reasoning:|Thinking\.{0,3}(?=\s*_))/u;
+
 function resolveHeartbeatReasoningPayloads(
   replyResult: ReplyPayload | ReplyPayload[] | undefined,
 ): ReplyPayload[] {
   const payloads = Array.isArray(replyResult) ? replyResult : replyResult ? [replyResult] : [];
   const reasoningPayloads: ReplyPayload[] = [];
   for (const payload of payloads) {
     const text = typeof payload.text === "string" ? payload.text : "";
-    const hasFormattedReasoningPrefix = /^(?:Reasoning:|Thinking\.{0,3}(?=\s*_))/u.test(
-      text.trimStart(),
-    );
-    if (payload.isReasoning !== true && !hasFormattedReasoningPrefix) {
+    // Shared classifier keeps this lane in lockstep with the heartbeat reply
+    // selector so a legacy-formatted reasoning payload is never both skipped
+    // here and surfaced as the visible reply (or vice versa). See #92242.
+    if (!isReasoningReplyPayload(payload)) {
       continue;
     }
 
-    const formattedText = hasFormattedReasoningPrefix ? text : formatReasoningMessage(text);
+    const formattedText = HEARTBEAT_REASONING_DISPLAY_PREFIX.test(text.trimStart())
+      ? text
+      : formatReasoningMessage(text);
     if (!formattedText.trim()) {
       continue;
     }
@@ -1878,7 +1887,15 @@ export async function runHeartbeatOnce(opts: {
       return { status: "ran", durationMs: Date.now() - startedAt };
     }
 
-    if (!heartbeatToolResponse && (!replyPayload || !hasOutboundReplyContent(replyPayload))) {
+    if (
+      !heartbeatToolResponse &&
+      (!replyPayload || !hasOutboundReplyContent(replyPayload)) &&
+      reasoningPayloads.length === 0
+    ) {
+      // No main reply to send. Only treat this as an empty heartbeat when there
+      // is also no opt-in reasoning to deliver; otherwise fall through so the
+      // includeReasoning Thinking payload is still sent (mirrors the
+      // shouldSkipMain guard below). See #92242 follow-up.
       await restoreHeartbeatUpdatedAt({
         storePath,
         sessionKey,
