From cd3793185bfa88adb2808bcf8e787db6bd83150e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E8=B4=B5=E8=90=8D0668001030?= Date: Fri, 19 Jun 2026 10:13:37 +0800 Subject: [PATCH] fix(outbound): preserve reserved Telegram directory targets --- src/infra/outbound/agent-delivery.test.ts | 125 +++++++++++++++++-- src/infra/outbound/agent-delivery.ts | 37 ++++-- src/infra/outbound/target-resolver.test.ts | 72 +++++++++++ src/infra/outbound/target-resolver.ts | 39 ++++-- src/infra/outbound/targets-resolve-shared.ts | 2 +- 5 files changed, 245 insertions(+), 30 deletions(-) diff --git a/src/infra/outbound/agent-delivery.test.ts b/src/infra/outbound/agent-delivery.test.ts index c5bb917a93e8..683650ef4711 100644 --- a/src/infra/outbound/agent-delivery.test.ts +++ b/src/infra/outbound/agent-delivery.test.ts @@ -4,6 +4,15 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ resolveOutboundChannelPlugin: vi.fn<() => unknown>(() => null), + resolveChannelTarget: vi.fn<() => Promise>(async () => ({ + ok: true, + target: { + to: "+1999", + kind: "group", + source: "normalized", + resolutionSource: "normalized", + }, + })), resolveOutboundTarget: vi.fn<() => { ok: true; to: string } | { ok: false; error: Error }>( () => ({ ok: true, to: "+1999" }), ), @@ -82,11 +91,16 @@ vi.mock("./outbound-session.js", () => ({ resolveOutboundSessionRoute: mocks.resolveOutboundSessionRoute, })); +vi.mock("./target-resolver.js", () => ({ + resolveChannelTarget: mocks.resolveChannelTarget, +})); + vi.mock("../../utils/message-channel.js", () => ({ INTERNAL_MESSAGE_CHANNEL: "webchat", - isDeliverableMessageChannel: (channel: string) => ["directchat", "workspace"].includes(channel), + isDeliverableMessageChannel: (channel: string) => + ["directchat", "workspace", "telegram"].includes(channel), isGatewayMessageChannel: (channel: string) => - ["directchat", "workspace", "webchat"].includes(channel), + ["directchat", "workspace", "telegram", "webchat"].includes(channel), normalizeMessageChannel: (value: string) => value.trim().toLowerCase(), })); @@ -106,7 +120,18 @@ beforeAll(async () => { beforeEach(() => { mocks.resolveOutboundChannelPlugin.mockReset(); mocks.resolveOutboundChannelPlugin.mockReturnValue(null); - mocks.resolveOutboundTarget.mockClear(); + mocks.resolveChannelTarget.mockReset(); + mocks.resolveChannelTarget.mockResolvedValue({ + ok: true, + target: { + to: "+1999", + kind: "group", + source: "normalized", + resolutionSource: "normalized", + }, + }); + mocks.resolveOutboundTarget.mockReset(); + mocks.resolveOutboundTarget.mockReturnValue({ ok: true, to: "+1999" }); mocks.resolveOutboundSessionRoute.mockReset(); mocks.resolveOutboundSessionRoute.mockResolvedValue(null); mocks.resolveSessionDeliveryTarget.mockClear(); @@ -313,31 +338,109 @@ describe("agent delivery helpers", () => { expect(plan.resolvedTo).toBe("1470130713209602050"); }); - it("defers reserved-literal errors to async session-route resolution", async () => { + it("resolves reserved explicit targets through directory-capable resolution before session routing", async () => { mocks.resolveOutboundChannelPlugin.mockReturnValue({ - messaging: { resolveOutboundSessionRoute: vi.fn() }, + messaging: { resolveOutboundSessionRoute: vi.fn(), targetResolver: {} }, }); mocks.resolveOutboundTarget.mockReturnValueOnce({ ok: false, error: new Error('Reserved target "current" for Telegram'), }); + mocks.resolveChannelTarget.mockResolvedValueOnce({ + ok: true, + target: { + to: "telegram:-1002458651455", + kind: "group", + source: "directory", + resolutionSource: "directory", + }, + }); + mocks.resolveOutboundSessionRoute.mockResolvedValueOnce({ + sessionKey: "agent:telegram:group:-1002458651455", + baseSessionKey: "agent:telegram:group:-1002458651455", + peer: { kind: "group", id: "-1002458651455" }, + chatType: "group", + from: "telegram:group:-1002458651455", + to: "telegram:-1002458651455", + }); + + const plan = await resolveAgentDeliveryPlanWithSessionRoute({ + cfg: {} as OpenClawConfig, + agentId: "agent", + currentSessionKey: "agent:main", + sessionEntry: undefined, + requestedChannel: "telegram", + explicitTo: "current", + accountId: "work", + wantsDelivery: true, + }); + + expect(mocks.resolveChannelTarget).toHaveBeenCalledWith({ + cfg: {}, + channel: "telegram", + input: "current", + accountId: "work", + unknownTargetMode: "normalized", + plugin: { + messaging: { resolveOutboundSessionRoute: expect.any(Function), targetResolver: {} }, + }, + }); + expect(mocks.resolveOutboundSessionRoute).toHaveBeenCalledWith({ + cfg: {}, + channel: "telegram", + agentId: "agent", + accountId: "work", + target: "telegram:-1002458651455", + resolvedTarget: { + to: "telegram:-1002458651455", + kind: "group", + source: "directory", + resolutionSource: "directory", + }, + currentSessionKey: "agent:main", + threadId: undefined, + }); + expect(plan.resolvedTo).toBe("telegram:-1002458651455"); + expect(plan.targetResolutionError).toBeUndefined(); + }); + + it("keeps reserved explicit target errors when directory-capable resolution misses", async () => { + const reservedError = new Error('Reserved target "current" for Telegram'); + mocks.resolveOutboundChannelPlugin.mockReturnValue({ + messaging: { resolveOutboundSessionRoute: vi.fn(), targetResolver: {} }, + }); + mocks.resolveOutboundTarget.mockReturnValueOnce({ + ok: false, + error: reservedError, + }); + mocks.resolveChannelTarget.mockResolvedValueOnce({ + ok: false, + error: reservedError, + }); const plan = await resolveAgentDeliveryPlanWithSessionRoute({ cfg: {} as OpenClawConfig, agentId: "agent", sessionEntry: undefined, - requestedChannel: "workspace", + requestedChannel: "telegram", explicitTo: "current", accountId: undefined, wantsDelivery: true, }); - // Reserved-literal errors do not block session route resolution; the - // async resolver (resolveMessagingTarget) does directory-first lookup - // before rejecting reserved literals. - expect(mocks.resolveOutboundSessionRoute).toHaveBeenCalled(); + expect(mocks.resolveChannelTarget).toHaveBeenCalledWith({ + cfg: {}, + channel: "telegram", + input: "current", + accountId: undefined, + unknownTargetMode: "normalized", + plugin: { + messaging: { resolveOutboundSessionRoute: expect.any(Function), targetResolver: {} }, + }, + }); + expect(mocks.resolveOutboundSessionRoute).not.toHaveBeenCalled(); expect(plan.resolvedTo).toBe("current"); - expect(plan.targetResolutionError).toBeUndefined(); + expect(plan.targetResolutionError).toBe(reservedError); }); it("surfaces stored explicit target errors even when explicit validation is disabled", () => { diff --git a/src/infra/outbound/agent-delivery.ts b/src/infra/outbound/agent-delivery.ts index 792e300f1d8f..0722e97e65b3 100644 --- a/src/infra/outbound/agent-delivery.ts +++ b/src/infra/outbound/agent-delivery.ts @@ -15,6 +15,7 @@ import { } from "../../utils/message-channel.js"; import { resolveOutboundChannelPlugin } from "./channel-resolution.js"; import { resolveOutboundSessionRoute } from "./outbound-session.js"; +import { resolveChannelTarget, type ResolvedMessagingTarget } from "./target-resolver.js"; import type { OutboundTargetResolution } from "./targets.js"; import { resolveOutboundTarget, @@ -156,6 +157,11 @@ export async function resolveAgentDeliveryPlanWithSessionRoute( ) { return plan; } + const plugin = resolveOutboundChannelPlugin({ + channel: plan.resolvedChannel, + cfg: params.cfg, + allowBootstrap: true, + }); const normalizedTarget = resolveOutboundTarget({ channel: plan.resolvedChannel, to: plan.resolvedTo, @@ -163,16 +169,28 @@ export async function resolveAgentDeliveryPlanWithSessionRoute( accountId: plan.resolvedAccountId, mode: plan.deliveryTargetMode ?? "explicit", }); - // Reserved-literal errors are not fatal for explicit delivery: the async - // session-route resolver (resolveChannelTarget → resolveMessagingTarget) - // does directory-first lookup before rejecting reserved literals, so a - // configured directory entry named like a reserved word still resolves. - const isReservedLiteralError = - !normalizedTarget.ok && normalizedTarget.error.message.includes("Reserved target"); - if (!normalizedTarget.ok && !isReservedLiteralError) { - return { ...plan, targetResolutionError: normalizedTarget.error }; + let sessionRouteTarget: string; + let resolvedSessionRouteTarget: ResolvedMessagingTarget | undefined; + if (normalizedTarget.ok) { + sessionRouteTarget = normalizedTarget.to; + } else { + if (!normalizedTarget.error.message.includes("Reserved target")) { + return { ...plan, targetResolutionError: normalizedTarget.error }; + } + const resolvedTarget = await resolveChannelTarget({ + cfg: params.cfg, + channel: plan.resolvedChannel as ChannelId, + input: plan.resolvedTo, + accountId: plan.resolvedAccountId, + unknownTargetMode: "normalized", + plugin, + }); + if (!resolvedTarget.ok) { + return { ...plan, targetResolutionError: resolvedTarget.error }; + } + sessionRouteTarget = resolvedTarget.target.to; + resolvedSessionRouteTarget = resolvedTarget.target; } - const sessionRouteTarget = normalizedTarget.ok ? normalizedTarget.to : (plan.resolvedTo ?? ""); const explicitThreadId = params.explicitThreadId != null && params.explicitThreadId !== "" ? params.explicitThreadId @@ -185,6 +203,7 @@ export async function resolveAgentDeliveryPlanWithSessionRoute( agentId: params.agentId, accountId: plan.resolvedAccountId, target: sessionRouteTarget, + ...(resolvedSessionRouteTarget ? { resolvedTarget: resolvedSessionRouteTarget } : {}), currentSessionKey: params.currentSessionKey, threadId: plan.deliveryTargetMode === "explicit" ? explicitThreadId : plan.resolvedThreadId, }); diff --git a/src/infra/outbound/target-resolver.test.ts b/src/infra/outbound/target-resolver.test.ts index 12139cead5c2..6795355da129 100644 --- a/src/infra/outbound/target-resolver.test.ts +++ b/src/infra/outbound/target-resolver.test.ts @@ -171,6 +171,78 @@ describe("resolveMessagingTarget (directory fallback)", () => { expect(mocks.resolveTarget).not.toHaveBeenCalled(); }); + it("keeps reserved literals on the directory path before id-like plugin normalization", async () => { + mocks.getChannelPlugin.mockReturnValue({ + ...createChannelTestPluginBase({ id: "telegram", label: "Telegram" }), + directory: { + listPeers: mocks.listPeers, + listPeersLive: mocks.listPeersLive, + listGroups: mocks.listGroups, + listGroupsLive: mocks.listGroupsLive, + }, + messaging: { + normalizeTarget: (raw: string) => + raw === "current" || raw === "telegram:current" ? "telegram:@current" : raw, + targetResolver: { + looksLikeId: (raw: string) => raw === "current" || raw === "telegram:current", + reservedLiterals: ["current", "self", "this", "me"], + hint: "", + resolveTarget: mocks.resolveTarget, + }, + }, + }); + mocks.listGroups.mockResolvedValueOnce([ + { kind: "group", id: "room-1", name: "current" } satisfies ChannelDirectoryEntry, + ]); + + const hit = await resolveMessagingTarget({ + cfg, + channel: "telegram", + input: "current", + }); + + expect(hit.ok).toBe(true); + if (hit.ok) { + expect(hit.target.to).toBe("room-1"); + expect(hit.target.source).toBe("directory"); + } + expect(mocks.resolveTarget).not.toHaveBeenCalled(); + + resetDirectoryCache(); + mocks.listGroups.mockResolvedValueOnce([ + { kind: "group", id: "room-1", name: "current" } satisfies ChannelDirectoryEntry, + ]); + + const prefixedHit = await resolveMessagingTarget({ + cfg, + channel: "telegram", + input: "telegram:current", + }); + + expect(prefixedHit.ok).toBe(true); + if (prefixedHit.ok) { + expect(prefixedHit.target.to).toBe("room-1"); + expect(prefixedHit.target.source).toBe("directory"); + } + + resetDirectoryCache(); + mocks.listGroups.mockResolvedValueOnce([]); + mocks.listGroupsLive.mockResolvedValueOnce([]); + + const miss = await resolveMessagingTarget({ + cfg, + channel: "telegram", + input: "current", + }); + + expect(miss.ok).toBe(false); + if (!miss.ok) { + expect(miss.error.message).toContain('Reserved target "current"'); + expect(miss.error.message).toContain("Telegram"); + } + expect(mocks.resolveTarget).not.toHaveBeenCalled(); + }); + it("rejects reserved literal targets after directory miss", async () => { mocks.getChannelPlugin.mockReturnValue({ ...createChannelTestPluginBase({ id: "telegram", label: "Telegram" }), diff --git a/src/infra/outbound/target-resolver.ts b/src/infra/outbound/target-resolver.ts index c84060009f6b..84a7061d23f1 100644 --- a/src/infra/outbound/target-resolver.ts +++ b/src/infra/outbound/target-resolver.ts @@ -96,9 +96,21 @@ function normalizeQuery(value: string): string { return normalizeLowercaseStringOrEmpty(value); } -function stripTargetPrefixes(value: string): string { - return value - .replace(/^(channel|user):/i, "") +function stripTargetPrefixes(value: string, channel?: ChannelId, plugin?: ChannelPlugin): string { + const providerPrefixes = [channel, plugin?.id, ...(plugin?.messaging?.targetPrefixes ?? [])] + .map((prefix) => (prefix ? String(prefix).trim().toLowerCase() : "")) + .filter(Boolean); + let target = value.trim(); + while (target) { + const lowered = target.toLowerCase(); + const prefix = providerPrefixes.find((candidate) => lowered.startsWith(`${candidate}:`)); + if (!prefix) { + break; + } + target = target.slice(prefix.length + 1).trim(); + } + return target + .replace(/^(channel|group|user):/i, "") .replace(/^[@#]/, "") .trim(); } @@ -222,9 +234,15 @@ function matchesDirectoryEntry(params: { } const id = stripTargetPrefixes( normalizeDirectoryEntryId(params.channel, params.entry, params.plugin), + params.channel, + params.plugin, ); - const name = params.entry.name ? stripTargetPrefixes(params.entry.name) : ""; - const handle = params.entry.handle ? stripTargetPrefixes(params.entry.handle) : ""; + const name = params.entry.name + ? stripTargetPrefixes(params.entry.name, params.channel, params.plugin) + : ""; + const handle = params.entry.handle + ? stripTargetPrefixes(params.entry.handle, params.channel, params.plugin) + : ""; const candidates = [id, name, handle].map((value) => normalizeQuery(value)).filter(Boolean); return candidates.some((value) => value === query || value.includes(query)); } @@ -403,8 +421,10 @@ export async function resolveMessagingTarget(params: { const kind = detectTargetKind(params.channel, raw, params.preferredKind, plugin); const normalizedInput = resolveNormalizedTargetInput(params.channel, raw, plugin); const normalized = normalizedInput?.normalized ?? raw; + const reservedLiteral = resolveReservedTargetLiteral({ raw, plugin }); if ( normalizedInput && + !reservedLiteral && looksLikeTargetId({ channel: params.channel, raw: normalizedInput.raw, @@ -431,7 +451,7 @@ export async function resolveMessagingTarget(params: { kind, }); } - const query = stripTargetPrefixes(raw); + const query = stripTargetPrefixes(raw, params.channel, plugin); const entries = await getDirectoryEntries({ cfg: params.cfg, channel: params.channel, @@ -450,7 +470,8 @@ export async function resolveMessagingTarget(params: { target: { to: normalizeDirectoryEntryId(params.channel, entry, plugin), kind, - display: entry.name ?? entry.handle ?? stripTargetPrefixes(entry.id), + display: + entry.name ?? entry.handle ?? stripTargetPrefixes(entry.id, params.channel, plugin), source: "directory", resolutionSource: "directory", }, @@ -466,7 +487,8 @@ export async function resolveMessagingTarget(params: { target: { to: normalizeDirectoryEntryId(params.channel, best, plugin), kind, - display: best.name ?? best.handle ?? stripTargetPrefixes(best.id), + display: + best.name ?? best.handle ?? stripTargetPrefixes(best.id, params.channel, plugin), source: "directory", resolutionSource: "directory", }, @@ -482,7 +504,6 @@ export async function resolveMessagingTarget(params: { // Directory miss: reject reserved literals before falling back to plugin // resolution, so a bare reserved word without a matching directory entry // does not accidentally resolve to a public channel or incorrect target. - const reservedLiteral = resolveReservedTargetLiteral({ raw, plugin }); if (reservedLiteral) { return { ok: false, error: reservedTargetLiteralError(providerLabel, reservedLiteral, hint) }; } diff --git a/src/infra/outbound/targets-resolve-shared.ts b/src/infra/outbound/targets-resolve-shared.ts index a591d2df3b11..d6bcd39eb00b 100644 --- a/src/infra/outbound/targets-resolve-shared.ts +++ b/src/infra/outbound/targets-resolve-shared.ts @@ -80,13 +80,13 @@ export function resolveOutboundTargetWithPlugin(params: { if (targetPrefixError) { return { ok: false, error: targetPrefixError }; } + const hint = plugin.messaging?.targetResolver?.hint; // Reserved-literal rejection is skipped for heartbeat mode so the // async directory-capable resolver (resolveChannelTarget → // resolveMessagingTarget) can do directory-first lookup before deciding. // Rejecting here would suppress a heartbeat route to an existing directory // entry whose name matches a reserved literal. if (params.target.mode !== "heartbeat") { - const hint = plugin.messaging?.targetResolver?.hint; const reservedLiteral = resolveReservedTargetLiteral({ raw: effectiveTo, plugin }); if (reservedLiteral) { return {