mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(hooks): preserve multi-account agent delivery (#116095)
* fix(hooks): preserve account routing for agent delivery Refs #43866 Co-authored-by: Paul Desmond Parker <paul.parker@dcconnect.cn> * test(gateway): prove hook account delivery handoff --------- Co-authored-by: Paul Desmond Parker <paul.parker@dcconnect.cn>
This commit is contained in:
@@ -532,7 +532,7 @@ Query-string tokens are rejected.
|
||||
-d '{"message":"Summarize inbox","name":"Email","model":"openai/gpt-5.6-sol"}'
|
||||
```
|
||||
|
||||
Fields: `message` (required), `name`, `agentId`, `sessionKey` (requires `hooks.allowRequestSessionKey=true`), `sessionMode` (`isolated` or `persistent`), `idempotencyKey`, `wakeMode`, `deliver`, `channel`, `to`, `model`, `thinking`, `timeoutSeconds`.
|
||||
Fields: `message` (required), `name`, `agentId`, `sessionKey` (requires `hooks.allowRequestSessionKey=true`), `sessionMode` (`isolated` or `persistent`), `idempotencyKey`, `wakeMode`, `deliver`, `channel`, `to`, `accountId`, `model`, `thinking`, `timeoutSeconds`.
|
||||
|
||||
Set `sessionMode: "persistent"` only when repeated deliveries should reuse prior context. Direct persistent hooks require an explicit `sessionKey`, `hooks.allowRequestSessionKey: true`, and a non-empty `hooks.allowedSessionKeyPrefixes` allowlist. Omit `sessionMode` or use `"isolated"` for a fresh run session.
|
||||
|
||||
@@ -543,6 +543,7 @@ Query-string tokens are rejected.
|
||||
- Announce delivery requires a concrete channel; webhook hooks never inherit the main session's `last` channel or recipient.
|
||||
- Setting `deliver: false` keeps the run completion-only and ignores any delivery destination.
|
||||
- Supplying both a concrete `channel` and `to` enables direct announce delivery.
|
||||
- Set `accountId` with `channel` and `to` to select a configured account on multi-account channels.
|
||||
|
||||
The HTTP response waits only for runner admission, not for the agent turn to finish. A `200` may take up to 15 seconds and means the run entered its agent runner. Pre-run failures return `{ ok: false, error, runId }` with:
|
||||
|
||||
|
||||
@@ -930,10 +930,11 @@ Validation and safety notes:
|
||||
**Endpoints:**
|
||||
|
||||
- `POST /hooks/wake` → `{ text, mode?: "now"|"next-heartbeat" }`
|
||||
- `POST /hooks/agent` → `{ message, name?, agentId?, sessionKey?, sessionMode?, wakeMode?, deliver?, channel?, to?, model?, thinking?, timeoutSeconds? }`
|
||||
- `POST /hooks/agent` → `{ message, name?, agentId?, sessionKey?, sessionMode?, wakeMode?, deliver?, channel?, to?, accountId?, model?, thinking?, timeoutSeconds? }`
|
||||
- `sessionKey` from request payload is accepted only when `hooks.allowRequestSessionKey=true` (default: `false`).
|
||||
- `sessionMode` is `"isolated"` by default. `"persistent"` reuses the resolved session and requires an explicit request `sessionKey`, `hooks.allowRequestSessionKey=true`, and non-empty `hooks.allowedSessionKeyPrefixes`.
|
||||
- Direct announce delivery requires both a concrete `channel` and `to`; supplying only one fails before the run is scheduled.
|
||||
- `accountId` selects a configured account for direct announce delivery and requires both `channel` and `to`.
|
||||
- Omit both delivery fields for completion-only hooks, or set `deliver: false` to ignore supplied destination data.
|
||||
- The request waits up to 15 seconds for runner admission, not run completion. `200` means the agent runner was entered.
|
||||
- Pre-run failures return `{ ok: false, error, runId }`: `409` for session admission conflicts, `502` for other preparation failures, and `503` when the 15-second admission deadline expires. Timed-out queued work is canceled and will not start later.
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
// QA Lab product proof exercises hook delivery through a real Gateway child and qa-channel bus.
|
||||
import { setTimeout as sleep } from "node:timers/promises";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { startQaGatewayChild } from "./gateway-child.js";
|
||||
import { startQaLabServer } from "./lab-server.js";
|
||||
import { startQaProviderServer } from "./providers/server-runtime.js";
|
||||
import { createQaChannelTransport } from "./qa-channel-transport.js";
|
||||
|
||||
const HOOK_TOKEN = "qa-hook-account-routing-token";
|
||||
const MARKER = "QA_HOOK_ACCOUNT_ROUTING_OK";
|
||||
const MODEL = "mock-openai/gpt-5.6-luna";
|
||||
|
||||
async function postJson(url: string, body: unknown, headers: Record<string, string>) {
|
||||
const response = await fetch(url, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
...headers,
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
signal: AbortSignal.timeout(30_000),
|
||||
});
|
||||
return {
|
||||
status: response.status,
|
||||
json: (await response.json()) as unknown,
|
||||
};
|
||||
}
|
||||
|
||||
describe("hook agent account routing product proof", () => {
|
||||
it(
|
||||
"delivers exactly once through the selected qa-channel account",
|
||||
{ timeout: 180_000 },
|
||||
async () => {
|
||||
const repoRoot = fileURLToPath(new URL("../../../", import.meta.url));
|
||||
const lab = await startQaLabServer({
|
||||
repoRoot,
|
||||
embeddedGateway: "disabled",
|
||||
});
|
||||
const mock = await startQaProviderServer("mock-openai", {
|
||||
modelRefs: [MODEL],
|
||||
});
|
||||
let gateway: Awaited<ReturnType<typeof startQaGatewayChild>> | undefined;
|
||||
|
||||
try {
|
||||
if (!mock) {
|
||||
throw new Error("mock-openai provider server did not start");
|
||||
}
|
||||
const transport = createQaChannelTransport(lab.state);
|
||||
gateway = await startQaGatewayChild({
|
||||
repoRoot,
|
||||
useRepoCli: true,
|
||||
providerBaseUrl: `${mock.baseUrl}/v1`,
|
||||
providerMode: "mock-openai",
|
||||
primaryModel: MODEL,
|
||||
alternateModel: MODEL,
|
||||
transportBaseUrl: lab.listenUrl,
|
||||
transport,
|
||||
controlUiEnabled: false,
|
||||
mutateConfig: (config) => {
|
||||
const qaChannel = config.channels?.["qa-channel"];
|
||||
if (!qaChannel) {
|
||||
throw new Error("qa-channel transport config missing");
|
||||
}
|
||||
return {
|
||||
...config,
|
||||
hooks: {
|
||||
enabled: true,
|
||||
token: HOOK_TOKEN,
|
||||
path: "/hooks",
|
||||
},
|
||||
channels: {
|
||||
...config.channels,
|
||||
"qa-channel": {
|
||||
...qaChannel,
|
||||
defaultAccount: "default",
|
||||
accounts: {
|
||||
default: { name: "Default" },
|
||||
work: { name: "Work" },
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
},
|
||||
});
|
||||
|
||||
const response = await postJson(
|
||||
`${gateway.baseUrl}/hooks/agent`,
|
||||
{
|
||||
message: `Reply exactly: ${MARKER}`,
|
||||
deliver: true,
|
||||
channel: "qa-channel",
|
||||
to: "dm:hook-recipient",
|
||||
accountId: "work",
|
||||
},
|
||||
{ Authorization: `Bearer ${HOOK_TOKEN}` },
|
||||
);
|
||||
expect(response.status, JSON.stringify(response.json)).toBe(200);
|
||||
|
||||
const body = response.json as { ok?: boolean; runId?: string };
|
||||
expect(body.ok).toBe(true);
|
||||
expect(body.runId).toEqual(expect.any(String));
|
||||
|
||||
await vi.waitFor(
|
||||
() => {
|
||||
const outbound = lab.state
|
||||
.getSnapshot()
|
||||
.messages.filter((message) => message.direction === "outbound");
|
||||
expect(outbound).toHaveLength(1);
|
||||
expect(outbound[0]).toMatchObject({
|
||||
accountId: "work",
|
||||
conversation: { id: "hook-recipient", kind: "direct" },
|
||||
text: MARKER,
|
||||
});
|
||||
},
|
||||
{ interval: 50, timeout: 60_000 },
|
||||
);
|
||||
await sleep(500);
|
||||
|
||||
const outbound = lab.state
|
||||
.getSnapshot()
|
||||
.messages.filter((message) => message.direction === "outbound");
|
||||
expect(outbound).toHaveLength(1);
|
||||
expect(outbound.filter((message) => message.accountId === "default")).toHaveLength(0);
|
||||
} finally {
|
||||
await gateway?.stop().catch(() => undefined);
|
||||
await mock?.stop().catch(() => undefined);
|
||||
await lab.stop().catch(() => undefined);
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -283,18 +283,47 @@ describe("gateway hooks helpers", () => {
|
||||
ok: false,
|
||||
error: "channel must name a concrete channel for hook delivery",
|
||||
});
|
||||
expect(
|
||||
normalizeAgentPayload({
|
||||
message: "hello",
|
||||
accountId: "work",
|
||||
}),
|
||||
).toEqual({
|
||||
ok: false,
|
||||
error: "accountId requires channel and to for hook delivery",
|
||||
});
|
||||
for (const accountId of [123, " "]) {
|
||||
expect(
|
||||
normalizeAgentPayload({
|
||||
message: "hello",
|
||||
channel: "demo-alias-channel",
|
||||
to: "123456",
|
||||
accountId,
|
||||
}),
|
||||
).toEqual({
|
||||
ok: false,
|
||||
error: "accountId must be a non-empty string for hook delivery",
|
||||
});
|
||||
}
|
||||
|
||||
const explicit = normalizeAgentPayload({
|
||||
message: "hello",
|
||||
channel: "demo-alias-channel",
|
||||
to: "123456",
|
||||
accountId: " work ",
|
||||
});
|
||||
expect(explicit).toMatchObject({
|
||||
ok: true,
|
||||
value: {
|
||||
channel: "demo-alias-channel",
|
||||
to: "123456",
|
||||
delivery: { mode: "announce", channel: "demo-alias-channel", to: "123456" },
|
||||
accountId: "work",
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel: "demo-alias-channel",
|
||||
to: "123456",
|
||||
accountId: "work",
|
||||
},
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
+29
-3
@@ -231,12 +231,14 @@ type HookAgentPayload = {
|
||||
deliver: boolean;
|
||||
channel: HookMessageChannel;
|
||||
to?: string;
|
||||
accountId?: string;
|
||||
delivery:
|
||||
| { mode: "none" }
|
||||
| {
|
||||
mode: "announce";
|
||||
channel: HookMessageChannel;
|
||||
to?: string;
|
||||
accountId?: string;
|
||||
};
|
||||
model?: string;
|
||||
thinking?: string;
|
||||
@@ -280,10 +282,15 @@ export function resolveHookDeliver(raw: unknown): boolean {
|
||||
}
|
||||
|
||||
/** Normalize webhook delivery intent before any isolated cron work is scheduled. */
|
||||
function normalizeHookAgentDelivery(params: { deliver: unknown; channel: unknown; to: unknown }):
|
||||
function normalizeHookAgentDelivery(params: {
|
||||
deliver: unknown;
|
||||
channel: unknown;
|
||||
to: unknown;
|
||||
accountId: unknown;
|
||||
}):
|
||||
| {
|
||||
ok: true;
|
||||
value: Pick<HookAgentPayload, "deliver" | "channel" | "to" | "delivery">;
|
||||
value: Pick<HookAgentPayload, "deliver" | "channel" | "to" | "accountId" | "delivery">;
|
||||
}
|
||||
| { ok: false; error: string } {
|
||||
const deliver = resolveHookDeliver(params.deliver);
|
||||
@@ -294,24 +301,28 @@ function normalizeHookAgentDelivery(params: { deliver: unknown; channel: unknown
|
||||
deliver,
|
||||
channel: "last",
|
||||
to: undefined,
|
||||
accountId: undefined,
|
||||
delivery: { mode: "none" },
|
||||
},
|
||||
};
|
||||
}
|
||||
const to = normalizeOptionalString(params.to);
|
||||
const accountId = normalizeOptionalString(params.accountId);
|
||||
const channel = resolveHookChannel(params.channel);
|
||||
if (!channel) {
|
||||
return { ok: false, error: getHookChannelError() };
|
||||
}
|
||||
const hasChannel = params.channel !== undefined;
|
||||
const hasTo = params.to !== undefined;
|
||||
if (!hasChannel && !hasTo) {
|
||||
const hasAccountId = params.accountId !== undefined;
|
||||
if (!hasChannel && !hasTo && !hasAccountId) {
|
||||
return {
|
||||
ok: true,
|
||||
value: {
|
||||
deliver,
|
||||
channel,
|
||||
to,
|
||||
accountId,
|
||||
delivery: { mode: "none" },
|
||||
},
|
||||
};
|
||||
@@ -322,6 +333,18 @@ function normalizeHookAgentDelivery(params: { deliver: unknown; channel: unknown
|
||||
error: "to must be a non-empty string for hook delivery",
|
||||
};
|
||||
}
|
||||
if (hasAccountId && !accountId) {
|
||||
return {
|
||||
ok: false,
|
||||
error: "accountId must be a non-empty string for hook delivery",
|
||||
};
|
||||
}
|
||||
if (hasAccountId && (!hasChannel || !to)) {
|
||||
return {
|
||||
ok: false,
|
||||
error: "accountId requires channel and to for hook delivery",
|
||||
};
|
||||
}
|
||||
if (!hasChannel || !to) {
|
||||
return {
|
||||
ok: false,
|
||||
@@ -340,10 +363,12 @@ function normalizeHookAgentDelivery(params: { deliver: unknown; channel: unknown
|
||||
deliver,
|
||||
channel,
|
||||
to,
|
||||
accountId,
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel,
|
||||
to,
|
||||
...(accountId ? { accountId } : {}),
|
||||
},
|
||||
},
|
||||
};
|
||||
@@ -532,6 +557,7 @@ export function normalizeAgentPayload(payload: Record<string, unknown>):
|
||||
deliver: payload.deliver,
|
||||
channel: payload.channel,
|
||||
to: payload.to,
|
||||
accountId: payload.accountId,
|
||||
});
|
||||
if (!delivery.ok) {
|
||||
return delivery;
|
||||
|
||||
@@ -141,11 +141,22 @@ describe("hook request delivery normalization", () => {
|
||||
const explicit = await dispatchPayload({
|
||||
handler,
|
||||
path: "/hooks/agent",
|
||||
payload: { message: "Explicit", channel: "delivery-test", to: "123456" },
|
||||
payload: {
|
||||
message: "Explicit",
|
||||
channel: "delivery-test",
|
||||
to: "123456",
|
||||
accountId: "work",
|
||||
},
|
||||
});
|
||||
expect(explicit.res.statusCode).toBe(200);
|
||||
expect(dispatchAgentHook.mock.calls[2]?.[0]).toMatchObject({
|
||||
delivery: { mode: "announce", channel: "delivery-test", to: "123456" },
|
||||
accountId: "work",
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel: "delivery-test",
|
||||
to: "123456",
|
||||
accountId: "work",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -392,6 +392,7 @@ export function createHooksRequestHandler(
|
||||
deliver: normalized.value.deliver,
|
||||
channel: normalized.value.channel,
|
||||
to: normalized.value.to ?? null,
|
||||
accountId: normalized.value.accountId ?? null,
|
||||
model: normalized.value.model ?? null,
|
||||
thinking: normalized.value.thinking ?? null,
|
||||
timeoutSeconds: normalized.value.timeoutSeconds ?? null,
|
||||
|
||||
@@ -172,6 +172,7 @@ describe("dispatchAgentHook trust handling", () => {
|
||||
mode: "announce" as const,
|
||||
channel: "telegram" as const,
|
||||
to: "123456",
|
||||
accountId: "work",
|
||||
};
|
||||
runCronIsolatedAgentTurnMock.mockResolvedValueOnce({
|
||||
status: "ok",
|
||||
@@ -184,6 +185,7 @@ describe("dispatchAgentHook trust handling", () => {
|
||||
deliver: true,
|
||||
channel: delivery.channel,
|
||||
to: delivery.to,
|
||||
accountId: delivery.accountId,
|
||||
delivery,
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user