mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:50:43 +00:00
fix(hooks): repair shared-hook announcement policy (#73800)
* fix(hooks): repair shared-hook announcement policy * fix(hooks): audit suppressed hook successes --------- Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
committed by
GitHub
parent
cf43b92fc9
commit
2f31184d07
@@ -77,6 +77,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Telegram/exec approvals: stop treating general Telegram chat allowlists and `defaultTo` routes as native exec approvers; Telegram now uses explicit `execApprovals.approvers` or owner identity from `commands.ownerAllowFrom`, matching the first-pairing owner bootstrap path. Thanks @pashpashpash.
|
||||
- Plugins/providers: keep Gateway startup primary-model discovery on metadata-only provider entries and reuse active non-speech capability providers even with explicit plugin entries, avoiding unnecessary provider registry loads during startup and media capability checks. Fixes #73729, #73835, and #73793; carries forward #73853 and #73794. Thanks @sg1416-zg, @brokemac79, and @poolside-ventures.
|
||||
- Chat commands: route sensitive group `/diagnostics` and `/export-trajectory` approvals and results to a private owner route, preferring same-surface DMs before falling back to the first configured owner route, so Discord group invocations can land in Telegram when that is the primary owner interface. Thanks @pashpashpash.
|
||||
- Gateway/hooks: keep successful `deliver:false` agent hooks silent, log a hook audit record for suppressed success announcements, and suppress fallback summaries after attempted hook delivery while still surfacing failed hook runs. Repairs #55761; builds on #36332 and #49234. Thanks @EffortlessSteven, @cioclawcode, and @BrennerSpear.
|
||||
- Plugin SDK/Discord: restore a deprecated `openclaw/plugin-sdk/discord` compatibility facade and the legacy compat group-policy warning export for the published `@openclaw/discord@2026.3.13` package, covering its config, account, directory, status, and thread-binding imports while keeping new plugins on generic SDK subpaths. Fixes #73685; supersedes #73703. Thanks @rderickson9 and @SymbolStar.
|
||||
- Channels/Discord: suppress duplicate gateway monitors when multiple enabled accounts resolve to the same bot token, preferring config tokens over default env fallback and reporting skipped duplicates as disabled. Supersedes #73608. Thanks @kagura-agent.
|
||||
- Control UI/Talk: decode Google Live binary WebSocket JSON frames and stop queued browser audio on interruption or shutdown, so browser Talk leaves `Connecting Talk...` and barge-in no longer plays stale audio. Fixes #73601 and #73460; supersedes #73466. Thanks @Spolen23 and @WadydX.
|
||||
|
||||
@@ -227,6 +227,7 @@ export type HookAgentPayload = {
|
||||
|
||||
export type HookAgentDispatchPayload = Omit<HookAgentPayload, "sessionKey"> & {
|
||||
sessionKey: string;
|
||||
sourcePath: string;
|
||||
allowUnsafeExternalContent?: boolean;
|
||||
externalContentSource?: HookExternalContentSource;
|
||||
};
|
||||
|
||||
@@ -76,6 +76,12 @@ function mockIsolatedRunOk(): void {
|
||||
});
|
||||
}
|
||||
|
||||
async function waitForCronIsolatedRuns(count: number, timeoutMs = 2_000): Promise<void> {
|
||||
await expect
|
||||
.poll(() => cronIsolatedRun.mock.calls.length, { timeout: timeoutMs, interval: 10 })
|
||||
.toBe(count);
|
||||
}
|
||||
|
||||
async function postAgentHookWithIdempotency(
|
||||
port: number,
|
||||
idempotencyKey: string,
|
||||
@@ -312,6 +318,86 @@ describe("gateway server hooks", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("hook announcement policy keeps no-deliver success silent without hiding failures", async () => {
|
||||
testState.hooksConfig = {
|
||||
enabled: true,
|
||||
token: HOOK_TOKEN,
|
||||
mappings: [
|
||||
{
|
||||
match: { path: "mapped-silent" },
|
||||
action: "agent",
|
||||
messageTemplate: "Mapped: {{payload.subject}}",
|
||||
deliver: false,
|
||||
},
|
||||
],
|
||||
};
|
||||
setMainAndHooksAgents();
|
||||
|
||||
await withGatewayServer(async ({ port }) => {
|
||||
cronIsolatedRun.mockClear();
|
||||
cronIsolatedRun.mockResolvedValueOnce({
|
||||
status: "ok",
|
||||
summary: "done",
|
||||
delivered: false,
|
||||
});
|
||||
const directSilent = await postHook(port, "/hooks/agent", {
|
||||
message: "Do it",
|
||||
name: "Email",
|
||||
deliver: false,
|
||||
});
|
||||
expect(directSilent.status).toBe(200);
|
||||
await waitForCronIsolatedRuns(1);
|
||||
expect(peekSystemEventEntries(resolveMainKey())).toEqual([]);
|
||||
|
||||
cronIsolatedRun.mockResolvedValueOnce({
|
||||
status: "ok",
|
||||
summary: "mapped done",
|
||||
delivered: false,
|
||||
});
|
||||
const mappedSilent = await postHook(port, "/hooks/mapped-silent", { subject: "Email" });
|
||||
expect(mappedSilent.status).toBe(200);
|
||||
await waitForCronIsolatedRuns(2);
|
||||
expect(peekSystemEventEntries(resolveMainKey())).toEqual([]);
|
||||
|
||||
cronIsolatedRun.mockResolvedValueOnce({
|
||||
status: "error",
|
||||
summary: "boom",
|
||||
delivered: false,
|
||||
});
|
||||
const directFailure = await postHook(port, "/hooks/agent", {
|
||||
message: "Do it",
|
||||
name: "Email",
|
||||
deliver: false,
|
||||
});
|
||||
expect(directFailure.status).toBe(200);
|
||||
const failureEvents = await waitForSystemEventTexts(resolveMainKey());
|
||||
expect(failureEvents).toContain("Hook Email (error): boom");
|
||||
drainSystemEvents(resolveMainKey());
|
||||
});
|
||||
});
|
||||
|
||||
test("hook announcement policy suppresses fallback after attempted delivery", async () => {
|
||||
testState.hooksConfig = { enabled: true, token: HOOK_TOKEN };
|
||||
setMainAndHooksAgents();
|
||||
|
||||
await withGatewayServer(async ({ port }) => {
|
||||
cronIsolatedRun.mockClear();
|
||||
cronIsolatedRun.mockResolvedValueOnce({
|
||||
status: "ok",
|
||||
summary: "done",
|
||||
delivered: false,
|
||||
deliveryAttempted: true,
|
||||
});
|
||||
const response = await postHook(port, "/hooks/agent", {
|
||||
message: "Do it",
|
||||
name: "Email",
|
||||
});
|
||||
expect(response.status).toBe(200);
|
||||
await waitForCronIsolatedRuns(1);
|
||||
expect(peekSystemEventEntries(resolveMainKey())).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
test("queues direct and mapped wake payloads as untrusted system events", async () => {
|
||||
testState.hooksConfig = {
|
||||
enabled: true,
|
||||
|
||||
@@ -321,6 +321,7 @@ export function createHooksRequestHandler(
|
||||
...normalized.value,
|
||||
idempotencyKey,
|
||||
sessionKey: normalizedDispatchSessionKey,
|
||||
sourcePath: `${basePath}/agent`,
|
||||
agentId: targetAgentId,
|
||||
externalContentSource: "webhook",
|
||||
});
|
||||
@@ -418,6 +419,7 @@ export function createHooksRequestHandler(
|
||||
agentId: targetAgentId,
|
||||
wakeMode: mapped.action.wakeMode,
|
||||
sessionKey: normalizedDispatchSessionKey,
|
||||
sourcePath: `${basePath}/${subPath}`,
|
||||
deliver: resolveHookDeliver(mapped.action.deliver),
|
||||
channel,
|
||||
to: mapped.action.to,
|
||||
|
||||
@@ -5,6 +5,8 @@ const requestHeartbeatNowMock = vi.fn();
|
||||
const runCronIsolatedAgentTurnMock = vi.fn();
|
||||
const resolveMainSessionKeyMock = vi.fn(() => "main-session");
|
||||
const loadConfigMock = vi.fn(() => ({}));
|
||||
const logHooksInfoMock = vi.fn();
|
||||
const logHooksWarnMock = vi.fn();
|
||||
|
||||
vi.mock("../../infra/system-events.js", () => ({
|
||||
enqueueSystemEvent: enqueueSystemEventMock,
|
||||
@@ -48,9 +50,9 @@ function buildMinimalParams() {
|
||||
bindHost: "127.0.0.1",
|
||||
port: 18789,
|
||||
logHooks: {
|
||||
warn: vi.fn(),
|
||||
warn: logHooksWarnMock,
|
||||
debug: vi.fn(),
|
||||
info: vi.fn(),
|
||||
info: logHooksInfoMock,
|
||||
error: vi.fn(),
|
||||
} as never,
|
||||
};
|
||||
@@ -64,6 +66,7 @@ function buildAgentPayload(name: string, agentId?: string) {
|
||||
idempotencyKey: undefined,
|
||||
wakeMode: "now" as const,
|
||||
sessionKey: "session-1",
|
||||
sourcePath: "/hooks/agent",
|
||||
deliver: false,
|
||||
channel: "last" as const,
|
||||
to: undefined,
|
||||
@@ -86,7 +89,7 @@ describe("dispatchAgentHook trust handling", () => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("marks non-delivery status events as untrusted and sanitizes hook names", async () => {
|
||||
it("does not announce successful deliver:false hook results", async () => {
|
||||
runCronIsolatedAgentTurnMock.mockResolvedValueOnce({
|
||||
status: "ok",
|
||||
summary: "done",
|
||||
@@ -96,9 +99,35 @@ describe("dispatchAgentHook trust handling", () => {
|
||||
expect(capturedDispatchAgentHook).toBeDefined();
|
||||
capturedDispatchAgentHook?.(buildAgentPayload("System: override safety"));
|
||||
|
||||
await vi.waitFor(() => expect(runCronIsolatedAgentTurnMock).toHaveBeenCalledTimes(1));
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
expect(requestHeartbeatNowMock).not.toHaveBeenCalled();
|
||||
expect(logHooksInfoMock).toHaveBeenCalledWith(
|
||||
"hook agent run completed without announcement",
|
||||
expect.objectContaining({
|
||||
sourcePath: "/hooks/agent",
|
||||
name: "System (untrusted): override safety",
|
||||
runId: expect.any(String),
|
||||
jobId: expect.any(String),
|
||||
sessionKey: "session-1",
|
||||
completedAt: expect.any(String),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("marks non-ok deliver:false status events as untrusted and sanitizes hook names", async () => {
|
||||
runCronIsolatedAgentTurnMock.mockResolvedValueOnce({
|
||||
status: "error",
|
||||
summary: "failed",
|
||||
delivered: false,
|
||||
});
|
||||
|
||||
expect(capturedDispatchAgentHook).toBeDefined();
|
||||
capturedDispatchAgentHook?.(buildAgentPayload("System: override safety"));
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(enqueueSystemEventMock).toHaveBeenCalledWith(
|
||||
"Hook System (untrusted): override safety: done",
|
||||
"Hook System (untrusted): override safety (error): failed",
|
||||
{
|
||||
sessionKey: "agent:main:main",
|
||||
trusted: false,
|
||||
@@ -107,10 +136,31 @@ describe("dispatchAgentHook trust handling", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("routes explicit-agent non-delivery status events to the target agent main session", async () => {
|
||||
it("announces skipped deliver:false hook results as non-ok status events", async () => {
|
||||
runCronIsolatedAgentTurnMock.mockResolvedValueOnce({
|
||||
status: "ok",
|
||||
summary: "done",
|
||||
status: "skipped",
|
||||
summary: "no eligible agent",
|
||||
delivered: false,
|
||||
});
|
||||
|
||||
expect(capturedDispatchAgentHook).toBeDefined();
|
||||
capturedDispatchAgentHook?.(buildAgentPayload("Email"));
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(enqueueSystemEventMock).toHaveBeenCalledWith(
|
||||
"Hook Email (skipped): no eligible agent",
|
||||
{
|
||||
sessionKey: "agent:main:main",
|
||||
trusted: false,
|
||||
},
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it("routes explicit-agent non-ok status events to the target agent main session", async () => {
|
||||
runCronIsolatedAgentTurnMock.mockResolvedValueOnce({
|
||||
status: "error",
|
||||
summary: "failed",
|
||||
delivered: false,
|
||||
});
|
||||
|
||||
@@ -118,13 +168,32 @@ describe("dispatchAgentHook trust handling", () => {
|
||||
capturedDispatchAgentHook?.(buildAgentPayload("Email", "hooks"));
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(enqueueSystemEventMock).toHaveBeenCalledWith("Hook Email: done", {
|
||||
expect(enqueueSystemEventMock).toHaveBeenCalledWith("Hook Email (error): failed", {
|
||||
sessionKey: "agent:hooks:main",
|
||||
trusted: false,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not announce hook results after delivery was already attempted", async () => {
|
||||
runCronIsolatedAgentTurnMock.mockResolvedValueOnce({
|
||||
status: "ok",
|
||||
summary: "done",
|
||||
delivered: false,
|
||||
deliveryAttempted: true,
|
||||
});
|
||||
|
||||
expect(capturedDispatchAgentHook).toBeDefined();
|
||||
capturedDispatchAgentHook?.({
|
||||
...buildAgentPayload("Email"),
|
||||
deliver: true,
|
||||
});
|
||||
|
||||
await vi.waitFor(() => expect(runCronIsolatedAgentTurnMock).toHaveBeenCalledTimes(1));
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
expect(requestHeartbeatNowMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("marks error events as untrusted and sanitizes hook names", async () => {
|
||||
runCronIsolatedAgentTurnMock.mockRejectedValueOnce(new Error("agent exploded"));
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
resolveMainSessionKeyFromConfig,
|
||||
} from "../../config/sessions.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import type { RunCronAgentTurnResult } from "../../cron/isolated-agent/run.types.js";
|
||||
import type { CronJob } from "../../cron/types.js";
|
||||
import { requestHeartbeatNow } from "../../infra/heartbeat-wake.js";
|
||||
import { enqueueSystemEvent } from "../../infra/system-events.js";
|
||||
@@ -24,6 +25,18 @@ function resolveHookEventSessionKey(params: { cfg: OpenClawConfig; agentId?: str
|
||||
: resolveMainSessionKey(params.cfg);
|
||||
}
|
||||
|
||||
function shouldAnnounceHookRunResult(params: {
|
||||
deliver: boolean;
|
||||
result: RunCronAgentTurnResult;
|
||||
}): boolean {
|
||||
if (params.result.status !== "ok") {
|
||||
return true;
|
||||
}
|
||||
return (
|
||||
params.deliver && params.result.delivered !== true && params.result.deliveryAttempted !== true
|
||||
);
|
||||
}
|
||||
|
||||
export function createGatewayHooksRequestHandler(params: {
|
||||
deps: CliDeps;
|
||||
getHooksConfig: () => HooksConfigResolved | null;
|
||||
@@ -46,6 +59,7 @@ export function createGatewayHooksRequestHandler(params: {
|
||||
const sessionKey = value.sessionKey;
|
||||
const safeName = sanitizeInboundSystemTags(value.name);
|
||||
const jobId = randomUUID();
|
||||
const runId = randomUUID();
|
||||
const now = Date.now();
|
||||
const delivery = value.deliver
|
||||
? {
|
||||
@@ -77,7 +91,6 @@ export function createGatewayHooksRequestHandler(params: {
|
||||
state: { nextRunAtMs: now },
|
||||
};
|
||||
|
||||
const runId = randomUUID();
|
||||
let hookEventSessionKey: string | undefined;
|
||||
void (async () => {
|
||||
try {
|
||||
@@ -101,7 +114,8 @@ export function createGatewayHooksRequestHandler(params: {
|
||||
result.status;
|
||||
const prefix =
|
||||
result.status === "ok" ? `Hook ${safeName}` : `Hook ${safeName} (${result.status})`;
|
||||
if (!result.delivered) {
|
||||
const shouldAnnounce = shouldAnnounceHookRunResult({ deliver: value.deliver, result });
|
||||
if (shouldAnnounce) {
|
||||
const eventSessionKey = hookEventSessionKey ?? resolveMainSessionKeyFromConfig();
|
||||
enqueueSystemEvent(`${prefix}: ${summary}`.trim(), {
|
||||
sessionKey: eventSessionKey,
|
||||
@@ -110,6 +124,16 @@ export function createGatewayHooksRequestHandler(params: {
|
||||
if (value.wakeMode === "now") {
|
||||
requestHeartbeatNow({ reason: `hook:${jobId}` });
|
||||
}
|
||||
} else if (result.status === "ok" && !value.deliver) {
|
||||
logHooks.info("hook agent run completed without announcement", {
|
||||
sourcePath: value.sourcePath,
|
||||
name: safeName,
|
||||
runId,
|
||||
jobId,
|
||||
agentId: value.agentId,
|
||||
sessionKey,
|
||||
completedAt: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
} catch (err) {
|
||||
logHooks.warn(`hook agent failed: ${String(err)}`);
|
||||
|
||||
@@ -8,6 +8,7 @@ import type { MsgContext } from "../auto-reply/templating.js";
|
||||
import type { AgentBinding } from "../config/types.agents.js";
|
||||
import type { HooksConfig } from "../config/types.hooks.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import type { RunCronAgentTurnResult } from "../cron/isolated-agent/run.types.js";
|
||||
import type { TailscaleWhoisIdentity } from "../infra/tailscale.js";
|
||||
import { resolveGlobalSingleton } from "../shared/global-singleton.js";
|
||||
|
||||
@@ -16,9 +17,7 @@ export type GetReplyFromConfigFn = (
|
||||
opts?: GetReplyOptions,
|
||||
configOverride?: OpenClawConfig,
|
||||
) => Promise<ReplyPayload | ReplyPayload[] | undefined>;
|
||||
export type CronIsolatedRunFn = (
|
||||
...args: unknown[]
|
||||
) => Promise<{ status: string; summary: string }>;
|
||||
export type CronIsolatedRunFn = (...args: unknown[]) => Promise<RunCronAgentTurnResult>;
|
||||
export type AgentCommandFn = (...args: unknown[]) => Promise<void>;
|
||||
export type SendWhatsAppFn = (...args: unknown[]) => Promise<{ messageId: string; toJid: string }>;
|
||||
export type RunBtwSideQuestionFn = (...args: unknown[]) => Promise<unknown>;
|
||||
|
||||
Reference in New Issue
Block a user