fix(hooks): repair shared-hook announcement policy (#73800)

openclaw-clownfish[bot] · vincentkoc · web-flow · commit 2f31184d07c2 · 2026-04-29T05:28:16.000-07:00
* fix(hooks): repair shared-hook announcement policy

* fix(hooks): audit suppressed hook successes

---------

Co-authored-by: openclaw-clownfish[bot] &lt;280122609+openclaw-clownfish[bot]@users.noreply.github.com&gt;
Co-authored-by: Vincent Koc &lt;vincentkoc@ieee.org&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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.
diff --git a/src/gateway/hooks.ts b/src/gateway/hooks.ts
@@ -227,6 +227,7 @@ export type HookAgentPayload = {
 
 export type HookAgentDispatchPayload = Omit<HookAgentPayload, "sessionKey"> & {
   sessionKey: string;
+  sourcePath: string;
   allowUnsafeExternalContent?: boolean;
   externalContentSource?: HookExternalContentSource;
 };
diff --git a/src/gateway/server.hooks.test.ts b/src/gateway/server.hooks.test.ts
@@ -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,
diff --git a/src/gateway/server/hooks-request-handler.ts b/src/gateway/server/hooks-request-handler.ts
@@ -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,
diff --git a/src/gateway/server/hooks.agent-trust.test.ts b/src/gateway/server/hooks.agent-trust.test.ts
@@ -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,24 +136,64 @@ 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,
     });
 
     expect(capturedDispatchAgentHook).toBeDefined();
     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"));
 
diff --git a/src/gateway/server/hooks.ts b/src/gateway/server/hooks.ts
@@ -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)}`);
diff --git a/src/gateway/test-helpers.runtime-state.ts b/src/gateway/test-helpers.runtime-state.ts
@@ -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>;
