fix: force message through empty allowlists

This commit is contained in:
Peter Steinberger
2026-05-15 09:56:45 +01:00
parent 63ad5b4f97
commit df70ed2b9c
6 changed files with 83 additions and 3 deletions

View File

@@ -918,6 +918,37 @@ describe("runCodexAppServerAttempt", () => {
expect(dynamicToolNames).toContain("music_generate");
});
it("keeps forced message dynamic tool when toolsAllow is empty", async () => {
__testing.setOpenClawCodingToolsFactoryForTests(() => [
createRuntimeDynamicTool("message"),
createRuntimeDynamicTool("music_generate"),
]);
const harness = createStartedThreadHarness();
const params = createParams(
path.join(tempDir, "session.jsonl"),
path.join(tempDir, "workspace"),
);
params.disableTools = false;
params.runtimePlan = createCodexRuntimePlanFixture();
params.sourceReplyDeliveryMode = "message_tool_only";
params.toolsAllow = [];
const run = runCodexAppServerAttempt(params, {
pluginConfig: { appServer: { mode: "yolo" } },
});
await harness.waitForMethod("turn/start", 120_000);
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
await run;
const startRequest = harness.requests.find((entry) => entry.method === "thread/start");
const dynamicToolNames =
(
startRequest?.params as { dynamicTools?: Array<{ name?: string }> } | undefined
)?.dynamicTools?.map((tool) => tool.name) ?? [];
expect(dynamicToolNames).toEqual(["message"]);
});
it("starts Codex threads with searchable OpenClaw dynamic tools by default", async () => {
__testing.setOpenClawCodingToolsFactoryForTests(() => [
createRuntimeDynamicTool("message"),

View File

@@ -2344,9 +2344,12 @@ function includeForcedMessageToolAllow(
if (!shouldForceMessageTool(params)) {
return toolsAllow;
}
if (!toolsAllow?.length) {
if (toolsAllow === undefined) {
return toolsAllow;
}
if (toolsAllow.length === 0) {
return ["message"];
}
const normalized = new Set(toolsAllow.map((name) => normalizeCodexDynamicToolName(name)));
return normalized.has("message") ? toolsAllow : [...toolsAllow, "message"];
}

View File

@@ -59,6 +59,18 @@ describe("applyEmbeddedAttemptToolsAllow", () => {
]);
});
it("materializes forced message tool through empty runtime allowlists", () => {
const tools = [{ name: "music_generate" }, { name: "message" }];
const toolsAllow = mergeForcedEmbeddedAttemptToolsAllow([], {
forceMessageTool: true,
});
expect(toolsAllow).toEqual(["message"]);
expect(applyEmbeddedAttemptToolsAllow(tools, toolsAllow).map((tool) => tool.name)).toEqual([
"message",
]);
});
it("normalizes explicit toolsAllow entries before filtering", () => {
const tools = [{ name: "cron" }, { name: "read" }, { name: "message" }];
@@ -155,6 +167,24 @@ describe("resolveEmbeddedAttemptToolConstructionPlan", () => {
});
});
it("constructs message tool for forced message delivery on explicit no-tools runs", () => {
expectConstructionPlan(
resolveEmbeddedAttemptToolConstructionPlan({ toolsAllow: [], forceMessageTool: true }),
{
constructTools: true,
includeCoreTools: true,
runtimeToolAllowlist: ["message"],
coding: {
includeBaseCodingTools: false,
includeShellTools: false,
includeChannelTools: false,
includeOpenClawTools: true,
includePluginTools: false,
},
},
);
});
it("materializes only plugin candidates for plugin-only allowlists", () => {
expectConstructionPlan(
resolveEmbeddedAttemptToolConstructionPlan({ toolsAllow: ["memory_search"] }),

View File

@@ -113,9 +113,16 @@ export function mergeForcedEmbeddedAttemptToolsAllow(
toolsAllow: string[] | undefined,
params: { forceMessageTool?: boolean },
): string[] | undefined {
if (!params.forceMessageTool || !toolsAllow?.length || hasWildcardToolAllowlist(toolsAllow)) {
if (
!params.forceMessageTool ||
toolsAllow === undefined ||
hasWildcardToolAllowlist(toolsAllow)
) {
return toolsAllow;
}
if (toolsAllow.length === 0) {
return ["message"];
}
const normalized = new Set(toolsAllow.map((entry) => normalizeToolName(entry)));
return normalized.has("message") ? toolsAllow : [...toolsAllow, "message"];
}

View File

@@ -39,12 +39,19 @@ describe("extractToolResultMediaPaths", () => {
attachments: [
{ type: "audio", path: "/tmp/song.mp3", mimeType: "audio/mpeg" },
{ type: "image", url: "https://example.test/cover.png" },
{ type: "file", media: "/tmp/stems.zip" },
{ type: "file", fileUrl: "https://example.test/stems.zip" },
],
},
},
}),
).toEqual({
mediaUrls: ["/tmp/song.mp3", "https://example.test/cover.png"],
mediaUrls: [
"/tmp/song.mp3",
"https://example.test/cover.png",
"/tmp/stems.zip",
"https://example.test/stems.zip",
],
});
});

View File

@@ -379,10 +379,12 @@ function collectStructuredMediaUrls(media: Record<string, unknown>): string[] {
return;
}
const attachment = value as Record<string, unknown>;
pushString(attachment.media);
pushString(attachment.path);
pushString(attachment.url);
pushString(attachment.mediaUrl);
pushString(attachment.filePath);
pushString(attachment.fileUrl);
};
if (typeof media.mediaUrl === "string" && media.mediaUrl.trim()) {
urls.push(media.mediaUrl.trim());