diff --git a/extensions/synology-chat/src/channel.test-mocks.ts b/extensions/synology-chat/src/channel.test-mocks.ts index e743859ea4f..5d103291bdc 100644 --- a/extensions/synology-chat/src/channel.test-mocks.ts +++ b/extensions/synology-chat/src/channel.test-mocks.ts @@ -75,7 +75,7 @@ vi.mock("openclaw/plugin-sdk/webhook-ingress", async () => { vi.mock("./client.js", () => ({ sendMessage: vi.fn().mockResolvedValue(true), sendFileUrl: vi.fn().mockResolvedValue(true), - resolveChatUserId: vi.fn().mockResolvedValue(undefined), + resolveLegacyWebhookNameToChatUserId: vi.fn().mockResolvedValue(undefined), })); vi.mock("./runtime.js", () => ({ diff --git a/extensions/synology-chat/src/client.test.ts b/extensions/synology-chat/src/client.test.ts index 2ae24f42904..d977b08b05f 100644 --- a/extensions/synology-chat/src/client.test.ts +++ b/extensions/synology-chat/src/client.test.ts @@ -15,7 +15,8 @@ vi.mock("node:http", () => { }); // Import after mocks are set up -const { sendMessage, sendFileUrl, fetchChatUsers, resolveChatUserId } = await import("./client.js"); +const { sendMessage, sendFileUrl, fetchChatUsers, resolveLegacyWebhookNameToChatUserId } = + await import("./client.js"); const https = await import("node:https"); let fakeNowMs = 1_700_000_000_000; @@ -109,7 +110,7 @@ describe("sendFileUrl", () => { }); }); -// Helper to mock the user_list API response for fetchChatUsers / resolveChatUserId +// Helper to mock the user_list API response for fetchChatUsers / resolveLegacyWebhookNameToChatUserId function mockUserListResponse( users: Array<{ user_id: number; username: string; nickname: string }>, ) { @@ -146,7 +147,7 @@ function mockUserListResponseImpl( httpsGet.mockImplementation(impl); } -describe("resolveChatUserId", () => { +describe("resolveLegacyWebhookNameToChatUserId", () => { const baseUrl = "https://nas.example.com/webapi/entry.cgi?api=SYNO.Chat.External&method=chatbot&version=2&token=%22test%22"; const baseUrl2 = @@ -169,7 +170,7 @@ describe("resolveChatUserId", () => { { user_id: 4, username: "jmn67", nickname: "jmn" }, { user_id: 7, username: "she67", nickname: "sarah" }, ]); - const result = await resolveChatUserId(baseUrl, "jmn"); + const result = await resolveLegacyWebhookNameToChatUserId(baseUrl, "jmn"); expect(result).toBe(4); }); @@ -181,7 +182,7 @@ describe("resolveChatUserId", () => { // Advance time to invalidate cache fakeNowMs += 10 * 60 * 1000; vi.setSystemTime(fakeNowMs); - const result = await resolveChatUserId(baseUrl, "jmn67"); + const result = await resolveLegacyWebhookNameToChatUserId(baseUrl, "jmn67"); expect(result).toBe(4); }); @@ -189,7 +190,7 @@ describe("resolveChatUserId", () => { mockUserListResponse([{ user_id: 4, username: "JMN67", nickname: "JMN" }]); fakeNowMs += 10 * 60 * 1000; vi.setSystemTime(fakeNowMs); - const result = await resolveChatUserId(baseUrl, "jmn"); + const result = await resolveLegacyWebhookNameToChatUserId(baseUrl, "jmn"); expect(result).toBe(4); }); @@ -197,7 +198,7 @@ describe("resolveChatUserId", () => { mockUserListResponse([{ user_id: 4, username: "jmn67", nickname: "jmn" }]); fakeNowMs += 10 * 60 * 1000; vi.setSystemTime(fakeNowMs); - const result = await resolveChatUserId(baseUrl, "unknown_user"); + const result = await resolveLegacyWebhookNameToChatUserId(baseUrl, "unknown_user"); expect(result).toBeUndefined(); }); @@ -205,7 +206,7 @@ describe("resolveChatUserId", () => { mockUserListResponse([]); fakeNowMs += 10 * 60 * 1000; vi.setSystemTime(fakeNowMs); - await resolveChatUserId(baseUrl, "anyone"); + await resolveLegacyWebhookNameToChatUserId(baseUrl, "anyone"); const httpsGet = vi.mocked((https as any).get); expect(httpsGet).toHaveBeenCalledWith( expect.stringContaining("method=user_list"), @@ -218,8 +219,8 @@ describe("resolveChatUserId", () => { mockUserListResponseOnce([{ user_id: 4, username: "jmn67", nickname: "jmn" }]); mockUserListResponseOnce([{ user_id: 9, username: "jmn67", nickname: "jmn" }]); - const result1 = await resolveChatUserId(baseUrl, "jmn"); - const result2 = await resolveChatUserId(baseUrl2, "jmn"); + const result1 = await resolveLegacyWebhookNameToChatUserId(baseUrl, "jmn"); + const result2 = await resolveLegacyWebhookNameToChatUserId(baseUrl2, "jmn"); expect(result1).toBe(4); expect(result2).toBe(9); diff --git a/extensions/synology-chat/src/client.ts b/extensions/synology-chat/src/client.ts index d66f1b720f4..03af80b3765 100644 --- a/extensions/synology-chat/src/client.ts +++ b/extensions/synology-chat/src/client.ts @@ -173,25 +173,25 @@ export async function fetchChatUsers( } /** - * Resolve a webhook username to the correct Chat API user_id. + * Resolve a mutable webhook username/nickname to the correct Chat API user_id. * * Synology Chat outgoing webhooks send a user_id that may NOT match the * Chat-internal user_id needed by the chatbot API (method=chatbot). * The webhook's "username" field corresponds to the Chat user's "nickname". * * @param incomingUrl - Bot incoming webhook URL (used to derive user_list URL) - * @param webhookUsername - The username from the outgoing webhook payload + * @param mutableWebhookUsername - The username from the outgoing webhook payload * @param allowInsecureSsl - Skip TLS verification * @returns The correct Chat user_id, or undefined if not found */ -export async function resolveChatUserId( +export async function resolveLegacyWebhookNameToChatUserId( incomingUrl: string, - webhookUsername: string, + mutableWebhookUsername: string, allowInsecureSsl = true, log?: { warn: (...args: unknown[]) => void }, ): Promise { const users = await fetchChatUsers(incomingUrl, allowInsecureSsl, log); - const lower = webhookUsername.toLowerCase(); + const lower = mutableWebhookUsername.toLowerCase(); // Match by nickname first (webhook "username" field = Chat "nickname") const byNickname = users.find((u) => u.nickname.toLowerCase() === lower); diff --git a/extensions/synology-chat/src/webhook-handler.test.ts b/extensions/synology-chat/src/webhook-handler.test.ts index d7d8123403b..7946a870d6f 100644 --- a/extensions/synology-chat/src/webhook-handler.test.ts +++ b/extensions/synology-chat/src/webhook-handler.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { resolveChatUserId, sendMessage } from "./client.js"; +import { resolveLegacyWebhookNameToChatUserId, sendMessage } from "./client.js"; import { makeFormBody, makeReq, makeRes, makeStalledReq } from "./test-http-utils.js"; import type { ResolvedSynologyChatAccount } from "./types.js"; import type { WebhookHandlerDeps } from "./webhook-handler.js"; @@ -8,10 +8,10 @@ import { createWebhookHandler, } from "./webhook-handler.js"; -// Mock sendMessage and resolveChatUserId to prevent real HTTP calls +// Mock sendMessage and resolveLegacyWebhookNameToChatUserId to prevent real HTTP calls vi.mock("./client.js", () => ({ sendMessage: vi.fn().mockResolvedValue(true), - resolveChatUserId: vi.fn().mockResolvedValue(undefined), + resolveLegacyWebhookNameToChatUserId: vi.fn().mockResolvedValue(undefined), })); function makeAccount( @@ -342,7 +342,7 @@ describe("createWebhookHandler", () => { await handler(req, res); expect(res._status).toBe(204); - expect(resolveChatUserId).not.toHaveBeenCalled(); + expect(resolveLegacyWebhookNameToChatUserId).not.toHaveBeenCalled(); expect(deliver).toHaveBeenCalledWith( expect.objectContaining({ from: "123", @@ -358,7 +358,7 @@ describe("createWebhookHandler", () => { }); it("only resolves reply recipient by username when break-glass mode is enabled", async () => { - vi.mocked(resolveChatUserId).mockResolvedValueOnce(456); + vi.mocked(resolveLegacyWebhookNameToChatUserId).mockResolvedValueOnce(456); const deliver = vi.fn().mockResolvedValue("Bot reply"); const handler = createWebhookHandler({ account: makeAccount({ @@ -374,7 +374,7 @@ describe("createWebhookHandler", () => { await handler(req, res); expect(res._status).toBe(204); - expect(resolveChatUserId).toHaveBeenCalledWith( + expect(resolveLegacyWebhookNameToChatUserId).toHaveBeenCalledWith( "https://nas.example.com/incoming", "testuser", true, @@ -395,7 +395,7 @@ describe("createWebhookHandler", () => { }); it("falls back to payload.user_id when break-glass resolution does not find a match", async () => { - vi.mocked(resolveChatUserId).mockResolvedValueOnce(undefined); + vi.mocked(resolveLegacyWebhookNameToChatUserId).mockResolvedValueOnce(undefined); const deliver = vi.fn().mockResolvedValue("Bot reply"); const handler = createWebhookHandler({ account: makeAccount({ @@ -411,7 +411,7 @@ describe("createWebhookHandler", () => { await handler(req, res); expect(res._status).toBe(204); - expect(resolveChatUserId).toHaveBeenCalledWith( + expect(resolveLegacyWebhookNameToChatUserId).toHaveBeenCalledWith( "https://nas.example.com/incoming", "testuser", true, diff --git a/extensions/synology-chat/src/webhook-handler.ts b/extensions/synology-chat/src/webhook-handler.ts index aab77c34bd2..ed1a2db123a 100644 --- a/extensions/synology-chat/src/webhook-handler.ts +++ b/extensions/synology-chat/src/webhook-handler.ts @@ -10,7 +10,7 @@ import { readRequestBodyWithLimit, requestBodyErrorToText, } from "openclaw/plugin-sdk/webhook-ingress"; -import { sendMessage, resolveChatUserId } from "./client.js"; +import { sendMessage, resolveLegacyWebhookNameToChatUserId } from "./client.js"; import { validateToken, authorizeUserForDm, sanitizeInput, RateLimiter } from "./security.js"; import type { SynologyWebhookPayload, ResolvedSynologyChatAccount } from "./types.js"; @@ -369,7 +369,7 @@ async function parseAndAuthorizeSynologyWebhook(params: { }; } -async function resolveSynologyReplyUserId(params: { +async function resolveSynologyDeliveryUserId(params: { account: ResolvedSynologyChatAccount; payload: SynologyWebhookPayload; log?: WebhookHandlerDeps["log"]; @@ -378,14 +378,14 @@ async function resolveSynologyReplyUserId(params: { return params.payload.user_id; } - const chatUserId = await resolveChatUserId( + const resolvedChatApiUserId = await resolveLegacyWebhookNameToChatUserId( params.account.incomingUrl, params.payload.username, params.account.allowInsecureSsl, params.log, ); - if (chatUserId !== undefined) { - return String(chatUserId); + if (resolvedChatApiUserId !== undefined) { + return String(resolvedChatApiUserId); } params.log?.warn( `Could not resolve Chat API user_id for "${params.payload.username}" — falling back to webhook user_id ${params.payload.user_id}. Reply delivery may fail.`, @@ -399,9 +399,10 @@ async function processAuthorizedSynologyWebhook(params: { log?: WebhookHandlerDeps["log"]; message: AuthorizedSynologyWebhook; }): Promise { - let replyUserId = params.message.payload.user_id; + const authorizedWebhookUserId = params.message.payload.user_id; + let deliveryUserId = authorizedWebhookUserId; try { - replyUserId = await resolveSynologyReplyUserId({ + deliveryUserId = await resolveSynologyDeliveryUserId({ account: params.account, payload: params.message.payload, log: params.log, @@ -409,13 +410,13 @@ async function processAuthorizedSynologyWebhook(params: { const deliverPromise = params.deliver({ body: params.message.body, - from: params.message.payload.user_id, + from: authorizedWebhookUserId, senderName: params.message.payload.username, provider: "synology-chat", chatType: "direct", accountId: params.account.accountId, commandAuthorized: params.message.commandAuthorized, - chatUserId: replyUserId, + chatUserId: deliveryUserId, }); const timeoutPromise = new Promise((_, reject) => setTimeout(() => reject(new Error("Agent response timeout (120s)")), 120_000), @@ -428,12 +429,12 @@ async function processAuthorizedSynologyWebhook(params: { await sendMessage( params.account.incomingUrl, reply, - replyUserId, + deliveryUserId, params.account.allowInsecureSsl, ); const replyPreview = reply.length > 100 ? `${reply.slice(0, 100)}...` : reply; params.log?.info?.( - `Reply sent to ${params.message.payload.username} (${replyUserId}): ${replyPreview}`, + `Reply sent to ${params.message.payload.username} (${deliveryUserId}): ${replyPreview}`, ); } catch (err) { const errMsg = err instanceof Error ? `${err.message}\n${err.stack}` : String(err); @@ -443,7 +444,7 @@ async function processAuthorizedSynologyWebhook(params: { await sendMessage( params.account.incomingUrl, "Sorry, an error occurred while processing your message.", - replyUserId, + deliveryUserId, params.account.allowInsecureSsl, ); }