fix(regression): stop cross-channel plugin thread defaults

This commit is contained in:
Tak Hoffman 2026-03-27 19:35:31 -05:00
parent 835441233d
commit b1eeca3b00
No known key found for this signature in database
4 changed files with 43 additions and 151 deletions

View File

@ -112,6 +112,26 @@ describe("executeZalouserTool", () => {
});
});
it("does not route send actions from foreign ambient thread defaults", async () => {
const tool = createZalouserTool({
deliveryContext: {
channel: "slack",
to: "channel:C123",
threadId: "1710000000.000100",
},
});
const result = await tool.execute("tool-1", {
action: "send",
message: "hello",
});
expect(mockSendMessage).not.toHaveBeenCalled();
expect(extractDetails(result)).toEqual({
error: "threadId and message required for send action",
});
});
it("returns tool error when send action fails", async () => {
mockSendMessage.mockResolvedValueOnce({ ok: false, error: "blocked" });
const result = await executeZalouserTool("tool-1", {

View File

@ -63,15 +63,23 @@ function resolveAmbientZalouserTarget(context?: ZalouserToolContext): {
threadId?: string;
isGroup?: boolean;
} {
const rawTarget = context?.deliveryContext?.to;
if (typeof rawTarget === "string" && rawTarget.trim()) {
const deliveryContext = context?.deliveryContext;
const rawTarget = deliveryContext?.to;
if (
(deliveryContext?.channel === undefined || deliveryContext.channel === "zalouser") &&
typeof rawTarget === "string" &&
rawTarget.trim()
) {
try {
return parseZalouserOutboundTarget(rawTarget);
} catch {
// Ignore unrelated delivery targets; explicit tool params still win.
}
}
const ambientThreadId = context?.deliveryContext?.threadId;
if (deliveryContext?.channel && deliveryContext.channel !== "zalouser") {
return {};
}
const ambientThreadId = deliveryContext?.threadId;
if (typeof ambientThreadId === "string" && ambientThreadId.trim()) {
return { threadId: ambientThreadId.trim() };
}

View File

@ -127,7 +127,7 @@ describe("createOpenClawTools plugin context", () => {
);
});
it("injects ambient thread defaults without mutating shared plugin tool instances", async () => {
it("does not inject ambient thread defaults into plugin tools", async () => {
const executeMock = vi.fn(async () => ({
content: [{ type: "text" as const, text: "ok" }],
details: {},
@ -157,18 +157,17 @@ describe("createOpenClawTools plugin context", () => {
expect(first).toBeDefined();
expect(second).toBeDefined();
expect(first).not.toBe(sharedTool);
expect(second).not.toBe(sharedTool);
expect(first).not.toBe(second);
expect(first).toBe(sharedTool);
expect(second).toBe(sharedTool);
await first?.execute("call-1", {});
await second?.execute("call-2", {});
expect(executeMock).toHaveBeenNthCalledWith(1, "call-1", { threadId: "111.222" });
expect(executeMock).toHaveBeenNthCalledWith(2, "call-2", { threadId: "333.444" });
expect(executeMock).toHaveBeenNthCalledWith(1, "call-1", {});
expect(executeMock).toHaveBeenNthCalledWith(2, "call-2", {});
});
it("injects messageThreadId defaults for missing params objects", async () => {
it("does not inject messageThreadId defaults for missing params objects", async () => {
const executeMock = vi.fn(async () => ({
content: [{ type: "text" as const, text: "ok" }],
details: {},
@ -194,10 +193,10 @@ describe("createOpenClawTools plugin context", () => {
await wrapped?.execute("call-1", undefined);
expect(executeMock).toHaveBeenCalledWith("call-1", { messageThreadId: 77 });
expect(executeMock).toHaveBeenCalledWith("call-1", undefined);
});
it("preserves string thread ids for tools that declare string thread parameters", async () => {
it("does not infer string thread ids for tools that declare thread parameters", async () => {
const executeMock = vi.fn(async () => ({
content: [{ type: "text" as const, text: "ok" }],
details: {},
@ -223,10 +222,10 @@ describe("createOpenClawTools plugin context", () => {
await wrapped?.execute("call-1", {});
expect(executeMock).toHaveBeenCalledWith("call-1", { threadId: "77" });
expect(executeMock).toHaveBeenCalledWith("call-1", {});
});
it("does not override explicit thread params when ambient defaults exist", async () => {
it("preserves explicit thread params when ambient defaults exist", async () => {
const executeMock = vi.fn(async () => ({
content: [{ type: "text" as const, text: "ok" }],
details: {},

View File

@ -1,145 +1,10 @@
import { readSnakeCaseParamRaw } from "../param-key.js";
import { copyPluginToolMeta } from "../plugins/tools.js";
import type { DeliveryContext } from "../utils/delivery-context.js";
import type { AnyAgentTool } from "./tools/common.js";
type ThreadInjectionKey = "threadId" | "messageThreadId";
function coerceAmbientThreadIdForSchema(params: {
value: unknown;
expectedType?: "string" | "number";
}): string | number | undefined {
const { value, expectedType } = params;
if (value === undefined || value === null) {
return undefined;
}
if (expectedType === "string") {
if (typeof value === "string") {
const trimmed = value.trim();
return trimmed || undefined;
}
if (typeof value === "number" && Number.isFinite(value)) {
return String(value);
}
return undefined;
}
if (expectedType === "number") {
if (typeof value === "number" && Number.isFinite(value)) {
return value;
}
if (typeof value !== "string") {
return undefined;
}
const trimmed = value.trim();
if (!trimmed) {
return undefined;
}
const parsed = Number(trimmed);
if (!Number.isFinite(parsed)) {
return undefined;
}
if (/^-?\d+$/.test(trimmed) && !Number.isSafeInteger(parsed)) {
return undefined;
}
return parsed;
}
if (typeof value === "number" && Number.isFinite(value)) {
return value;
}
if (typeof value === "string") {
const trimmed = value.trim();
return trimmed || undefined;
}
return undefined;
}
function resolveThreadInjectionTarget(tool: AnyAgentTool): {
key: ThreadInjectionKey;
expectedType?: "string" | "number";
} | null {
const schema =
tool.parameters && typeof tool.parameters === "object"
? (tool.parameters as Record<string, unknown>)
: null;
const properties =
schema?.properties && typeof schema.properties === "object"
? (schema.properties as Record<string, unknown>)
: null;
if (!properties) {
return null;
}
for (const key of ["threadId", "messageThreadId"] as const) {
const property =
properties[key] && typeof properties[key] === "object"
? (properties[key] as Record<string, unknown>)
: null;
if (!property) {
continue;
}
const type = property.type;
const expectedType =
type === "string" ? "string" : type === "number" || type === "integer" ? "number" : undefined;
return { key, expectedType };
}
return null;
}
function wrapPluginToolWithAmbientThreadDefaults(params: {
tool: AnyAgentTool;
ambientThreadId: string | number;
}): AnyAgentTool {
const target = resolveThreadInjectionTarget(params.tool);
if (!params.tool.execute || !target) {
return params.tool;
}
const defaultThreadId = coerceAmbientThreadIdForSchema({
value: params.ambientThreadId,
expectedType: target.expectedType,
});
if (defaultThreadId === undefined) {
return params.tool;
}
const originalExecute = params.tool.execute.bind(params.tool);
const wrappedTool: AnyAgentTool = {
...params.tool,
execute: async (...args: unknown[]) => {
const existingParams = args[1];
const paramsRecord =
existingParams == null
? {}
: existingParams && typeof existingParams === "object" && !Array.isArray(existingParams)
? (existingParams as Record<string, unknown>)
: null;
if (!paramsRecord) {
return await originalExecute(...(args as Parameters<typeof originalExecute>));
}
if (
readSnakeCaseParamRaw(paramsRecord, "threadId") !== undefined ||
readSnakeCaseParamRaw(paramsRecord, "messageThreadId") !== undefined
) {
return await originalExecute(...(args as Parameters<typeof originalExecute>));
}
const nextArgs = [...args];
nextArgs[1] = { ...paramsRecord, [target.key]: defaultThreadId };
return await originalExecute(...(nextArgs as Parameters<typeof originalExecute>));
},
};
copyPluginToolMeta(params.tool, wrappedTool);
return wrappedTool;
}
export function applyPluginToolDeliveryDefaults(params: {
tools: AnyAgentTool[];
deliveryContext?: DeliveryContext;
}): AnyAgentTool[] {
const ambientThreadId = params.deliveryContext?.threadId;
if (ambientThreadId == null) {
return params.tools;
}
return params.tools.map((tool) =>
wrapPluginToolWithAmbientThreadDefaults({
tool,
ambientThreadId,
}),
);
void params.deliveryContext;
return params.tools;
}