fix(webchat): fire before_message_write on chat.send user persistence [AI-assisted]

orihakimi · claude · orihakimi · commit d7390e18b1f6 · 2026-05-23T15:47:54.000+03:00
ClawSweeper rated this PR 🦞 diamond lobster on proof but 🧂 unranked
krab on patch quality, with two P1 findings at chat.ts:2684-2690:

  [P1] Keep agent user turns on the SessionManager writer
  [P1] Preserve transcript hooks for gateway user writes

The original write at chat.ts:2684-2690 called appendSessionTranscriptMessage
directly and bypassed plugin-registered before_message_write hooks. The
canonical hook-respecting pattern lives at extensions/codex/src/app-server/
transcript-mirror.ts:147 — run runAgentHarnessBeforeMessageWriteHook first,
write only the (possibly transformed) message it returns, skip the write
entirely if a plugin blocks.

Mirror that pattern for the webchat user-persistence path. Now plugin
policy (redaction, provenance stamping, block-list) applies to gateway
user writes the same way it applies to codex-side assistant writes. Also
drop the silent .catch() that swallowed real append failures, and emit
the post-hook (and post-append) message to subscribers so what they see
matches what is on disk.

Tests added in chat.user-transcript-persistence.test.ts:
  - hook returns block=true → nothing written, transcript unchanged.
  - hook rewrites content → on-disk content is the rewritten value,
    the original content does not land on disk.
  - no hooks registered → identity passthrough preserved.

Targeted vitest: 14/14 in chat.user-transcript-persistence.test.ts.
Full server-methods suite: 1638/1638 across 109 files, no regressions.
pnpm lint --quiet, pnpm tsgo, pnpm tsgo:test all clean.

Co-Authored-By: Claude Opus 4.7 &lt;noreply@anthropic.com&gt;
diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts
@@ -10,6 +10,7 @@ import {
   resolveSendableOutboundReplyParts,
 } from "openclaw/plugin-sdk/reply-payload";
 import { resolveAgentWorkspaceDir, resolveSessionAgentId } from "../../agents/agent-scope.js";
+import { runAgentHarnessBeforeMessageWriteHook } from "../../agents/harness/hook-helpers.js";
 import { resolveDefaultModelForAgent } from "../../agents/model-selection.js";
 import { rewriteTranscriptEntriesInSessionFile } from "../../agents/pi-embedded-runner/transcript-rewrite.js";
 import { resolveProviderIdForAuth } from "../../agents/provider-auth-aliases.js";
@@ -2678,25 +2679,34 @@ export const chatHandlers: GatewayRequestHandlers = {
                 savedImages: persistedImages,
                 timestamp: now,
               });
-              // Call sites that invoke emitUserTranscriptUpdate are gated on
-              // !hasBeforeAgentRunGate, so this append never collides with
-              // attempt.ts's redacted-user-message write on the hook-block path.
-              // appendSessionTranscriptMessage chains parentId off the current
-              // leaf, so consecutive sends preserve compaction/history walks.
-              await appendSessionTranscriptMessage({
+              // Fire before_message_write so plugin policy (redaction,
+              // provenance, block-list) applies to webchat user writes the
+              // same way it does for the codex transcript mirror path at
+              // extensions/codex/src/app-server/transcript-mirror.ts:147 .
+              // appendSessionTranscriptMessage chains parentId off the
+              // current leaf, so consecutive sends preserve compaction and
+              // history walks.
+              const hookedMessage = runAgentHarnessBeforeMessageWriteHook({
+                message: userMessage as AgentMessage,
+                ...(agentId ? { agentId } : {}),
+                sessionKey,
+              });
+              if (!hookedMessage) {
+                context.logGateway.info(
+                  `webchat user transcript append blocked by before_message_write hook sessionKey=${sessionKey}`,
+                );
+                return;
+              }
+              const { message: appendedMessage } = await appendSessionTranscriptMessage({
                 transcriptPath,
-                message: userMessage,
+                message: hookedMessage,
                 sessionId: resolvedSessionId,
                 config: cfg,
-              }).catch((err) => {
-                context.logGateway.warn(
-                  `webchat user transcript append failed: ${formatForLog(err)}`,
-                );
               });
               emitSessionTranscriptUpdate({
                 sessionFile: transcriptPath,
                 sessionKey,
-                message: userMessage,
+                message: appendedMessage,
               });
             },
             {
diff --git a/src/gateway/server-methods/chat.user-transcript-persistence.test.ts b/src/gateway/server-methods/chat.user-transcript-persistence.test.ts
@@ -1,6 +1,13 @@
 import fs from "node:fs";
-import { describe, expect, it } from "vitest";
+import type { AgentMessage } from "@earendil-works/pi-agent-core";
+import { afterEach, describe, expect, it } from "vitest";
+import { runAgentHarnessBeforeMessageWriteHook } from "../../agents/harness/hook-helpers.js";
 import { appendSessionTranscriptMessage } from "../../config/sessions/transcript-append.js";
+import {
+  initializeGlobalHookRunner,
+  resetGlobalHookRunner,
+} from "../../plugins/hook-runner-global.js";
+import { createMockPluginRegistry } from "../../plugins/hooks.test-helpers.js";
 import { createTranscriptFixtureSync } from "./chat.test-helpers.js";
 
 function readTranscriptLines(transcriptPath: string): Array<Record<string, unknown>> {
@@ -126,7 +133,7 @@ describe("gateway chat.send webchat user-transcript persistence", () => {
       });
       expect(userEntries).toHaveLength(2);
       expect((userEntries[0] as { id?: unknown }).id).toBe(first.messageId);
-      expect((userEntries[1] as { id?: unknown }).parentId).toBe(first.messageId);
+      expect((userEntries[1] as { parentId?: unknown }).parentId).toBe(first.messageId);
     } finally {
       fs.rmSync(dir, { recursive: true, force: true });
     }
@@ -148,9 +155,7 @@ describe("gateway chat.send webchat user-transcript persistence", () => {
       const entries = readTranscriptLines(transcriptPath);
       // Walk leaves forward to build the conversation order, the same shape
       // chat.history reconstructs after reconnect.
-      const messageEntries = entries.filter((entry) => entry.type === "message") as Array<
-        Record<string, unknown>
-      >;
+      const messageEntries = entries.filter((entry) => entry.type === "message");
       const byId = new Map<string, Record<string, unknown>>();
       for (const entry of messageEntries) {
         const id = entry.id as string;
@@ -167,3 +172,137 @@ describe("gateway chat.send webchat user-transcript persistence", () => {
     }
   });
 });
+
+// The chat.send webchat user-write path must fire `before_message_write` so
+// plugin-registered policy (redaction, provenance, block-list) applies to
+// gateway user writes the same way it applies to the codex transcript mirror
+// at extensions/codex/src/app-server/transcript-mirror.ts:147. These tests
+// pin the composition: hook → append → emit, mirroring exactly what
+// chat.ts does at the webchat persistence site.
+describe("gateway chat.send webchat user-transcript before_message_write hook", () => {
+  afterEach(() => {
+    resetGlobalHookRunner();
+  });
+
+  it("does not write to disk when the hook blocks the user message", async () => {
+    initializeGlobalHookRunner(
+      createMockPluginRegistry([
+        {
+          hookName: "before_message_write",
+          handler: () => ({ block: true }),
+        },
+      ]),
+    );
+
+    const { dir, transcriptPath } = createTranscriptFixtureSync({
+      prefix: "openclaw-webchat-user-hook-block-",
+      sessionId: "sess-hook-block",
+    });
+
+    try {
+      const initialLineCount = readTranscriptLines(transcriptPath).length;
+      const userMessage = buildUserMessage("blocked content", 1_700_000_100_000);
+
+      // Mirror exactly what chat.ts does at the webchat persistence site.
+      const hookedMessage = runAgentHarnessBeforeMessageWriteHook({
+        message: userMessage as AgentMessage,
+        agentId: "test-agent",
+        sessionKey: "sess-hook-block",
+      });
+
+      expect(hookedMessage).toBeNull();
+      // If the hook blocks, chat.ts returns without appending — no disk write,
+      // no emit. Assert that the transcript file is unchanged.
+      expect(readTranscriptLines(transcriptPath).length).toBe(initialLineCount);
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it("writes the hook-transformed message rather than the original", async () => {
+    initializeGlobalHookRunner(
+      createMockPluginRegistry([
+        {
+          hookName: "before_message_write",
+          handler: (event) => {
+            const original = (event as { message: Record<string, unknown> }).message;
+            return {
+              message: {
+                ...original,
+                content: "<redacted by plugin policy>",
+              } as AgentMessage,
+            };
+          },
+        },
+      ]),
+    );
+
+    const { dir, transcriptPath } = createTranscriptFixtureSync({
+      prefix: "openclaw-webchat-user-hook-transform-",
+      sessionId: "sess-hook-transform",
+    });
+
+    try {
+      const userMessage = buildUserMessage(
+        "secret-token-abc123 should not land on disk",
+        1_700_000_101_000,
+      );
+
+      const hookedMessage = runAgentHarnessBeforeMessageWriteHook({
+        message: userMessage as AgentMessage,
+        agentId: "test-agent",
+        sessionKey: "sess-hook-transform",
+      });
+
+      expect(hookedMessage).not.toBeNull();
+      const result = await appendSessionTranscriptMessage({
+        transcriptPath,
+        message: hookedMessage,
+        sessionId: "sess-hook-transform",
+      });
+
+      const entries = readTranscriptLines(transcriptPath);
+      const last = entries.at(-1) as Record<string, unknown>;
+      const lastMessage = last.message as Record<string, unknown>;
+      expect(lastMessage.content).toBe("<redacted by plugin policy>");
+      expect(JSON.stringify(entries)).not.toContain("secret-token-abc123");
+      expect(last).toHaveProperty("id", result.messageId);
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it("passes the original message through unchanged when no hooks are registered", async () => {
+    // No initializeGlobalHookRunner — the helper short-circuits to identity.
+    const { dir, transcriptPath } = createTranscriptFixtureSync({
+      prefix: "openclaw-webchat-user-hook-none-",
+      sessionId: "sess-hook-none",
+    });
+
+    try {
+      const userMessage = buildUserMessage("plain user content", 1_700_000_102_000);
+
+      const hookedMessage = runAgentHarnessBeforeMessageWriteHook({
+        message: userMessage as AgentMessage,
+        agentId: "test-agent",
+        sessionKey: "sess-hook-none",
+      });
+
+      // With no global hook runner, the helper returns the original message.
+      expect(hookedMessage).toBe(userMessage);
+
+      await appendSessionTranscriptMessage({
+        transcriptPath,
+        message: hookedMessage,
+        sessionId: "sess-hook-none",
+      });
+
+      const entries = readTranscriptLines(transcriptPath);
+      const last = entries.at(-1) as Record<string, unknown>;
+      const lastMessage = last.message as Record<string, unknown>;
+      expect(lastMessage.content).toBe("plain user content");
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+});
