nyrn
/
openclaw
mirror of https://github.com/openclaw/openclaw.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

refactor: clarify synology delivery identity names

This commit is contained in:
Peter Steinberger 2026-03-22 23:11:58 -07:00
parent 6c1ea41472
commit ea800dd4ef
No known key found for this signature in database
5 changed files with 38 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
extensions/synology-chat/src/channel.test-mocks.ts
View File

@ -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", () => ({

21
extensions/synology-chat/src/client.test.ts
View File

@ -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);

10
extensions/synology-chat/src/client.ts
View File

@ -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<number | undefined> {
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);

16
extensions/synology-chat/src/webhook-handler.test.ts
View File

@ -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,

25
extensions/synology-chat/src/webhook-handler.ts
View File

@ -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<void> {
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<null>((_, 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,
);
}