vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-03-12 07:20:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(synology-chat): prevent restart loop in startAccount (#23074)

Browse Source 
* fix(synology-chat): prevent restart loop in startAccount

startAccount must return a Promise that stays pending while the channel
is running. The gateway wraps the return value in Promise.resolve(), and
when it resolves, the gateway thinks the channel crashed and auto-restarts
with exponential backoff (5s → 10s → 20s..., up to 10 attempts).

Replace the synchronous { stop } return with a Promise<void> that resolves
only when ctx.abortSignal fires, keeping the channel alive until shutdown.

Tested on Synology DS923+ with DSM 7.2 — single startup, no restart loop.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(synology-chat): add type guards for startAccount return value

startAccount returns `void | { stop: () => void }` — TypeScript requires
a type guard before accessing .stop on the union type. Added proper checks
in both integration and unit tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(synology-chat): use Readable stream in integration test for Windows compat

Replace EventEmitter + process.nextTick with Readable stream for
request body simulation. The process.nextTick approach caused the test
to hang on Windows CI (120s timeout) because events were not reliably
delivered to readBody() listeners.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: stabilize synology gateway account lifecycle (#23074) (thanks @druide67)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Jean-Marc
2026-03-02 21:06:16 +01:00
committed by GitHub
parent 4b50018406
commit b52561bfa3
4 changed files with 87 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -50,6 +50,7 @@ Docs: https://docs.openclaw.ai
- Synology Chat/webhook compatibility: accept JSON and alias payload fields, allow token resolution from body/query/header sources, and ACK webhook requests with `204` to avoid persistent `Processing...` states in Synology Chat clients. (#26635) Thanks @memphislee09-source.
- Synology Chat/webhook ingress hardening: enforce bounded body reads (size + timeout) via shared request-body guards to prevent unauthenticated slow-body hangs before token validation. (#25831) Thanks @bmendonca3.
- Synology Chat/reply delivery: resolve webhook usernames to Chat API `user_id` values for outbound chatbot replies, avoiding mismatches between webhook user IDs and `method=chatbot` recipient IDs in multi-account setups. (#23709) Thanks @druide67.
- Synology Chat/gateway lifecycle: keep `startAccount` pending until abort for inactive and active account paths to prevent webhook route restart loops under gateway supervision. (#23074) Thanks @druide67.
- Auto-reply/followup queue: avoid stale callback reuse across idle-window restarts by caching the followup runner only when a drain actually starts, preserving enqueue ordering after empty-finalize paths. (#31902) Thanks @Lanfei.
- Cron/HEARTBEAT_OK summary leak: suppress fallback main-session enqueue for heartbeat/internal ack summaries in isolated announce mode so `HEARTBEAT_OK` noise never appears in user chat while real summaries still forward. (#32093) Thanks @scoootscooob.
- Sessions/lock recovery: reclaim orphan legacy same-PID lock files missing `starttime` when no in-process lock ownership exists, avoiding false lock timeouts after PID reuse while preserving active lock safety checks. (#32081) Thanks @bmendonca3.

11
extensions/synology-chat/src/channel.integration.test.ts
View File

@@ -44,7 +44,6 @@ vi.mock("./client.js", () => ({
}));
const { createSynologyChatPlugin } = await import("./channel.js");
describe("Synology channel wiring integration", () => {
beforeEach(() => {
registerPluginHttpRouteMock.mockClear();
@@ -53,6 +52,7 @@ describe("Synology channel wiring integration", () => {
it("registers real webhook handler with resolved account config and enforces allowlist", async () => {
const plugin = createSynologyChatPlugin();
const abortController = new AbortController();
const ctx = {
cfg: {
channels: {
@@ -73,9 +73,10 @@ describe("Synology channel wiring integration", () => {
},
accountId: "alerts",
log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
abortSignal: abortController.signal,
};
const started = await plugin.gateway.startAccount(ctx);
const started = plugin.gateway.startAccount(ctx);
expect(registerPluginHttpRouteMock).toHaveBeenCalledTimes(1);
const firstCall = registerPluginHttpRouteMock.mock.calls[0];
@@ -101,9 +102,7 @@ describe("Synology channel wiring integration", () => {
expect(res._status).toBe(403);
expect(res._body).toContain("not authorized");
expect(dispatchReplyWithBufferedBlockDispatcher).not.toHaveBeenCalled();
if (started && typeof started === "object" && "stop" in started) {
(started as { stop: () => void }).stop();
}
abortController.abort();
await started;
});
});

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

@@ -317,35 +317,56 @@ describe("createSynologyChatPlugin", () => {
});
describe("gateway", () => {
it("startAccount returns stop function for disabled account", async () => {
it("startAccount returns pending promise for disabled account", async () => {
const plugin = createSynologyChatPlugin();
const abortController = new AbortController();
const ctx = {
cfg: {
channels: { "synology-chat": { enabled: false } },
},
accountId: "default",
log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
abortSignal: abortController.signal,
};
const result = await plugin.gateway.startAccount(ctx);
expect(typeof result.stop).toBe("function");
const result = plugin.gateway.startAccount(ctx);
expect(result).toBeInstanceOf(Promise);
// Promise should stay pending (never resolve) to prevent restart loop
const resolved = await Promise.race([
result,
new Promise((r) => setTimeout(() => r("pending"), 50)),
]);
expect(resolved).toBe("pending");
abortController.abort();
await result;
});
it("startAccount returns stop function for account without token", async () => {
it("startAccount returns pending promise for account without token", async () => {
const plugin = createSynologyChatPlugin();
const abortController = new AbortController();
const ctx = {
cfg: {
channels: { "synology-chat": { enabled: true } },
},
accountId: "default",
log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
abortSignal: abortController.signal,
};
const result = await plugin.gateway.startAccount(ctx);
expect(typeof result.stop).toBe("function");
const result = plugin.gateway.startAccount(ctx);
expect(result).toBeInstanceOf(Promise);
// Promise should stay pending (never resolve) to prevent restart loop
const resolved = await Promise.race([
result,
new Promise((r) => setTimeout(() => r("pending"), 50)),
]);
expect(resolved).toBe("pending");
abortController.abort();
await result;
});
it("startAccount refuses allowlist accounts with empty allowedUserIds", async () => {
const registerMock = vi.mocked(registerPluginHttpRoute);
registerMock.mockClear();
const abortController = new AbortController();
const plugin = createSynologyChatPlugin();
const ctx = {
@@ -362,12 +383,20 @@ describe("createSynologyChatPlugin", () => {
},
accountId: "default",
log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
abortSignal: abortController.signal,
};
const result = await plugin.gateway.startAccount(ctx);
expect(typeof result.stop).toBe("function");
const result = plugin.gateway.startAccount(ctx);
expect(result).toBeInstanceOf(Promise);
const resolved = await Promise.race([
result,
new Promise((r) => setTimeout(() => r("pending"), 50)),
]);
expect(resolved).toBe("pending");
expect(ctx.log.warn).toHaveBeenCalledWith(expect.stringContaining("empty allowedUserIds"));
expect(registerMock).not.toHaveBeenCalled();
abortController.abort();
await result;
});
it("deregisters stale route before re-registering same account/path", async () => {
@@ -377,7 +406,9 @@ describe("createSynologyChatPlugin", () => {
registerMock.mockReturnValueOnce(unregisterFirst).mockReturnValueOnce(unregisterSecond);
const plugin = createSynologyChatPlugin();
const ctx = {
const abortFirst = new AbortController();
const abortSecond = new AbortController();
const makeCtx = (abortCtrl: AbortController) => ({
cfg: {
channels: {
"synology-chat": {
@@ -392,18 +423,25 @@ describe("createSynologyChatPlugin", () => {
},
accountId: "default",
log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
};
abortSignal: abortCtrl.signal,
});
const first = await plugin.gateway.startAccount(ctx);
const second = await plugin.gateway.startAccount(ctx);
// Start first account (returns a pending promise)
const firstPromise = plugin.gateway.startAccount(makeCtx(abortFirst));
// Start second account on same path — should deregister the first route
const secondPromise = plugin.gateway.startAccount(makeCtx(abortSecond));
// Give microtasks time to settle
await new Promise((r) => setTimeout(r, 10));
expect(registerMock).toHaveBeenCalledTimes(2);
expect(unregisterFirst).toHaveBeenCalledTimes(1);
expect(unregisterSecond).not.toHaveBeenCalled();
// Clean up active route map so this module-level state doesn't leak across tests.
first.stop();
second.stop();
// Clean up: abort both to resolve promises and prevent test leak
abortFirst.abort();
abortSecond.abort();
await Promise.allSettled([firstPromise, secondPromise]);
});
});
});

38
extensions/synology-chat/src/channel.ts
View File

@@ -22,6 +22,23 @@ const SynologyChatConfigSchema = buildChannelConfigSchema(z.object({}).passthrou
const activeRouteUnregisters = new Map<string, () => void>();
function waitUntilAbort(signal?: AbortSignal, onAbort?: () => void): Promise<void> {
return new Promise((resolve) => {
const complete = () => {
onAbort?.();
resolve();
};
if (!signal) {
return;
}
if (signal.aborted) {
complete();
return;
}
signal.addEventListener("abort", complete, { once: true });
});
}
export function createSynologyChatPlugin() {
return {
id: CHANNEL_ID,
@@ -217,20 +234,20 @@ export function createSynologyChatPlugin() {
if (!account.enabled) {
log?.info?.(`Synology Chat account ${accountId} is disabled, skipping`);
return { stop: () => {} };
return waitUntilAbort(ctx.abortSignal);
}
if (!account.token || !account.incomingUrl) {
log?.warn?.(
`Synology Chat account ${accountId} not fully configured (missing token or incomingUrl)`,
);
return { stop: () => {} };
return waitUntilAbort(ctx.abortSignal);
}
if (account.dmPolicy === "allowlist" && account.allowedUserIds.length === 0) {
log?.warn?.(
`Synology Chat account ${accountId} has dmPolicy=allowlist but empty allowedUserIds; refusing to start route`,
);
return { stop: () => {} };
return waitUntilAbort(ctx.abortSignal);
}
log?.info?.(
@@ -318,13 +335,14 @@ export function createSynologyChatPlugin() {
log?.info?.(`Registered HTTP route: ${account.webhookPath} for Synology Chat`);
return {
stop: () => {
log?.info?.(`Stopping Synology Chat channel (account: ${accountId})`);
if (typeof unregister === "function") unregister();
activeRouteUnregisters.delete(routeKey);
},
};
// Keep alive until abort signal fires.
// The gateway expects a Promise that stays pending while the channel is running.
// Resolving immediately triggers a restart loop.
return waitUntilAbort(ctx.abortSignal, () => {
log?.info?.(`Stopping Synology Chat channel (account: ${accountId})`);
if (typeof unregister === "function") unregister();
activeRouteUnregisters.delete(routeKey);
});
},
stopAccount: async (ctx: any) => {