Skip to content

Commit 100d9a7

Browse files
steipetesteipete
committed
refactor: share boundary open and gateway test helpers
1 parent b21bcf6 commit 100d9a7

11 files changed

+140
-91
lines changed

‎src/gateway/control-ui.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import fs from "node:fs";
22
import type { IncomingMessage, ServerResponse } from "node:http";
33
import path from "node:path";
44
import type { OpenClawConfig } from "../config/config.js";
5-
import { openBoundaryFileSync } from "../infra/boundary-file-read.js";
5+
import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js";
66
import {
77
isPackageProvenControlUiRootSync,
88
resolveControlUiRootSync,
@@ -271,10 +271,12 @@ function resolveSafeControlUiFile(
271271
rejectHardlinks,
272272
});
273273
if (!opened.ok) {
274-
if (opened.reason === "io") {
275-
throw opened.error;
276-
}
277-
return null;
274+
return matchBoundaryFileOpenFailure(opened, {
275+
io: (failure) => {
276+
throw failure.error;
277+
},
278+
fallback: () => null,
279+
});
278280
}
279281
return { path: opened.path, fd: opened.fd };
280282
}

‎src/gateway/server.chat.gateway-server-chat-b.test.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
connectOk,
99
getReplyFromConfig,
1010
installGatewayTestHooks,
11+
mockGetReplyFromConfigOnce,
1112
onceMessage,
1213
rpcReq,
1314
startServerWithClient,
@@ -166,9 +167,8 @@ describe("gateway server chat", () => {
166167
await writeMainSessionStore();
167168
testState.agentConfig = { blockStreamingDefault: "on" };
168169
try {
169-
spy.mockClear();
170170
let capturedOpts: GetReplyOptions | undefined;
171-
spy.mockImplementationOnce(async (_ctx: unknown, opts?: GetReplyOptions) => {
171+
mockGetReplyFromConfigOnce(async (_ctx, opts) => {
172172
capturedOpts = opts;
173173
return undefined;
174174
});
@@ -386,8 +386,7 @@ describe("gateway server chat", () => {
386386
await createSessionDir();
387387
await writeMainSessionStore();
388388

389-
spy.mockClear();
390-
spy.mockImplementationOnce(async (_ctx, opts) => {
389+
mockGetReplyFromConfigOnce(async (_ctx, opts) => {
391390
opts?.onAgentRunStart?.(opts.runId ?? "idem-abort-1");
392391
const signal = opts?.abortSignal;
393392
await new Promise<void>((resolve) => {

‎src/gateway/server.chat.gateway-server-chat.test.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,13 @@ import os from "node:os";
33
import path from "node:path";
44
import { describe, expect, test, vi } from "vitest";
55
import { WebSocket } from "ws";
6-
import type { GetReplyOptions } from "../auto-reply/types.js";
76
import { emitAgentEvent, registerAgentRunContext } from "../infra/agent-events.js";
87
import { extractFirstTextBlock } from "../shared/chat-message-content.js";
98
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
109
import {
1110
connectOk,
12-
getReplyFromConfig,
1311
installGatewayTestHooks,
12+
mockGetReplyFromConfigOnce,
1413
onceMessage,
1514
rpcReq,
1615
testState,
@@ -166,8 +165,7 @@ describe("gateway server chat", () => {
166165
const blockedReply = new Promise<void>((resolve) => {
167166
releaseBlockedReply = resolve;
168167
});
169-
const replySpy = vi.mocked(getReplyFromConfig);
170-
replySpy.mockImplementationOnce(async (_ctx: unknown, opts?: GetReplyOptions) => {
168+
mockGetReplyFromConfigOnce(async (_ctx, opts) => {
171169
await new Promise<void>((resolve) => {
172170
let settled = false;
173171
const finish = () => {
@@ -564,11 +562,10 @@ describe("gateway server chat", () => {
564562

565563
test("routes /btw replies through side-result events without transcript injection", async () => {
566564
await withMainSessionStore(async () => {
567-
const replyMock = vi.mocked(getReplyFromConfig);
568-
replyMock.mockResolvedValueOnce({
565+
mockGetReplyFromConfigOnce(async () => ({
569566
text: "323",
570567
btw: { question: "what is 17 * 19?" },
571-
});
568+
}));
572569
const sideResultPromise = onceMessage(
573570
ws,
574571
(o) =>
@@ -620,8 +617,7 @@ describe("gateway server chat", () => {
620617

621618
test("routes block-streamed /btw replies through side-result events", async () => {
622619
await withMainSessionStore(async () => {
623-
const replyMock = vi.mocked(getReplyFromConfig);
624-
replyMock.mockImplementationOnce(async (_ctx: unknown, opts?: GetReplyOptions) => {
620+
mockGetReplyFromConfigOnce(async (_ctx, opts) => {
625621
await opts?.onBlockReply?.({
626622
text: "first chunk",
627623
btw: { question: "what changed?" },

‎src/gateway/test-helpers.mocks.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,9 @@ export const piSdkMock = hoisted.piSdkMock;
298298
export const cronIsolatedRun = hoisted.cronIsolatedRun;
299299
export const agentCommand = hoisted.agentCommand;
300300
export const getReplyFromConfig: Mock<GetReplyFromConfigFn> = hoisted.getReplyFromConfig;
301+
export const mockGetReplyFromConfigOnce = (impl: GetReplyFromConfigFn) => {
302+
getReplyFromConfig.mockImplementationOnce(impl);
303+
};
301304
export const sendWhatsAppMock = hoisted.sendWhatsAppMock;
302305

303306
export const testState = hoisted.testState;

‎src/infra/boundary-file-read.test.ts‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,19 @@ vi.mock("./safe-open-sync.js", () => ({
1515
}));
1616

1717
let canUseBoundaryFileOpen: typeof import("./boundary-file-read.js").canUseBoundaryFileOpen;
18+
let matchBoundaryFileOpenFailure: typeof import("./boundary-file-read.js").matchBoundaryFileOpenFailure;
1819
let openBoundaryFile: typeof import("./boundary-file-read.js").openBoundaryFile;
1920
let openBoundaryFileSync: typeof import("./boundary-file-read.js").openBoundaryFileSync;
2021

2122
describe("boundary-file-read", () => {
2223
beforeEach(async () => {
2324
vi.resetModules();
24-
({ canUseBoundaryFileOpen, openBoundaryFile, openBoundaryFileSync } =
25-
await import("./boundary-file-read.js"));
25+
({
26+
canUseBoundaryFileOpen,
27+
matchBoundaryFileOpenFailure,
28+
openBoundaryFile,
29+
openBoundaryFileSync,
30+
} = await import("./boundary-file-read.js"));
2631
resolveBoundaryPathSyncMock.mockReset();
2732
resolveBoundaryPathMock.mockReset();
2833
openVerifiedFileSyncMock.mockReset();
@@ -205,4 +210,31 @@ describe("boundary-file-read", () => {
205210
});
206211
expect(openVerifiedFileSyncMock).not.toHaveBeenCalled();
207212
});
213+
214+
it("matches boundary file failures by reason with fallback support", () => {
215+
const missing = matchBoundaryFileOpenFailure(
216+
{ ok: false, reason: "path", error: new Error("missing") },
217+
{
218+
path: () => "missing",
219+
fallback: () => "fallback",
220+
},
221+
);
222+
const io = matchBoundaryFileOpenFailure(
223+
{ ok: false, reason: "io", error: new Error("io") },
224+
{
225+
io: () => "io",
226+
fallback: () => "fallback",
227+
},
228+
);
229+
const validation = matchBoundaryFileOpenFailure(
230+
{ ok: false, reason: "validation", error: new Error("blocked") },
231+
{
232+
fallback: (failure) => failure.reason,
233+
},
234+
);
235+
236+
expect(missing).toBe("missing");
237+
expect(io).toBe("io");
238+
expect(validation).toBe("validation");
239+
});
208240
});

‎src/infra/boundary-file-read.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export type BoundaryFileOpenResult =
2929
| { ok: true; path: string; fd: number; stat: fs.Stats; rootRealPath: string }
3030
| { ok: false; reason: BoundaryFileOpenFailureReason; error?: unknown };
3131

32+
export type BoundaryFileOpenFailure = Extract<BoundaryFileOpenResult, { ok: false }>;
33+
3234
export type OpenBoundaryFileSyncParams = {
3335
absolutePath: string;
3436
rootPath: string;
@@ -89,6 +91,25 @@ export function openBoundaryFileSync(params: OpenBoundaryFileSyncParams): Bounda
8991
});
9092
}
9193

94+
export function matchBoundaryFileOpenFailure<T>(
95+
failure: BoundaryFileOpenFailure,
96+
handlers: {
97+
path?: (failure: BoundaryFileOpenFailure) => T;
98+
validation?: (failure: BoundaryFileOpenFailure) => T;
99+
io?: (failure: BoundaryFileOpenFailure) => T;
100+
fallback: (failure: BoundaryFileOpenFailure) => T;
101+
},
102+
): T {
103+
switch (failure.reason) {
104+
case "path":
105+
return handlers.path ? handlers.path(failure) : handlers.fallback(failure);
106+
case "validation":
107+
return handlers.validation ? handlers.validation(failure) : handlers.fallback(failure);
108+
case "io":
109+
return handlers.io ? handlers.io(failure) : handlers.fallback(failure);
110+
}
111+
}
112+
92113
function openBoundaryFileResolved(params: {
93114
absolutePath: string;
94115
resolvedPath: string;

‎src/plugins/bundle-manifest.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import fs from "node:fs";
22
import path from "node:path";
3-
import { openBoundaryFileSync } from "../infra/boundary-file-read.js";
3+
import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js";
44
import { isRecord } from "../utils.js";
55
import { DEFAULT_PLUGIN_ENTRY_CANDIDATES, PLUGIN_MANIFEST_FILENAME } from "./manifest.js";
66
import type { PluginBundleFormat } from "./types.js";
@@ -102,17 +102,19 @@ function loadBundleManifestFile(params: {
102102
rejectHardlinks: params.rejectHardlinks,
103103
});
104104
if (!opened.ok) {
105-
if (opened.reason === "path") {
106-
if (params.allowMissing) {
107-
return { ok: true, raw: {}, manifestPath };
108-
}
109-
return { ok: false, error: `plugin manifest not found: ${manifestPath}`, manifestPath };
110-
}
111-
return {
112-
ok: false,
113-
error: `unsafe plugin manifest path: ${manifestPath} (${opened.reason})`,
114-
manifestPath,
115-
};
105+
return matchBoundaryFileOpenFailure(opened, {
106+
path: () => {
107+
if (params.allowMissing) {
108+
return { ok: true, raw: {}, manifestPath };
109+
}
110+
return { ok: false, error: `plugin manifest not found: ${manifestPath}`, manifestPath };
111+
},
112+
fallback: (failure) => ({
113+
ok: false,
114+
error: `unsafe plugin manifest path: ${manifestPath} (${failure.reason})`,
115+
manifestPath,
116+
}),
117+
});
116118
}
117119
try {
118120
const raw = JSON.parse(fs.readFileSync(opened.fd, "utf-8")) as unknown;

‎src/plugins/bundle-mcp.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import fs from "node:fs";
22
import path from "node:path";
33
import type { OpenClawConfig } from "../config/config.js";
44
import { applyMergePatch } from "../config/merge-patch.js";
5-
import { openBoundaryFileSync } from "../infra/boundary-file-read.js";
5+
import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js";
66
import { isRecord } from "../utils.js";
77
import {
88
CLAUDE_BUNDLE_MANIFEST_RELATIVE_PATH,
@@ -57,10 +57,18 @@ function readPluginJsonObject(params: {
5757
rejectHardlinks: true,
5858
});
5959
if (!opened.ok) {
60-
if (opened.reason === "path" && params.allowMissing) {
61-
return { ok: true, raw: {} };
62-
}
63-
return { ok: false, error: `unable to read ${params.relativePath}: ${opened.reason}` };
60+
return matchBoundaryFileOpenFailure(opened, {
61+
path: () => {
62+
if (params.allowMissing) {
63+
return { ok: true, raw: {} };
64+
}
65+
return { ok: false, error: `unable to read ${params.relativePath}: path` };
66+
},
67+
fallback: (failure) => ({
68+
ok: false,
69+
error: `unable to read ${params.relativePath}: ${failure.reason}`,
70+
}),
71+
});
6472
}
6573
try {
6674
const raw = JSON.parse(fs.readFileSync(opened.fd, "utf-8")) as unknown;

‎src/plugins/conversation-binding.test.ts‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,34 +7,12 @@ import type {
77
SessionBindingAdapter,
88
SessionBindingRecord,
99
} from "../infra/outbound/session-binding-service.js";
10+
import { createEmptyPluginRegistry } from "./registry-empty.js";
1011
import type { PluginRegistry } from "./registry.js";
1112

1213
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-plugin-binding-"));
1314
const approvalsPath = path.join(tempRoot, "plugin-binding-approvals.json");
1415

15-
function createEmptyPluginRegistry(): PluginRegistry {
16-
return {
17-
plugins: [],
18-
tools: [],
19-
hooks: [],
20-
typedHooks: [],
21-
channels: [],
22-
channelSetups: [],
23-
providers: [],
24-
speechProviders: [],
25-
mediaUnderstandingProviders: [],
26-
imageGenerationProviders: [],
27-
webSearchProviders: [],
28-
gatewayHandlers: {},
29-
httpRoutes: [],
30-
cliRegistrars: [],
31-
services: [],
32-
commands: [],
33-
conversationBindingResolvedHandlers: [],
34-
diagnostics: [],
35-
};
36-
}
37-
3816
const sessionBindingState = vi.hoisted(() => {
3917
const records = new Map<string, SessionBindingRecord>();
4018
let nextId = 1;
@@ -105,9 +83,13 @@ const sessionBindingState = vi.hoisted(() => {
10583
};
10684
});
10785

108-
const pluginRuntimeState = vi.hoisted(() => ({
109-
registry: createEmptyPluginRegistry(),
110-
}));
86+
const pluginRuntimeState = vi.hoisted(
87+
() =>
88+
({
89+
// The runtime mock is initialized before imports; beforeEach installs the real shared stub.
90+
registry: null as unknown as PluginRegistry,
91+
}) satisfies { registry: PluginRegistry },
92+
);
11193

11294
vi.mock("../infra/home-dir.js", async (importOriginal) => {
11395
const actual = await importOriginal<typeof import("../infra/home-dir.js")>();

‎src/plugins/discovery.ts‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import fs from "node:fs";
22
import path from "node:path";
3-
import { openBoundaryFileSync } from "../infra/boundary-file-read.js";
3+
import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js";
44
import { resolveUserPath } from "../utils.js";
55
import { detectBundleManifestFormat, loadBundleManifest } from "./bundle-manifest.js";
66
import {
@@ -476,25 +476,25 @@ function resolvePackageEntrySource(params: {
476476
rejectHardlinks: params.rejectHardlinks ?? true,
477477
});
478478
if (!opened.ok) {
479-
if (opened.reason === "path") {
480-
// File missing (ENOENT) — skip, not a security violation.
481-
return null;
482-
}
483-
if (opened.reason === "io") {
484-
// Filesystem error (EACCES, EMFILE, etc.) — warn but don't abort.
485-
params.diagnostics.push({
486-
level: "warn",
487-
message: `extension entry unreadable (I/O error): ${params.entryPath}`,
488-
source: params.sourceLabel,
489-
});
490-
return null;
491-
}
492-
params.diagnostics.push({
493-
level: "error",
494-
message: `extension entry escapes package directory: ${params.entryPath}`,
495-
source: params.sourceLabel,
479+
return matchBoundaryFileOpenFailure(opened, {
480+
path: () => null,
481+
io: () => {
482+
params.diagnostics.push({
483+
level: "warn",
484+
message: `extension entry unreadable (I/O error): ${params.entryPath}`,
485+
source: params.sourceLabel,
486+
});
487+
return null;
488+
},
489+
fallback: () => {
490+
params.diagnostics.push({
491+
level: "error",
492+
message: `extension entry escapes package directory: ${params.entryPath}`,
493+
source: params.sourceLabel,
494+
});
495+
return null;
496+
},
496497
});
497-
return null;
498498
}
499499
const safeSource = opened.path;
500500
fs.closeSync(opened.fd);

0 commit comments

Comments
 (0)