From 32ecd6f5796c3a136529a2df0d65d1c84c85d2c0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 23:47:14 +0000 Subject: [PATCH] refactor(auto-reply,telegram,config): extract guard and forum helpers --- .../reply/agent-runner-reminder-guard.ts | 64 +++++++++++++++++ src/auto-reply/reply/agent-runner.ts | 71 ++----------------- .../config.backup-rotation.test-helpers.ts | 19 +++++ src/config/config.backup-rotation.test.ts | 38 ++++------ ...t-message-context.implicit-mention.test.ts | 10 +-- src/telegram/bot-message-context.ts | 29 +------- src/telegram/forum-service-message.ts | 23 ++++++ 7 files changed, 125 insertions(+), 129 deletions(-) create mode 100644 src/auto-reply/reply/agent-runner-reminder-guard.ts create mode 100644 src/config/config.backup-rotation.test-helpers.ts create mode 100644 src/telegram/forum-service-message.ts diff --git a/src/auto-reply/reply/agent-runner-reminder-guard.ts b/src/auto-reply/reply/agent-runner-reminder-guard.ts new file mode 100644 index 00000000000..2a0d1ad7bd7 --- /dev/null +++ b/src/auto-reply/reply/agent-runner-reminder-guard.ts @@ -0,0 +1,64 @@ +import { loadCronStore, resolveCronStorePath } from "../../cron/store.js"; +import type { ReplyPayload } from "../types.js"; + +export const UNSCHEDULED_REMINDER_NOTE = + "Note: I did not schedule a reminder in this turn, so this will not trigger automatically."; + +const REMINDER_COMMITMENT_PATTERNS: RegExp[] = [ + /\b(?:i\s*['’]?ll|i will)\s+(?:make sure to\s+)?(?:remember|remind|ping|follow up|follow-up|check back|circle back)\b/i, + /\b(?:i\s*['’]?ll|i will)\s+(?:set|create|schedule)\s+(?:a\s+)?reminder\b/i, +]; + +export function hasUnbackedReminderCommitment(text: string): boolean { + const normalized = text.toLowerCase(); + if (!normalized.trim()) { + return false; + } + if (normalized.includes(UNSCHEDULED_REMINDER_NOTE.toLowerCase())) { + return false; + } + return REMINDER_COMMITMENT_PATTERNS.some((pattern) => pattern.test(text)); +} + +/** + * Returns true when the cron store has at least one enabled job that shares the + * current session key. Used to suppress the "no reminder scheduled" guard note + * when an existing cron (created in a prior turn) already covers the commitment. + */ +export async function hasSessionRelatedCronJobs(params: { + cronStorePath?: string; + sessionKey?: string; +}): Promise { + try { + const storePath = resolveCronStorePath(params.cronStorePath); + const store = await loadCronStore(storePath); + if (store.jobs.length === 0) { + return false; + } + if (params.sessionKey) { + return store.jobs.some((job) => job.enabled && job.sessionKey === params.sessionKey); + } + return false; + } catch { + // If we cannot read the cron store, do not suppress the note. + return false; + } +} + +export function appendUnscheduledReminderNote(payloads: ReplyPayload[]): ReplyPayload[] { + let appended = false; + return payloads.map((payload) => { + if (appended || payload.isError || typeof payload.text !== "string") { + return payload; + } + if (!hasUnbackedReminderCommitment(payload.text)) { + return payload; + } + appended = true; + const trimmed = payload.text.trimEnd(); + return { + ...payload, + text: `${trimmed}\n\n${UNSCHEDULED_REMINDER_NOTE}`, + }; + }); +} diff --git a/src/auto-reply/reply/agent-runner.ts b/src/auto-reply/reply/agent-runner.ts index 937262ea803..5896bf1c163 100644 --- a/src/auto-reply/reply/agent-runner.ts +++ b/src/auto-reply/reply/agent-runner.ts @@ -15,7 +15,6 @@ import { updateSessionStoreEntry, } from "../../config/sessions.js"; import type { TypingMode } from "../../config/types.js"; -import { loadCronStore, resolveCronStorePath } from "../../cron/store.js"; import { emitAgentEvent } from "../../infra/agent-events.js"; import { emitDiagnosticEvent, isDiagnosticsEnabled } from "../../infra/diagnostic-events.js"; import { generateSecureUuid } from "../../infra/secure-random.js"; @@ -40,6 +39,11 @@ import { } from "./agent-runner-helpers.js"; import { runMemoryFlushIfNeeded } from "./agent-runner-memory.js"; import { buildReplyPayloads } from "./agent-runner-payloads.js"; +import { + appendUnscheduledReminderNote, + hasSessionRelatedCronJobs, + hasUnbackedReminderCommitment, +} from "./agent-runner-reminder-guard.js"; import { appendUsageLine, formatResponseUsageLine } from "./agent-runner-utils.js"; import { createAudioAsVoiceBuffer, createBlockReplyPipeline } from "./block-reply-pipeline.js"; import { resolveEffectiveBlockStreamingConfig } from "./block-streaming.js"; @@ -54,71 +58,6 @@ import { createTypingSignaler } from "./typing-mode.js"; import type { TypingController } from "./typing.js"; const BLOCK_REPLY_SEND_TIMEOUT_MS = 15_000; -const UNSCHEDULED_REMINDER_NOTE = - "Note: I did not schedule a reminder in this turn, so this will not trigger automatically."; -const REMINDER_COMMITMENT_PATTERNS: RegExp[] = [ - /\b(?:i\s*['’]?ll|i will)\s+(?:make sure to\s+)?(?:remember|remind|ping|follow up|follow-up|check back|circle back)\b/i, - /\b(?:i\s*['’]?ll|i will)\s+(?:set|create|schedule)\s+(?:a\s+)?reminder\b/i, -]; - -function hasUnbackedReminderCommitment(text: string): boolean { - const normalized = text.toLowerCase(); - if (!normalized.trim()) { - return false; - } - if (normalized.includes(UNSCHEDULED_REMINDER_NOTE.toLowerCase())) { - return false; - } - return REMINDER_COMMITMENT_PATTERNS.some((pattern) => pattern.test(text)); -} - -/** - * Returns true when the cron store has at least one enabled job that shares the - * current session key. Used to suppress the "no reminder scheduled" guard note - * when an existing cron (created in a prior turn) already covers the commitment. - */ -async function hasSessionRelatedCronJobs(params: { - cronStorePath?: string; - sessionKey?: string; -}): Promise { - try { - const storePath = resolveCronStorePath(params.cronStorePath); - const store = await loadCronStore(storePath); - if (store.jobs.length === 0) { - return false; - } - // If we have a session key, only consider cron jobs from the same session. - // This avoids suppressing the note due to unrelated cron jobs. - if (params.sessionKey) { - return store.jobs.some((job) => job.enabled && job.sessionKey === params.sessionKey); - } - // No session key available — cannot scope the check, so do not suppress - // the note. Broadening to all enabled jobs could silently swallow the - // guard note due to unrelated sessions. - return false; - } catch { - // If we cannot read the cron store, do not suppress the note. - return false; - } -} - -function appendUnscheduledReminderNote(payloads: ReplyPayload[]): ReplyPayload[] { - let appended = false; - return payloads.map((payload) => { - if (appended || payload.isError || typeof payload.text !== "string") { - return payload; - } - if (!hasUnbackedReminderCommitment(payload.text)) { - return payload; - } - appended = true; - const trimmed = payload.text.trimEnd(); - return { - ...payload, - text: `${trimmed}\n\n${UNSCHEDULED_REMINDER_NOTE}`, - }; - }); -} export async function runReplyAgent(params: { commandBody: string; diff --git a/src/config/config.backup-rotation.test-helpers.ts b/src/config/config.backup-rotation.test-helpers.ts new file mode 100644 index 00000000000..77374324443 --- /dev/null +++ b/src/config/config.backup-rotation.test-helpers.ts @@ -0,0 +1,19 @@ +import path from "node:path"; +import { expect } from "vitest"; + +export const IS_WINDOWS = process.platform === "win32"; + +export function resolveConfigPathFromTempState(fileName = "openclaw.json"): string { + const stateDir = process.env.OPENCLAW_STATE_DIR?.trim(); + if (!stateDir) { + throw new Error("Expected OPENCLAW_STATE_DIR to be set by withTempHome"); + } + return path.join(stateDir, fileName); +} + +export function expectPosixMode(statMode: number, expectedMode: number): void { + if (IS_WINDOWS) { + return; + } + expect(statMode & 0o777).toBe(expectedMode); +} diff --git a/src/config/config.backup-rotation.test.ts b/src/config/config.backup-rotation.test.ts index b89f0653379..8c12db78b82 100644 --- a/src/config/config.backup-rotation.test.ts +++ b/src/config/config.backup-rotation.test.ts @@ -1,5 +1,4 @@ import fs from "node:fs/promises"; -import path from "node:path"; import { describe, expect, it } from "vitest"; import { maintainConfigBackups, @@ -7,19 +6,18 @@ import { hardenBackupPermissions, cleanOrphanBackups, } from "./backup-rotation.js"; +import { + expectPosixMode, + IS_WINDOWS, + resolveConfigPathFromTempState, +} from "./config.backup-rotation.test-helpers.js"; import { withTempHome } from "./test-helpers.js"; import type { OpenClawConfig } from "./types.js"; -const IS_WINDOWS = process.platform === "win32"; - describe("config backup rotation", () => { it("keeps a 5-deep backup ring for config writes", async () => { await withTempHome(async () => { - const stateDir = process.env.OPENCLAW_STATE_DIR?.trim(); - if (!stateDir) { - throw new Error("Expected OPENCLAW_STATE_DIR to be set by withTempHome"); - } - const configPath = path.join(stateDir, "openclaw.json"); + const configPath = resolveConfigPathFromTempState(); const buildConfig = (version: number): OpenClawConfig => ({ agents: { list: [{ id: `v${version}` }] }, @@ -60,11 +58,7 @@ describe("config backup rotation", () => { // chmod is a no-op on Windows — 0o600 can never be observed there. it.skipIf(IS_WINDOWS)("hardenBackupPermissions sets 0o600 on all backup files", async () => { await withTempHome(async () => { - const stateDir = process.env.OPENCLAW_STATE_DIR?.trim(); - if (!stateDir) { - throw new Error("Expected OPENCLAW_STATE_DIR to be set by withTempHome"); - } - const configPath = path.join(stateDir, "openclaw.json"); + const configPath = resolveConfigPathFromTempState(); // Create .bak and .bak.1 with permissive mode await fs.writeFile(`${configPath}.bak`, "secret", { mode: 0o644 }); @@ -75,18 +69,14 @@ describe("config backup rotation", () => { const bakStat = await fs.stat(`${configPath}.bak`); const bak1Stat = await fs.stat(`${configPath}.bak.1`); - expect(bakStat.mode & 0o777).toBe(0o600); - expect(bak1Stat.mode & 0o777).toBe(0o600); + expectPosixMode(bakStat.mode, 0o600); + expectPosixMode(bak1Stat.mode, 0o600); }); }); it("cleanOrphanBackups removes stale files outside the rotation ring", async () => { await withTempHome(async () => { - const stateDir = process.env.OPENCLAW_STATE_DIR?.trim(); - if (!stateDir) { - throw new Error("Expected OPENCLAW_STATE_DIR to be set by withTempHome"); - } - const configPath = path.join(stateDir, "openclaw.json"); + const configPath = resolveConfigPathFromTempState(); // Create valid backups await fs.writeFile(configPath, "current"); @@ -118,11 +108,7 @@ describe("config backup rotation", () => { it("maintainConfigBackups composes rotate/copy/harden/prune flow", async () => { await withTempHome(async () => { - const stateDir = process.env.OPENCLAW_STATE_DIR?.trim(); - if (!stateDir) { - throw new Error("Expected OPENCLAW_STATE_DIR to be set by withTempHome"); - } - const configPath = path.join(stateDir, "openclaw.json"); + const configPath = resolveConfigPathFromTempState(); await fs.writeFile(configPath, JSON.stringify({ token: "secret" }), { mode: 0o600 }); await fs.writeFile(`${configPath}.bak`, "previous", { mode: 0o644 }); await fs.writeFile(`${configPath}.bak.orphan`, "old"); @@ -139,7 +125,7 @@ describe("config backup rotation", () => { // should still run there. if (!IS_WINDOWS) { const primaryBackupStat = await fs.stat(`${configPath}.bak`); - expect(primaryBackupStat.mode & 0o777).toBe(0o600); + expectPosixMode(primaryBackupStat.mode, 0o600); } // Out-of-ring orphan gets pruned. await expect(fs.stat(`${configPath}.bak.orphan`)).rejects.toThrow(); diff --git a/src/telegram/bot-message-context.implicit-mention.test.ts b/src/telegram/bot-message-context.implicit-mention.test.ts index c6ece03108b..4ed40719be5 100644 --- a/src/telegram/bot-message-context.implicit-mention.test.ts +++ b/src/telegram/bot-message-context.implicit-mention.test.ts @@ -1,16 +1,8 @@ import { describe, expect, it } from "vitest"; import { buildTelegramMessageContextForTest } from "./bot-message-context.test-harness.js"; +import { TELEGRAM_FORUM_SERVICE_FIELDS } from "./forum-service-message.js"; describe("buildTelegramMessageContext implicitMention forum service messages", () => { - const TELEGRAM_FORUM_SERVICE_FIELDS = [ - "forum_topic_created", - "forum_topic_edited", - "forum_topic_closed", - "forum_topic_reopened", - "general_forum_topic_hidden", - "general_forum_topic_unhidden", - ] as const; - /** * Build a group message context where the user sends a message inside a * forum topic that has `reply_to_message` pointing to a message from the diff --git a/src/telegram/bot-message-context.ts b/src/telegram/bot-message-context.ts index 536ed711fcc..7927af7f94d 100644 --- a/src/telegram/bot-message-context.ts +++ b/src/telegram/bot-message-context.ts @@ -67,6 +67,7 @@ import { } from "./bot/helpers.js"; import type { StickerMetadata, TelegramContext } from "./bot/types.js"; import { enforceTelegramDmAccess } from "./dm-access.js"; +import { isTelegramForumServiceMessage } from "./forum-service-message.js"; import { evaluateTelegramGroupBaseAccess } from "./group-access.js"; import { resolveTelegramGroupPromptSettings } from "./group-config-helpers.js"; import { @@ -867,31 +868,3 @@ export const buildTelegramMessageContext = async ({ export type TelegramMessageContext = NonNullable< Awaited> >; - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -/** Telegram forum-topic service-message fields (Bot API). */ -const FORUM_SERVICE_FIELDS = [ - "forum_topic_created", - "forum_topic_edited", - "forum_topic_closed", - "forum_topic_reopened", - "general_forum_topic_hidden", - "general_forum_topic_unhidden", -] as const; - -/** - * Returns `true` when the message is a Telegram forum service message (e.g. - * "Topic created"). These auto-generated messages carry one of the - * `forum_topic_*` / `general_forum_topic_*` fields and should not count as - * regular bot replies for implicit-mention purposes. - */ -function isTelegramForumServiceMessage(msg: unknown): boolean { - if (!msg || typeof msg !== "object") { - return false; - } - const record = msg as Record; - return FORUM_SERVICE_FIELDS.some((f) => record[f] != null); -} diff --git a/src/telegram/forum-service-message.ts b/src/telegram/forum-service-message.ts new file mode 100644 index 00000000000..d6d23f2b92d --- /dev/null +++ b/src/telegram/forum-service-message.ts @@ -0,0 +1,23 @@ +/** Telegram forum-topic service-message fields (Bot API). */ +export const TELEGRAM_FORUM_SERVICE_FIELDS = [ + "forum_topic_created", + "forum_topic_edited", + "forum_topic_closed", + "forum_topic_reopened", + "general_forum_topic_hidden", + "general_forum_topic_unhidden", +] as const; + +/** + * Returns `true` when the message is a Telegram forum service message (e.g. + * "Topic created"). These auto-generated messages carry one of the + * `forum_topic_*` / `general_forum_topic_*` fields and should not count as + * regular bot replies for implicit-mention purposes. + */ +export function isTelegramForumServiceMessage(msg: unknown): boolean { + if (!msg || typeof msg !== "object") { + return false; + } + const record = msg as Record; + return TELEGRAM_FORUM_SERVICE_FIELDS.some((field) => record[field] != null); +}