Skip to content

Commit a47cb0a

Browse files
committed
refactor: dedupe approval gateway resolver setup
1 parent c7cc899 commit a47cb0a

7 files changed

Lines changed: 222 additions & 146 deletions

File tree

extensions/matrix/src/exec-approval-resolver.test.ts

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,21 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
22

33
const gatewayRuntimeHoisted = vi.hoisted(() => ({
44
requestSpy: vi.fn(),
5-
startSpy: vi.fn(),
6-
stopSpy: vi.fn(),
7-
stopAndWaitSpy: vi.fn(async () => undefined),
8-
createClientSpy: vi.fn(),
5+
withClientSpy: vi.fn(),
96
}));
107

118
vi.mock("openclaw/plugin-sdk/gateway-runtime", () => ({
12-
createOperatorApprovalsGatewayClient: gatewayRuntimeHoisted.createClientSpy,
9+
withOperatorApprovalsGatewayClient: gatewayRuntimeHoisted.withClientSpy,
1310
}));
1411

1512
describe("resolveMatrixExecApproval", () => {
1613
beforeEach(() => {
1714
gatewayRuntimeHoisted.requestSpy.mockReset();
18-
gatewayRuntimeHoisted.startSpy.mockReset();
19-
gatewayRuntimeHoisted.stopSpy.mockReset();
20-
gatewayRuntimeHoisted.stopAndWaitSpy.mockReset().mockResolvedValue(undefined);
21-
gatewayRuntimeHoisted.createClientSpy.mockReset().mockImplementation((opts) => ({
22-
start: () => {
23-
gatewayRuntimeHoisted.startSpy();
24-
opts.onHelloOk?.();
25-
},
26-
request: gatewayRuntimeHoisted.requestSpy,
27-
stop: gatewayRuntimeHoisted.stopSpy,
28-
stopAndWait: gatewayRuntimeHoisted.stopAndWaitSpy,
29-
}));
15+
gatewayRuntimeHoisted.withClientSpy.mockReset().mockImplementation(async (_params, run) => {
16+
await run({
17+
request: gatewayRuntimeHoisted.requestSpy,
18+
} as never);
19+
});
3020
});
3121

3222
it("submits exec approval resolutions through the gateway approvals client", async () => {
Lines changed: 12 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-runtime";
22
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
33
import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime";
4-
import { createOperatorApprovalsGatewayClient } from "openclaw/plugin-sdk/gateway-runtime";
4+
import { withOperatorApprovalsGatewayClient } from "openclaw/plugin-sdk/gateway-runtime";
55

66
export { isApprovalNotFoundError };
77

@@ -12,53 +12,17 @@ export async function resolveMatrixExecApproval(params: {
1212
senderId?: string | null;
1313
gatewayUrl?: string;
1414
}): Promise<void> {
15-
let readySettled = false;
16-
let resolveReady!: () => void;
17-
let rejectReady!: (err: unknown) => void;
18-
const ready = new Promise<void>((resolve, reject) => {
19-
resolveReady = resolve;
20-
rejectReady = reject;
21-
});
22-
const markReady = () => {
23-
if (readySettled) {
24-
return;
25-
}
26-
readySettled = true;
27-
resolveReady();
28-
};
29-
const failReady = (err: unknown) => {
30-
if (readySettled) {
31-
return;
32-
}
33-
readySettled = true;
34-
rejectReady(err);
35-
};
36-
37-
const gatewayClient = await createOperatorApprovalsGatewayClient({
38-
config: params.cfg,
39-
gatewayUrl: params.gatewayUrl,
40-
clientDisplayName: `Matrix approval (${params.senderId?.trim() || "unknown"})`,
41-
onHelloOk: () => {
42-
markReady();
43-
},
44-
onConnectError: (err) => {
45-
failReady(err);
15+
await withOperatorApprovalsGatewayClient(
16+
{
17+
config: params.cfg,
18+
gatewayUrl: params.gatewayUrl,
19+
clientDisplayName: `Matrix approval (${params.senderId?.trim() || "unknown"})`,
4620
},
47-
onClose: (code, reason) => {
48-
failReady(new Error(`gateway closed (${code}): ${reason}`));
21+
async (gatewayClient) => {
22+
await gatewayClient.request("exec.approval.resolve", {
23+
id: params.approvalId,
24+
decision: params.decision,
25+
});
4926
},
50-
});
51-
52-
try {
53-
gatewayClient.start();
54-
await ready;
55-
await gatewayClient.request("exec.approval.resolve", {
56-
id: params.approvalId,
57-
decision: params.decision,
58-
});
59-
} finally {
60-
await gatewayClient.stopAndWait().catch(() => {
61-
gatewayClient.stop();
62-
});
63-
}
27+
);
6428
}

extensions/telegram/src/exec-approval-resolver.test.ts

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,21 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
22

33
const gatewayRuntimeHoisted = vi.hoisted(() => ({
44
requestSpy: vi.fn(),
5-
startSpy: vi.fn(),
6-
stopSpy: vi.fn(),
7-
stopAndWaitSpy: vi.fn(async () => undefined),
8-
createClientSpy: vi.fn(),
5+
withClientSpy: vi.fn(),
96
}));
107

118
vi.mock("openclaw/plugin-sdk/gateway-runtime", () => ({
12-
createOperatorApprovalsGatewayClient: gatewayRuntimeHoisted.createClientSpy,
9+
withOperatorApprovalsGatewayClient: gatewayRuntimeHoisted.withClientSpy,
1310
}));
1411

1512
describe("resolveTelegramExecApproval", () => {
1613
beforeEach(() => {
1714
gatewayRuntimeHoisted.requestSpy.mockReset();
18-
gatewayRuntimeHoisted.startSpy.mockReset();
19-
gatewayRuntimeHoisted.stopSpy.mockReset();
20-
gatewayRuntimeHoisted.stopAndWaitSpy.mockReset().mockResolvedValue(undefined);
21-
gatewayRuntimeHoisted.createClientSpy.mockReset().mockImplementation((opts) => ({
22-
start: () => {
23-
gatewayRuntimeHoisted.startSpy();
24-
opts.onHelloOk?.();
25-
},
26-
request: gatewayRuntimeHoisted.requestSpy,
27-
stop: gatewayRuntimeHoisted.stopSpy,
28-
stopAndWait: gatewayRuntimeHoisted.stopAndWaitSpy,
29-
}));
15+
gatewayRuntimeHoisted.withClientSpy.mockReset().mockImplementation(async (_params, run) => {
16+
await run({
17+
request: gatewayRuntimeHoisted.requestSpy,
18+
} as never);
19+
});
3020
});
3121

3222
it("routes plugin approval ids through plugin.approval.resolve", async () => {
Lines changed: 27 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
22
import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime";
3-
import { createOperatorApprovalsGatewayClient } from "openclaw/plugin-sdk/gateway-runtime";
3+
import { withOperatorApprovalsGatewayClient } from "openclaw/plugin-sdk/gateway-runtime";
44
import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/infra-runtime";
55

66
export type ResolveTelegramExecApprovalParams = {
@@ -15,69 +15,33 @@ export type ResolveTelegramExecApprovalParams = {
1515
export async function resolveTelegramExecApproval(
1616
params: ResolveTelegramExecApprovalParams,
1717
): Promise<void> {
18-
let readySettled = false;
19-
let resolveReady!: () => void;
20-
let rejectReady!: (err: unknown) => void;
21-
const ready = new Promise<void>((resolve, reject) => {
22-
resolveReady = resolve;
23-
rejectReady = reject;
24-
});
25-
const markReady = () => {
26-
if (readySettled) {
27-
return;
28-
}
29-
readySettled = true;
30-
resolveReady();
31-
};
32-
const failReady = (err: unknown) => {
33-
if (readySettled) {
34-
return;
35-
}
36-
readySettled = true;
37-
rejectReady(err);
38-
};
39-
40-
const gatewayClient = await createOperatorApprovalsGatewayClient({
41-
config: params.cfg,
42-
gatewayUrl: params.gatewayUrl,
43-
clientDisplayName: `Telegram approval (${params.senderId?.trim() || "unknown"})`,
44-
onHelloOk: () => {
45-
markReady();
46-
},
47-
onConnectError: (err) => {
48-
failReady(err);
18+
await withOperatorApprovalsGatewayClient(
19+
{
20+
config: params.cfg,
21+
gatewayUrl: params.gatewayUrl,
22+
clientDisplayName: `Telegram approval (${params.senderId?.trim() || "unknown"})`,
4923
},
50-
onClose: (code, reason) => {
51-
// Once onHelloOk resolves `ready`, in-flight request failures must come from
52-
// gatewayClient.request() itself; failReady only covers the pre-ready phase.
53-
failReady(new Error(`gateway closed (${code}): ${reason}`));
54-
},
55-
});
56-
57-
try {
58-
gatewayClient.start();
59-
await ready;
60-
const requestApproval = async (method: "exec.approval.resolve" | "plugin.approval.resolve") => {
61-
await gatewayClient.request(method, {
62-
id: params.approvalId,
63-
decision: params.decision,
64-
});
65-
};
66-
if (params.approvalId.startsWith("plugin:")) {
67-
await requestApproval("plugin.approval.resolve");
68-
} else {
69-
try {
70-
await requestApproval("exec.approval.resolve");
71-
} catch (err) {
72-
if (!params.allowPluginFallback || !isApprovalNotFoundError(err)) {
73-
throw err;
74-
}
24+
async (gatewayClient) => {
25+
const requestApproval = async (
26+
method: "exec.approval.resolve" | "plugin.approval.resolve",
27+
) => {
28+
await gatewayClient.request(method, {
29+
id: params.approvalId,
30+
decision: params.decision,
31+
});
32+
};
33+
if (params.approvalId.startsWith("plugin:")) {
7534
await requestApproval("plugin.approval.resolve");
35+
} else {
36+
try {
37+
await requestApproval("exec.approval.resolve");
38+
} catch (err) {
39+
if (!params.allowPluginFallback || !isApprovalNotFoundError(err)) {
40+
throw err;
41+
}
42+
await requestApproval("plugin.approval.resolve");
43+
}
7644
}
77-
}
78-
} finally {
79-
await gatewayClient.stopAndWait().catch(() => {
80-
gatewayClient.stop();
81-
});
82-
}
45+
},
46+
);
8347
}
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
const clientState = vi.hoisted(() => ({
4+
options: null as Record<string, unknown> | null,
5+
startMode: "hello" as "hello" | "close",
6+
close: { code: 1008, reason: "pairing required" },
7+
requestSpy: vi.fn(),
8+
stopSpy: vi.fn(),
9+
stopAndWaitSpy: vi.fn(async () => undefined),
10+
}));
11+
12+
class MockGatewayClient {
13+
private readonly opts: Record<string, unknown>;
14+
15+
constructor(opts: Record<string, unknown>) {
16+
this.opts = opts;
17+
clientState.options = opts;
18+
}
19+
20+
start(): void {
21+
void Promise.resolve()
22+
.then(async () => {
23+
if (clientState.startMode === "close") {
24+
const onClose = this.opts.onClose;
25+
if (typeof onClose === "function") {
26+
onClose(clientState.close.code, clientState.close.reason);
27+
}
28+
return;
29+
}
30+
const onHelloOk = this.opts.onHelloOk;
31+
if (typeof onHelloOk === "function") {
32+
await onHelloOk();
33+
}
34+
})
35+
.catch(() => {});
36+
}
37+
38+
async request(method: string, params: unknown): Promise<unknown> {
39+
return await clientState.requestSpy(method, params);
40+
}
41+
42+
stop(): void {
43+
clientState.stopSpy();
44+
}
45+
46+
async stopAndWait(): Promise<void> {
47+
await clientState.stopAndWaitSpy();
48+
}
49+
}
50+
51+
vi.mock("./client-bootstrap.js", () => ({
52+
resolveGatewayClientBootstrap: vi.fn(async () => ({
53+
url: "ws://127.0.0.1:18789",
54+
auth: { token: "secret", password: undefined },
55+
})),
56+
}));
57+
58+
vi.mock("./client.js", () => ({
59+
GatewayClient: MockGatewayClient,
60+
}));
61+
62+
const { withOperatorApprovalsGatewayClient } = await import("./operator-approvals-client.js");
63+
64+
describe("withOperatorApprovalsGatewayClient", () => {
65+
beforeEach(() => {
66+
clientState.options = null;
67+
clientState.startMode = "hello";
68+
clientState.close = { code: 1008, reason: "pairing required" };
69+
clientState.requestSpy.mockReset().mockResolvedValue(undefined);
70+
clientState.stopSpy.mockReset();
71+
clientState.stopAndWaitSpy.mockReset().mockResolvedValue(undefined);
72+
});
73+
74+
it("waits for hello before running the callback and stops cleanly", async () => {
75+
await withOperatorApprovalsGatewayClient(
76+
{
77+
config: {} as never,
78+
clientDisplayName: "Matrix approval (@owner:example.org)",
79+
},
80+
async (client) => {
81+
await client.request("exec.approval.resolve", {
82+
id: "req-123",
83+
decision: "allow-once",
84+
});
85+
},
86+
);
87+
88+
expect(clientState.options?.scopes).toEqual(["operator.approvals"]);
89+
expect(clientState.requestSpy).toHaveBeenCalledWith("exec.approval.resolve", {
90+
id: "req-123",
91+
decision: "allow-once",
92+
});
93+
expect(clientState.stopAndWaitSpy).toHaveBeenCalledTimes(1);
94+
});
95+
96+
it("surfaces close failures before hello", async () => {
97+
clientState.startMode = "close";
98+
99+
await expect(
100+
withOperatorApprovalsGatewayClient(
101+
{
102+
config: {} as never,
103+
clientDisplayName: "Matrix approval (@owner:example.org)",
104+
},
105+
async () => undefined,
106+
),
107+
).rejects.toThrow("gateway closed (1008): pairing required");
108+
});
109+
});

0 commit comments

Comments
 (0)