From 21ec1546e5a83975e5ffb3535bc8bc8b0cf2365b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 14 Jul 2026 15:02:56 -0700 Subject: [PATCH] refactor(extensions): trim internal plugin surfaces (#107796) * refactor(diffs): privatize internal plugin surfaces * refactor(file-transfer): privatize internal plugin surfaces * refactor(nextcloud-talk): privatize internal plugin surfaces * refactor(synology-chat): privatize internal plugin surfaces * chore(deadcode): shrink extension export baseline --- extensions/diffs/src/browser.test.ts | 13 +- extensions/diffs/src/browser.ts | 5 - .../diffs/src/shiki-curated-languages.ts | 9 - .../node-host/dir-fetch.stream-errors.test.ts | 95 ++++--- .../file-transfer/src/node-host/dir-fetch.ts | 6 - .../node-invoke-policy.stream-errors.test.ts | 88 ++++--- .../src/shared/node-invoke-policy.test.ts | 9 +- .../src/shared/node-invoke-policy.ts | 4 - .../src/tools/dir-fetch-tool.test.ts | 234 +++++++++--------- .../file-transfer/src/tools/dir-fetch-tool.ts | 6 - .../nextcloud-talk/src/monitor.replay.test.ts | 85 ++++--- extensions/nextcloud-talk/src/monitor.ts | 35 +-- .../src/room-info.fetch-timeout.test.ts | 62 +++-- .../nextcloud-talk/src/room-info.test.ts | 3 +- extensions/nextcloud-talk/src/room-info.ts | 6 - .../src/channel.integration.test.ts | 10 +- extensions/synology-chat/src/channel.test.ts | 78 +++--- extensions/synology-chat/src/channel.ts | 2 +- extensions/synology-chat/src/client.test.ts | 20 +- extensions/synology-chat/src/client.ts | 2 +- .../synology-chat/src/webhook-handler.test.ts | 8 +- .../synology-chat/src/webhook-handler.ts | 12 - scripts/deadcode-exports.baseline.mjs | 12 - scripts/diffs-shiki-curated.ts | 18 +- 24 files changed, 417 insertions(+), 405 deletions(-) diff --git a/extensions/diffs/src/browser.test.ts b/extensions/diffs/src/browser.test.ts index 52b58a341fe..629f1b23324 100644 --- a/extensions/diffs/src/browser.test.ts +++ b/extensions/diffs/src/browser.test.ts @@ -5,7 +5,7 @@ import path from "node:path"; import { expectDefined } from "@openclaw/normalization-core"; import { createTestPluginApi } from "openclaw/plugin-sdk/plugin-test-api"; import { createMockServerResponse } from "openclaw/plugin-sdk/test-env"; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../api.js"; import type { OpenClawPluginApi, OpenClawPluginToolContext } from "../api.js"; import { registerDiffsPlugin } from "./plugin.js"; @@ -16,7 +16,6 @@ const { launchMock } = vi.hoisted(() => ({ })); let PlaywrightDiffScreenshotter: typeof import("./browser.js").PlaywrightDiffScreenshotter; -let resetSharedBrowserStateForTests: typeof import("./browser.js").resetSharedBrowserStateForTests; vi.mock("playwright-core", () => ({ chromium: { @@ -45,21 +44,17 @@ describe("PlaywrightDiffScreenshotter", () => { let outputPath: string; let cleanupRootDir: () => Promise; - beforeAll(async () => { - ({ PlaywrightDiffScreenshotter, resetSharedBrowserStateForTests } = - await import("./browser.js")); - }); - beforeEach(async () => { vi.useFakeTimers(); + vi.resetModules(); + ({ PlaywrightDiffScreenshotter } = await import("./browser.js")); ({ rootDir, cleanup: cleanupRootDir } = await createTempDiffRoot("openclaw-diffs-browser-")); outputPath = path.join(rootDir, "preview.png"); launchMock.mockReset(); - await resetSharedBrowserStateForTests(); }); afterEach(async () => { - await resetSharedBrowserStateForTests(); + await vi.runAllTimersAsync(); vi.useRealTimers(); await cleanupRootDir(); }); diff --git a/extensions/diffs/src/browser.ts b/extensions/diffs/src/browser.ts index 0891afd76d4..51fb2a02708 100644 --- a/extensions/diffs/src/browser.ts +++ b/extensions/diffs/src/browser.ts @@ -313,11 +313,6 @@ async function writeExternalArtifactFile(params: { }); } -export async function resetSharedBrowserStateForTests(): Promise { - executablePathCache = null; - await closeSharedBrowser(); -} - function injectBaseHref(html: string): string { if (html.includes(" - getBundledLanguageAliases(language).map((alias) => [alias, language.import]), - ), -); -export const bundledLanguages = { - ...bundledLanguagesBase, - ...bundledLanguagesAlias, -}; diff --git a/extensions/file-transfer/src/node-host/dir-fetch.stream-errors.test.ts b/extensions/file-transfer/src/node-host/dir-fetch.stream-errors.test.ts index f8d1d2c025c..589bf1590f7 100644 --- a/extensions/file-transfer/src/node-host/dir-fetch.stream-errors.test.ts +++ b/extensions/file-transfer/src/node-host/dir-fetch.stream-errors.test.ts @@ -1,5 +1,8 @@ -// File Transfer tests cover canonical process-wrapper failures during dir fetch. -import { afterEach, describe, expect, it, vi } from "vitest"; +// File Transfer tests cover canonical process-wrapper failures through dir fetch. +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const { runCommandBufferedMock } = vi.hoisted(() => ({ runCommandBufferedMock: vi.fn() })); @@ -7,7 +10,9 @@ vi.mock("openclaw/plugin-sdk/process-runtime", () => ({ runCommandBuffered: runCommandBufferedMock, })); -import { testing } from "./dir-fetch.js"; +import { handleDirFetch } from "./dir-fetch.js"; + +let tmpRoot: string; function commandResult(overrides: Record = {}) { return { @@ -21,53 +26,83 @@ function commandResult(overrides: Record = {}) { }; } -afterEach(() => { +beforeEach(async () => { + tmpRoot = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "dir-fetch-errors-"))); + await fs.writeFile(path.join(tmpRoot, "ok.txt"), "ok"); +}); + +afterEach(async () => { runCommandBufferedMock.mockReset(); + await fs.rm(tmpRoot, { recursive: true, force: true }); }); describe("dir.fetch process wrapper", () => { it("falls back to capped tar when the optional du probe fails", async () => { - runCommandBufferedMock.mockRejectedValueOnce(new Error("du failed")); + runCommandBufferedMock + .mockRejectedValueOnce(new Error("du failed")) + .mockResolvedValueOnce(commandResult({ stdout: Buffer.from("archive") })) + .mockResolvedValueOnce(commandResult({ stdout: Buffer.from("./ok.txt\n") })); - await expect(testing.preflightDu("/tmp/project", 1024)).resolves.toBe(true); - expect(runCommandBufferedMock).toHaveBeenCalledWith( - ["du", "-sk", "/tmp/project"], + await expect(handleDirFetch({ path: tmpRoot, maxBytes: 1024 })).resolves.toMatchObject({ + ok: true, + entries: ["ok.txt"], + }); + expect(runCommandBufferedMock).toHaveBeenNthCalledWith( + 1, + ["du", "-sk", tmpRoot], expect.objectContaining({ discardOutput: { stderr: true } }), ); }); it("fails tar entry listing closed on wrapper errors", async () => { - runCommandBufferedMock.mockResolvedValueOnce( - commandResult({ code: null, termination: "error", error: new Error("listing failed") }), - ); + runCommandBufferedMock + .mockResolvedValueOnce(commandResult({ stdout: Buffer.from("1\tproject\n") })) + .mockResolvedValueOnce(commandResult({ stdout: Buffer.from("archive") })) + .mockResolvedValueOnce( + commandResult({ code: null, termination: "error", error: new Error("listing failed") }), + ); - await expect(testing.listTarEntries(Buffer.from("archive"))).resolves.toBeNull(); - expect(runCommandBufferedMock).toHaveBeenCalledWith( - ["tar", "-tzf", "-"], - expect.objectContaining({ discardOutput: { stderr: true } }), - ); + await expect(handleDirFetch({ path: tmpRoot, maxBytes: 1024 })).resolves.toMatchObject({ + ok: false, + code: "READ_ERROR", + message: "tar entry listing failed", + }); }); - it("classifies archive output caps, timeouts, and launch errors", async () => { - runCommandBufferedMock.mockResolvedValueOnce( - commandResult({ + it.each([ + { + label: "output cap", + result: commandResult({ code: null, termination: "output-limit", outputLimitStream: "stdout", }), - ); - await expect(testing.createTarArchive("/tmp/project", 1024)).resolves.toBe("TOO_LARGE"); - expect(runCommandBufferedMock).toHaveBeenLastCalledWith( - expect.any(Array), - expect.objectContaining({ discardOutput: { stderr: true } }), - ); - + message: "tarball exceeded 1024 byte limit mid-stream", + }, + { + label: "timeout", + result: commandResult({ code: null, termination: "timeout" }), + message: "tar command exceeded 60s wall-clock timeout", + }, + { + label: "launch error", + result: new Error("spawn failed"), + message: "tar command failed", + }, + ])("classifies $label failures through handleDirFetch", async ({ result, message }) => { runCommandBufferedMock.mockResolvedValueOnce( - commandResult({ code: null, termination: "timeout" }), + commandResult({ stdout: Buffer.from("1\tproject\n") }), ); - await expect(testing.createTarArchive("/tmp/project", 1024)).resolves.toBe("TIMEOUT"); + if (result instanceof Error) { + runCommandBufferedMock.mockRejectedValueOnce(result); + } else { + runCommandBufferedMock.mockResolvedValueOnce(result); + } - runCommandBufferedMock.mockRejectedValueOnce(new Error("spawn failed")); - await expect(testing.createTarArchive("/tmp/project", 1024)).resolves.toBe("ERROR"); + const response = await handleDirFetch({ path: tmpRoot, maxBytes: 1024 }); + expect(response).toMatchObject({ ok: false, code: expect.any(String) }); + if (!response.ok) { + expect(response.message).toContain(message); + } }); }); diff --git a/extensions/file-transfer/src/node-host/dir-fetch.ts b/extensions/file-transfer/src/node-host/dir-fetch.ts index 3f843413f2a..ee4e05a49cc 100644 --- a/extensions/file-transfer/src/node-host/dir-fetch.ts +++ b/extensions/file-transfer/src/node-host/dir-fetch.ts @@ -285,9 +285,3 @@ export async function handleDirFetch(params: DirFetchParams): Promise ({ runCommandWithTimeout: runCommandWithTimeoutMock, })); -import { testing } from "./node-invoke-policy.js"; +import { createFileTransferNodeInvokePolicy } from "./node-invoke-policy.js"; function commandResult(overrides: Record = {}) { return { @@ -46,6 +47,51 @@ function mockCommandResult(overrides: Record = {}) { ); } +function createDirFetchContext(): OpenClawPluginNodeInvokePolicyContext { + const archive = Buffer.from("archive"); + const invokeNode = vi + .fn() + .mockResolvedValueOnce({ + ok: true, + payload: { + ok: true, + path: "/tmp/project", + entries: ["ok.txt"], + preflightOnly: true, + }, + }) + .mockResolvedValueOnce({ + ok: true, + payload: { + ok: true, + path: "/tmp/project", + tarBase64: archive.toString("base64"), + tarBytes: archive.byteLength, + sha256: crypto.createHash("sha256").update(archive).digest("hex"), + fileCount: 1, + }, + }); + return { + nodeId: "node-1", + command: "dir.fetch", + params: { path: "/tmp/project" }, + config: {}, + pluginConfig: { + nodes: { + "node-1": { + allowReadPaths: ["/tmp/**"], + }, + }, + }, + node: { nodeId: "node-1", displayName: "Node One" }, + invokeNode, + }; +} + +async function runPolicy() { + return await createFileTransferNodeInvokePolicy().handle(createDirFetchContext()); +} + afterEach(() => { runCommandWithTimeoutMock.mockReset(); }); @@ -54,29 +100,17 @@ describe("dir.fetch archive policy process wrapper", () => { it("fails archive listing closed on wrapper errors", async () => { runCommandWithTimeoutMock.mockRejectedValueOnce(new Error("policy listing read failed")); - await expect( - testing.listDirFetchArchiveEntries({ - tarBase64: Buffer.from("archive").toString("base64"), - }), - ).resolves.toEqual({ + await expect(runPolicy()).resolves.toMatchObject({ ok: false, code: "ARCHIVE_ENTRIES_UNREADABLE", - reason: "tar -tzf error: policy listing read failed", + message: expect.stringContaining("tar -tzf error: policy listing read failed"), }); }); it("normalizes successful archive entries", async () => { mockCommandResult({ stdout: "./ok.txt\n" }); - const archive = Buffer.from("archive"); - await expect( - testing.listDirFetchArchiveEntries({ tarBase64: archive.toString("base64") }), - ).resolves.toEqual({ - ok: true, - entries: ["ok.txt"], - sizeBytes: archive.byteLength, - sha256: crypto.createHash("sha256").update(archive).digest("hex"), - }); + await expect(runPolicy()).resolves.toMatchObject({ ok: true }); expect(runCommandWithTimeoutMock).toHaveBeenCalledWith( expect.any(Array), expect.objectContaining({ tolerateOutputError: { stderr: true } }), @@ -88,14 +122,13 @@ describe("dir.fetch archive policy process wrapper", () => { const recent = "🤖" + "f".repeat(199); mockCommandResult({ code: 2, stderr: oldNoise + recent }); - const result = await testing.listDirFetchArchiveEntries({ - tarBase64: Buffer.from("archive").toString("base64"), - }); + const result = await runPolicy(); expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toContain(projectBoundedTextTail(recent, 200)); - expect(result.reason).not.toContain("🤖"); + if (result.ok) { + throw new Error("expected archive policy failure"); } + expect(result.message).toContain(projectBoundedTextTail(recent, 200)); + expect(result.message).not.toContain("🤖"); }); it("stops archive listing as soon as the entry cap is crossed", async () => { @@ -103,10 +136,9 @@ describe("dir.fetch archive policy process wrapper", () => { stdout: Array.from({ length: 5_001 }, (_, index) => `file-${index}`).join("\n") + "\n", }); - await expect( - testing.listDirFetchArchiveEntries({ - tarBase64: Buffer.from("archive").toString("base64"), - }), - ).resolves.toMatchObject({ ok: false, code: "ARCHIVE_ENTRIES_TOO_MANY" }); + await expect(runPolicy()).resolves.toMatchObject({ + ok: false, + code: "ARCHIVE_ENTRIES_TOO_MANY", + }); }); }); diff --git a/extensions/file-transfer/src/shared/node-invoke-policy.test.ts b/extensions/file-transfer/src/shared/node-invoke-policy.test.ts index 1c0aa7b0e32..d54753dd3e7 100644 --- a/extensions/file-transfer/src/shared/node-invoke-policy.test.ts +++ b/extensions/file-transfer/src/shared/node-invoke-policy.test.ts @@ -5,7 +5,7 @@ import { gzipSync } from "node:zlib"; import type { OpenClawPluginNodeInvokePolicyContext } from "openclaw/plugin-sdk/plugin-entry"; import { afterAll, afterEach, describe, expect, it, vi } from "vitest"; import { appendFileTransferAudit } from "./audit.js"; -import { createFileTransferNodeInvokePolicy, testing } from "./node-invoke-policy.js"; +import { createFileTransferNodeInvokePolicy } from "./node-invoke-policy.js"; vi.mock("./audit.js", () => ({ appendFileTransferAudit: vi.fn(async () => undefined), @@ -155,13 +155,6 @@ function requireInvokeParams( } describe("file-transfer node invoke policy", () => { - it("maps only transfer payload sizes into audit records", () => { - expect(testing.readAuditSizeBytes("file.fetch", { size: 3 })).toBe(3); - expect(testing.readAuditSizeBytes("file.write", { size: 4 })).toBe(4); - expect(testing.readAuditSizeBytes("dir.fetch", { tarBytes: 999 }, 5)).toBe(5); - expect(testing.readAuditSizeBytes("dir.list", { size: 6 })).toBeUndefined(); - }); - it("injects policy-owned limits before invoking the node", async () => { const policy = createFileTransferNodeInvokePolicy(); const { ctx, invokeNode } = createCtx({ diff --git a/extensions/file-transfer/src/shared/node-invoke-policy.ts b/extensions/file-transfer/src/shared/node-invoke-policy.ts index 58855066d32..e032bfe6247 100644 --- a/extensions/file-transfer/src/shared/node-invoke-policy.ts +++ b/extensions/file-transfer/src/shared/node-invoke-policy.ts @@ -964,8 +964,4 @@ export function createFileTransferNodeInvokePolicy(): OpenClawPluginNodeInvokePo }; } -export const testing = { - listDirFetchArchiveEntries, - readAuditSizeBytes, -}; /* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ diff --git a/extensions/file-transfer/src/tools/dir-fetch-tool.test.ts b/extensions/file-transfer/src/tools/dir-fetch-tool.test.ts index 1cbea19bead..a6bf686ed22 100644 --- a/extensions/file-transfer/src/tools/dir-fetch-tool.test.ts +++ b/extensions/file-transfer/src/tools/dir-fetch-tool.test.ts @@ -1,11 +1,10 @@ -// File Transfer tests cover dir fetch tar validation through the canonical process wrapper. -import { spawn } from "node:child_process"; +// File Transfer tests cover dir fetch tar validation through the tool boundary. +import crypto from "node:crypto"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { projectBoundedTextTail } from "../shared/append-bounded-text-tail.js"; -import { testing } from "./dir-fetch-tool.js"; let tmpRoot: string; @@ -14,34 +13,14 @@ beforeEach(async () => { }); afterEach(async () => { + vi.doUnmock("openclaw/plugin-sdk/media-store"); vi.doUnmock("openclaw/plugin-sdk/process-runtime"); + vi.doUnmock("../shared/audit.js"); + vi.doUnmock("./node-tool-invoke.js"); vi.resetModules(); await fs.rm(tmpRoot, { recursive: true, force: true }); }); -async function tarDirectory(dir: string): Promise { - return await new Promise((resolve, reject) => { - const tarBin = process.platform !== "win32" ? "/usr/bin/tar" : "tar"; - const child = spawn(tarBin, ["-czf", "-", "-C", dir, "."], { - stdio: ["ignore", "pipe", "pipe"], - }); - const chunks: Buffer[] = []; - let stderr = ""; - child.stdout.on("data", (chunk: Buffer) => chunks.push(chunk)); - child.stderr.on("data", (chunk: Buffer) => { - stderr += chunk.toString(); - }); - child.on("close", (code) => { - if (code !== 0) { - reject(new Error(`tar exited ${code}: ${stderr}`)); - return; - } - resolve(Buffer.concat(chunks)); - }); - child.on("error", reject); - }); -} - function commandResult(overrides: Record = {}) { return { stdout: "", @@ -54,17 +33,11 @@ function commandResult(overrides: Record = {}) { }; } -function bufferedCommandResult(overrides: Record = {}) { - return { - ...commandResult(), - stdout: Buffer.alloc(0), - stderr: Buffer.alloc(0), - ...overrides, - }; -} +type MockCommandResult = Record & { + outputByteLength?: number; +}; -async function importWithCommandResults(...results: Array>) { - const runCommandBuffered = vi.fn().mockResolvedValue(bufferedCommandResult()); +async function importToolWithCommandResults(tarBuffer: Buffer, ...results: MockCommandResult[]) { const runCommandWithTimeout = vi.fn(); for (const result of results) { runCommandWithTimeout.mockImplementationOnce( @@ -75,10 +48,15 @@ async function importWithCommandResults(...results: Array ({ - runCommandBuffered, runCommandWithTimeout, })); + vi.doMock("openclaw/plugin-sdk/media-store", () => ({ + saveMediaBuffer: vi.fn(async () => ({ path: path.join(tmpRoot, "archive.tar.gz") })), + })); + vi.doMock("../shared/audit.js", () => ({ + appendFileTransferAudit: vi.fn(async () => undefined), + })); + vi.doMock("./node-tool-invoke.js", () => ({ + readRequiredNodePath: (params: Record) => ({ + node: String(params.node), + requestedPath: String(params.path), + }), + invokeNodeToolPayload: vi.fn(async () => ({ + nodeId: "node-1", + nodeDisplayName: "Node One", + payload: { + ok: true, + path: "/tmp/project", + tarBase64: tarBuffer.toString("base64"), + tarBytes: tarBuffer.byteLength, + sha256: crypto.createHash("sha256").update(tarBuffer).digest("hex"), + fileCount: 1, + }, + startedAt: Date.now(), + })), + })); return { module: await import("./dir-fetch-tool.js"), - runCommandBuffered, runCommandWithTimeout, }; } -const testUnlessWindows = process.platform === "win32" ? it.skip : it; +async function executeDirFetch(module: typeof import("./dir-fetch-tool.js")) { + return await module.createDirFetchTool().execute("tool-call-1", { + node: "node-1", + path: "/tmp/project", + }); +} -describe("validateTarUncompressedBudget", () => { - testUnlessWindows( - "rejects an archive before extraction when expanded bytes exceed budget", - async () => { - await fs.writeFile(path.join(tmpRoot, "zeros.txt"), "0".repeat(128)); - const tarBuffer = await tarDirectory(tmpRoot); +const validListingResults = [{ stdout: "./ok.txt\n" }, { stdout: "-ok.txt\n" }] as const; - await expect(testing.validateTarUncompressedBudget(tarBuffer, 64)).resolves.toEqual({ - ok: false, - reason: "archive expands past uncompressed budget 64 bytes", - }); - await expect(testing.validateTarUncompressedBudget(tarBuffer, 256)).resolves.toEqual({ - ok: true, - }); - }, - ); +describe("dir.fetch tar validation", () => { + it("rejects an archive before extraction when expanded bytes exceed budget", async () => { + const { module } = await importToolWithCommandResults( + Buffer.from("archive"), + ...validListingResults, + { outputByteLength: 64 * 1024 * 1024 + 1 }, + ); - it("fails closed on wrapper errors", async () => { - const { module, runCommandWithTimeout } = await importWithCommandResults({ - code: null, - termination: "error", - error: new Error("budget read failed"), - }); + await expect(executeDirFetch(module)).rejects.toThrow( + "dir.fetch UNCOMPRESSED_TOO_LARGE: archive expands past uncompressed budget 67108864 bytes", + ); + }); - await expect(module.testing.validateTarUncompressedBudget(Buffer.from("x"))).resolves.toEqual({ - ok: false, - reason: "tar uncompressed budget validation error: budget read failed", - }); - expect(runCommandWithTimeout).toHaveBeenCalledWith( + it("fails uncompressed budget checks closed on wrapper errors", async () => { + const { module, runCommandWithTimeout } = await importToolWithCommandResults( + Buffer.from("archive"), + ...validListingResults, + { + code: null, + termination: "error", + error: new Error("budget read failed"), + }, + ); + + await expect(executeDirFetch(module)).rejects.toThrow( + "dir.fetch UNCOMPRESSED_TOO_LARGE: tar uncompressed budget validation error: budget read failed", + ); + expect(runCommandWithTimeout).toHaveBeenLastCalledWith( expect.any(Array), expect.objectContaining({ tolerateOutputError: { stderr: true } }), ); }); -}); -describe("dir.fetch tar validation", () => { it("fails tar listing closed on wrapper errors", async () => { - const { module } = await importWithCommandResults({ + const { module } = await importToolWithCommandResults(Buffer.from("archive"), { code: null, termination: "error", error: new Error("listing read failed"), }); - await expect(module.testing.preValidateTarball(Buffer.from("x"))).resolves.toEqual({ - ok: false, - reason: "tar -tzf error: listing read failed", - }); + await expect(executeDirFetch(module)).rejects.toThrow( + "dir.fetch UNSAFE_ARCHIVE: tar -tzf error: listing read failed", + ); }); - it("accepts successful unpack", async () => { - const { module, runCommandWithTimeout } = await importWithCommandResults(); + it("accepts successful validation and unpack", async () => { + const { module, runCommandWithTimeout } = await importToolWithCommandResults( + Buffer.from("archive"), + ...validListingResults, + {}, + {}, + ); - await expect(module.testing.unpackTar(Buffer.from("x"), tmpRoot)).resolves.toBeUndefined(); - expect(runCommandWithTimeout).toHaveBeenCalledWith( + await expect(executeDirFetch(module)).resolves.toMatchObject({ + details: { + path: "/tmp/project", + fileCount: 1, + }, + }); + expect(runCommandWithTimeout).toHaveBeenLastCalledWith( expect.any(Array), expect.objectContaining({ outputCapture: { stdout: "discard", stderr: "tail" }, @@ -167,69 +181,59 @@ describe("dir.fetch tar validation", () => { }); it("keeps tar exit diagnostics", async () => { - const { module } = await importWithCommandResults({ + const { module } = await importToolWithCommandResults(Buffer.from("archive"), { code: 2, stderr: "invalid archive", }); - await expect(module.testing.preValidateTarball(Buffer.from("x"))).resolves.toEqual({ - ok: false, - reason: "tar -tzf exited 2: invalid archive", - }); + await expect(executeDirFetch(module)).rejects.toThrow( + "dir.fetch UNSAFE_ARCHIVE: tar -tzf exited 2: invalid archive", + ); }); it("stops name validation at the entry cap", async () => { const tarLines = Array.from({ length: 5001 }, (_, index) => `file-${index}`).join("\n") + "\n"; - const { module, runCommandWithTimeout } = await importWithCommandResults({ - stdout: tarLines, - }); - - await expect(module.testing.preValidateTarball(Buffer.from("x"))).resolves.toEqual({ - ok: false, - reason: "archive contains 5001 entries; limit 5000", - }); - expect(runCommandWithTimeout).toHaveBeenCalledOnce(); - expect(runCommandWithTimeout).toHaveBeenCalledWith( - expect.any(Array), - expect.objectContaining({ tolerateOutputError: { stderr: true } }), + const { module, runCommandWithTimeout } = await importToolWithCommandResults( + Buffer.from("archive"), + { stdout: tarLines }, ); + + await expect(executeDirFetch(module)).rejects.toThrow( + "dir.fetch UNSAFE_ARCHIVE: archive contains 5001 entries; limit 5000", + ); + expect(runCommandWithTimeout).toHaveBeenCalledOnce(); }); it("keeps recent tar stderr when listing fails noisily", async () => { const oldNoise = "old-noise\n".repeat(600); const recent = "recent-invalid-archive-details\n".repeat(12); - const { module } = await importWithCommandResults({ + const { module } = await importToolWithCommandResults(Buffer.from("archive"), { code: 2, stderr: oldNoise + recent, }); - const result = await module.testing.preValidateTarball(Buffer.from("x")); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toContain(projectBoundedTextTail(recent, 200)); - expect(result.reason).not.toContain(oldNoise.slice(0, 40)); - } + await expect(executeDirFetch(module)).rejects.toThrow(projectBoundedTextTail(recent, 200)); }); it("surfaces a UTF-16-safe tar stderr tail", async () => { const oldNoise = "n".repeat(250); const recent = "🤖" + "f".repeat(199); - const { module } = await importWithCommandResults({ + const { module } = await importToolWithCommandResults(Buffer.from("archive"), { code: 2, stderr: oldNoise + recent, }); - const result = await module.testing.preValidateTarball(Buffer.from("x")); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toContain(projectBoundedTextTail(recent, 200)); - expect(result.reason).toContain("f".repeat(199)); - expect(result.reason).not.toContain("🤖"); - expect( - /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { - it("reads valid body within max bytes", async () => { - const req = createMockIncomingRequest(['{"type":"Create"}']); - const body = await readNextcloudTalkWebhookBody(req, 1024); - expect(body).toBe('{"type":"Create"}'); +async function invokeWebhookServerRequest(params: { + body: string; + headers: Record; + maxBodyBytes: number; +}) { + const { server } = createNextcloudTalkWebhookServer({ + host: "127.0.0.1", + port: 0, + path: "/nextcloud-body-limit", + secret: "nextcloud-secret", // pragma: allowlist secret + maxBodyBytes: params.maxBodyBytes, + onMessage: vi.fn(), }); + const listener = server.listeners("request")[0] as + | ((req: IncomingMessage, res: ServerResponse) => void) + | undefined; + if (!listener) { + throw new Error("expected Nextcloud Talk request listener"); + } + const req = Object.assign(createMockIncomingRequest([params.body]), { + method: "POST", + url: "/nextcloud-body-limit", + headers: params.headers, + socket: { remoteAddress: "127.0.0.1" }, + }) as unknown as IncomingMessage; - it("rejects when payload exceeds max bytes", async () => { - const req = createMockIncomingRequest(["x".repeat(300)]); - await expect(readNextcloudTalkWebhookBody(req, 128)).rejects.toThrow("PayloadTooLarge"); + return await new Promise<{ body: string; status: number }>((resolve) => { + let status = 0; + const res = { + headersSent: false, + writeHead(code: number) { + status = code; + this.headersSent = true; + return this; + }, + end(body?: string) { + resolve({ body: body ?? "", status }); + return this; + }, + } as unknown as ServerResponse; + listener(req, res); }); -}); +} describe("createNextcloudTalkWebhookServer auth order", () => { it("rejects missing signature headers before reading request body", async () => { @@ -49,6 +80,19 @@ describe("createNextcloudTalkWebhookServer auth order", () => { expect(await response.json()).toEqual({ error: "Missing signature headers" }); expect(readBody).not.toHaveBeenCalled(); }); + + it("rejects signed payloads over the configured body limit", async () => { + const { body, headers } = createSignedCreateMessageRequest(); + + const response = await invokeWebhookServerRequest({ + body, + headers, + maxBodyBytes: 128, + }); + + expect(response.status).toBe(413); + expect(JSON.parse(response.body)).toEqual({ error: "Payload too large" }); + }); }); describe("createNextcloudTalkWebhookServer backend allowlist", () => { @@ -143,25 +187,6 @@ describe("createNextcloudTalkWebhookServer replay handling", () => { expect(onMessage).toHaveBeenCalledTimes(1); }); - it("allows a retry after replay-guarded processing fails before commit", async () => { - let attempts = 0; - const handleMessage = vi.fn(async () => { - attempts += 1; - if (attempts === 1) { - throw new NextcloudTalkRetryableWebhookError("transient nextcloud failure"); - } - }); - const processMessage = createReplayGuardedProcess({ - handleMessage, - }); - const message = buildInboundMessage(); - - await expect(processMessage(message)).rejects.toThrow("transient nextcloud failure"); - await expect(processMessage(message)).resolves.toBe("processed"); - - expect(handleMessage).toHaveBeenCalledTimes(2); - }); - it("keeps replay committed after a non-retryable replay-guarded processing failure", async () => { const visibleSideEffect = vi.fn(); const handleMessage = vi.fn(async () => { diff --git a/extensions/nextcloud-talk/src/monitor.ts b/extensions/nextcloud-talk/src/monitor.ts index 349647986ac..ae5d4e7fc24 100644 --- a/extensions/nextcloud-talk/src/monitor.ts +++ b/extensions/nextcloud-talk/src/monitor.ts @@ -61,13 +61,6 @@ const WEBHOOK_ERRORS = { internalServerError: "Internal server error", } as const; -export class NextcloudTalkRetryableWebhookError extends Error { - constructor(message: string, options?: ErrorOptions) { - super(message, options); - this.name = "NextcloudTalkRetryableWebhookError"; - } -} - export async function processNextcloudTalkReplayGuardedMessage(params: { replayGuard: NextcloudTalkReplayGuard; accountId: string; @@ -92,22 +85,13 @@ export async function processNextcloudTalkReplayGuardedMessage(params: { }); return "processed"; } catch (error) { - if (error instanceof NextcloudTalkRetryableWebhookError) { - params.replayGuard.releaseMessage({ - accountId: params.accountId, - roomToken: params.message.roomToken, - messageId: params.message.messageId, - error, - }); - } else { - // Generic failures are treated as non-retryable because the handler may already - // have produced a visible side effect, and replaying the webhook would duplicate it. - await params.replayGuard.commitMessage({ - accountId: params.accountId, - roomToken: params.message.roomToken, - messageId: params.message.messageId, - }); - } + // Failures are treated as non-retryable because the handler may already + // have produced a visible side effect, and replaying the webhook would duplicate it. + await params.replayGuard.commitMessage({ + accountId: params.accountId, + roomToken: params.message.roomToken, + messageId: params.message.messageId, + }); throw error; } } @@ -231,10 +215,7 @@ function payloadToInboundMessage( }; } -export function readNextcloudTalkWebhookBody( - req: IncomingMessage, - maxBodyBytes: number, -): Promise { +function readNextcloudTalkWebhookBody(req: IncomingMessage, maxBodyBytes: number): Promise { return readRequestBodyWithLimit(req, { // This read happens before signature verification, so keep the unauthenticated // body budget bounded even if the operator-configured post-parse limit is larger. diff --git a/extensions/nextcloud-talk/src/room-info.fetch-timeout.test.ts b/extensions/nextcloud-talk/src/room-info.fetch-timeout.test.ts index dba43180b6d..30732993e3e 100644 --- a/extensions/nextcloud-talk/src/room-info.fetch-timeout.test.ts +++ b/extensions/nextcloud-talk/src/room-info.fetch-timeout.test.ts @@ -1,47 +1,43 @@ // Nextcloud Talk room info lookup tests cover real HTTP timeout behavior. import { withServer } from "openclaw/plugin-sdk/test-env"; import { describe, expect, it, vi } from "vitest"; -import { resolveNextcloudTalkRoomKind, testing } from "./room-info.js"; +import { resolveNextcloudTalkRoomKind } from "./room-info.js"; describe("nextcloud talk room info fetch timeout", () => { it("bounds hanging room info GET requests", async () => { let received = false; const runtimeError = vi.fn(); - try { - await withServer( - (request) => { - received = true; - expect(request.method).toBe("GET"); - expect(request.url).toBe("/ocs/v2.php/apps/spreed/api/v4/room/abc123"); - request.resume(); - }, - async (baseUrl) => { - const kind = await resolveNextcloudTalkRoomKind({ - account: { - accountId: "acct-hanging-room-info", - baseUrl, - config: { - apiUser: "bot", - apiPassword: "secret", - network: { dangerouslyAllowPrivateNetwork: true }, - }, - } as never, - roomToken: "abc123", - runtime: { - error: runtimeError, - exit: vi.fn(), - log: vi.fn(), + await withServer( + (request) => { + received = true; + expect(request.method).toBe("GET"); + expect(request.url).toBe("/ocs/v2.php/apps/spreed/api/v4/room/abc123"); + request.resume(); + }, + async (baseUrl) => { + const kind = await resolveNextcloudTalkRoomKind({ + account: { + accountId: "acct-hanging-room-info", + baseUrl, + config: { + apiUser: "bot", + apiPassword: "secret", + network: { dangerouslyAllowPrivateNetwork: true }, }, - timeoutMs: 50, - }); + } as never, + roomToken: "abc123", + runtime: { + error: runtimeError, + exit: vi.fn(), + log: vi.fn(), + }, + timeoutMs: 50, + }); - expect(kind).toBeUndefined(); - }, - ); - } finally { - testing.resetRoomCache(); - } + expect(kind).toBeUndefined(); + }, + ); expect(received).toBe(true); expect(String(runtimeError.mock.calls[0]?.[0] ?? "")).toMatch(/abort|timeout/i); diff --git a/extensions/nextcloud-talk/src/room-info.test.ts b/extensions/nextcloud-talk/src/room-info.test.ts index b542888e86f..407bd53ef2f 100644 --- a/extensions/nextcloud-talk/src/room-info.test.ts +++ b/extensions/nextcloud-talk/src/room-info.test.ts @@ -3,7 +3,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { resolveNextcloudTalkRoomKind, testing } from "./room-info.js"; +import { resolveNextcloudTalkRoomKind } from "./room-info.js"; const fetchWithSsrFGuard = vi.hoisted(() => vi.fn()); const tempDirs: string[] = []; @@ -14,7 +14,6 @@ vi.mock("../runtime-api.js", () => { afterEach(() => { fetchWithSsrFGuard.mockReset(); - testing.resetRoomCache(); for (const dir of tempDirs.splice(0)) { rmSync(dir, { force: true, recursive: true }); } diff --git a/extensions/nextcloud-talk/src/room-info.ts b/extensions/nextcloud-talk/src/room-info.ts index 673f5bf30fc..2feb6eb1ef6 100644 --- a/extensions/nextcloud-talk/src/room-info.ts +++ b/extensions/nextcloud-talk/src/room-info.ts @@ -18,12 +18,6 @@ const roomCache = new Map< { kind?: "direct" | "group"; fetchedAt: number; error?: string } >(); -export const testing = { - resetRoomCache() { - roomCache.clear(); - }, -}; - function resolveRoomCacheKey(params: { accountId: string; roomToken: string }) { return `${params.accountId}:${params.roomToken}`; } diff --git a/extensions/synology-chat/src/channel.integration.test.ts b/extensions/synology-chat/src/channel.integration.test.ts index 9a1c50fc0ec..adc71c0b67f 100644 --- a/extensions/synology-chat/src/channel.integration.test.ts +++ b/extensions/synology-chat/src/channel.integration.test.ts @@ -10,7 +10,7 @@ import { } from "./channel.test-mocks.js"; import { makeFormBody, makeReq, makeRes } from "./test-http-utils.js"; -let createSynologyChatPlugin: typeof import("./channel.js").createSynologyChatPlugin; +let synologyChatPlugin: typeof import("./channel.js").synologyChatPlugin; function makeStartContext(cfg: T, accountId: string, abortSignal: AbortSignal) { setSynologyRuntimeConfigForTest(cfg); @@ -43,7 +43,7 @@ function requireMockCall( describe("Synology channel wiring integration", () => { beforeAll(async () => { - ({ createSynologyChatPlugin } = await import("./channel.js")); + ({ synologyChatPlugin } = await import("./channel.js")); }); beforeEach(() => { @@ -56,7 +56,7 @@ describe("Synology channel wiring integration", () => { }); it("registers real webhook handler with resolved account config and enforces allowlist", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const abortController = new AbortController(); const cfg = { channels: { @@ -109,7 +109,7 @@ describe("Synology channel wiring integration", () => { }); it("uses gateway trusted proxy settings for pre-auth invalid-token throttling", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const abortController = new AbortController(); const cfg = { gateway: { @@ -172,7 +172,7 @@ describe("Synology channel wiring integration", () => { }); it("isolates same user_id across different accounts", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const alphaAbortController = new AbortController(); const betaAbortController = new AbortController(); const cfg = { diff --git a/extensions/synology-chat/src/channel.test.ts b/extensions/synology-chat/src/channel.test.ts index af2d6d0784a..ee385261888 100644 --- a/extensions/synology-chat/src/channel.test.ts +++ b/extensions/synology-chat/src/channel.test.ts @@ -50,7 +50,7 @@ vi.mock("./webhook-handler.js", () => ({ createWebhookHandler: vi.fn(() => vi.fn()), })); -const { createSynologyChatPlugin, synologyChatPlugin } = await import("./channel.js"); +const { synologyChatPlugin } = await import("./channel.js"); const getSynologyChatSetupStatus = createPluginSetupWizardStatus(synologyChatPlugin); describe("createSynologyChatPlugin", () => { @@ -71,7 +71,7 @@ describe("createSynologyChatPlugin", () => { describe("meta", () => { it("has correct id and label", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; expect(plugin.meta.id).toBe("synology-chat"); expect(plugin.meta.label).toBe("Synology Chat"); expect(plugin.meta.docsPath).toBe("/channels/synology-chat"); @@ -80,7 +80,7 @@ describe("createSynologyChatPlugin", () => { describe("messaging", () => { it("isolates stable Chat API recipients from inbound webhook identities", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const route = await plugin.messaging?.resolveOutboundSessionRoute?.({ cfg: {}, agentId: "ops", @@ -100,7 +100,7 @@ describe("createSynologyChatPlugin", () => { }); it("rejects non-numeric Chat API recipients for session routing", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const route = await plugin.messaging?.resolveOutboundSessionRoute?.({ cfg: {}, agentId: "ops", @@ -111,7 +111,7 @@ describe("createSynologyChatPlugin", () => { }); it("canonicalizes safe Chat API recipient IDs", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const route = await plugin.messaging?.resolveOutboundSessionRoute?.({ cfg: {}, agentId: "ops", @@ -126,7 +126,7 @@ describe("createSynologyChatPlugin", () => { }); it("rejects Chat API recipient IDs beyond the safe integer range", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const route = await plugin.messaging?.resolveOutboundSessionRoute?.({ cfg: {}, agentId: "ops", @@ -139,7 +139,7 @@ describe("createSynologyChatPlugin", () => { describe("capabilities", () => { it("supports direct chat with media", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; expect(plugin.capabilities.chatTypes).toEqual(["direct"]); expect(plugin.capabilities.media).toBe(true); expect(plugin.capabilities.threads).toBe(false); @@ -148,7 +148,7 @@ describe("createSynologyChatPlugin", () => { describe("config", () => { it("listAccountIds includes default and named accounts when configured", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const result = plugin.config.listAccountIds({ channels: { "synology-chat": { @@ -181,7 +181,7 @@ describe("createSynologyChatPlugin", () => { }, }, }; - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = plugin.config.resolveAccount(cfg, "office"); expect(account.accountId).toBe("office"); expect(account.token).toBe("office-token"); @@ -194,7 +194,7 @@ describe("createSynologyChatPlugin", () => { }); it("defaultAccountId returns 'default'", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; expect(plugin.config.defaultAccountId?.({})).toBe("default"); }); @@ -228,7 +228,7 @@ describe("createSynologyChatPlugin", () => { }); it("formats allowFrom entries through the shared adapter", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; expect( plugin.config.formatAllowFrom?.({ cfg: {}, @@ -240,7 +240,7 @@ describe("createSynologyChatPlugin", () => { describe("security", () => { it("resolveDmPolicy returns policy, allowFrom, normalizeEntry", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = { accountId: "default", enabled: true, @@ -269,7 +269,7 @@ describe("createSynologyChatPlugin", () => { describe("pairing", () => { it("normalizes entries and notifies approved users", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; expect(plugin.pairing.idLabel).toBe("synologyChatUserId"); const normalize = plugin.pairing.normalizeAllowEntry; const notifyApproval = plugin.pairing.notifyApproval; @@ -322,28 +322,28 @@ describe("createSynologyChatPlugin", () => { } it("warns when token is missing", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = makeSecurityAccount({ token: "" }); const warnings = plugin.security.collectWarnings({ cfg: {}, account }); expectIncludesSubstring(warnings, "token"); }); it("warns when allowInsecureSsl is true", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = makeSecurityAccount({ allowInsecureSsl: true }); const warnings = plugin.security.collectWarnings({ cfg: {}, account }); expectIncludesSubstring(warnings, "SSL"); }); it("warns when dangerous name matching is enabled", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = makeSecurityAccount({ dangerouslyAllowNameMatching: true }); const warnings = plugin.security.collectWarnings({ cfg: {}, account }); expectIncludesSubstring(warnings, "dangerouslyAllowNameMatching"); }); it("warns when inherited shared webhookPath is dangerously re-enabled", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = makeSecurityAccount({ accountId: "alerts", webhookPathSource: "inherited-base", @@ -354,28 +354,28 @@ describe("createSynologyChatPlugin", () => { }); it("warns when dmPolicy is open", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = makeSecurityAccount({ dmPolicy: "open", allowedUserIds: ["*"] }); const warnings = plugin.security.collectWarnings({ cfg: {}, account }); expectIncludesSubstring(warnings, "open"); }); it("warns when dmPolicy is open and allowedUserIds is empty", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = makeSecurityAccount({ dmPolicy: "open", allowedUserIds: [] }); const warnings = plugin.security.collectWarnings({ cfg: {}, account }); expectIncludesSubstring(warnings, "empty allowedUserIds"); }); it("warns when dmPolicy is allowlist and allowedUserIds is empty", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = makeSecurityAccount(); const warnings = plugin.security.collectWarnings({ cfg: {}, account }); expectIncludesSubstring(warnings, "empty allowedUserIds"); }); it("warns when named multi-account routes inherit a shared webhookPath", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const cfg = makeSharedWebhookConfig(); const account = plugin.config.resolveAccount(cfg, "alerts"); const warnings = plugin.security.collectWarnings({ cfg, account }); @@ -383,7 +383,7 @@ describe("createSynologyChatPlugin", () => { }); it("warns when enabled accounts share the same exact webhookPath", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const base = makeSharedWebhookConfig({ webhookPath: "/webhook/shared" }).channels[ "synology-chat" ]; @@ -403,7 +403,7 @@ describe("createSynologyChatPlugin", () => { }); it("returns no warnings for fully configured account", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const account = makeSecurityAccount({ allowedUserIds: ["user1"] }); const warnings = plugin.security.collectWarnings({ cfg: {}, account }); expect(warnings).toHaveLength(0); @@ -412,7 +412,7 @@ describe("createSynologyChatPlugin", () => { describe("messaging", () => { it("normalizeTarget strips prefix and trims", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; expect(plugin.messaging.normalizeTarget("synology-chat:123")).toBe("123"); expect(plugin.messaging.normalizeTarget("synology_chat:123")).toBe("123"); expect(plugin.messaging.normalizeTarget("synology:123")).toBe("123"); @@ -421,7 +421,7 @@ describe("createSynologyChatPlugin", () => { }); it("targetResolver.looksLikeId matches numeric IDs", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; expect(plugin.messaging.targetResolver.looksLikeId("12345")).toBe(true); expect(plugin.messaging.targetResolver.looksLikeId("synology-chat:99")).toBe(true); expect(plugin.messaging.targetResolver.looksLikeId("synology_chat:99")).toBe(true); @@ -433,7 +433,7 @@ describe("createSynologyChatPlugin", () => { describe("directory", () => { it("returns empty stubs", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const params = { cfg: {}, runtime: {} as never }; expect(await plugin.directory.self?.(params)).toBeNull(); expect(await plugin.directory.listPeers?.(params)).toStrictEqual([]); @@ -443,7 +443,7 @@ describe("createSynologyChatPlugin", () => { describe("agentPrompt", () => { it("returns formatting hints", () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const hints = plugin.agentPrompt.messageToolHints(); expect(hints).toContain("### Synology Chat Formatting"); expect(hints).toContain("**Links**: Use `` to create clickable links."); @@ -453,7 +453,7 @@ describe("createSynologyChatPlugin", () => { describe("outbound", () => { it("declares message adapter durable text and media with receipt proofs", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const cfg = { channels: { "synology-chat": { @@ -503,7 +503,7 @@ describe("createSynologyChatPlugin", () => { }); it("sendText throws when no incomingUrl", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; await expect( plugin.outbound.sendText({ cfg: { @@ -518,7 +518,7 @@ describe("createSynologyChatPlugin", () => { }); it("sendText returns OutboundDeliveryResult on success", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const result = await plugin.outbound.sendText({ cfg: { channels: { @@ -541,7 +541,7 @@ describe("createSynologyChatPlugin", () => { }); it("sendMedia throws when missing incomingUrl", async () => { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; await expect( plugin.outbound.sendMedia({ cfg: { @@ -557,7 +557,7 @@ describe("createSynologyChatPlugin", () => { it("sanitizeText strips internal tool-trace banners from outbound text", () => { const text = "Done.\n⚠️ 🛠️ `search repos (agent)` failed"; - const sanitizeText = createSynologyChatPlugin().outbound.sanitizeText; + const sanitizeText = synologyChatPlugin.outbound.sanitizeText; expect(sanitizeText({ text, payload: { text } })).toBe("Done."); const prose = "The pipeline has 3 open deals."; @@ -567,7 +567,7 @@ describe("createSynologyChatPlugin", () => { it("sanitizeText returns empty string for trace-only replies", () => { const traceOnly = "⚠️ 🛠️ `search repos (agent)` failed"; expect( - createSynologyChatPlugin().outbound.sanitizeText({ + synologyChatPlugin.outbound.sanitizeText({ text: traceOnly, payload: { text: traceOnly }, }), @@ -648,7 +648,7 @@ describe("createSynologyChatPlugin", () => { } async function expectPendingStartAccount(accountConfig: Record) { - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const { ctx, abortController } = makeStartAccountCtx(accountConfig); const result = plugin.gateway.startAccount(ctx); await expectPendingStartAccountPromise(result, abortController); @@ -665,7 +665,7 @@ describe("createSynologyChatPlugin", () => { it("startAccount refuses allowlist accounts with empty allowedUserIds", async () => { const registerMock = registerSynologyWebhookRouteMock; registerMock.mockClear(); - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const { ctx, abortController } = makeStartAccountCtx({ enabled: true, token: "t", @@ -683,7 +683,7 @@ describe("createSynologyChatPlugin", () => { it("startAccount refuses open accounts with empty allowedUserIds", async () => { const registerMock = registerSynologyWebhookRouteMock; registerMock.mockClear(); - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const { ctx, abortController } = makeStartAccountCtx({ enabled: true, token: "t", @@ -703,7 +703,7 @@ describe("createSynologyChatPlugin", () => { it("startAccount refuses named accounts without explicit webhookPath in multi-account setups", async () => { const registerMock = registerSynologyWebhookRouteMock; - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const { ctx, abortController } = makeNamedStartAccountCtx({ dmPolicy: "allowlist", allowedUserIds: ["123"], @@ -717,7 +717,7 @@ describe("createSynologyChatPlugin", () => { it("startAccount refuses duplicate exact webhook paths across accounts", async () => { const registerMock = registerSynologyWebhookRouteMock; - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const { ctx, abortController } = makeNamedStartAccountCtx({ webhookPath: "/webhook/synology-shared", dmPolicy: "open", @@ -736,7 +736,7 @@ describe("createSynologyChatPlugin", () => { const registerMock = registerSynologyWebhookRouteMock; registerMock.mockReturnValueOnce(unregisterFirst).mockReturnValueOnce(unregisterSecond); - const plugin = createSynologyChatPlugin(); + const plugin = synologyChatPlugin; const abortFirst = new AbortController(); const abortSecond = new AbortController(); const makeCtx = (abortCtrl: AbortController) => ({ diff --git a/extensions/synology-chat/src/channel.ts b/extensions/synology-chat/src/channel.ts index 8acc2b7c8a5..08c140b0b91 100644 --- a/extensions/synology-chat/src/channel.ts +++ b/extensions/synology-chat/src/channel.ts @@ -300,7 +300,7 @@ const synologyChatMessageAdapter = defineChannelMessageAdapter({ }, }); -export function createSynologyChatPlugin(): SynologyChatPlugin { +function createSynologyChatPlugin(): SynologyChatPlugin { return createChatChannelPlugin({ base: { id: CHANNEL_ID, diff --git a/extensions/synology-chat/src/client.test.ts b/extensions/synology-chat/src/client.test.ts index 750a0bee8ea..7166d4b089f 100644 --- a/extensions/synology-chat/src/client.test.ts +++ b/extensions/synology-chat/src/client.test.ts @@ -33,7 +33,6 @@ const https = await import("node:https"); let fakeNowMs = 1_700_000_000_000; let sendMessage: typeof import("./client.js").sendMessage; let sendFileUrl: typeof import("./client.js").sendFileUrl; -let fetchChatUsers: typeof import("./client.js").fetchChatUsers; let resolveLegacyWebhookNameToChatUserId: typeof import("./client.js").resolveLegacyWebhookNameToChatUserId; type RequestCallback = (res: IncomingMessage) => void; @@ -108,7 +107,7 @@ function mockFailureResponse(statusCode = 500) { function installFakeTimerHarness() { beforeAll(async () => { - ({ sendMessage, sendFileUrl, fetchChatUsers, resolveLegacyWebhookNameToChatUserId } = + ({ sendMessage, sendFileUrl, resolveLegacyWebhookNameToChatUserId } = await import("./client.js")); }); @@ -416,7 +415,7 @@ describe("resolveLegacyWebhookNameToChatUserId", () => { }); }); -describe("fetchChatUsers", () => { +describe("resolveLegacyWebhookNameToChatUserId user lookup", () => { installFakeTimerHarness(); it("filters malformed user entries while keeping valid ones", async () => { @@ -425,11 +424,13 @@ describe("fetchChatUsers", () => { { user_id: "bad", username: "broken" }, ]); - const users = await fetchChatUsers( - "https://nas.example.com/webapi/entry.cgi?api=SYNO.Chat.External&method=chatbot&version=2&token=%22test%22", - ); + const userId = await resolveLegacyWebhookNameToChatUserId({ + incomingUrl: + "https://nas.example.com/webapi/entry.cgi?api=SYNO.Chat.External&method=chatbot&version=2&token=%22test%22", + mutableWebhookUsername: "jmn", + }); - expect(users).toEqual([{ user_id: 4, username: "jmn67", nickname: "jmn" }]); + expect(userId).toBe(4); }); it("verifies TLS by default for user_list lookups", async () => { @@ -437,7 +438,10 @@ describe("fetchChatUsers", () => { const freshUrl = "https://fresh-nas.example.com/webapi/entry.cgi?api=SYNO.Chat.External&method=chatbot&version=2&token=%22fresh%22"; - await fetchChatUsers(freshUrl); + await resolveLegacyWebhookNameToChatUserId({ + incomingUrl: freshUrl, + mutableWebhookUsername: "jmn", + }); const firstCall = firstHttpsGetCall(); expect(firstCall[1]?.rejectUnauthorized).toBe(true); diff --git a/extensions/synology-chat/src/client.ts b/extensions/synology-chat/src/client.ts index cbb662e48f0..8baeb729dda 100644 --- a/extensions/synology-chat/src/client.ts +++ b/extensions/synology-chat/src/client.ts @@ -147,7 +147,7 @@ export async function sendFileUrl( * The user_list endpoint uses the same base URL as the chatbot API but * with method=user_list instead of method=chatbot. */ -export async function fetchChatUsers( +async function fetchChatUsers( incomingUrl: string, allowInsecureSsl = false, log?: { warn: (...args: unknown[]) => void }, diff --git a/extensions/synology-chat/src/webhook-handler.test.ts b/extensions/synology-chat/src/webhook-handler.test.ts index b4bb5f5d6ae..7af89204f2b 100644 --- a/extensions/synology-chat/src/webhook-handler.test.ts +++ b/extensions/synology-chat/src/webhook-handler.test.ts @@ -9,8 +9,7 @@ const sendMessage = vi.spyOn(clientModule, "sendMessage").mockResolvedValue(true const resolveLegacyWebhookNameToChatUserId = vi .spyOn(clientModule, "resolveLegacyWebhookNameToChatUserId") .mockResolvedValue(undefined); -const { clearSynologyWebhookRateLimiterStateForTest, createWebhookHandler } = - await import("./webhook-handler.js"); +const { createWebhookHandler } = await import("./webhook-handler.js"); type TestLog = { info: (...args: unknown[]) => void; @@ -48,11 +47,13 @@ function deliveredMessage(deliver: ReturnType) { return message; } +let accountSequence = 0; + function makeAccount( overrides: Partial = {}, ): ResolvedSynologyChatAccount { return { - accountId: "default", + accountId: `test-account-${++accountSequence}`, enabled: true, token: "valid-token", incomingUrl: "https://nas.example.com/incoming", @@ -114,7 +115,6 @@ describe("createWebhookHandler", () => { let log: TestLog; beforeEach(() => { - clearSynologyWebhookRateLimiterStateForTest(); sendMessage.mockClear(); sendMessage.mockResolvedValue(true); resolveLegacyWebhookNameToChatUserId.mockClear(); diff --git a/extensions/synology-chat/src/webhook-handler.ts b/extensions/synology-chat/src/webhook-handler.ts index e8af85eb2b9..f586ab3236b 100644 --- a/extensions/synology-chat/src/webhook-handler.ts +++ b/extensions/synology-chat/src/webhook-handler.ts @@ -120,18 +120,6 @@ function getInvalidTokenRateLimiter(account: ResolvedSynologyChatAccount): Inval return rl; } -export function clearSynologyWebhookRateLimiterStateForTest(): void { - for (const limiter of rateLimiters.values()) { - limiter.clear(); - } - rateLimiters.clear(); - for (const limiter of invalidTokenRateLimiters.values()) { - limiter.clear(); - } - invalidTokenRateLimiters.clear(); - webhookInFlightLimiter.clear(); -} - function getSynologyWebhookInvalidTokenRateLimitKey(params: { req: IncomingMessage; trustedProxies?: string[]; diff --git a/scripts/deadcode-exports.baseline.mjs b/scripts/deadcode-exports.baseline.mjs index b889186881e..9032415c16f 100644 --- a/scripts/deadcode-exports.baseline.mjs +++ b/scripts/deadcode-exports.baseline.mjs @@ -7,9 +7,6 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [ "extensions/codex/src/session-upstream-activity.ts: checkCodexUpstreamActivity (upstream)", "extensions/codex/src/session-upstream-activity.ts: classifyCodexUpstreamTurns", "extensions/diagnostics-prometheus/src/service.ts: testApi", - "extensions/diffs/src/browser.ts: resetSharedBrowserStateForTests", - "extensions/diffs/src/shiki-curated-languages.ts: bundledLanguages", - "extensions/diffs/src/shiki-curated-languages.ts: bundledLanguagesAlias", "extensions/discord/src/actions/runtime.guild.ts: discordGuildActionRuntime", "extensions/discord/src/actions/runtime.moderation.ts: discordModerationActionRuntime", "extensions/discord/src/components-registry.ts: clearDiscordComponentEntries", @@ -22,9 +19,6 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [ "extensions/discord/src/monitor/native-command.runtime.ts: testing", "extensions/discord/src/monitor/native-command.ts: testing", "extensions/discord/src/monitor/provider.ts: testing", - "extensions/file-transfer/src/node-host/dir-fetch.ts: testing", - "extensions/file-transfer/src/shared/node-invoke-policy.ts: testing", - "extensions/file-transfer/src/tools/dir-fetch-tool.ts: testing", "extensions/google-meet/src/transports/chrome.ts: testing", "extensions/googlechat/src/approval-card-actions.ts: clearGoogleChatApprovalCardBindingsForTest", "extensions/googlechat/src/auth.ts: testing", @@ -55,9 +49,6 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [ "extensions/msteams/src/team-identity.ts: _teamGroupIdCacheForTest", "extensions/msteams/src/thread-parent-context.ts: resetThreadParentContextCachesForTest", "extensions/msteams/src/user-agent.ts: resetUserAgentCache", - "extensions/nextcloud-talk/src/monitor.ts: NextcloudTalkRetryableWebhookError", - "extensions/nextcloud-talk/src/monitor.ts: readNextcloudTalkWebhookBody", - "extensions/nextcloud-talk/src/room-info.ts: testing", "extensions/ollama/src/provider-models.ts: resetOllamaModelShowInfoCacheForTest", "extensions/ollama/src/web-search-provider.ts: testing", "extensions/openshell/src/backend.ts: ENSURE_OPEN_SHELL_REMOTE_REAL_DIRECTORY_SCRIPT", @@ -74,9 +65,6 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [ "extensions/qa-matrix/src/substrate/e2ee-client.ts: testing", "extensions/qa-matrix/src/substrate/harness.runtime.ts: testing", "extensions/signal/src/reply-authors.ts: clearSignalReplyAuthorsForTest", - "extensions/synology-chat/src/channel.ts: createSynologyChatPlugin", - "extensions/synology-chat/src/client.ts: fetchChatUsers (synologyClient)", - "extensions/synology-chat/src/webhook-handler.ts: clearSynologyWebhookRateLimiterStateForTest", "extensions/telegram/src/account-throttler.ts: clearAccountThrottlersForTest", "extensions/telegram/src/bot-info-cache.ts: setTelegramBotInfoCacheStoreForTest", "extensions/telegram/src/bot-message-dispatch.ts: resetTelegramReplyFenceForTests", diff --git a/scripts/diffs-shiki-curated.ts b/scripts/diffs-shiki-curated.ts index 7aaeec39cc0..9fcff1bdb74 100644 --- a/scripts/diffs-shiki-curated.ts +++ b/scripts/diffs-shiki-curated.ts @@ -13,17 +13,25 @@ import { defaultJavaScriptRegexConstructor, } from "@shikijs/engine-javascript"; import { createOnigurumaEngine, loadWasm } from "@shikijs/engine-oniguruma"; -import { bundledLanguages } from "../extensions/diffs/src/shiki-curated-languages.js"; -export * from "@shikijs/core"; -export { - bundledLanguages, - bundledLanguagesAlias, +import { bundledLanguagesBase, bundledLanguagesInfo, } from "../extensions/diffs/src/shiki-curated-languages.js"; +export * from "@shikijs/core"; +export { bundledLanguagesBase, bundledLanguagesInfo }; export { bundledThemes, bundledThemesInfo } from "shiki/themes"; import { bundledThemes } from "shiki/themes"; +export const bundledLanguagesAlias = Object.fromEntries( + bundledLanguagesInfo.flatMap((language) => + ("aliases" in language ? language.aliases : []).map((alias) => [alias, language.import]), + ), +); +export const bundledLanguages = { + ...bundledLanguagesBase, + ...bundledLanguagesAlias, +}; + export const createHighlighter = createBundledHighlighter({ langs: bundledLanguages, themes: bundledThemes,