From 9e041cd3867f31a0f39403ca4becb3b31fa57ab4 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 30 Jul 2026 07:10:21 +0800 Subject: [PATCH] fix(hooks): preserve multi-account agent delivery (#116095) * fix(hooks): preserve account routing for agent delivery Refs #43866 Co-authored-by: Paul Desmond Parker * test(gateway): prove hook account delivery handoff --------- Co-authored-by: Paul Desmond Parker --- docs/automation/cron-jobs.md | 3 +- docs/gateway/configuration-reference.md | 3 +- .../hook-agent-account-routing.e2e.test.ts | 132 ++++++++++++++++++ src/gateway/hooks.test.ts | 31 +++- src/gateway/hooks.ts | 32 ++++- .../server-http.hooks-delivery.test.ts | 15 +- src/gateway/server/hooks-request-handler.ts | 1 + src/gateway/server/hooks.agent-trust.test.ts | 2 + 8 files changed, 211 insertions(+), 8 deletions(-) create mode 100644 extensions/qa-lab/src/hook-agent-account-routing.e2e.test.ts diff --git a/docs/automation/cron-jobs.md b/docs/automation/cron-jobs.md index 2ef677a5ecf5..578caea07e43 100644 --- a/docs/automation/cron-jobs.md +++ b/docs/automation/cron-jobs.md @@ -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: diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index b7a4645c4fa6..055e780292e2 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -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. diff --git a/extensions/qa-lab/src/hook-agent-account-routing.e2e.test.ts b/extensions/qa-lab/src/hook-agent-account-routing.e2e.test.ts new file mode 100644 index 000000000000..849d909668f9 --- /dev/null +++ b/extensions/qa-lab/src/hook-agent-account-routing.e2e.test.ts @@ -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) { + 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> | 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); + } + }, + ); +}); diff --git a/src/gateway/hooks.test.ts b/src/gateway/hooks.test.ts index e6c7bbcde139..ef02d32e472b 100644 --- a/src/gateway/hooks.test.ts +++ b/src/gateway/hooks.test.ts @@ -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", + }, }, }); }); diff --git a/src/gateway/hooks.ts b/src/gateway/hooks.ts index 96c840da100d..8d8e3ed4a8ed 100644 --- a/src/gateway/hooks.ts +++ b/src/gateway/hooks.ts @@ -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; + value: Pick; } | { 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): deliver: payload.deliver, channel: payload.channel, to: payload.to, + accountId: payload.accountId, }); if (!delivery.ok) { return delivery; diff --git a/src/gateway/server-http.hooks-delivery.test.ts b/src/gateway/server-http.hooks-delivery.test.ts index e0f634c42dd0..0c14643239ba 100644 --- a/src/gateway/server-http.hooks-delivery.test.ts +++ b/src/gateway/server-http.hooks-delivery.test.ts @@ -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", + }, }); }); diff --git a/src/gateway/server/hooks-request-handler.ts b/src/gateway/server/hooks-request-handler.ts index 96a026180823..927089a68b09 100644 --- a/src/gateway/server/hooks-request-handler.ts +++ b/src/gateway/server/hooks-request-handler.ts @@ -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, diff --git a/src/gateway/server/hooks.agent-trust.test.ts b/src/gateway/server/hooks.agent-trust.test.ts index f674b8ccb060..e7945fda0f94 100644 --- a/src/gateway/server/hooks.agent-trust.test.ts +++ b/src/gateway/server/hooks.agent-trust.test.ts @@ -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, });