From 4c2812b42976100a18da4202b09890fcda258c78 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 2 Jan 2026 01:42:27 +0100 Subject: [PATCH] fix: refine HEARTBEAT_OK handling --- CHANGELOG.md | 2 +- docs/heartbeat.md | 12 ++- src/agents/system-prompt.ts | 2 +- src/auto-reply/heartbeat.test.ts | 96 ++++++++++++++++-------- src/auto-reply/heartbeat.ts | 71 ++++++++++++++++-- src/auto-reply/reply.triggers.test.ts | 25 +++++++ src/auto-reply/reply.ts | 104 ++++++++++++++------------ src/infra/heartbeat-runner.ts | 5 +- src/web/auto-reply.ts | 35 ++++++++- 9 files changed, 261 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c2e8c912b0..2c8948a28a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,7 @@ - Chat UI: keep the chat scrolled to the latest message after switching sessions. - WebChat: stream live updates for sessions even when runs start outside the chat UI. - Gateway CLI: read `CLAWDIS_GATEWAY_PASSWORD` from environment in `callGateway()` — allows `doctor`/`health` commands to auth without explicit `--password` flag. -- Auto-reply: suppress stray `HEARTBEAT_OK` acks so they never get delivered as messages. +- Auto-reply: strip stray leading/trailing `HEARTBEAT_OK` from normal replies; drop short (≤ 30 chars) heartbeat acks. - Logging: trim provider prefix duplication in Discord/Signal/Telegram runtime log lines. - Discord: include recent guild context when replying to mentions and add `discord.historyLimit` to tune how many messages are captured. - Skills: switch imsg installer to brew tap formula. diff --git a/docs/heartbeat.md b/docs/heartbeat.md index 33b76208447..9edd2f0587b 100644 --- a/docs/heartbeat.md +++ b/docs/heartbeat.md @@ -10,10 +10,18 @@ surface anything that needs attention without spamming the user. ## Prompt contract - Heartbeat body defaults to `HEARTBEAT` (configurable via `agent.heartbeat.prompt`). -- If nothing needs attention, the model must reply **exactly** `HEARTBEAT_OK`. -- Any response containing `HEARTBEAT_OK` is treated as an ack and discarded. +- If nothing needs attention, the model should reply **exactly** `HEARTBEAT_OK`. +- During heartbeat runs, Clawdis treats `HEARTBEAT_OK` as an ack when it appears at + the **start or end** of the reply. Clawdis strips the token and discards the + reply if the remaining content is **≤ 30 characters**. +- If `HEARTBEAT_OK` is in the **middle** of a reply, it is not treated specially. - For alerts, do **not** include `HEARTBEAT_OK`; return only the alert text. +### Stray `HEARTBEAT_OK` outside heartbeats +If the model accidentally includes `HEARTBEAT_OK` at the start or end of a +normal (non-heartbeat) reply, Clawdis strips the token and logs a verbose +message. If the reply is only `HEARTBEAT_OK`, it is dropped. + ## Config ```json5 diff --git a/src/agents/system-prompt.ts b/src/agents/system-prompt.ts index 00bfb165c09..3eed242a542 100644 --- a/src/agents/system-prompt.ts +++ b/src/agents/system-prompt.ts @@ -95,7 +95,7 @@ export function buildAgentSystemPromptAppend(params: { "## Heartbeats", 'If you receive a heartbeat poll (a user message containing just "HEARTBEAT"), and there is nothing that needs attention, reply exactly:', "HEARTBEAT_OK", - 'Any response containing "HEARTBEAT_OK" is treated as a heartbeat ack and will not be delivered.', + 'Clawdis treats a leading/trailing "HEARTBEAT_OK" as a heartbeat ack (and may discard it).', 'If something needs attention, do NOT include "HEARTBEAT_OK"; reply with the alert text instead.', "", "## Runtime", diff --git a/src/auto-reply/heartbeat.test.ts b/src/auto-reply/heartbeat.test.ts index 500c17e3961..9cf47e276ed 100644 --- a/src/auto-reply/heartbeat.test.ts +++ b/src/auto-reply/heartbeat.test.ts @@ -5,55 +5,89 @@ import { HEARTBEAT_TOKEN } from "./tokens.js"; describe("stripHeartbeatToken", () => { it("skips empty or token-only replies", () => { - expect(stripHeartbeatToken(undefined)).toEqual({ + expect(stripHeartbeatToken(undefined, { mode: "heartbeat" })).toEqual({ shouldSkip: true, text: "", + didStrip: false, }); - expect(stripHeartbeatToken(" ")).toEqual({ + expect(stripHeartbeatToken(" ", { mode: "heartbeat" })).toEqual({ shouldSkip: true, text: "", + didStrip: false, }); - expect(stripHeartbeatToken(HEARTBEAT_TOKEN)).toEqual({ + expect(stripHeartbeatToken(HEARTBEAT_TOKEN, { mode: "heartbeat" })).toEqual( + { + shouldSkip: true, + text: "", + didStrip: true, + }, + ); + }); + + it("drops heartbeats with small junk in heartbeat mode", () => { + expect( + stripHeartbeatToken("HEARTBEAT_OK 🦞", { mode: "heartbeat" }), + ).toEqual({ shouldSkip: true, text: "", + didStrip: true, + }); + expect( + stripHeartbeatToken(`🦞 ${HEARTBEAT_TOKEN}`, { mode: "heartbeat" }), + ).toEqual({ + shouldSkip: true, + text: "", + didStrip: true, }); }); - it("skips any reply that includes the heartbeat token", () => { - expect(stripHeartbeatToken(`ALERT ${HEARTBEAT_TOKEN}`)).toEqual({ - shouldSkip: true, - text: "", - }); - expect(stripHeartbeatToken("HEARTBEAT_OK 🦞")).toEqual({ - shouldSkip: true, - text: "", - }); - expect(stripHeartbeatToken("HEARTBEAT_OK_OK_OK")).toEqual({ - shouldSkip: true, - text: "", - }); - expect(stripHeartbeatToken("HEARTBEAT_OK_OK")).toEqual({ - shouldSkip: true, - text: "", - }); - expect(stripHeartbeatToken("HEARTBEAT_OK _OK")).toEqual({ - shouldSkip: true, - text: "", - }); - expect(stripHeartbeatToken("HEARTBEAT_OK OK")).toEqual({ - shouldSkip: true, - text: "", - }); - expect(stripHeartbeatToken("ALERT HEARTBEAT_OK_OK")).toEqual({ + it("drops short remainder in heartbeat mode", () => { + expect( + stripHeartbeatToken(`ALERT ${HEARTBEAT_TOKEN}`, { mode: "heartbeat" }), + ).toEqual({ shouldSkip: true, text: "", + didStrip: true, }); }); - it("keeps non-heartbeat content", () => { - expect(stripHeartbeatToken("hello")).toEqual({ + it("keeps heartbeat replies when remaining content exceeds threshold", () => { + const long = "A".repeat(31); + expect( + stripHeartbeatToken(`${long} ${HEARTBEAT_TOKEN}`, { mode: "heartbeat" }), + ).toEqual({ + shouldSkip: false, + text: long, + didStrip: true, + }); + }); + + it("strips token at edges for normal messages", () => { + expect( + stripHeartbeatToken(`${HEARTBEAT_TOKEN} hello`, { mode: "message" }), + ).toEqual({ shouldSkip: false, text: "hello", + didStrip: true, + }); + expect( + stripHeartbeatToken(`hello ${HEARTBEAT_TOKEN}`, { mode: "message" }), + ).toEqual({ + shouldSkip: false, + text: "hello", + didStrip: true, + }); + }); + + it("does not touch token in the middle", () => { + expect( + stripHeartbeatToken(`hello ${HEARTBEAT_TOKEN} there`, { + mode: "message", + }), + ).toEqual({ + shouldSkip: false, + text: `hello ${HEARTBEAT_TOKEN} there`, + didStrip: false, }); }); }); diff --git a/src/auto-reply/heartbeat.ts b/src/auto-reply/heartbeat.ts index 8c64f25072c..ba0fa9868ec 100644 --- a/src/auto-reply/heartbeat.ts +++ b/src/auto-reply/heartbeat.ts @@ -2,12 +2,69 @@ import { HEARTBEAT_TOKEN } from "./tokens.js"; export const HEARTBEAT_PROMPT = "HEARTBEAT"; -export function stripHeartbeatToken(raw?: string) { - if (!raw) return { shouldSkip: true, text: "" }; - const trimmed = raw.trim(); - if (!trimmed) return { shouldSkip: true, text: "" }; - if (trimmed.includes(HEARTBEAT_TOKEN)) { - return { shouldSkip: true, text: "" }; +export type StripHeartbeatMode = "heartbeat" | "message"; + +function stripTokenAtEdges(raw: string): { text: string; didStrip: boolean } { + let text = raw.trim(); + if (!text) return { text: "", didStrip: false }; + + const token = HEARTBEAT_TOKEN; + if (!text.includes(token)) return { text, didStrip: false }; + + let didStrip = false; + let changed = true; + while (changed) { + changed = false; + const next = text.trim(); + if (next.startsWith(token)) { + const after = next.slice(token.length).trimStart(); + text = after; + didStrip = true; + changed = true; + continue; + } + if (next.endsWith(token)) { + const before = next.slice(0, Math.max(0, next.length - token.length)); + text = before.trimEnd(); + didStrip = true; + changed = true; + } } - return { shouldSkip: false, text: trimmed }; + + const collapsed = text.replace(/\s+/g, " ").trim(); + return { text: collapsed, didStrip }; +} + +export function stripHeartbeatToken( + raw?: string, + opts: { mode?: StripHeartbeatMode; maxAckChars?: number } = {}, +) { + if (!raw) return { shouldSkip: true, text: "", didStrip: false }; + const trimmed = raw.trim(); + if (!trimmed) return { shouldSkip: true, text: "", didStrip: false }; + + const mode: StripHeartbeatMode = opts.mode ?? "message"; + const maxAckChars = Math.max(0, opts.maxAckChars ?? 30); + + if (!trimmed.includes(HEARTBEAT_TOKEN)) { + return { shouldSkip: false, text: trimmed, didStrip: false }; + } + + const stripped = stripTokenAtEdges(trimmed); + if (!stripped.didStrip) { + return { shouldSkip: false, text: trimmed, didStrip: false }; + } + + if (!stripped.text) { + return { shouldSkip: true, text: "", didStrip: true }; + } + + if (mode === "heartbeat") { + const rest = stripped.text.trim(); + if (rest.length <= maxAckChars) { + return { shouldSkip: true, text: "", didStrip: true }; + } + } + + return { shouldSkip: false, text: stripped.text, didStrip: true }; } diff --git a/src/auto-reply/reply.triggers.test.ts b/src/auto-reply/reply.triggers.test.ts index bd8358b43cb..62c77995b1b 100644 --- a/src/auto-reply/reply.triggers.test.ts +++ b/src/auto-reply/reply.triggers.test.ts @@ -186,6 +186,31 @@ describe("trigger handling", () => { }); }); + it("strips HEARTBEAT_OK at edges outside heartbeat runs", async () => { + await withTempHome(async (home) => { + vi.mocked(runEmbeddedPiAgent).mockResolvedValue({ + payloads: [{ text: `${HEARTBEAT_TOKEN} hello` }], + meta: { + durationMs: 1, + agentMeta: { sessionId: "s", provider: "p", model: "m" }, + }, + }); + + const res = await getReplyFromConfig( + { + Body: "hello", + From: "+1002", + To: "+2000", + }, + {}, + makeCfg(home), + ); + + const text = Array.isArray(res) ? res[0]?.text : res?.text; + expect(text).toBe("hello"); + }); + }); + it("updates group activation when the owner sends /activation", async () => { await withTempHome(async (home) => { const cfg = makeCfg(home); diff --git a/src/auto-reply/reply.ts b/src/auto-reply/reply.ts index 737ff6e6192..916c16d4a37 100644 --- a/src/auto-reply/reply.ts +++ b/src/auto-reply/reply.ts @@ -53,6 +53,7 @@ import { normalizeGroupActivation, parseActivationCommand, } from "./group-activation.js"; +import { stripHeartbeatToken } from "./heartbeat.js"; import { extractModelDirective } from "./model.js"; import { buildStatusMessage } from "./status.js"; import type { MsgContext, TemplateContext } from "./templating.js"; @@ -62,7 +63,7 @@ import { type ThinkLevel, type VerboseLevel, } from "./thinking.js"; -import { HEARTBEAT_TOKEN, SILENT_REPLY_TOKEN } from "./tokens.js"; +import { SILENT_REPLY_TOKEN } from "./tokens.js"; import { isAudio, transcribeInboundAudio } from "./transcription.js"; import type { GetReplyOptions, ReplyPayload } from "./types.js"; @@ -1191,7 +1192,7 @@ export async function getReplyFromConfig( return undefined; } - let suppressedByHeartbeatAck = false; + let didLogHeartbeatStrip = false; try { if (shouldEagerType) { await startTypingLoop(); @@ -1221,20 +1222,24 @@ export async function getReplyFromConfig( runId, onPartialReply: opts?.onPartialReply ? async (payload) => { - if ( - !opts?.isHeartbeat && - payload.text?.includes(HEARTBEAT_TOKEN) - ) { - suppressedByHeartbeatAck = true; - logVerbose( - "Suppressing partial reply: detected HEARTBEAT_OK token", - ); - return; + let text = payload.text; + if (!opts?.isHeartbeat && text?.includes("HEARTBEAT_OK")) { + const stripped = stripHeartbeatToken(text, { mode: "message" }); + if (stripped.didStrip && !didLogHeartbeatStrip) { + didLogHeartbeatStrip = true; + logVerbose("Stripped stray HEARTBEAT_OK token from reply"); + } + if ( + stripped.shouldSkip && + (payload.mediaUrls?.length ?? 0) === 0 + ) { + return; + } + text = stripped.text; } - if (suppressedByHeartbeatAck) return; - await startTypingOnText(payload.text); + await startTypingOnText(text); await opts.onPartialReply?.({ - text: payload.text, + text, mediaUrls: payload.mediaUrls, }); } @@ -1242,22 +1247,23 @@ export async function getReplyFromConfig( shouldEmitToolResult, onToolResult: opts?.onToolResult ? async (payload) => { - if ( - !opts?.isHeartbeat && - payload.text?.includes(HEARTBEAT_TOKEN) - ) { - suppressedByHeartbeatAck = true; - logVerbose( - "Suppressing tool result: detected HEARTBEAT_OK token", - ); - return; + let text = payload.text; + if (!opts?.isHeartbeat && text?.includes("HEARTBEAT_OK")) { + const stripped = stripHeartbeatToken(text, { mode: "message" }); + if (stripped.didStrip && !didLogHeartbeatStrip) { + didLogHeartbeatStrip = true; + logVerbose("Stripped stray HEARTBEAT_OK token from reply"); + } + if ( + stripped.shouldSkip && + (payload.mediaUrls?.length ?? 0) === 0 + ) { + return; + } + text = stripped.text; } - if (suppressedByHeartbeatAck) return; - await startTypingOnText(payload.text); - await opts.onToolResult?.({ - text: payload.text, - mediaUrls: payload.mediaUrls, - }); + await startTypingOnText(text); + await opts.onToolResult?.({ text, mediaUrls: payload.mediaUrls }); } : undefined, }); @@ -1288,22 +1294,28 @@ export async function getReplyFromConfig( const payloadArray = runResult.payloads ?? []; if (payloadArray.length === 0) return undefined; - if ( - suppressedByHeartbeatAck || - (!opts?.isHeartbeat && - payloadArray.some((payload) => payload.text?.includes(HEARTBEAT_TOKEN))) - ) { - logVerbose("Suppressing reply: detected HEARTBEAT_OK token"); - return undefined; - } - const shouldSignalTyping = payloadArray.some((payload) => { + + const sanitizedPayloads = opts?.isHeartbeat + ? payloadArray + : payloadArray.flatMap((payload) => { + const text = payload.text; + if (!text || !text.includes("HEARTBEAT_OK")) return [payload]; + const stripped = stripHeartbeatToken(text, { mode: "message" }); + if (stripped.didStrip && !didLogHeartbeatStrip) { + didLogHeartbeatStrip = true; + logVerbose("Stripped stray HEARTBEAT_OK token from reply"); + } + const hasMedia = + Boolean(payload.mediaUrl) || (payload.mediaUrls?.length ?? 0) > 0; + if (stripped.shouldSkip && !hasMedia) return []; + return [{ ...payload, text: stripped.text }]; + }); + + if (sanitizedPayloads.length === 0) return undefined; + + const shouldSignalTyping = sanitizedPayloads.some((payload) => { const trimmed = payload.text?.trim(); - if ( - trimmed && - trimmed !== SILENT_REPLY_TOKEN && - !trimmed.includes(HEARTBEAT_TOKEN) - ) - return true; + if (trimmed && trimmed !== SILENT_REPLY_TOKEN) return true; if (payload.mediaUrl) return true; if (payload.mediaUrls && payload.mediaUrls.length > 0) return true; return false; @@ -1356,11 +1368,11 @@ export async function getReplyFromConfig( } // If verbose is enabled and this is a new session, prepend a session hint. - let finalPayloads = payloadArray; + let finalPayloads = sanitizedPayloads; if (resolvedVerboseLevel === "on" && isNewSession) { finalPayloads = [ { text: `🧭 New session: ${sessionIdFinal}` }, - ...payloadArray, + ...finalPayloads, ]; } diff --git a/src/infra/heartbeat-runner.ts b/src/infra/heartbeat-runner.ts index f3c69913b4a..b94d77f7b75 100644 --- a/src/infra/heartbeat-runner.ts +++ b/src/infra/heartbeat-runner.ts @@ -266,7 +266,10 @@ function normalizeHeartbeatReply( payload: ReplyPayload, responsePrefix?: string, ) { - const stripped = stripHeartbeatToken(payload.text); + const stripped = stripHeartbeatToken(payload.text, { + mode: "heartbeat", + maxAckChars: 30, + }); const hasMedia = Boolean( payload.mediaUrl || (payload.mediaUrls?.length ?? 0) > 0, ); diff --git a/src/web/auto-reply.ts b/src/web/auto-reply.ts index c844625afd0..74ff33072a6 100644 --- a/src/web/auto-reply.ts +++ b/src/web/auto-reply.ts @@ -185,7 +185,6 @@ export { stripHeartbeatToken }; function isSilentReply(payload?: ReplyPayload): boolean { if (!payload) return false; const text = payload.text?.trim(); - if (text?.includes(HEARTBEAT_TOKEN)) return true; if (!text || text !== SILENT_REPLY_TOKEN) return false; if (payload.mediaUrl || payload.mediaUrls?.length) return false; return true; @@ -337,7 +336,10 @@ export async function runWebHeartbeatOnce(opts: { const hasMedia = Boolean( replyPayload.mediaUrl || (replyPayload.mediaUrls?.length ?? 0) > 0, ); - const stripped = stripHeartbeatToken(replyPayload.text); + const stripped = stripHeartbeatToken(replyPayload.text, { + mode: "heartbeat", + maxAckChars: 30, + }); if (stripped.shouldSkip && !hasMedia) { // Don't let heartbeats keep sessions alive: restore previous updatedAt so idle expiry still works. const storePath = resolveStorePath(cfg.session?.store); @@ -1034,6 +1036,7 @@ export async function monitorWebProvider( } const responsePrefix = cfg.messages?.responsePrefix; + let didLogHeartbeatStrip = false; let didSendReply = false; let toolSendChain: Promise = Promise.resolve(); const sendToolResult = (payload: ReplyPayload) => { @@ -1046,6 +1049,20 @@ export async function monitorWebProvider( } if (isSilentReply(payload)) return; const toolPayload: ReplyPayload = { ...payload }; + if (toolPayload.text?.includes(HEARTBEAT_TOKEN)) { + const stripped = stripHeartbeatToken(toolPayload.text, { + mode: "message", + }); + if (stripped.didStrip && !didLogHeartbeatStrip) { + didLogHeartbeatStrip = true; + logVerbose("Stripped stray HEARTBEAT_OK token from web reply"); + } + const hasMedia = Boolean( + toolPayload.mediaUrl || (toolPayload.mediaUrls?.length ?? 0) > 0, + ); + if (stripped.shouldSkip && !hasMedia) return; + toolPayload.text = stripped.text; + } if ( responsePrefix && toolPayload.text && @@ -1134,6 +1151,20 @@ export async function monitorWebProvider( await toolSendChain; for (const replyPayload of sendableReplies) { + if (replyPayload.text?.includes(HEARTBEAT_TOKEN)) { + const stripped = stripHeartbeatToken(replyPayload.text, { + mode: "message", + }); + if (stripped.didStrip && !didLogHeartbeatStrip) { + didLogHeartbeatStrip = true; + logVerbose("Stripped stray HEARTBEAT_OK token from web reply"); + } + const hasMedia = Boolean( + replyPayload.mediaUrl || (replyPayload.mediaUrls?.length ?? 0) > 0, + ); + if (stripped.shouldSkip && !hasMedia) continue; + replyPayload.text = stripped.text; + } if ( responsePrefix && replyPayload.text &&