From c0bf42f2a8754b4736dc5991c63313db9525b90a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 04:04:02 +0000 Subject: [PATCH] refactor: centralize delivery/path/media/version lifecycle --- extensions/mattermost/src/channel.ts | 3 +- .../mattermost/src/mattermost/send.test.ts | 100 +++++++++++ extensions/mattermost/src/mattermost/send.ts | 6 +- extensions/msteams/src/outbound.ts | 13 +- extensions/msteams/src/send.test.ts | 109 ++++++++++++ extensions/msteams/src/send.ts | 16 +- src/agents/apply-patch.ts | 21 +-- src/agents/path-policy.ts | 72 ++++++++ src/agents/pi-tools.read.ts | 31 +--- src/infra/outbound/deliver.ts | 2 +- src/plugin-sdk/index.ts | 2 + src/plugin-sdk/outbound-media.test.ts | 43 +++++ src/plugin-sdk/outbound-media.ts | 16 ++ src/plugins/loader.test.ts | 31 ++++ src/plugins/loader.ts | 10 +- src/telegram/bot/delivery.ts | 155 ++++++++---------- src/telegram/bot/reply-threading.ts | 76 +++++++++ src/version.test.ts | 37 +++++ src/version.ts | 25 ++- 19 files changed, 616 insertions(+), 152 deletions(-) create mode 100644 extensions/mattermost/src/mattermost/send.test.ts create mode 100644 extensions/msteams/src/send.test.ts create mode 100644 src/agents/path-policy.ts create mode 100644 src/plugin-sdk/outbound-media.test.ts create mode 100644 src/plugin-sdk/outbound-media.ts create mode 100644 src/telegram/bot/reply-threading.ts diff --git a/extensions/mattermost/src/channel.ts b/extensions/mattermost/src/channel.ts index 5053026f49a..ea9ad100a9c 100644 --- a/extensions/mattermost/src/channel.ts +++ b/extensions/mattermost/src/channel.ts @@ -279,10 +279,11 @@ export const mattermostPlugin: ChannelPlugin = { }); return { channel: "mattermost", ...result }; }, - sendMedia: async ({ to, text, mediaUrl, accountId, replyToId }) => { + sendMedia: async ({ to, text, mediaUrl, mediaLocalRoots, accountId, replyToId }) => { const result = await sendMessageMattermost(to, text, { accountId: accountId ?? undefined, mediaUrl, + mediaLocalRoots, replyToId: replyToId ?? undefined, }); return { channel: "mattermost", ...result }; diff --git a/extensions/mattermost/src/mattermost/send.test.ts b/extensions/mattermost/src/mattermost/send.test.ts new file mode 100644 index 00000000000..1176cbfa7d1 --- /dev/null +++ b/extensions/mattermost/src/mattermost/send.test.ts @@ -0,0 +1,100 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { sendMessageMattermost } from "./send.js"; + +const mockState = vi.hoisted(() => ({ + loadOutboundMediaFromUrl: vi.fn(), + createMattermostClient: vi.fn(), + createMattermostDirectChannel: vi.fn(), + createMattermostPost: vi.fn(), + fetchMattermostMe: vi.fn(), + fetchMattermostUserByUsername: vi.fn(), + normalizeMattermostBaseUrl: vi.fn((input: string | undefined) => input?.trim() ?? ""), + uploadMattermostFile: vi.fn(), +})); + +vi.mock("openclaw/plugin-sdk", () => ({ + loadOutboundMediaFromUrl: mockState.loadOutboundMediaFromUrl, +})); + +vi.mock("./accounts.js", () => ({ + resolveMattermostAccount: () => ({ + accountId: "default", + botToken: "bot-token", + baseUrl: "https://mattermost.example.com", + }), +})); + +vi.mock("./client.js", () => ({ + createMattermostClient: mockState.createMattermostClient, + createMattermostDirectChannel: mockState.createMattermostDirectChannel, + createMattermostPost: mockState.createMattermostPost, + fetchMattermostMe: mockState.fetchMattermostMe, + fetchMattermostUserByUsername: mockState.fetchMattermostUserByUsername, + normalizeMattermostBaseUrl: mockState.normalizeMattermostBaseUrl, + uploadMattermostFile: mockState.uploadMattermostFile, +})); + +vi.mock("../runtime.js", () => ({ + getMattermostRuntime: () => ({ + config: { + loadConfig: () => ({}), + }, + logging: { + shouldLogVerbose: () => false, + getChildLogger: () => ({ debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }), + }, + channel: { + text: { + resolveMarkdownTableMode: () => "off", + convertMarkdownTables: (text: string) => text, + }, + activity: { + record: vi.fn(), + }, + }, + }), +})); + +describe("sendMessageMattermost", () => { + beforeEach(() => { + mockState.loadOutboundMediaFromUrl.mockReset(); + mockState.createMattermostClient.mockReset(); + mockState.createMattermostDirectChannel.mockReset(); + mockState.createMattermostPost.mockReset(); + mockState.fetchMattermostMe.mockReset(); + mockState.fetchMattermostUserByUsername.mockReset(); + mockState.uploadMattermostFile.mockReset(); + mockState.createMattermostClient.mockReturnValue({}); + mockState.createMattermostPost.mockResolvedValue({ id: "post-1" }); + mockState.uploadMattermostFile.mockResolvedValue({ id: "file-1" }); + }); + + it("loads outbound media with trusted local roots before upload", async () => { + mockState.loadOutboundMediaFromUrl.mockResolvedValueOnce({ + buffer: Buffer.from("media-bytes"), + fileName: "photo.png", + contentType: "image/png", + kind: "image", + }); + + await sendMessageMattermost("channel:town-square", "hello", { + mediaUrl: "file:///tmp/agent-workspace/photo.png", + mediaLocalRoots: ["/tmp/agent-workspace"], + }); + + expect(mockState.loadOutboundMediaFromUrl).toHaveBeenCalledWith( + "file:///tmp/agent-workspace/photo.png", + { + mediaLocalRoots: ["/tmp/agent-workspace"], + }, + ); + expect(mockState.uploadMattermostFile).toHaveBeenCalledWith( + {}, + expect.objectContaining({ + channelId: "town-square", + fileName: "photo.png", + contentType: "image/png", + }), + ); + }); +}); diff --git a/extensions/mattermost/src/mattermost/send.ts b/extensions/mattermost/src/mattermost/send.ts index b3e40e39ca3..8732d2400db 100644 --- a/extensions/mattermost/src/mattermost/send.ts +++ b/extensions/mattermost/src/mattermost/send.ts @@ -1,3 +1,4 @@ +import { loadOutboundMediaFromUrl } from "openclaw/plugin-sdk"; import { getMattermostRuntime } from "../runtime.js"; import { resolveMattermostAccount } from "./accounts.js"; import { @@ -16,6 +17,7 @@ export type MattermostSendOpts = { baseUrl?: string; accountId?: string; mediaUrl?: string; + mediaLocalRoots?: readonly string[]; replyToId?: string; }; @@ -176,7 +178,9 @@ export async function sendMessageMattermost( const mediaUrl = opts.mediaUrl?.trim(); if (mediaUrl) { try { - const media = await core.media.loadWebMedia(mediaUrl); + const media = await loadOutboundMediaFromUrl(mediaUrl, { + mediaLocalRoots: opts.mediaLocalRoots, + }); const fileInfo = await uploadMattermostFile(client, { channelId, buffer: media.buffer, diff --git a/extensions/msteams/src/outbound.ts b/extensions/msteams/src/outbound.ts index 48f5d0c61af..3a401f13d9c 100644 --- a/extensions/msteams/src/outbound.ts +++ b/extensions/msteams/src/outbound.ts @@ -14,11 +14,18 @@ export const msteamsOutbound: ChannelOutboundAdapter = { const result = await send(to, text); return { channel: "msteams", ...result }; }, - sendMedia: async ({ cfg, to, text, mediaUrl, deps }) => { + sendMedia: async ({ cfg, to, text, mediaUrl, mediaLocalRoots, deps }) => { const send = deps?.sendMSTeams ?? - ((to, text, opts) => sendMessageMSTeams({ cfg, to, text, mediaUrl: opts?.mediaUrl })); - const result = await send(to, text, { mediaUrl }); + ((to, text, opts) => + sendMessageMSTeams({ + cfg, + to, + text, + mediaUrl: opts?.mediaUrl, + mediaLocalRoots: opts?.mediaLocalRoots, + })); + const result = await send(to, text, { mediaUrl, mediaLocalRoots }); return { channel: "msteams", ...result }; }, sendPoll: async ({ cfg, to, poll }) => { diff --git a/extensions/msteams/src/send.test.ts b/extensions/msteams/src/send.test.ts new file mode 100644 index 00000000000..cbab8459dd9 --- /dev/null +++ b/extensions/msteams/src/send.test.ts @@ -0,0 +1,109 @@ +import type { OpenClawConfig } from "openclaw/plugin-sdk"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { sendMessageMSTeams } from "./send.js"; + +const mockState = vi.hoisted(() => ({ + loadOutboundMediaFromUrl: vi.fn(), + resolveMSTeamsSendContext: vi.fn(), + requiresFileConsent: vi.fn(), + prepareFileConsentActivity: vi.fn(), + extractFilename: vi.fn(async () => "fallback.bin"), + sendMSTeamsMessages: vi.fn(), +})); + +vi.mock("openclaw/plugin-sdk", () => ({ + loadOutboundMediaFromUrl: mockState.loadOutboundMediaFromUrl, +})); + +vi.mock("./send-context.js", () => ({ + resolveMSTeamsSendContext: mockState.resolveMSTeamsSendContext, +})); + +vi.mock("./file-consent-helpers.js", () => ({ + requiresFileConsent: mockState.requiresFileConsent, + prepareFileConsentActivity: mockState.prepareFileConsentActivity, +})); + +vi.mock("./media-helpers.js", () => ({ + extractFilename: mockState.extractFilename, + extractMessageId: () => "message-1", +})); + +vi.mock("./messenger.js", () => ({ + sendMSTeamsMessages: mockState.sendMSTeamsMessages, + buildConversationReference: () => ({}), +})); + +vi.mock("./runtime.js", () => ({ + getMSTeamsRuntime: () => ({ + channel: { + text: { + resolveMarkdownTableMode: () => "off", + convertMarkdownTables: (text: string) => text, + }, + }, + }), +})); + +describe("sendMessageMSTeams", () => { + beforeEach(() => { + mockState.loadOutboundMediaFromUrl.mockReset(); + mockState.resolveMSTeamsSendContext.mockReset(); + mockState.requiresFileConsent.mockReset(); + mockState.prepareFileConsentActivity.mockReset(); + mockState.extractFilename.mockReset(); + mockState.sendMSTeamsMessages.mockReset(); + + mockState.extractFilename.mockResolvedValue("fallback.bin"); + mockState.requiresFileConsent.mockReturnValue(false); + mockState.resolveMSTeamsSendContext.mockResolvedValue({ + adapter: {}, + appId: "app-id", + conversationId: "19:conversation@thread.tacv2", + ref: {}, + log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + conversationType: "personal", + tokenProvider: { getAccessToken: vi.fn(async () => "token") }, + mediaMaxBytes: 8 * 1024, + sharePointSiteId: undefined, + }); + mockState.sendMSTeamsMessages.mockResolvedValue(["message-1"]); + }); + + it("loads media through shared helper and forwards mediaLocalRoots", async () => { + const mediaBuffer = Buffer.from("tiny-image"); + mockState.loadOutboundMediaFromUrl.mockResolvedValueOnce({ + buffer: mediaBuffer, + contentType: "image/png", + fileName: "inline.png", + kind: "image", + }); + + await sendMessageMSTeams({ + cfg: {} as OpenClawConfig, + to: "conversation:19:conversation@thread.tacv2", + text: "hello", + mediaUrl: "file:///tmp/agent-workspace/inline.png", + mediaLocalRoots: ["/tmp/agent-workspace"], + }); + + expect(mockState.loadOutboundMediaFromUrl).toHaveBeenCalledWith( + "file:///tmp/agent-workspace/inline.png", + { + maxBytes: 8 * 1024, + mediaLocalRoots: ["/tmp/agent-workspace"], + }, + ); + + expect(mockState.sendMSTeamsMessages).toHaveBeenCalledWith( + expect.objectContaining({ + messages: [ + expect.objectContaining({ + text: "hello", + mediaUrl: `data:image/png;base64,${mediaBuffer.toString("base64")}`, + }), + ], + }), + ); + }); +}); diff --git a/extensions/msteams/src/send.ts b/extensions/msteams/src/send.ts index c4f801b0332..2ddb12df116 100644 --- a/extensions/msteams/src/send.ts +++ b/extensions/msteams/src/send.ts @@ -1,5 +1,5 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk"; -import { loadWebMedia, resolveChannelMediaMaxBytes } from "openclaw/plugin-sdk"; +import { loadOutboundMediaFromUrl } from "openclaw/plugin-sdk"; import { createMSTeamsConversationStoreFs } from "./conversation-store-fs.js"; import { classifyMSTeamsSendError, @@ -28,6 +28,7 @@ export type SendMSTeamsMessageParams = { text: string; /** Optional media URL */ mediaUrl?: string; + mediaLocalRoots?: readonly string[]; }; export type SendMSTeamsMessageResult = { @@ -93,7 +94,7 @@ export type SendMSTeamsCardResult = { export async function sendMessageMSTeams( params: SendMSTeamsMessageParams, ): Promise { - const { cfg, to, text, mediaUrl } = params; + const { cfg, to, text, mediaUrl, mediaLocalRoots } = params; const tableMode = getMSTeamsRuntime().channel.text.resolveMarkdownTableMode({ cfg, channel: "msteams", @@ -120,12 +121,11 @@ export async function sendMessageMSTeams( // Handle media if present if (mediaUrl) { - const mediaMaxBytes = - resolveChannelMediaMaxBytes({ - cfg, - resolveChannelLimitMb: ({ cfg }) => cfg.channels?.msteams?.mediaMaxMb, - }) ?? MSTEAMS_MAX_MEDIA_BYTES; - const media = await loadWebMedia(mediaUrl, mediaMaxBytes); + const mediaMaxBytes = ctx.mediaMaxBytes ?? MSTEAMS_MAX_MEDIA_BYTES; + const media = await loadOutboundMediaFromUrl(mediaUrl, { + maxBytes: mediaMaxBytes, + mediaLocalRoots, + }); const isLargeFile = media.buffer.length >= FILE_CONSENT_THRESHOLD_BYTES; const isImage = media.contentType?.startsWith("image/") ?? false; const fallbackFileName = await extractFilename(mediaUrl); diff --git a/src/agents/apply-patch.ts b/src/agents/apply-patch.ts index cc3bf7df07c..9c948cb3971 100644 --- a/src/agents/apply-patch.ts +++ b/src/agents/apply-patch.ts @@ -7,7 +7,8 @@ import { openBoundaryFile, type BoundaryFileOpenResult } from "../infra/boundary import { writeFileWithinRoot } from "../infra/fs-safe.js"; import { PATH_ALIAS_POLICIES, type PathAliasPolicy } from "../infra/path-alias-guards.js"; import { applyUpdateHunk } from "./apply-patch-update.js"; -import { assertSandboxPath, resolveSandboxInputPath } from "./sandbox-paths.js"; +import { toRelativeSandboxPath, resolvePathFromInput } from "./path-policy.js"; +import { assertSandboxPath } from "./sandbox-paths.js"; import type { SandboxFsBridge } from "./sandbox/fs-bridge.js"; const BEGIN_PATCH_MARKER = "*** Begin Patch"; @@ -261,7 +262,7 @@ function resolvePatchFileOps(options: ApplyPatchOptions): PatchFileOps { await fs.writeFile(filePath, content, "utf8"); return; } - const relative = toRelativeWorkspacePath(options.cwd, filePath); + const relative = toRelativeSandboxPath(options.cwd, filePath); await writeFileWithinRoot({ rootDir: options.cwd, relativePath: relative, @@ -318,27 +319,13 @@ async function resolvePatchPath( allowFinalHardlinkForUnlink: aliasPolicy.allowFinalHardlinkForUnlink, }) ).resolved - : resolvePathFromCwd(filePath, options.cwd); + : resolvePathFromInput(filePath, options.cwd); return { resolved, display: toDisplayPath(resolved, options.cwd), }; } -function resolvePathFromCwd(filePath: string, cwd: string): string { - return path.normalize(resolveSandboxInputPath(filePath, cwd)); -} - -function toRelativeWorkspacePath(workspaceRoot: string, absolutePath: string): string { - const rootResolved = path.resolve(workspaceRoot); - const resolved = path.resolve(absolutePath); - const relative = path.relative(rootResolved, resolved); - if (!relative || relative === "." || relative.startsWith("..") || path.isAbsolute(relative)) { - throw new Error(`Path escapes sandbox root (${workspaceRoot}): ${absolutePath}`); - } - return relative; -} - function assertBoundaryRead( opened: BoundaryFileOpenResult, targetPath: string, diff --git a/src/agents/path-policy.ts b/src/agents/path-policy.ts new file mode 100644 index 00000000000..f4eb8e32292 --- /dev/null +++ b/src/agents/path-policy.ts @@ -0,0 +1,72 @@ +import path from "node:path"; +import { resolveSandboxInputPath } from "./sandbox-paths.js"; + +type RelativePathOptions = { + allowRoot?: boolean; + cwd?: string; + boundaryLabel?: string; + includeRootInError?: boolean; +}; + +function toRelativePathUnderRoot(params: { + root: string; + candidate: string; + options?: RelativePathOptions; +}): string { + const rootResolved = path.resolve(params.root); + const resolvedCandidate = path.resolve( + resolveSandboxInputPath(params.candidate, params.options?.cwd ?? params.root), + ); + const relative = path.relative(rootResolved, resolvedCandidate); + if (relative === "" || relative === ".") { + if (params.options?.allowRoot) { + return ""; + } + const boundary = params.options?.boundaryLabel ?? "workspace root"; + const suffix = params.options?.includeRootInError ? ` (${rootResolved})` : ""; + throw new Error(`Path escapes ${boundary}${suffix}: ${params.candidate}`); + } + if (relative.startsWith("..") || path.isAbsolute(relative)) { + const boundary = params.options?.boundaryLabel ?? "workspace root"; + const suffix = params.options?.includeRootInError ? ` (${rootResolved})` : ""; + throw new Error(`Path escapes ${boundary}${suffix}: ${params.candidate}`); + } + return relative; +} + +export function toRelativeWorkspacePath( + root: string, + candidate: string, + options?: Pick, +): string { + return toRelativePathUnderRoot({ + root, + candidate, + options: { + allowRoot: options?.allowRoot, + cwd: options?.cwd, + boundaryLabel: "workspace root", + }, + }); +} + +export function toRelativeSandboxPath( + root: string, + candidate: string, + options?: Pick, +): string { + return toRelativePathUnderRoot({ + root, + candidate, + options: { + allowRoot: options?.allowRoot, + cwd: options?.cwd, + boundaryLabel: "sandbox root", + includeRootInError: true, + }, + }); +} + +export function resolvePathFromInput(filePath: string, cwd: string): string { + return path.normalize(resolveSandboxInputPath(filePath, cwd)); +} diff --git a/src/agents/pi-tools.read.ts b/src/agents/pi-tools.read.ts index 0b5ff58478d..f0fa6d2e2e3 100644 --- a/src/agents/pi-tools.read.ts +++ b/src/agents/pi-tools.read.ts @@ -12,6 +12,7 @@ import { import { detectMime } from "../media/mime.js"; import { sniffMimeFromBase64 } from "../media/sniff-mime-from-base64.js"; import type { ImageSanitizationLimits } from "./image-sanitization.js"; +import { toRelativeWorkspacePath } from "./path-policy.js"; import type { AnyAgentTool } from "./pi-tools.types.js"; import { assertSandboxPath } from "./sandbox-paths.js"; import type { SandboxFsBridge } from "./sandbox/fs-bridge.js"; @@ -784,13 +785,13 @@ function createHostWriteOperations(root: string, options?: { workspaceOnly?: boo // When workspaceOnly is true, enforce workspace boundary return { mkdir: async (dir: string) => { - const relative = toRelativePathInRoot(root, dir, { allowRoot: true }); + const relative = toRelativeWorkspacePath(root, dir, { allowRoot: true }); const resolved = relative ? path.resolve(root, relative) : path.resolve(root); await assertSandboxPath({ filePath: resolved, cwd: root, root }); await fs.mkdir(resolved, { recursive: true }); }, writeFile: async (absolutePath: string, content: string) => { - const relative = toRelativePathInRoot(root, absolutePath); + const relative = toRelativeWorkspacePath(root, absolutePath); await writeFileWithinRoot({ rootDir: root, relativePath: relative, @@ -827,7 +828,7 @@ function createHostEditOperations(root: string, options?: { workspaceOnly?: bool // When workspaceOnly is true, enforce workspace boundary return { readFile: async (absolutePath: string) => { - const relative = toRelativePathInRoot(root, absolutePath); + const relative = toRelativeWorkspacePath(root, absolutePath); const safeRead = await readFileWithinRoot({ rootDir: root, relativePath: relative, @@ -835,7 +836,7 @@ function createHostEditOperations(root: string, options?: { workspaceOnly?: bool return safeRead.buffer; }, writeFile: async (absolutePath: string, content: string) => { - const relative = toRelativePathInRoot(root, absolutePath); + const relative = toRelativeWorkspacePath(root, absolutePath); await writeFileWithinRoot({ rootDir: root, relativePath: relative, @@ -846,7 +847,7 @@ function createHostEditOperations(root: string, options?: { workspaceOnly?: bool access: async (absolutePath: string) => { let relative: string; try { - relative = toRelativePathInRoot(root, absolutePath); + relative = toRelativeWorkspacePath(root, absolutePath); } catch { // Path escapes workspace root. Don't throw here – the upstream // library replaces any `access` error with a misleading "File not @@ -876,26 +877,6 @@ function createHostEditOperations(root: string, options?: { workspaceOnly?: bool } as const; } -function toRelativePathInRoot( - root: string, - candidate: string, - options?: { allowRoot?: boolean }, -): string { - const rootResolved = path.resolve(root); - const resolved = path.resolve(candidate); - const relative = path.relative(rootResolved, resolved); - if (relative === "" || relative === ".") { - if (options?.allowRoot) { - return ""; - } - throw new Error(`Path escapes workspace root: ${candidate}`); - } - if (relative.startsWith("..") || path.isAbsolute(relative)) { - throw new Error(`Path escapes workspace root: ${candidate}`); - } - return relative; -} - function createFsAccessError(code: string, filePath: string): NodeJS.ErrnoException { const error = new Error(`Sandbox FS error (${code}): ${filePath}`) as NodeJS.ErrnoException; error.code = code; diff --git a/src/infra/outbound/deliver.ts b/src/infra/outbound/deliver.ts index 9002245ab3c..a6acc956941 100644 --- a/src/infra/outbound/deliver.ts +++ b/src/infra/outbound/deliver.ts @@ -59,7 +59,7 @@ export type OutboundSendDeps = { sendMSTeams?: ( to: string, text: string, - opts?: { mediaUrl?: string }, + opts?: { mediaUrl?: string; mediaLocalRoots?: readonly string[] }, ) => Promise<{ messageId: string; conversationId: string }>; }; diff --git a/src/plugin-sdk/index.ts b/src/plugin-sdk/index.ts index aeadabb5e0e..9299eb80532 100644 --- a/src/plugin-sdk/index.ts +++ b/src/plugin-sdk/index.ts @@ -234,6 +234,8 @@ export { sendMediaWithLeadingCaption, } from "./reply-payload.js"; export type { OutboundReplyPayload } from "./reply-payload.js"; +export type { OutboundMediaLoadOptions } from "./outbound-media.js"; +export { loadOutboundMediaFromUrl } from "./outbound-media.js"; export { resolveChannelAccountConfigBasePath } from "./config-paths.js"; export { buildMediaPayload } from "../channels/plugins/media-payload.js"; export type { MediaPayload, MediaPayloadInput } from "../channels/plugins/media-payload.js"; diff --git a/src/plugin-sdk/outbound-media.test.ts b/src/plugin-sdk/outbound-media.test.ts new file mode 100644 index 00000000000..bb1ef547973 --- /dev/null +++ b/src/plugin-sdk/outbound-media.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it, vi } from "vitest"; +import { loadOutboundMediaFromUrl } from "./outbound-media.js"; + +const loadWebMediaMock = vi.hoisted(() => vi.fn()); + +vi.mock("../web/media.js", () => ({ + loadWebMedia: loadWebMediaMock, +})); + +describe("loadOutboundMediaFromUrl", () => { + it("forwards maxBytes and mediaLocalRoots to loadWebMedia", async () => { + loadWebMediaMock.mockResolvedValueOnce({ + buffer: Buffer.from("x"), + kind: "image", + contentType: "image/png", + }); + + await loadOutboundMediaFromUrl("file:///tmp/image.png", { + maxBytes: 1024, + mediaLocalRoots: ["/tmp/workspace-agent"], + }); + + expect(loadWebMediaMock).toHaveBeenCalledWith("file:///tmp/image.png", { + maxBytes: 1024, + localRoots: ["/tmp/workspace-agent"], + }); + }); + + it("keeps options optional", async () => { + loadWebMediaMock.mockResolvedValueOnce({ + buffer: Buffer.from("x"), + kind: "image", + contentType: "image/png", + }); + + await loadOutboundMediaFromUrl("https://example.com/image.png"); + + expect(loadWebMediaMock).toHaveBeenCalledWith("https://example.com/image.png", { + maxBytes: undefined, + localRoots: undefined, + }); + }); +}); diff --git a/src/plugin-sdk/outbound-media.ts b/src/plugin-sdk/outbound-media.ts new file mode 100644 index 00000000000..49e8b92f681 --- /dev/null +++ b/src/plugin-sdk/outbound-media.ts @@ -0,0 +1,16 @@ +import { loadWebMedia } from "../web/media.js"; + +export type OutboundMediaLoadOptions = { + maxBytes?: number; + mediaLocalRoots?: readonly string[]; +}; + +export async function loadOutboundMediaFromUrl( + mediaUrl: string, + options: OutboundMediaLoadOptions = {}, +) { + return await loadWebMedia(mediaUrl, { + maxBytes: options.maxBytes, + localRoots: options.mediaLocalRoots, + }); +} diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index 8502a1da56a..ffa5be4be7d 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -4,6 +4,7 @@ import os from "node:os"; import path from "node:path"; import { afterAll, afterEach, describe, expect, it } from "vitest"; import { withEnv } from "../test-utils/env.js"; +import { getGlobalHookRunner, resetGlobalHookRunner } from "./hook-runner-global.js"; import { __testing, loadOpenClawPlugins } from "./loader.js"; type TempPlugin = { dir: string; file: string; id: string }; @@ -295,6 +296,36 @@ describe("loadOpenClawPlugins", () => { expect(Object.keys(registry.gatewayHandlers)).toContain("allowed.ping"); }); + it("re-initializes global hook runner when serving registry from cache", () => { + process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins"; + const plugin = writePlugin({ + id: "cache-hook-runner", + body: `export default { id: "cache-hook-runner", register() {} };`, + }); + + const options = { + workspaceDir: plugin.dir, + config: { + plugins: { + load: { paths: [plugin.file] }, + allow: ["cache-hook-runner"], + }, + }, + }; + + const first = loadOpenClawPlugins(options); + expect(getGlobalHookRunner()).not.toBeNull(); + + resetGlobalHookRunner(); + expect(getGlobalHookRunner()).toBeNull(); + + const second = loadOpenClawPlugins(options); + expect(second).toBe(first); + expect(getGlobalHookRunner()).not.toBeNull(); + + resetGlobalHookRunner(); + }); + it("loads plugins when source and root differ only by realpath alias", () => { process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins"; const plugin = writePlugin({ diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index c60acba7396..2a166a8638b 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -365,6 +365,11 @@ function warnAboutUntrackedLoadedPlugins(params: { } } +function activatePluginRegistry(registry: PluginRegistry, cacheKey: string): void { + setActivePluginRegistry(registry, cacheKey); + initializeGlobalHookRunner(registry); +} + export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegistry { // Test env: default-disable plugins unless explicitly configured. // This keeps unit/gateway suites fast and avoids loading heavyweight plugin deps by accident. @@ -380,7 +385,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi if (cacheEnabled) { const cached = registryCache.get(cacheKey); if (cached) { - setActivePluginRegistry(cached, cacheKey); + activatePluginRegistry(cached, cacheKey); return cached; } } @@ -711,8 +716,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi if (cacheEnabled) { registryCache.set(cacheKey, registry); } - setActivePluginRegistry(registry, cacheKey); - initializeGlobalHookRunner(registry); + activatePluginRegistry(registry, cacheKey); return registry; } diff --git a/src/telegram/bot/delivery.ts b/src/telegram/bot/delivery.ts index cf5f183ba19..a0624065d0e 100644 --- a/src/telegram/bot/delivery.ts +++ b/src/telegram/bot/delivery.ts @@ -30,6 +30,14 @@ import { resolveTelegramReplyId, type TelegramThreadSpec, } from "./helpers.js"; +import { + createDeliveryProgress, + markDelivered, + markReplyApplied, + resolveReplyToForSend, + sendChunkedTelegramReplyText, + type DeliveryProgress, +} from "./reply-threading.js"; import type { StickerMetadata, TelegramContext } from "./types.js"; const PARSE_ERR_RE = /can't parse entities|parse entities|find end of the entity/i; @@ -45,11 +53,6 @@ const TELEGRAM_MEDIA_SSRF_POLICY = { allowRfc2544BenchmarkRange: true, }; -type DeliveryProgress = { - hasReplied: boolean; - hasDelivered: boolean; -}; - type ChunkTextFn = (markdown: string) => ReturnType; function buildChunkTextResolver(params: { @@ -82,26 +85,6 @@ function buildChunkTextResolver(params: { }; } -function resolveReplyToForSend(params: { - replyToId?: number; - replyToMode: ReplyToMode; - progress: DeliveryProgress; -}): number | undefined { - return params.replyToId && (params.replyToMode === "all" || !params.progress.hasReplied) - ? params.replyToId - : undefined; -} - -function markReplyApplied(progress: DeliveryProgress, replyToId?: number): void { - if (replyToId && !progress.hasReplied) { - progress.hasReplied = true; - } -} - -function markDelivered(progress: DeliveryProgress): void { - progress.hasDelivered = true; -} - async function deliverTextReply(params: { bot: Bot; chatId: string; @@ -117,29 +100,26 @@ async function deliverTextReply(params: { progress: DeliveryProgress; }): Promise { const chunks = params.chunkText(params.replyText); - for (let i = 0; i < chunks.length; i += 1) { - const chunk = chunks[i]; - if (!chunk) { - continue; - } - const shouldAttachButtons = i === 0 && params.replyMarkup; - const replyToForChunk = resolveReplyToForSend({ - replyToId: params.replyToId, - replyToMode: params.replyToMode, - progress: params.progress, - }); - await sendTelegramText(params.bot, params.chatId, chunk.html, params.runtime, { - replyToMessageId: replyToForChunk, - replyQuoteText: params.replyQuoteText, - thread: params.thread, - textMode: "html", - plainText: chunk.text, - linkPreview: params.linkPreview, - replyMarkup: shouldAttachButtons ? params.replyMarkup : undefined, - }); - markReplyApplied(params.progress, replyToForChunk); - markDelivered(params.progress); - } + await sendChunkedTelegramReplyText({ + chunks, + progress: params.progress, + replyToId: params.replyToId, + replyToMode: params.replyToMode, + replyMarkup: params.replyMarkup, + replyQuoteText: params.replyQuoteText, + quoteOnlyOnFirstChunk: true, + sendChunk: async ({ chunk, replyToMessageId, replyMarkup, replyQuoteText }) => { + await sendTelegramText(params.bot, params.chatId, chunk.html, params.runtime, { + replyToMessageId, + replyQuoteText, + thread: params.thread, + textMode: "html", + plainText: chunk.text, + linkPreview: params.linkPreview, + replyMarkup, + }); + }, + }); } async function sendPendingFollowUpText(params: { @@ -156,24 +136,23 @@ async function sendPendingFollowUpText(params: { progress: DeliveryProgress; }): Promise { const chunks = params.chunkText(params.text); - for (let i = 0; i < chunks.length; i += 1) { - const chunk = chunks[i]; - const replyToForFollowUp = resolveReplyToForSend({ - replyToId: params.replyToId, - replyToMode: params.replyToMode, - progress: params.progress, - }); - await sendTelegramText(params.bot, params.chatId, chunk.html, params.runtime, { - replyToMessageId: replyToForFollowUp, - thread: params.thread, - textMode: "html", - plainText: chunk.text, - linkPreview: params.linkPreview, - replyMarkup: i === 0 ? params.replyMarkup : undefined, - }); - markReplyApplied(params.progress, replyToForFollowUp); - markDelivered(params.progress); - } + await sendChunkedTelegramReplyText({ + chunks, + progress: params.progress, + replyToId: params.replyToId, + replyToMode: params.replyToMode, + replyMarkup: params.replyMarkup, + sendChunk: async ({ chunk, replyToMessageId, replyMarkup }) => { + await sendTelegramText(params.bot, params.chatId, chunk.html, params.runtime, { + replyToMessageId, + thread: params.thread, + textMode: "html", + plainText: chunk.text, + linkPreview: params.linkPreview, + replyMarkup, + }); + }, + }); } async function deliverMediaReply(params: { @@ -409,10 +388,7 @@ export async function deliverReplies(params: { /** Optional quote text for Telegram reply_parameters. */ replyQuoteText?: string; }): Promise<{ delivered: boolean }> { - const progress: DeliveryProgress = { - hasReplied: false, - hasDelivered: false, - }; + const progress = createDeliveryProgress(); const chunkText = buildChunkTextResolver({ textLimit: params.textLimit, chunkMode: params.chunkMode ?? "length", @@ -679,24 +655,27 @@ async function sendTelegramVoiceFallbackText(opts: { replyQuoteText?: string; }): Promise { const chunks = opts.chunkText(opts.text); - let appliedReplyTo = false; - for (let i = 0; i < chunks.length; i += 1) { - const chunk = chunks[i]; - // Only apply reply reference, quote text, and buttons to the first chunk. - const replyToForChunk = !appliedReplyTo ? opts.replyToId : undefined; - await sendTelegramText(opts.bot, opts.chatId, chunk.html, opts.runtime, { - replyToMessageId: replyToForChunk, - replyQuoteText: !appliedReplyTo ? opts.replyQuoteText : undefined, - thread: opts.thread, - textMode: "html", - plainText: chunk.text, - linkPreview: opts.linkPreview, - replyMarkup: !appliedReplyTo ? opts.replyMarkup : undefined, - }); - if (replyToForChunk) { - appliedReplyTo = true; - } - } + const progress = createDeliveryProgress(); + await sendChunkedTelegramReplyText({ + chunks, + progress, + replyToId: opts.replyToId, + replyToMode: "first", + replyMarkup: opts.replyMarkup, + replyQuoteText: opts.replyQuoteText, + quoteOnlyOnFirstChunk: true, + sendChunk: async ({ chunk, replyToMessageId, replyMarkup, replyQuoteText }) => { + await sendTelegramText(opts.bot, opts.chatId, chunk.html, opts.runtime, { + replyToMessageId, + replyQuoteText, + thread: opts.thread, + textMode: "html", + plainText: chunk.text, + linkPreview: opts.linkPreview, + replyMarkup, + }); + }, + }); } function isTelegramThreadNotFoundError(err: unknown): boolean { diff --git a/src/telegram/bot/reply-threading.ts b/src/telegram/bot/reply-threading.ts new file mode 100644 index 00000000000..d80bbf63264 --- /dev/null +++ b/src/telegram/bot/reply-threading.ts @@ -0,0 +1,76 @@ +import type { ReplyToMode } from "../../config/config.js"; + +export type DeliveryProgress = { + hasReplied: boolean; + hasDelivered: boolean; +}; + +export function createDeliveryProgress(): DeliveryProgress { + return { + hasReplied: false, + hasDelivered: false, + }; +} + +export function resolveReplyToForSend(params: { + replyToId?: number; + replyToMode: ReplyToMode; + progress: DeliveryProgress; +}): number | undefined { + return params.replyToId && (params.replyToMode === "all" || !params.progress.hasReplied) + ? params.replyToId + : undefined; +} + +export function markReplyApplied(progress: DeliveryProgress, replyToId?: number): void { + if (replyToId && !progress.hasReplied) { + progress.hasReplied = true; + } +} + +export function markDelivered(progress: DeliveryProgress): void { + progress.hasDelivered = true; +} + +export async function sendChunkedTelegramReplyText(params: { + chunks: readonly TChunk[]; + progress: DeliveryProgress; + replyToId?: number; + replyToMode: ReplyToMode; + replyMarkup?: TReplyMarkup; + replyQuoteText?: string; + quoteOnlyOnFirstChunk?: boolean; + sendChunk: (opts: { + chunk: TChunk; + isFirstChunk: boolean; + replyToMessageId?: number; + replyMarkup?: TReplyMarkup; + replyQuoteText?: string; + }) => Promise; +}): Promise { + for (let i = 0; i < params.chunks.length; i += 1) { + const chunk = params.chunks[i]; + if (!chunk) { + continue; + } + const isFirstChunk = i === 0; + const replyToMessageId = resolveReplyToForSend({ + replyToId: params.replyToId, + replyToMode: params.replyToMode, + progress: params.progress, + }); + const shouldAttachQuote = + Boolean(replyToMessageId) && + Boolean(params.replyQuoteText) && + (params.quoteOnlyOnFirstChunk !== true || isFirstChunk); + await params.sendChunk({ + chunk, + isFirstChunk, + replyToMessageId, + replyMarkup: isFirstChunk ? params.replyMarkup : undefined, + replyQuoteText: shouldAttachQuote ? params.replyQuoteText : undefined, + }); + markReplyApplied(params.progress, replyToMessageId); + markDelivered(params.progress); + } +} diff --git a/src/version.test.ts b/src/version.test.ts index 856e4a908b8..028aac69be8 100644 --- a/src/version.test.ts +++ b/src/version.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it } from "vitest"; import { readVersionFromBuildInfoForModuleUrl, readVersionFromPackageJsonForModuleUrl, + resolveBinaryVersion, resolveRuntimeServiceVersion, resolveVersionFromModuleUrl, } from "./version.js"; @@ -94,6 +95,42 @@ describe("version resolution", () => { expect(resolveVersionFromModuleUrl("not-a-valid-url")).toBeNull(); }); + it("resolves binary version with explicit precedence", async () => { + await withTempDir(async (root) => { + await writeJsonFixture(root, "package.json", { name: "openclaw", version: "2.3.4" }); + const moduleUrl = await ensureModuleFixture(root); + expect( + resolveBinaryVersion({ + moduleUrl, + injectedVersion: "9.9.9", + bundledVersion: "8.8.8", + fallback: "0.0.0", + }), + ).toBe("9.9.9"); + expect( + resolveBinaryVersion({ + moduleUrl, + bundledVersion: "8.8.8", + fallback: "0.0.0", + }), + ).toBe("2.3.4"); + expect( + resolveBinaryVersion({ + moduleUrl: "not-a-valid-url", + bundledVersion: "8.8.8", + fallback: "0.0.0", + }), + ).toBe("8.8.8"); + expect( + resolveBinaryVersion({ + moduleUrl: "not-a-valid-url", + bundledVersion: " ", + fallback: "0.0.0", + }), + ).toBe("0.0.0"); + }); + }); + it("prefers OPENCLAW_VERSION over service and package versions", () => { expect( resolveRuntimeServiceVersion({ diff --git a/src/version.ts b/src/version.ts index 18c3c968dd7..2e974b6e1db 100644 --- a/src/version.ts +++ b/src/version.ts @@ -71,6 +71,21 @@ export function resolveVersionFromModuleUrl(moduleUrl: string): string | null { ); } +export function resolveBinaryVersion(params: { + moduleUrl: string; + injectedVersion?: string; + bundledVersion?: string; + fallback?: string; +}): string { + return ( + firstNonEmpty(params.injectedVersion) || + resolveVersionFromModuleUrl(params.moduleUrl) || + firstNonEmpty(params.bundledVersion) || + params.fallback || + "0.0.0" + ); +} + export type RuntimeVersionEnv = { [key: string]: string | undefined; }; @@ -91,8 +106,8 @@ export function resolveRuntimeServiceVersion( // Single source of truth for the current OpenClaw version. // - Embedded/bundled builds: injected define or env var. // - Dev/npm builds: package.json. -export const VERSION = - (typeof __OPENCLAW_VERSION__ === "string" && __OPENCLAW_VERSION__) || - process.env.OPENCLAW_BUNDLED_VERSION || - resolveVersionFromModuleUrl(import.meta.url) || - "0.0.0"; +export const VERSION = resolveBinaryVersion({ + moduleUrl: import.meta.url, + injectedVersion: typeof __OPENCLAW_VERSION__ === "string" ? __OPENCLAW_VERSION__ : undefined, + bundledVersion: process.env.OPENCLAW_BUNDLED_VERSION, +});