From 194c0ff4b312c7ecbbf8f7b5be2818dd533e4245 Mon Sep 17 00:00:00 2001 From: jesse-merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Wed, 6 May 2026 14:54:26 +1000 Subject: [PATCH] fix: wrap hook block messages with originator --- src/agents/cli-runner.reliability.test.ts | 15 +++++++++++---- src/agents/cli-runner.ts | 7 +++++-- .../pi-embedded-runner/run/attempt.test.ts | 12 ++++++++++-- src/agents/pi-embedded-runner/run/attempt.ts | 17 +++++++++++------ .../agent-runner.misc.runreplyagent.test.ts | 17 ++++++++++++++--- src/plugins/hook-decision-types.test.ts | 19 +++++++++++++------ src/plugins/hook-decision-types.ts | 19 ++++++++++++------- 7 files changed, 76 insertions(+), 30 deletions(-) diff --git a/src/agents/cli-runner.reliability.test.ts b/src/agents/cli-runner.reliability.test.ts index 9cf2c3280e9..b33f5f7cdb2 100644 --- a/src/agents/cli-runner.reliability.test.ts +++ b/src/agents/cli-runner.reliability.test.ts @@ -668,7 +668,10 @@ describe("runCliAgent reliability", () => { }); expect(result.payloads).toEqual([ - { text: "The agent cannot read this message.", isError: true }, + { + text: "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)", + isError: true, + }, ]); expect(result.meta.livenessState).toBe("blocked"); expect(supervisorSpawnMock).not.toHaveBeenCalled(); @@ -692,11 +695,13 @@ describe("runCliAgent reliability", () => { expect(hookRunner.runAgentEnd).toHaveBeenCalledWith( expect.objectContaining({ success: false, - error: "The agent cannot read this message.", + error: + "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)", messages: expect.arrayContaining([ expect.objectContaining({ role: "user", - content: "The agent cannot read this message.", + content: + "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)", }), ]), }), @@ -706,7 +711,9 @@ describe("runCliAgent reliability", () => { const lines = fs.readFileSync(sessionFile, "utf-8").trim().split("\n"); const blockedLine = JSON.parse(lines[lines.length - 1]); - expect(blockedLine.message.content[0].text).toBe("The agent cannot read this message."); + expect(blockedLine.message.content[0].text).toBe( + "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)", + ); expect(JSON.stringify(blockedLine)).not.toContain("secret prompt"); expect(JSON.stringify(blockedLine)).not.toContain("matched secret prompt"); expect(blockedLine.message.__openclaw.beforeAgentRunBlocked).toMatchObject({ diff --git a/src/agents/cli-runner.ts b/src/agents/cli-runner.ts index 88ef3cbcc77..c6029bca81f 100644 --- a/src/agents/cli-runner.ts +++ b/src/agents/cli-runner.ts @@ -4,7 +4,7 @@ import { SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; import { formatErrorMessage } from "../infra/errors.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { buildAgentHookContextChannelFields } from "../plugins/hook-agent-context.js"; -import { DEFAULT_BLOCK_MESSAGE, resolveBlockMessage } from "../plugins/hook-decision-types.js"; +import { resolveBlockMessage } from "../plugins/hook-decision-types.js"; import { getGlobalHookRunner } from "../plugins/hook-runner-global.js"; import { loadCliSessionHistoryMessages } from "./cli-runner/session-history.js"; import type { PreparedCliRunContext, RunCliAgentParams } from "./cli-runner/types.js"; @@ -417,7 +417,10 @@ export async function runPreparedCliAgent( buildAgentHookContext(hookContext), ); } catch (err) { - const blockMessage = `${DEFAULT_BLOCK_MESSAGE} by before_agent_run`; + const blockMessage = resolveBlockMessage( + { outcome: "block", reason: "before_agent_run hook failed" }, + { blockedBy: "before_agent_run" }, + ); await persistBlockedBeforeAgentRun({ message: blockMessage, pluginId: "before_agent_run", diff --git a/src/agents/pi-embedded-runner/run/attempt.test.ts b/src/agents/pi-embedded-runner/run/attempt.test.ts index 1d100182e5a..ad5f59f6614 100644 --- a/src/agents/pi-embedded-runner/run/attempt.test.ts +++ b/src/agents/pi-embedded-runner/run/attempt.test.ts @@ -155,7 +155,12 @@ describe("normalizeMessagesForLlmBoundary", () => { const input = [ { role: "user", - content: [{ type: "text", text: "The agent cannot read this message." }], + content: [ + { + type: "text", + text: "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)", + }, + ], timestamp: 1, __openclaw: { beforeAgentRunBlocked: { @@ -171,7 +176,10 @@ describe("normalizeMessagesForLlmBoundary", () => { ) as Array>; expect(output[0]?.content).toEqual([ - { type: "text", text: "The agent cannot read this message." }, + { + type: "text", + text: "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)", + }, ]); expect(output[0]).toHaveProperty("__openclaw.beforeAgentRunBlocked"); expect(output[0]).not.toHaveProperty("__openclaw.beforeAgentRunBlocked.reason"); diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index 161590a99b8..aa35f667067 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -25,10 +25,7 @@ import { MAX_IMAGE_BYTES } from "../../../media/constants.js"; import { listRegisteredPluginAgentPromptGuidance } from "../../../plugins/command-registry-state.js"; import { getCurrentPluginMetadataSnapshot } from "../../../plugins/current-plugin-metadata-snapshot.js"; import { buildAgentHookContextChannelFields } from "../../../plugins/hook-agent-context.js"; -import { - DEFAULT_BLOCK_MESSAGE, - resolveBlockMessage, -} from "../../../plugins/hook-decision-types.js"; +import { resolveBlockMessage } from "../../../plugins/hook-decision-types.js"; import { getGlobalHookRunner } from "../../../plugins/hook-runner-global.js"; import { extractModelCompat, @@ -2862,10 +2859,18 @@ export async function runEmbeddedAttempt( beforeAgentRunBlocked = true; beforeAgentRunBlockedBy = "before_agent_run"; await persistBlockedBeforeAgentRun({ - message: `${DEFAULT_BLOCK_MESSAGE} by before_agent_run`, + message: resolveBlockMessage( + { outcome: "block", reason: "before_agent_run hook failed" }, + { blockedBy: "before_agent_run" }, + ), pluginId: "before_agent_run", }); - promptError = new Error(`${DEFAULT_BLOCK_MESSAGE} by before_agent_run`); + promptError = new Error( + resolveBlockMessage( + { outcome: "block", reason: "before_agent_run hook failed" }, + { blockedBy: "before_agent_run" }, + ), + ); promptErrorSource = "hook:before_agent_run"; skipPromptSubmission = true; } diff --git a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts index d8028a87dc2..04f0bba35c0 100644 --- a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts +++ b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts @@ -1692,9 +1692,18 @@ describe("runReplyAgent claude-cli routing", () => { it("does not leak hook-blocked CLI input in raw trace payloads", async () => { runCliAgentMock.mockResolvedValueOnce({ - payloads: [{ text: "The agent cannot read this message.", isError: true }], + payloads: [ + { + text: "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)", + isError: true, + }, + ], meta: { - error: { kind: "hook_block", message: "The agent cannot read this message." }, + error: { + kind: "hook_block", + message: + "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)", + }, agentMeta: { provider: "claude-cli", model: "opus-4.5", @@ -1773,7 +1782,9 @@ describe("runReplyAgent claude-cli routing", () => { const texts = Array.isArray(result) ? result.map((payload) => payload.text ?? "").join("\n") : (result?.text ?? ""); - expect(texts).toContain("The agent cannot read this message."); + expect(texts).toContain( + "Your message could not be sent: The agent cannot read this message. (blocked by policy-plugin)", + ); expect(texts).not.toContain("secret hitl prompt"); }); diff --git a/src/plugins/hook-decision-types.test.ts b/src/plugins/hook-decision-types.test.ts index 284d6e104a3..f9c4b670478 100644 --- a/src/plugins/hook-decision-types.test.ts +++ b/src/plugins/hook-decision-types.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it } from "vitest"; import { + BLOCK_MESSAGE_PREFIX, type HookDecision, type HookDecisionBlock, mergeHookDecisions, isHookDecision, - DEFAULT_BLOCK_MESSAGE, resolveBlockMessage, } from "./hook-decision-types.js"; @@ -63,12 +63,19 @@ describe("HookDecision helpers", () => { reason: "policy", }; - expect(resolveBlockMessage(explicit)).toBe("Please rephrase your request."); - expect(resolveBlockMessage(fallback)).toBe(DEFAULT_BLOCK_MESSAGE); - expect(resolveBlockMessage(fallback, { blockedBy: "policy-plugin" })).toBe( - `${DEFAULT_BLOCK_MESSAGE} by policy-plugin`, + expect(resolveBlockMessage(explicit)).toBe( + `${BLOCK_MESSAGE_PREFIX}: Please rephrase your request.`, + ); + expect(resolveBlockMessage(fallback)).toBe(`${BLOCK_MESSAGE_PREFIX}: blocked`); + expect(resolveBlockMessage(fallback, { blockedBy: "policy-plugin" })).toBe( + `${BLOCK_MESSAGE_PREFIX}: blocked by policy-plugin`, + ); + expect(resolveBlockMessage(explicit, { blockedBy: "policy-plugin" })).toBe( + `${BLOCK_MESSAGE_PREFIX}: Please rephrase your request. (blocked by policy-plugin)`, + ); + expect(resolveBlockMessage({ ...explicit, message: " " })).toBe( + `${BLOCK_MESSAGE_PREFIX}: blocked`, ); - expect(resolveBlockMessage({ ...explicit, message: " " })).toBe(DEFAULT_BLOCK_MESSAGE); }); }); }); diff --git a/src/plugins/hook-decision-types.ts b/src/plugins/hook-decision-types.ts index 4f908a3ee07..e42f48f2622 100644 --- a/src/plugins/hook-decision-types.ts +++ b/src/plugins/hook-decision-types.ts @@ -10,8 +10,8 @@ export type HookDecisionPass = { outcome: "pass"; }; -/** Default user-facing replacement message when a `block` decision omits one. */ -export const DEFAULT_BLOCK_MESSAGE = "This request was blocked by policy"; +/** Prefix for user-facing replacement messages when a `block` decision stops a request. */ +export const BLOCK_MESSAGE_PREFIX = "Your message could not be sent"; /** * Content is blocked. `reason` is internal; `message` is user-facing. @@ -20,7 +20,7 @@ export type HookDecisionBlock = { outcome: "block"; /** Internal reason for logging/observability. Never shown to user. */ reason: string; - /** Optional user-facing replacement text. Defaults to `DEFAULT_BLOCK_MESSAGE`. */ + /** Optional user-facing detail included in the block response envelope. */ message?: string; /** Plugin-defined category for analytics (e.g. "violence", "pii", "cost_limit"). */ category?: string; @@ -32,11 +32,16 @@ export function resolveBlockMessage( decision: HookDecisionBlock, params: { blockedBy?: string } = {}, ): string { - if (typeof decision.message === "string" && decision.message.trim()) { - return decision.message; - } + const message = typeof decision.message === "string" ? decision.message.trim() : ""; const blockedBy = params.blockedBy?.trim(); - return blockedBy ? `${DEFAULT_BLOCK_MESSAGE} by ${blockedBy}` : DEFAULT_BLOCK_MESSAGE; + if (message) { + return blockedBy + ? `${BLOCK_MESSAGE_PREFIX}: ${message} (blocked by ${blockedBy})` + : `${BLOCK_MESSAGE_PREFIX}: ${message}`; + } + return blockedBy + ? `${BLOCK_MESSAGE_PREFIX}: blocked by ${blockedBy}` + : `${BLOCK_MESSAGE_PREFIX}: blocked`; } /** Outcome severity for most-restrictive-wins merging. Higher = more restrictive. */