Skip to content

Commit d0562a8

Browse files
steipetesteipete
committed
perf(agents): extract cli runner image and approval seams
1 parent 42ae213 commit d0562a8

7 files changed

Lines changed: 262 additions & 314 deletions

‎src/agents/bash-tools.exec-host-shared.test.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,4 +209,72 @@ describe("buildExecApprovalPendingToolResult", () => {
209209
expect(text).toContain("/approve approval-slug allow-once");
210210
expect(text).not.toContain("native chat exec approvals are not configured on Discord");
211211
});
212+
213+
it("returns an unavailable reply when Discord exec approvals are disabled", () => {
214+
const result = buildExecApprovalPendingToolResult({
215+
host: "gateway",
216+
command: "npm view diver name version description",
217+
cwd: process.cwd(),
218+
warningText: "",
219+
approvalId: "approval-id",
220+
approvalSlug: "approval-slug",
221+
expiresAtMs: Date.now() + 60_000,
222+
initiatingSurface: {
223+
kind: "disabled",
224+
channel: "discord",
225+
channelLabel: "Discord",
226+
accountId: "default",
227+
},
228+
sentApproverDms: false,
229+
unavailableReason: "initiating-platform-disabled",
230+
});
231+
232+
expect(result.details).toMatchObject({
233+
status: "approval-unavailable",
234+
reason: "initiating-platform-disabled",
235+
channel: "discord",
236+
channelLabel: "Discord",
237+
accountId: "default",
238+
host: "gateway",
239+
});
240+
const text = result.content.find((part) => part.type === "text")?.text ?? "";
241+
expect(text).toContain("native chat exec approvals are not configured on Discord");
242+
expect(text).not.toContain("/approve");
243+
expect(text).not.toContain("Pending command:");
244+
});
245+
246+
it("keeps the Telegram unavailable reply when Discord DM approvals are not fully configured", () => {
247+
const result = buildExecApprovalPendingToolResult({
248+
host: "gateway",
249+
command: "npm view diver name version description",
250+
cwd: process.cwd(),
251+
warningText: "",
252+
approvalId: "approval-id",
253+
approvalSlug: "approval-slug",
254+
expiresAtMs: Date.now() + 60_000,
255+
initiatingSurface: {
256+
kind: "disabled",
257+
channel: "telegram",
258+
channelLabel: "Telegram",
259+
accountId: "default",
260+
},
261+
sentApproverDms: false,
262+
unavailableReason: "initiating-platform-disabled",
263+
});
264+
265+
expect(result.details).toMatchObject({
266+
status: "approval-unavailable",
267+
reason: "initiating-platform-disabled",
268+
channel: "telegram",
269+
channelLabel: "Telegram",
270+
accountId: "default",
271+
sentApproverDms: false,
272+
host: "gateway",
273+
});
274+
const text = result.content.find((part) => part.type === "text")?.text ?? "";
275+
expect(text).toContain("native chat exec approvals are not configured on Telegram");
276+
expect(text).not.toContain("/approve");
277+
expect(text).not.toContain("Pending command:");
278+
expect(text).not.toContain("Approver DMs were sent");
279+
});
212280
});

‎src/agents/bash-tools.exec.approval-id.test.ts‎

Lines changed: 0 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@ import os from "node:os";
44
import path from "node:path";
55
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
66
import { clearConfigCache, clearRuntimeConfigSnapshot } from "../config/config.js";
7-
import { clearPluginDiscoveryCache } from "../plugins/discovery.js";
8-
import { clearPluginLoaderCache } from "../plugins/loader.js";
9-
import { clearPluginManifestRegistryCache } from "../plugins/manifest-registry.js";
10-
import { resetPluginRuntimeStateForTest } from "../plugins/runtime.js";
117
import { buildSystemRunPreparePayload } from "../test-utils/system-run-prepare-payload.js";
128

139
vi.mock("./tools/gateway.js", () => ({
@@ -28,7 +24,6 @@ vi.mock("../infra/outbound/message.js", () => ({
2824

2925
let callGatewayTool: typeof import("./tools/gateway.js").callGatewayTool;
3026
let createExecTool: typeof import("./bash-tools.exec.js").createExecTool;
31-
let getExecApprovalApproverDmNoticeText: typeof import("../infra/exec-approval-reply.js").getExecApprovalApproverDmNoticeText;
3227
let sendMessage: typeof import("../infra/outbound/message.js").sendMessage;
3328

3429
function buildPreparedSystemRunPayload(rawInvokeParams: unknown) {
@@ -45,42 +40,12 @@ function buildPreparedSystemRunPayload(rawInvokeParams: unknown) {
4540
return buildSystemRunPreparePayload(params);
4641
}
4742

48-
function getTestConfigPath() {
49-
return path.join(process.env.HOME ?? "", ".openclaw", "openclaw.json");
50-
}
51-
52-
async function writeOpenClawConfig(config: Record<string, unknown>, pretty = false) {
53-
const configPath = getTestConfigPath();
54-
await fs.mkdir(path.dirname(configPath), { recursive: true });
55-
await fs.writeFile(configPath, JSON.stringify(config, null, pretty ? 2 : undefined));
56-
}
57-
5843
async function writeExecApprovalsConfig(config: Record<string, unknown>) {
5944
const approvalsPath = path.join(process.env.HOME ?? "", ".openclaw", "exec-approvals.json");
6045
await fs.mkdir(path.dirname(approvalsPath), { recursive: true });
6146
await fs.writeFile(approvalsPath, JSON.stringify(config, null, 2));
6247
}
6348

64-
function resetPluginState() {
65-
clearPluginDiscoveryCache();
66-
clearPluginLoaderCache();
67-
clearPluginManifestRegistryCache();
68-
resetPluginRuntimeStateForTest();
69-
clearRuntimeConfigSnapshot();
70-
clearConfigCache();
71-
}
72-
73-
async function withBundledChannels<T>(run: () => Promise<T>) {
74-
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
75-
resetPluginState();
76-
try {
77-
return await run();
78-
} finally {
79-
process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS = "1";
80-
resetPluginState();
81-
}
82-
}
83-
8449
function acceptedApprovalResponse(params: unknown) {
8550
return { status: "accepted", id: (params as { id?: string })?.id };
8651
}
@@ -272,7 +237,6 @@ describe("exec approvals", () => {
272237
process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS = "1";
273238
({ callGatewayTool } = await import("./tools/gateway.js"));
274239
({ createExecTool } = await import("./bash-tools.exec.js"));
275-
({ getExecApprovalApproverDmNoticeText } = await import("../infra/exec-approval-reply.js"));
276240
({ sendMessage } = await import("../infra/outbound/message.js"));
277241
vi.mocked(callGatewayTool).mockReset();
278242
vi.mocked(sendMessage).mockClear();
@@ -1391,95 +1355,4 @@ describe("exec approvals", () => {
13911355
}),
13921356
).rejects.toThrow("Cron runs cannot wait for interactive exec approval");
13931357
});
1394-
1395-
it("returns an unavailable reply when discord exec approvals are disabled", async () => {
1396-
await writeOpenClawConfig({
1397-
channels: {
1398-
discord: {
1399-
enabled: true,
1400-
execApprovals: { enabled: false },
1401-
},
1402-
},
1403-
});
1404-
1405-
mockPendingApprovalRegistration();
1406-
1407-
const tool = createExecTool({
1408-
host: "gateway",
1409-
ask: "always",
1410-
approvalRunningNoticeMs: 0,
1411-
messageProvider: "discord",
1412-
accountId: "default",
1413-
currentChannelId: "1234567890",
1414-
});
1415-
1416-
const result = await withBundledChannels(async () =>
1417-
tool.execute("call-unavailable", {
1418-
command: "npm view diver name version description",
1419-
}),
1420-
);
1421-
1422-
expect(result.details.status).toBe("approval-unavailable");
1423-
expect(result.details).toMatchObject({
1424-
reason: "initiating-platform-disabled",
1425-
channel: "discord",
1426-
channelLabel: "Discord",
1427-
accountId: "default",
1428-
host: "gateway",
1429-
});
1430-
expect(getResultText(result)).toContain(
1431-
"native chat exec approvals are not configured on Discord",
1432-
);
1433-
expect(getResultText(result)).not.toContain("/approve");
1434-
expect(getResultText(result)).not.toContain("Pending command:");
1435-
});
1436-
1437-
it("keeps the Telegram unavailable reply when Discord DM approvals are not fully configured", async () => {
1438-
await writeOpenClawConfig(
1439-
{
1440-
channels: {
1441-
telegram: {
1442-
enabled: true,
1443-
execApprovals: { enabled: false },
1444-
},
1445-
discord: {
1446-
enabled: true,
1447-
execApprovals: { enabled: true, approvers: ["123"], target: "dm" },
1448-
},
1449-
},
1450-
},
1451-
true,
1452-
);
1453-
1454-
mockPendingApprovalRegistration();
1455-
1456-
const tool = createExecTool({
1457-
host: "gateway",
1458-
ask: "always",
1459-
approvalRunningNoticeMs: 0,
1460-
messageProvider: "telegram",
1461-
accountId: "default",
1462-
currentChannelId: "-1003841603622",
1463-
});
1464-
1465-
const result = await withBundledChannels(async () =>
1466-
tool.execute("call-tg-unavailable", {
1467-
command: "npm view diver name version description",
1468-
}),
1469-
);
1470-
1471-
expect(result.details.status).toBe("approval-unavailable");
1472-
expect(result.details).toMatchObject({
1473-
reason: "initiating-platform-disabled",
1474-
channel: "telegram",
1475-
channelLabel: "Telegram",
1476-
accountId: "default",
1477-
sentApproverDms: false,
1478-
host: "gateway",
1479-
});
1480-
expect(getResultText(result)).toContain(
1481-
"native chat exec approvals are not configured on Telegram",
1482-
);
1483-
expect(getResultText(result)).not.toContain(getExecApprovalApproverDmNoticeText());
1484-
});
14851358
});

‎src/agents/cli-runner.helpers.test.ts‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import fs from "node:fs/promises";
2+
import path from "node:path";
23
import type { ImageContent } from "@mariozechner/pi-ai";
34
import { beforeEach, describe, expect, it, vi } from "vitest";
45
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
56
import { MAX_IMAGE_BYTES } from "../media/constants.js";
67
import {
78
buildCliArgs,
89
loadPromptRefImages,
10+
prepareCliPromptImagePayload,
911
resolveCliRunQueueKey,
1012
writeCliImages,
1113
} from "./cli-runner/helpers.js";
@@ -179,6 +181,142 @@ describe("writeCliImages", () => {
179181
await fs.rm(written.paths[0], { force: true });
180182
}
181183
});
184+
185+
it("hydrates prompt media refs into codex image args through the helper seams", async () => {
186+
const tempDir = await fs.mkdtemp(
187+
path.join(resolvePreferredOpenClawTmpDir(), "openclaw-cli-prompt-image-"),
188+
);
189+
const sourceImage = path.join(tempDir, "bb-image.png");
190+
await fs.writeFile(
191+
sourceImage,
192+
Buffer.from(
193+
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/woAAn8B9FD5fHAAAAAASUVORK5CYII=",
194+
"base64",
195+
),
196+
);
197+
198+
try {
199+
const prepared = await prepareCliPromptImagePayload({
200+
backend: {
201+
command: "codex",
202+
imageArg: "--image",
203+
imageMode: "repeat",
204+
input: "arg",
205+
},
206+
prompt: `[media attached: ${sourceImage} (image/png)]\n\n<media:image>`,
207+
workspaceDir: tempDir,
208+
});
209+
const argv = buildCliArgs({
210+
backend: {
211+
command: "codex",
212+
imageArg: "--image",
213+
imageMode: "repeat",
214+
},
215+
baseArgs: ["exec", "--json"],
216+
modelId: "gpt-5.4",
217+
imagePaths: prepared.imagePaths,
218+
useResume: false,
219+
});
220+
221+
const imageArgIndex = argv.indexOf("--image");
222+
expect(imageArgIndex).toBeGreaterThanOrEqual(0);
223+
expect(argv[imageArgIndex + 1]).toContain("openclaw-cli-images");
224+
expect(argv[imageArgIndex + 1]).not.toBe(sourceImage);
225+
226+
await prepared.cleanupImages?.();
227+
} finally {
228+
await fs.rm(tempDir, { recursive: true, force: true });
229+
}
230+
});
231+
232+
it("appends hydrated prompt media refs for stdin backends through the helper seams", async () => {
233+
const tempDir = await fs.mkdtemp(
234+
path.join(resolvePreferredOpenClawTmpDir(), "openclaw-cli-prompt-image-generic-"),
235+
);
236+
const sourceImage = path.join(tempDir, "claude-image.png");
237+
await fs.writeFile(
238+
sourceImage,
239+
Buffer.from(
240+
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/woAAn8B9FD5fHAAAAAASUVORK5CYII=",
241+
"base64",
242+
),
243+
);
244+
245+
try {
246+
const prompt = `[media attached: ${sourceImage} (image/png)]\n\n<media:image>`;
247+
const prepared = await prepareCliPromptImagePayload({
248+
backend: {
249+
command: "claude",
250+
input: "stdin",
251+
},
252+
prompt,
253+
workspaceDir: tempDir,
254+
});
255+
const promptWithImages = prepared.prompt;
256+
257+
expect(promptWithImages).toContain("openclaw-cli-images");
258+
expect(promptWithImages).toContain(prepared.imagePaths?.[0] ?? "");
259+
expect(promptWithImages.trimEnd().endsWith(prepared.imagePaths?.[0] ?? "")).toBe(true);
260+
261+
await prepared.cleanupImages?.();
262+
} finally {
263+
await fs.rm(tempDir, { recursive: true, force: true });
264+
}
265+
});
266+
267+
it("prefers explicit images over prompt refs through the helper seams", async () => {
268+
const tempDir = await fs.mkdtemp(
269+
path.join(resolvePreferredOpenClawTmpDir(), "openclaw-cli-explicit-images-"),
270+
);
271+
const sourceImage = path.join(tempDir, "ignored-prompt-image.png");
272+
await fs.writeFile(
273+
sourceImage,
274+
Buffer.from(
275+
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/woAAn8B9FD5fHAAAAAASUVORK5CYII=",
276+
"base64",
277+
),
278+
);
279+
const explicitImage: ImageContent = {
280+
type: "image",
281+
data: "c29tZS1leHBsaWNpdC1pbWFnZQ==",
282+
mimeType: "image/png",
283+
};
284+
285+
try {
286+
const prepared = await prepareCliPromptImagePayload({
287+
backend: {
288+
command: "codex",
289+
imageArg: "--image",
290+
imageMode: "repeat",
291+
input: "arg",
292+
},
293+
prompt: `[media attached: ${sourceImage} (image/png)]\n\n<media:image>`,
294+
workspaceDir: tempDir,
295+
images: [explicitImage],
296+
});
297+
const argv = buildCliArgs({
298+
backend: {
299+
command: "codex",
300+
imageArg: "--image",
301+
imageMode: "repeat",
302+
},
303+
baseArgs: ["exec", "--json"],
304+
modelId: "gpt-5.4",
305+
imagePaths: prepared.imagePaths,
306+
useResume: false,
307+
});
308+
309+
expect(argv.filter((arg) => arg === "--image")).toHaveLength(1);
310+
expect(argv[argv.indexOf("--image") + 1]).toContain("openclaw-cli-images");
311+
await expect(fs.readFile(prepared.imagePaths?.[0] ?? "")).resolves.toEqual(
312+
Buffer.from(explicitImage.data, "base64"),
313+
);
314+
315+
await prepared.cleanupImages?.();
316+
} finally {
317+
await fs.rm(tempDir, { recursive: true, force: true });
318+
}
319+
});
182320
});
183321

184322
describe("resolveCliRunQueueKey", () => {

0 commit comments

Comments
 (0)