From 31fb5867e8836ee30d5f40d965a89c5790a36401 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 8 Jan 2026 21:03:11 +0100 Subject: [PATCH] refactor(outbound): centralize telegram account defaults --- CHANGELOG.md | 1 + src/commands/agent.test.ts | 4 +- src/commands/send.test.ts | 6 +-- src/infra/heartbeat-runner.test.ts | 71 +++++++++++++++++++++++++++++- src/infra/heartbeat-runner.ts | 8 ---- src/infra/outbound/deliver.test.ts | 28 +++++++++++- src/infra/outbound/deliver.ts | 29 ++++++------ src/telegram/accounts.test.ts | 69 +++++++++++++++++++++++++++++ src/telegram/accounts.ts | 41 ++++++++++++----- 9 files changed, 214 insertions(+), 43 deletions(-) create mode 100644 src/telegram/accounts.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 57a32842d..e39ad2dc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Outbound: default Telegram account selection for config-only tokens; remove heartbeat-specific accountId handling. - Heartbeat: resolve Telegram account IDs from config-only tokens; cron tool accepts canonical `jobId` and legacy `id` for job actions. (#516) — thanks @YuriNachos - Discord: stop provider when gateway reconnects are exhausted and surface errors. (#514) — thanks @joshp123 - Auto-reply: preserve block reply ordering with timeout fallback for streaming. (#503) — thanks @joshp123 diff --git a/src/commands/agent.test.ts b/src/commands/agent.test.ts index e7d647c16..7aac629d1 100644 --- a/src/commands/agent.test.ts +++ b/src/commands/agent.test.ts @@ -266,7 +266,7 @@ describe("agentCommand", () => { }); }); - it("passes telegram account id when delivering", async () => { + it("passes through telegram accountId when delivering", async () => { await withTempHome(async (home) => { const store = path.join(home, "sessions.json"); mockConfig(home, store, undefined, undefined, { botToken: "t-1" }); @@ -297,7 +297,7 @@ describe("agentCommand", () => { expect(deps.sendMessageTelegram).toHaveBeenCalledWith( "123", "ok", - expect.objectContaining({ accountId: "default", verbose: false }), + expect.objectContaining({ accountId: undefined, verbose: false }), ); } finally { if (prevTelegramToken === undefined) { diff --git a/src/commands/send.test.ts b/src/commands/send.test.ts index 51c83d489..0cf52f259 100644 --- a/src/commands/send.test.ts +++ b/src/commands/send.test.ts @@ -137,7 +137,7 @@ describe("sendCommand", () => { expect(deps.sendMessageTelegram).toHaveBeenCalledWith( "123", "hi", - expect.objectContaining({ accountId: "default", verbose: false }), + expect.objectContaining({ accountId: undefined, verbose: false }), ); expect(deps.sendMessageWhatsApp).not.toHaveBeenCalled(); }); @@ -158,7 +158,7 @@ describe("sendCommand", () => { expect(deps.sendMessageTelegram).toHaveBeenCalledWith( "123", "hi", - expect.objectContaining({ accountId: "default", verbose: false }), + expect.objectContaining({ accountId: undefined, verbose: false }), ); }); @@ -212,7 +212,7 @@ describe("sendCommand", () => { expect(deps.sendMessageSlack).toHaveBeenCalledWith( "channel:C123", "hi", - expect.objectContaining({ accountId: "default" }), + expect.objectContaining({ accountId: undefined }), ); expect(deps.sendMessageWhatsApp).not.toHaveBeenCalled(); }); diff --git a/src/infra/heartbeat-runner.test.ts b/src/infra/heartbeat-runner.test.ts index 54ad17b62..2cdc595a9 100644 --- a/src/infra/heartbeat-runner.test.ts +++ b/src/infra/heartbeat-runner.test.ts @@ -368,7 +368,7 @@ describe("runHeartbeatOnce", () => { } }); - it("passes telegram token from config to sendTelegram", async () => { + it("passes through accountId for telegram heartbeats", async () => { const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-hb-")); const storePath = path.join(tmpDir, "sessions.json"); const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); @@ -418,7 +418,74 @@ describe("runHeartbeatOnce", () => { expect(sendTelegram).toHaveBeenCalledWith( "123456", "Hello from heartbeat", - expect.objectContaining({ accountId: "default", verbose: false }), + expect.objectContaining({ accountId: undefined, verbose: false }), + ); + } finally { + replySpy.mockRestore(); + if (prevTelegramToken === undefined) { + delete process.env.TELEGRAM_BOT_TOKEN; + } else { + process.env.TELEGRAM_BOT_TOKEN = prevTelegramToken; + } + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }); + + it("does not pre-resolve telegram accountId (allows config-only account tokens)", async () => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-hb-")); + const storePath = path.join(tmpDir, "sessions.json"); + const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); + const prevTelegramToken = process.env.TELEGRAM_BOT_TOKEN; + process.env.TELEGRAM_BOT_TOKEN = ""; + try { + await fs.writeFile( + storePath, + JSON.stringify( + { + main: { + sessionId: "sid", + updatedAt: Date.now(), + lastProvider: "telegram", + lastTo: "123456", + }, + }, + null, + 2, + ), + ); + + const cfg: ClawdbotConfig = { + agent: { + heartbeat: { every: "5m", target: "telegram", to: "123456" }, + }, + telegram: { + accounts: { + work: { botToken: "test-bot-token-123" }, + }, + }, + session: { store: storePath }, + }; + + replySpy.mockResolvedValue({ text: "Hello from heartbeat" }); + const sendTelegram = vi.fn().mockResolvedValue({ + messageId: "m1", + chatId: "123456", + }); + + await runHeartbeatOnce({ + cfg, + deps: { + sendTelegram, + getQueueSize: () => 0, + nowMs: () => 0, + }, + }); + + expect(sendTelegram).toHaveBeenCalledTimes(1); + expect(sendTelegram).toHaveBeenCalledWith( + "123456", + "Hello from heartbeat", + expect.objectContaining({ accountId: undefined, verbose: false }), ); } finally { replySpy.mockRestore(); diff --git a/src/infra/heartbeat-runner.ts b/src/infra/heartbeat-runner.ts index 25b5aab8e..b015a0896 100644 --- a/src/infra/heartbeat-runner.ts +++ b/src/infra/heartbeat-runner.ts @@ -22,7 +22,6 @@ import { createSubsystemLogger } from "../logging.js"; import { getQueueSize } from "../process/command-queue.js"; import { webAuthExists } from "../providers/web/index.js"; import { defaultRuntime, type RuntimeEnv } from "../runtime.js"; -import { resolveDefaultTelegramAccountId } from "../telegram/accounts.js"; import { getActiveWebListener } from "../web/active-listener.js"; import { emitHeartbeatEvent } from "./heartbeat-events.js"; import { @@ -316,17 +315,10 @@ export async function runHeartbeatOnce(opts: { } } - // Resolve accountId for providers that support multiple accounts - const accountId = - delivery.provider === "telegram" - ? resolveDefaultTelegramAccountId(cfg) - : undefined; - await deliverOutboundPayloads({ cfg, provider: delivery.provider, to: delivery.to, - accountId, payloads: [ { text: normalized.text, diff --git a/src/infra/outbound/deliver.test.ts b/src/infra/outbound/deliver.test.ts index 0cf53236a..3dc29b2f1 100644 --- a/src/infra/outbound/deliver.test.ts +++ b/src/infra/outbound/deliver.test.ts @@ -7,7 +7,7 @@ import { } from "./deliver.js"; describe("deliverOutboundPayloads", () => { - it("chunks telegram markdown and passes account id", async () => { + it("chunks telegram markdown and passes through accountId", async () => { const sendTelegram = vi .fn() .mockResolvedValue({ messageId: "m1", chatId: "c1" }); @@ -28,7 +28,7 @@ describe("deliverOutboundPayloads", () => { expect(sendTelegram).toHaveBeenCalledTimes(2); for (const call of sendTelegram.mock.calls) { expect(call[2]).toEqual( - expect.objectContaining({ accountId: "default", verbose: false }), + expect.objectContaining({ accountId: undefined, verbose: false }), ); } expect(results).toHaveLength(2); @@ -42,6 +42,30 @@ describe("deliverOutboundPayloads", () => { } }); + it("passes explicit accountId to sendTelegram", async () => { + const sendTelegram = vi + .fn() + .mockResolvedValue({ messageId: "m1", chatId: "c1" }); + const cfg: ClawdbotConfig = { + telegram: { botToken: "tok-1", textChunkLimit: 2 }, + }; + + await deliverOutboundPayloads({ + cfg, + provider: "telegram", + to: "123", + accountId: "default", + payloads: [{ text: "hi" }], + deps: { sendTelegram }, + }); + + expect(sendTelegram).toHaveBeenCalledWith( + "123", + "hi", + expect.objectContaining({ accountId: "default", verbose: false }), + ); + }); + it("uses signal media maxBytes from config", async () => { const sendSignal = vi .fn() diff --git a/src/infra/outbound/deliver.ts b/src/infra/outbound/deliver.ts index 5644de226..7b915715d 100644 --- a/src/infra/outbound/deliver.ts +++ b/src/infra/outbound/deliver.ts @@ -86,7 +86,8 @@ function createProviderHandler(params: { deps: Required; }): ProviderHandler { const { cfg, to, deps } = params; - const accountId = normalizeAccountId(params.accountId); + const rawAccountId = params.accountId; + const accountId = normalizeAccountId(rawAccountId); const signalMaxBytes = params.provider === "signal" ? resolveMediaMaxBytes(cfg, "signal", accountId) @@ -103,7 +104,7 @@ function createProviderHandler(params: { provider: "whatsapp", ...(await deps.sendWhatsApp(to, text, { verbose: false, - accountId, + accountId: rawAccountId, })), }), sendMedia: async (caption, mediaUrl) => ({ @@ -111,7 +112,7 @@ function createProviderHandler(params: { ...(await deps.sendWhatsApp(to, caption, { verbose: false, mediaUrl, - accountId, + accountId: rawAccountId, })), }), }, @@ -121,7 +122,7 @@ function createProviderHandler(params: { provider: "telegram", ...(await deps.sendTelegram(to, text, { verbose: false, - accountId, + accountId: rawAccountId, })), }), sendMedia: async (caption, mediaUrl) => ({ @@ -129,7 +130,7 @@ function createProviderHandler(params: { ...(await deps.sendTelegram(to, caption, { verbose: false, mediaUrl, - accountId, + accountId: rawAccountId, })), }), }, @@ -139,7 +140,7 @@ function createProviderHandler(params: { provider: "discord", ...(await deps.sendDiscord(to, text, { verbose: false, - accountId, + accountId: rawAccountId, })), }), sendMedia: async (caption, mediaUrl) => ({ @@ -147,7 +148,7 @@ function createProviderHandler(params: { ...(await deps.sendDiscord(to, caption, { verbose: false, mediaUrl, - accountId, + accountId: rawAccountId, })), }), }, @@ -156,14 +157,14 @@ function createProviderHandler(params: { sendText: async (text) => ({ provider: "slack", ...(await deps.sendSlack(to, text, { - accountId, + accountId: rawAccountId, })), }), sendMedia: async (caption, mediaUrl) => ({ provider: "slack", ...(await deps.sendSlack(to, caption, { mediaUrl, - accountId, + accountId: rawAccountId, })), }), }, @@ -173,7 +174,7 @@ function createProviderHandler(params: { provider: "signal", ...(await deps.sendSignal(to, text, { maxBytes: signalMaxBytes, - accountId, + accountId: rawAccountId, })), }), sendMedia: async (caption, mediaUrl) => ({ @@ -181,7 +182,7 @@ function createProviderHandler(params: { ...(await deps.sendSignal(to, caption, { mediaUrl, maxBytes: signalMaxBytes, - accountId, + accountId: rawAccountId, })), }), }, @@ -191,7 +192,7 @@ function createProviderHandler(params: { provider: "imessage", ...(await deps.sendIMessage(to, text, { maxBytes: imessageMaxBytes, - accountId, + accountId: rawAccountId, })), }), sendMedia: async (caption, mediaUrl) => ({ @@ -199,7 +200,7 @@ function createProviderHandler(params: { ...(await deps.sendIMessage(to, caption, { mediaUrl, maxBytes: imessageMaxBytes, - accountId, + accountId: rawAccountId, })), }), }, @@ -220,7 +221,7 @@ export async function deliverOutboundPayloads(params: { onPayload?: (payload: NormalizedOutboundPayload) => void; }): Promise { const { cfg, provider, to, payloads } = params; - const accountId = normalizeAccountId(params.accountId); + const accountId = params.accountId; const deps = { sendWhatsApp: params.deps?.sendWhatsApp ?? sendMessageWhatsApp, sendTelegram: params.deps?.sendTelegram ?? sendMessageTelegram, diff --git a/src/telegram/accounts.test.ts b/src/telegram/accounts.test.ts new file mode 100644 index 000000000..d7fb60d6f --- /dev/null +++ b/src/telegram/accounts.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "vitest"; + +import type { ClawdbotConfig } from "../config/config.js"; +import { resolveTelegramAccount } from "./accounts.js"; + +describe("resolveTelegramAccount", () => { + it("falls back to the first configured account when accountId is omitted", () => { + const prevTelegramToken = process.env.TELEGRAM_BOT_TOKEN; + process.env.TELEGRAM_BOT_TOKEN = ""; + try { + const cfg: ClawdbotConfig = { + telegram: { accounts: { work: { botToken: "tok-work" } } }, + }; + + const account = resolveTelegramAccount({ cfg }); + expect(account.accountId).toBe("work"); + expect(account.token).toBe("tok-work"); + expect(account.tokenSource).toBe("config"); + } finally { + if (prevTelegramToken === undefined) { + delete process.env.TELEGRAM_BOT_TOKEN; + } else { + process.env.TELEGRAM_BOT_TOKEN = prevTelegramToken; + } + } + }); + + it("prefers TELEGRAM_BOT_TOKEN when accountId is omitted", () => { + const prevTelegramToken = process.env.TELEGRAM_BOT_TOKEN; + process.env.TELEGRAM_BOT_TOKEN = "tok-env"; + try { + const cfg: ClawdbotConfig = { + telegram: { accounts: { work: { botToken: "tok-work" } } }, + }; + + const account = resolveTelegramAccount({ cfg }); + expect(account.accountId).toBe("default"); + expect(account.token).toBe("tok-env"); + expect(account.tokenSource).toBe("env"); + } finally { + if (prevTelegramToken === undefined) { + delete process.env.TELEGRAM_BOT_TOKEN; + } else { + process.env.TELEGRAM_BOT_TOKEN = prevTelegramToken; + } + } + }); + + it("does not fall back when accountId is explicitly provided", () => { + const prevTelegramToken = process.env.TELEGRAM_BOT_TOKEN; + process.env.TELEGRAM_BOT_TOKEN = ""; + try { + const cfg: ClawdbotConfig = { + telegram: { accounts: { work: { botToken: "tok-work" } } }, + }; + + const account = resolveTelegramAccount({ cfg, accountId: "default" }); + expect(account.accountId).toBe("default"); + expect(account.tokenSource).toBe("none"); + expect(account.token).toBe(""); + } finally { + if (prevTelegramToken === undefined) { + delete process.env.TELEGRAM_BOT_TOKEN; + } else { + process.env.TELEGRAM_BOT_TOKEN = prevTelegramToken; + } + } + }); +}); diff --git a/src/telegram/accounts.ts b/src/telegram/accounts.ts index f76113880..e646a161a 100644 --- a/src/telegram/accounts.ts +++ b/src/telegram/accounts.ts @@ -56,20 +56,37 @@ export function resolveTelegramAccount(params: { cfg: ClawdbotConfig; accountId?: string | null; }): ResolvedTelegramAccount { - const accountId = normalizeAccountId(params.accountId); + const hasExplicitAccountId = Boolean(params.accountId?.trim()); const baseEnabled = params.cfg.telegram?.enabled !== false; - const merged = mergeTelegramAccountConfig(params.cfg, accountId); - const accountEnabled = merged.enabled !== false; - const enabled = baseEnabled && accountEnabled; - const tokenResolution = resolveTelegramToken(params.cfg, { accountId }); - return { - accountId, - enabled, - name: merged.name?.trim() || undefined, - token: tokenResolution.token, - tokenSource: tokenResolution.source, - config: merged, + + const resolve = (accountId: string) => { + const merged = mergeTelegramAccountConfig(params.cfg, accountId); + const accountEnabled = merged.enabled !== false; + const enabled = baseEnabled && accountEnabled; + const tokenResolution = resolveTelegramToken(params.cfg, { accountId }); + return { + accountId, + enabled, + name: merged.name?.trim() || undefined, + token: tokenResolution.token, + tokenSource: tokenResolution.source, + config: merged, + } satisfies ResolvedTelegramAccount; }; + + const normalized = normalizeAccountId(params.accountId); + const primary = resolve(normalized); + if (hasExplicitAccountId) return primary; + if (primary.tokenSource !== "none") return primary; + + // If accountId is omitted, prefer a configured account token over failing on + // the implicit "default" account. This keeps env-based setups working (env + // still wins) while making config-only tokens work for things like heartbeats. + const fallbackId = resolveDefaultTelegramAccountId(params.cfg); + if (fallbackId === primary.accountId) return primary; + const fallback = resolve(fallbackId); + if (fallback.tokenSource === "none") return primary; + return fallback; } export function listEnabledTelegramAccounts(