mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 11:55:47 -06:00
fix(outbound): preserve reserved Telegram directory targets
This commit is contained in:
committed by
Ayaan Zaidi
parent
5fccf06b5f
commit
cd3793185b
@@ -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<unknown>>(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", () => {
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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: "<chatId>",
|
||||
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" }),
|
||||
|
||||
@@ -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) };
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user