From f171d509bbd6cc9636cf153f656f1ea1fc030266 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 7 Jan 2026 01:15:53 +0000 Subject: [PATCH] refactor: centralize outbound target validation --- docs/refactor/send-refactor.md | 2 +- src/commands/agent.ts | 117 +++++++---------------------- src/commands/send.ts | 10 ++- src/infra/outbound/targets.test.ts | 39 ++++++++++ src/infra/outbound/targets.ts | 93 +++++++++++++++++++++++ 5 files changed, 168 insertions(+), 93 deletions(-) create mode 100644 src/infra/outbound/targets.test.ts diff --git a/docs/refactor/send-refactor.md b/docs/refactor/send-refactor.md index 6f92a4659..c914a4f69 100644 --- a/docs/refactor/send-refactor.md +++ b/docs/refactor/send-refactor.md @@ -1,6 +1,6 @@ # send-refactor scratchpad -- [ ] Commit + push current outbound refactor changes +- [x] Commit + push current outbound refactor changes - [ ] Step 1: centralize outbound target validation - [ ] Step 2: normalize payloads + single delivery call - [ ] Step 3: unify outbound JSON/result formatting diff --git a/src/commands/agent.ts b/src/commands/agent.ts index 3be991d56..3d0e6ab68 100644 --- a/src/commands/agent.ts +++ b/src/commands/agent.ts @@ -45,6 +45,7 @@ import { registerAgentRunContext, } from "../infra/agent-events.js"; import { deliverOutboundPayloads } from "../infra/outbound/deliver.js"; +import { resolveOutboundTarget } from "../infra/outbound/targets.js"; import { defaultRuntime, type RuntimeEnv } from "../runtime.js"; import { resolveSendPolicy } from "../sessions/send-policy.js"; import { normalizeE164 } from "../utils.js"; @@ -501,93 +502,39 @@ export async function agentCommand( const deliveryProvider = deliveryProviderRaw === "imsg" ? "imessage" : deliveryProviderRaw; - const whatsappTarget = opts.to ? normalizeE164(opts.to) : allowFrom[0]; - const telegramTarget = opts.to?.trim() || undefined; - const discordTarget = opts.to?.trim() || undefined; - const slackTarget = opts.to?.trim() || undefined; - const signalTarget = opts.to?.trim() || undefined; - const imessageTarget = opts.to?.trim() || undefined; - const logDeliveryError = (err: unknown) => { - const deliveryTarget = - deliveryProvider === "telegram" - ? telegramTarget - : deliveryProvider === "whatsapp" - ? whatsappTarget - : deliveryProvider === "discord" - ? discordTarget - : deliveryProvider === "slack" - ? slackTarget - : deliveryProvider === "signal" - ? signalTarget - : deliveryProvider === "imessage" - ? imessageTarget - : undefined; const message = `Delivery failed (${deliveryProvider}${deliveryTarget ? ` to ${deliveryTarget}` : ""}): ${String(err)}`; runtime.error?.(message); if (!runtime.error) runtime.log(message); }; + const isDeliveryProviderKnown = + deliveryProvider === "whatsapp" || + deliveryProvider === "telegram" || + deliveryProvider === "discord" || + deliveryProvider === "slack" || + deliveryProvider === "signal" || + deliveryProvider === "imessage" || + deliveryProvider === "webchat"; + + const resolvedTarget = + deliver && isDeliveryProviderKnown + ? resolveOutboundTarget({ + provider: deliveryProvider, + to: opts.to, + allowFrom, + }) + : null; + const deliveryTarget = resolvedTarget?.ok ? resolvedTarget.to : undefined; + if (deliver) { - if (deliveryProvider === "whatsapp" && !whatsappTarget) { - const err = new Error( - "Delivering to WhatsApp requires --to or whatsapp.allowFrom[0]", - ); - if (!bestEffortDeliver) throw err; - logDeliveryError(err); - } - if (deliveryProvider === "telegram" && !telegramTarget) { - const err = new Error("Delivering to Telegram requires --to "); - if (!bestEffortDeliver) throw err; - logDeliveryError(err); - } - if (deliveryProvider === "discord" && !discordTarget) { - const err = new Error( - "Delivering to Discord requires --to ", - ); - if (!bestEffortDeliver) throw err; - logDeliveryError(err); - } - if (deliveryProvider === "slack" && !slackTarget) { - const err = new Error( - "Delivering to Slack requires --to ", - ); - if (!bestEffortDeliver) throw err; - logDeliveryError(err); - } - if (deliveryProvider === "signal" && !signalTarget) { - const err = new Error( - "Delivering to Signal requires --to ", - ); - if (!bestEffortDeliver) throw err; - logDeliveryError(err); - } - if (deliveryProvider === "imessage" && !imessageTarget) { - const err = new Error( - "Delivering to iMessage requires --to ", - ); - if (!bestEffortDeliver) throw err; - logDeliveryError(err); - } - if (deliveryProvider === "webchat") { - const err = new Error( - "Delivering to WebChat is not supported via `clawdbot agent`; use WhatsApp/Telegram or run with --deliver=false.", - ); - if (!bestEffortDeliver) throw err; - logDeliveryError(err); - } - if ( - deliveryProvider !== "whatsapp" && - deliveryProvider !== "telegram" && - deliveryProvider !== "discord" && - deliveryProvider !== "slack" && - deliveryProvider !== "signal" && - deliveryProvider !== "imessage" && - deliveryProvider !== "webchat" - ) { + if (!isDeliveryProviderKnown) { const err = new Error(`Unknown provider: ${deliveryProvider}`); if (!bestEffortDeliver) throw err; logDeliveryError(err); + } else if (resolvedTarget && !resolvedTarget.ok) { + if (!bestEffortDeliver) throw resolvedTarget.error; + logDeliveryError(resolvedTarget.error); } } @@ -639,24 +586,12 @@ export async function agentCommand( deliveryProvider === "signal" || deliveryProvider === "imessage" ) { - const target = - deliveryProvider === "whatsapp" - ? whatsappTarget - : deliveryProvider === "telegram" - ? telegramTarget - : deliveryProvider === "discord" - ? discordTarget - : deliveryProvider === "slack" - ? slackTarget - : deliveryProvider === "signal" - ? signalTarget - : imessageTarget; - if (!target) continue; + if (!deliveryTarget) continue; try { await deliverOutboundPayloads({ cfg, provider: deliveryProvider, - to: target, + to: deliveryTarget, payloads: [payload], deps: { sendWhatsApp: deps.sendMessageWhatsApp, diff --git a/src/commands/send.ts b/src/commands/send.ts index 804f5263d..453ed7efb 100644 --- a/src/commands/send.ts +++ b/src/commands/send.ts @@ -4,6 +4,7 @@ import { callGateway, randomIdempotencyKey } from "../gateway/call.js"; import { success } from "../globals.js"; import { deliverOutboundPayloads } from "../infra/outbound/deliver.js"; import type { OutboundDeliveryResult } from "../infra/outbound/deliver.js"; +import { resolveOutboundTarget } from "../infra/outbound/targets.js"; import type { RuntimeEnv } from "../runtime.js"; export async function sendCommand( @@ -37,10 +38,17 @@ export async function sendCommand( provider === "signal" || provider === "imessage" ) { + const resolvedTarget = resolveOutboundTarget({ + provider, + to: opts.to, + }); + if (!resolvedTarget.ok) { + throw resolvedTarget.error; + } const results = await deliverOutboundPayloads({ cfg: loadConfig(), provider, - to: opts.to, + to: resolvedTarget.to, payloads: [{ text: opts.message, mediaUrl: opts.media }], deps: { sendWhatsApp: deps.sendMessageWhatsApp, diff --git a/src/infra/outbound/targets.test.ts b/src/infra/outbound/targets.test.ts new file mode 100644 index 000000000..725acb66a --- /dev/null +++ b/src/infra/outbound/targets.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; + +import { resolveOutboundTarget } from "./targets.js"; + +describe("resolveOutboundTarget", () => { + it("falls back to whatsapp allowFrom", () => { + const res = resolveOutboundTarget({ + provider: "whatsapp", + to: "", + allowFrom: ["+1555"], + }); + expect(res).toEqual({ ok: true, to: "+1555" }); + }); + + it("normalizes whatsapp target when provided", () => { + const res = resolveOutboundTarget({ + provider: "whatsapp", + to: " (555) 123-4567 ", + }); + if (!res.ok) throw res.error; + expect(res.to).toBe("+5551234567"); + }); + + it("rejects telegram with missing target", () => { + const res = resolveOutboundTarget({ provider: "telegram", to: " " }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error.message).toContain("Telegram"); + } + }); + + it("rejects webchat delivery", () => { + const res = resolveOutboundTarget({ provider: "webchat", to: "x" }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error.message).toContain("WebChat"); + } + }); +}); diff --git a/src/infra/outbound/targets.ts b/src/infra/outbound/targets.ts index a551a7dc9..122854ea8 100644 --- a/src/infra/outbound/targets.ts +++ b/src/infra/outbound/targets.ts @@ -19,6 +19,99 @@ export type OutboundTarget = { reason?: string; }; +export type OutboundTargetResolution = + | { ok: true; to: string } + | { ok: false; error: Error }; + +export function resolveOutboundTarget(params: { + provider: + | "whatsapp" + | "telegram" + | "discord" + | "slack" + | "signal" + | "imessage" + | "webchat"; + to?: string; + allowFrom?: string[]; +}): OutboundTargetResolution { + const trimmed = params.to?.trim() || ""; + if (params.provider === "whatsapp") { + if (trimmed) { + return { ok: true, to: normalizeE164(trimmed) }; + } + const fallback = params.allowFrom?.[0]?.trim(); + if (fallback) { + return { ok: true, to: fallback }; + } + return { + ok: false, + error: new Error( + "Delivering to WhatsApp requires --to or whatsapp.allowFrom[0]", + ), + }; + } + if (params.provider === "telegram") { + if (!trimmed) { + return { + ok: false, + error: new Error("Delivering to Telegram requires --to "), + }; + } + return { ok: true, to: trimmed }; + } + if (params.provider === "discord") { + if (!trimmed) { + return { + ok: false, + error: new Error( + "Delivering to Discord requires --to ", + ), + }; + } + return { ok: true, to: trimmed }; + } + if (params.provider === "slack") { + if (!trimmed) { + return { + ok: false, + error: new Error( + "Delivering to Slack requires --to ", + ), + }; + } + return { ok: true, to: trimmed }; + } + if (params.provider === "signal") { + if (!trimmed) { + return { + ok: false, + error: new Error( + "Delivering to Signal requires --to ", + ), + }; + } + return { ok: true, to: trimmed }; + } + if (params.provider === "imessage") { + if (!trimmed) { + return { + ok: false, + error: new Error( + "Delivering to iMessage requires --to ", + ), + }; + } + return { ok: true, to: trimmed }; + } + return { + ok: false, + error: new Error( + "Delivering to WebChat is not supported via `clawdbot agent`; use WhatsApp/Telegram or run with --deliver=false.", + ), + }; +} + export function resolveHeartbeatDeliveryTarget(params: { cfg: ClawdbotConfig; entry?: SessionEntry;