refactor(agents): dedupe plugin hooks and test helpers

src/agents/openclaw-gateway-tool.e2e.test.ts

@@ -16,15 +16,43 @@ vi.mock("./tools/gateway.js", () => ({
   readGatewayCallOptions: vi.fn(() => ({})),
 }));
 
+function requireGatewayTool(agentSessionKey?: string) {
+  const tool = createOpenClawTools({
+    ...(agentSessionKey ? { agentSessionKey } : {}),
+    config: { commands: { restart: true } },
+  }).find((candidate) => candidate.name === "gateway");
+  expect(tool).toBeDefined();
+  if (!tool) {
+    throw new Error("missing gateway tool");
+  }
+  return tool;
+}
+
+function expectConfigMutationCall(params: {
+  callGatewayTool: {
+    mock: {
+      calls: Array<[string, unknown, Record<string, unknown>]>;
+    };
+  };
+  action: "config.apply" | "config.patch";
+  raw: string;
+  sessionKey: string;
+}) {
+  expect(params.callGatewayTool).toHaveBeenCalledWith("config.get", expect.any(Object), {});
+  expect(params.callGatewayTool).toHaveBeenCalledWith(
+    params.action,
+    expect.any(Object),
+    expect.objectContaining({
+      raw: params.raw.trim(),
+      baseHash: "hash-1",
+      sessionKey: params.sessionKey,
+    }),
+  );
+}
+
 describe("gateway tool", () => {
   it("marks gateway as owner-only", async () => {
-    const tool = createOpenClawTools({
-      config: { commands: { restart: true } },
-    }).find((candidate) => candidate.name === "gateway");
-    expect(tool).toBeDefined();
-    if (!tool) {
-      throw new Error("missing gateway tool");
-    }
+    const tool = requireGatewayTool();
     expect(tool.ownerOnly).toBe(true);
   });
 
@@ -37,13 +65,7 @@ describe("gateway tool", () => {
       await withEnvAsync(
         { OPENCLAW_STATE_DIR: stateDir, OPENCLAW_PROFILE: "isolated" },
         async () => {
-          const tool = createOpenClawTools({
-            config: { commands: { restart: true } },
-          }).find((candidate) => candidate.name === "gateway");
-          expect(tool).toBeDefined();
-          if (!tool) {
-            throw new Error("missing gateway tool");
-          }
+          const tool = requireGatewayTool();
 
           const result = await tool.execute("call1", {
             action: "restart",
@@ -80,13 +102,8 @@ describe("gateway tool", () => {
 
   it("passes config.apply through gateway call", async () => {
     const { callGatewayTool } = await import("./tools/gateway.js");
-    const tool = createOpenClawTools({
-      agentSessionKey: "agent:main:whatsapp:dm:+15555550123",
-    }).find((candidate) => candidate.name === "gateway");
-    expect(tool).toBeDefined();
-    if (!tool) {
-      throw new Error("missing gateway tool");
-    }
+    const sessionKey = "agent:main:whatsapp:dm:+15555550123";
+    const tool = requireGatewayTool(sessionKey);
 
     const raw = '{\n  agents: { defaults: { workspace: "~/openclaw" } }\n}\n';
     await tool.execute("call2", {
@@ -94,27 +111,18 @@ describe("gateway tool", () => {
       raw,
     });
 
-    expect(callGatewayTool).toHaveBeenCalledWith("config.get", expect.any(Object), {});
-    expect(callGatewayTool).toHaveBeenCalledWith(
-      "config.apply",
-      expect.any(Object),
-      expect.objectContaining({
-        raw: raw.trim(),
-        baseHash: "hash-1",
-        sessionKey: "agent:main:whatsapp:dm:+15555550123",
-      }),
-    );
+    expectConfigMutationCall({
+      callGatewayTool: vi.mocked(callGatewayTool),
+      action: "config.apply",
+      raw,
+      sessionKey,
+    });
   });
 
   it("passes config.patch through gateway call", async () => {
     const { callGatewayTool } = await import("./tools/gateway.js");
-    const tool = createOpenClawTools({
-      agentSessionKey: "agent:main:whatsapp:dm:+15555550123",
-    }).find((candidate) => candidate.name === "gateway");
-    expect(tool).toBeDefined();
-    if (!tool) {
-      throw new Error("missing gateway tool");
-    }
+    const sessionKey = "agent:main:whatsapp:dm:+15555550123";
+    const tool = requireGatewayTool(sessionKey);
 
     const raw = '{\n  channels: { telegram: { groups: { "*": { requireMention: false } } } }\n}\n';
     await tool.execute("call4", {
@@ -122,27 +130,18 @@ describe("gateway tool", () => {
       raw,
     });
 
-    expect(callGatewayTool).toHaveBeenCalledWith("config.get", expect.any(Object), {});
-    expect(callGatewayTool).toHaveBeenCalledWith(
-      "config.patch",
-      expect.any(Object),
-      expect.objectContaining({
-        raw: raw.trim(),
-        baseHash: "hash-1",
-        sessionKey: "agent:main:whatsapp:dm:+15555550123",
-      }),
-    );
+    expectConfigMutationCall({
+      callGatewayTool: vi.mocked(callGatewayTool),
+      action: "config.patch",
+      raw,
+      sessionKey,
+    });
   });
 
   it("passes update.run through gateway call", async () => {
     const { callGatewayTool } = await import("./tools/gateway.js");
-    const tool = createOpenClawTools({
-      agentSessionKey: "agent:main:whatsapp:dm:+15555550123",
-    }).find((candidate) => candidate.name === "gateway");
-    expect(tool).toBeDefined();
-    if (!tool) {
-      throw new Error("missing gateway tool");
-    }
+    const sessionKey = "agent:main:whatsapp:dm:+15555550123";
+    const tool = requireGatewayTool(sessionKey);
 
     await tool.execute("call3", {
       action: "update.run",
@@ -154,7 +153,7 @@ describe("gateway tool", () => {
       expect.any(Object),
       expect.objectContaining({
         note: "test update",
-        sessionKey: "agent:main:whatsapp:dm:+15555550123",
+        sessionKey,
       }),
     );
     const updateCall = vi

src/agents/pi-embedded-runner/google.e2e.test.ts

@@ -3,67 +3,82 @@ import { describe, expect, it } from "vitest";
 import { sanitizeToolsForGoogle } from "./google.js";
 
 describe("sanitizeToolsForGoogle", () => {
-  it("strips unsupported schema keywords for Google providers", () => {
-    const tool = {
+  const createTool = (parameters: Record<string, unknown>) =>
+    ({
       name: "test",
       description: "test",
-      parameters: {
-        type: "object",
-        additionalProperties: false,
-        properties: {
-          foo: {
-            type: "string",
-            format: "uuid",
-          },
+      parameters,
+      execute: async () => ({ ok: true, content: [] }),
+    }) as unknown as AgentTool;
+
+  const expectFormatRemoved = (
+    sanitized: AgentTool,
+    key: "additionalProperties" | "patternProperties",
+  ) => {
+    const params = sanitized.parameters as {
+      additionalProperties?: unknown;
+      patternProperties?: unknown;
+      properties?: Record<string, { format?: unknown }>;
+    };
+    expect(params[key]).toBeUndefined();
+    expect(params.properties?.foo?.format).toBeUndefined();
+  };
+
+  it("strips unsupported schema keywords for Google providers", () => {
+    const tool = createTool({
+      type: "object",
+      additionalProperties: false,
+      properties: {
+        foo: {
+          type: "string",
+          format: "uuid",
         },
       },
-      execute: async () => ({ ok: true, content: [] }),
-    } as unknown as AgentTool;
-
+    });
     const [sanitized] = sanitizeToolsForGoogle({
       tools: [tool],
       provider: "google-gemini-cli",
     });
-
-    const params = sanitized.parameters as {
-      additionalProperties?: unknown;
-      properties?: Record<string, { format?: unknown }>;
-    };
-
-    expect(params.additionalProperties).toBeUndefined();
-    expect(params.properties?.foo?.format).toBeUndefined();
+    expectFormatRemoved(sanitized, "additionalProperties");
   });
 
   it("strips unsupported schema keywords for google-antigravity", () => {
-    const tool = {
-      name: "test",
-      description: "test",
-      parameters: {
-        type: "object",
-        patternProperties: {
-          "^x-": { type: "string" },
-        },
-        properties: {
-          foo: {
-            type: "string",
-            format: "uuid",
-          },
+    const tool = createTool({
+      type: "object",
+      patternProperties: {
+        "^x-": { type: "string" },
+      },
+      properties: {
+        foo: {
+          type: "string",
+          format: "uuid",
         },
       },
-      execute: async () => ({ ok: true, content: [] }),
-    } as unknown as AgentTool;
-
+    });
     const [sanitized] = sanitizeToolsForGoogle({
       tools: [tool],
       provider: "google-antigravity",
     });
+    expectFormatRemoved(sanitized, "patternProperties");
+  });
 
-    const params = sanitized.parameters as {
-      patternProperties?: unknown;
-      properties?: Record<string, { format?: unknown }>;
-    };
+  it("returns original tools for non-google providers", () => {
+    const tool = createTool({
+      type: "object",
+      additionalProperties: false,
+      properties: {
+        foo: {
+          type: "string",
+          format: "uuid",
+        },
+      },
+    });
+    const sanitized = sanitizeToolsForGoogle({
+      tools: [tool],
+      provider: "openai",
+    });
 
-    expect(params.patternProperties).toBeUndefined();
-    expect(params.properties?.foo?.format).toBeUndefined();
+    expect(sanitized).toEqual([tool]);
+    expect(sanitized[0]).toBe(tool);
   });
 });

src/agents/subagent-registry-completion.test.ts

@@ -26,6 +26,21 @@ function createRunEntry(): SubagentRunRecord {
 }
 
 describe("emitSubagentEndedHookOnce", () => {
+  const createEmitParams = (
+    overrides?: Partial<Parameters<typeof emitSubagentEndedHookOnce>[0]>,
+  ) => {
+    const entry = overrides?.entry ?? createRunEntry();
+    return {
+      entry,
+      reason: SUBAGENT_ENDED_REASON_COMPLETE,
+      sendFarewell: true,
+      accountId: "acct-1",
+      inFlightRunIds: new Set<string>(),
+      persist: vi.fn(),
+      ...overrides,
+    };
+  };
+
   beforeEach(() => {
     lifecycleMocks.getGlobalHookRunner.mockReset();
     lifecycleMocks.runSubagentEnded.mockClear();
@@ -37,21 +52,13 @@ describe("emitSubagentEndedHookOnce", () => {
       runSubagentEnded: lifecycleMocks.runSubagentEnded,
     });
 
-    const entry = createRunEntry();
-    const persist = vi.fn();
-    const emitted = await emitSubagentEndedHookOnce({
-      entry,
-      reason: SUBAGENT_ENDED_REASON_COMPLETE,
-      sendFarewell: true,
-      accountId: "acct-1",
-      inFlightRunIds: new Set<string>(),
-      persist,
-    });
+    const params = createEmitParams();
+    const emitted = await emitSubagentEndedHookOnce(params);
 
     expect(emitted).toBe(true);
     expect(lifecycleMocks.runSubagentEnded).not.toHaveBeenCalled();
-    expect(typeof entry.endedHookEmittedAt).toBe("number");
-    expect(persist).toHaveBeenCalledTimes(1);
+    expect(typeof params.entry.endedHookEmittedAt).toBe("number");
+    expect(params.persist).toHaveBeenCalledTimes(1);
   });
 
   it("runs subagent_ended hooks when available", async () => {
@@ -60,20 +67,60 @@ describe("emitSubagentEndedHookOnce", () => {
       runSubagentEnded: lifecycleMocks.runSubagentEnded,
     });
 
-    const entry = createRunEntry();
-    const persist = vi.fn();
-    const emitted = await emitSubagentEndedHookOnce({
-      entry,
-      reason: SUBAGENT_ENDED_REASON_COMPLETE,
-      sendFarewell: true,
-      accountId: "acct-1",
-      inFlightRunIds: new Set<string>(),
-      persist,
-    });
+    const params = createEmitParams();
+    const emitted = await emitSubagentEndedHookOnce(params);
 
     expect(emitted).toBe(true);
     expect(lifecycleMocks.runSubagentEnded).toHaveBeenCalledTimes(1);
-    expect(typeof entry.endedHookEmittedAt).toBe("number");
-    expect(persist).toHaveBeenCalledTimes(1);
+    expect(typeof params.entry.endedHookEmittedAt).toBe("number");
+    expect(params.persist).toHaveBeenCalledTimes(1);
+  });
+
+  it("returns false when runId is blank", async () => {
+    const params = createEmitParams({
+      entry: { ...createRunEntry(), runId: "   " },
+    });
+    const emitted = await emitSubagentEndedHookOnce(params);
+    expect(emitted).toBe(false);
+    expect(params.persist).not.toHaveBeenCalled();
+    expect(lifecycleMocks.runSubagentEnded).not.toHaveBeenCalled();
+  });
+
+  it("returns false when ended hook marker already exists", async () => {
+    const params = createEmitParams({
+      entry: { ...createRunEntry(), endedHookEmittedAt: Date.now() },
+    });
+    const emitted = await emitSubagentEndedHookOnce(params);
+    expect(emitted).toBe(false);
+    expect(params.persist).not.toHaveBeenCalled();
+    expect(lifecycleMocks.runSubagentEnded).not.toHaveBeenCalled();
+  });
+
+  it("returns false when runId is already in flight", async () => {
+    const entry = createRunEntry();
+    const inFlightRunIds = new Set<string>([entry.runId]);
+    const params = createEmitParams({ entry, inFlightRunIds });
+    const emitted = await emitSubagentEndedHookOnce(params);
+    expect(emitted).toBe(false);
+    expect(params.persist).not.toHaveBeenCalled();
+    expect(lifecycleMocks.runSubagentEnded).not.toHaveBeenCalled();
+  });
+
+  it("returns false when subagent hook execution throws", async () => {
+    lifecycleMocks.runSubagentEnded.mockRejectedValueOnce(new Error("boom"));
+    lifecycleMocks.getGlobalHookRunner.mockReturnValue({
+      hasHooks: () => true,
+      runSubagentEnded: lifecycleMocks.runSubagentEnded,
+    });
+
+    const entry = createRunEntry();
+    const inFlightRunIds = new Set<string>();
+    const params = createEmitParams({ entry, inFlightRunIds });
+    const emitted = await emitSubagentEndedHookOnce(params);
+
+    expect(emitted).toBe(false);
+    expect(params.persist).not.toHaveBeenCalled();
+    expect(inFlightRunIds.has(entry.runId)).toBe(false);
+    expect(entry.endedHookEmittedAt).toBeUndefined();
   });
 });

src/agents/system-prompt-report.test.ts

@@ -13,33 +13,42 @@ function makeBootstrapFile(overrides: Partial<WorkspaceBootstrapFile>): Workspac
 }
 
 describe("buildSystemPromptReport", () => {
-  it("counts injected chars when injected file paths are absolute", () => {
-    const file = makeBootstrapFile({ path: "/tmp/workspace/policies/AGENTS.md" });
-    const report = buildSystemPromptReport({
+  const makeReport = (params: {
+    file: WorkspaceBootstrapFile;
+    injectedPath: string;
+    injectedContent: string;
+    bootstrapMaxChars?: number;
+    bootstrapTotalMaxChars?: number;
+  }) =>
+    buildSystemPromptReport({
       source: "run",
       generatedAt: 0,
-      bootstrapMaxChars: 20_000,
+      bootstrapMaxChars: params.bootstrapMaxChars ?? 20_000,
+      bootstrapTotalMaxChars: params.bootstrapTotalMaxChars,
       systemPrompt: "system",
-      bootstrapFiles: [file],
-      injectedFiles: [{ path: "/tmp/workspace/policies/AGENTS.md", content: "trimmed" }],
+      bootstrapFiles: [params.file],
+      injectedFiles: [{ path: params.injectedPath, content: params.injectedContent }],
       skillsPrompt: "",
       tools: [],
     });
 
+  it("counts injected chars when injected file paths are absolute", () => {
+    const file = makeBootstrapFile({ path: "/tmp/workspace/policies/AGENTS.md" });
+    const report = makeReport({
+      file,
+      injectedPath: "/tmp/workspace/policies/AGENTS.md",
+      injectedContent: "trimmed",
+    });
+
     expect(report.injectedWorkspaceFiles[0]?.injectedChars).toBe("trimmed".length);
   });
 
   it("keeps legacy basename matching for injected files", () => {
     const file = makeBootstrapFile({ path: "/tmp/workspace/policies/AGENTS.md" });
-    const report = buildSystemPromptReport({
-      source: "run",
-      generatedAt: 0,
-      bootstrapMaxChars: 20_000,
-      systemPrompt: "system",
-      bootstrapFiles: [file],
-      injectedFiles: [{ path: "AGENTS.md", content: "trimmed" }],
-      skillsPrompt: "",
-      tools: [],
+    const report = makeReport({
+      file,
+      injectedPath: "AGENTS.md",
+      injectedContent: "trimmed",
     });
 
     expect(report.injectedWorkspaceFiles[0]?.injectedChars).toBe("trimmed".length);
@@ -50,15 +59,10 @@ describe("buildSystemPromptReport", () => {
       path: "/tmp/workspace/policies/AGENTS.md",
       content: "abcdefghijklmnopqrstuvwxyz",
     });
-    const report = buildSystemPromptReport({
-      source: "run",
-      generatedAt: 0,
-      bootstrapMaxChars: 20_000,
-      systemPrompt: "system",
-      bootstrapFiles: [file],
-      injectedFiles: [{ path: "/tmp/workspace/policies/AGENTS.md", content: "trimmed" }],
-      skillsPrompt: "",
-      tools: [],
+    const report = makeReport({
+      file,
+      injectedPath: "/tmp/workspace/policies/AGENTS.md",
+      injectedContent: "trimmed",
     });
 
     expect(report.injectedWorkspaceFiles[0]?.truncated).toBe(true);
@@ -66,19 +70,27 @@ describe("buildSystemPromptReport", () => {
 
   it("includes both bootstrap caps in the report payload", () => {
     const file = makeBootstrapFile({ path: "/tmp/workspace/policies/AGENTS.md" });
-    const report = buildSystemPromptReport({
-      source: "run",
-      generatedAt: 0,
+    const report = makeReport({
+      file,
+      injectedPath: "AGENTS.md",
+      injectedContent: "trimmed",
       bootstrapMaxChars: 11_111,
       bootstrapTotalMaxChars: 22_222,
-      systemPrompt: "system",
-      bootstrapFiles: [file],
-      injectedFiles: [{ path: "AGENTS.md", content: "trimmed" }],
-      skillsPrompt: "",
-      tools: [],
     });
 
     expect(report.bootstrapMaxChars).toBe(11_111);
     expect(report.bootstrapTotalMaxChars).toBe(22_222);
   });
+
+  it("reports injectedChars=0 when injected file does not match by path or basename", () => {
+    const file = makeBootstrapFile({ path: "/tmp/workspace/policies/AGENTS.md" });
+    const report = makeReport({
+      file,
+      injectedPath: "/tmp/workspace/policies/OTHER.md",
+      injectedContent: "trimmed",
+    });
+
+    expect(report.injectedWorkspaceFiles[0]?.injectedChars).toBe(0);
+    expect(report.injectedWorkspaceFiles[0]?.truncated).toBe(true);
+  });
 });

src/agents/workspace.bootstrap-cache.test.ts

@@ -11,6 +11,19 @@ describe("workspace bootstrap file caching", () => {
     workspaceDir = await makeTempWorkspace("openclaw-bootstrap-cache-test-");
   });
 
+  const loadAgentsFile = async (dir: string) => {
+    const result = await loadWorkspaceBootstrapFiles(dir);
+    return result.find((f) => f.name === DEFAULT_AGENTS_FILENAME);
+  };
+
+  const expectAgentsContent = (
+    agentsFile: Awaited<ReturnType<typeof loadAgentsFile>>,
+    content: string,
+  ) => {
+    expect(agentsFile?.content).toBe(content);
+    expect(agentsFile?.missing).toBe(false);
+  };
+
   it("returns cached content when mtime unchanged", async () => {
     const content1 = "# Initial content";
     await writeWorkspaceFile({
@@ -20,16 +33,12 @@ describe("workspace bootstrap file caching", () => {
     });
 
     // First load
-    const result1 = await loadWorkspaceBootstrapFiles(workspaceDir);
-    const agentsFile1 = result1.find((f) => f.name === DEFAULT_AGENTS_FILENAME);
-    expect(agentsFile1?.content).toBe(content1);
-    expect(agentsFile1?.missing).toBe(false);
+    const agentsFile1 = await loadAgentsFile(workspaceDir);
+    expectAgentsContent(agentsFile1, content1);
 
     // Second load should use cached content (same mtime)
-    const result2 = await loadWorkspaceBootstrapFiles(workspaceDir);
-    const agentsFile2 = result2.find((f) => f.name === DEFAULT_AGENTS_FILENAME);
-    expect(agentsFile2?.content).toBe(content1);
-    expect(agentsFile2?.missing).toBe(false);
+    const agentsFile2 = await loadAgentsFile(workspaceDir);
+    expectAgentsContent(agentsFile2, content1);
 
     // Verify both calls returned the same content without re-reading
     expect(agentsFile1?.content).toBe(agentsFile2?.content);
@@ -46,9 +55,8 @@ describe("workspace bootstrap file caching", () => {
     });
 
     // First load
-    const result1 = await loadWorkspaceBootstrapFiles(workspaceDir);
-    const agentsFile1 = result1.find((f) => f.name === DEFAULT_AGENTS_FILENAME);
-    expect(agentsFile1?.content).toBe(content1);
+    const agentsFile1 = await loadAgentsFile(workspaceDir);
+    expectAgentsContent(agentsFile1, content1);
 
     // Wait a bit to ensure mtime will be different
     await new Promise((resolve) => setTimeout(resolve, 10));
@@ -61,10 +69,8 @@ describe("workspace bootstrap file caching", () => {
     });
 
     // Second load should detect the change and return new content
-    const result2 = await loadWorkspaceBootstrapFiles(workspaceDir);
-    const agentsFile2 = result2.find((f) => f.name === DEFAULT_AGENTS_FILENAME);
-    expect(agentsFile2?.content).toBe(content2);
-    expect(agentsFile2?.missing).toBe(false);
+    const agentsFile2 = await loadAgentsFile(workspaceDir);
+    expectAgentsContent(agentsFile2, content2);
   });
 
   it("handles file deletion gracefully", async () => {
@@ -74,10 +80,8 @@ describe("workspace bootstrap file caching", () => {
     await writeWorkspaceFile({ dir: workspaceDir, name: DEFAULT_AGENTS_FILENAME, content });
 
     // First load
-    const result1 = await loadWorkspaceBootstrapFiles(workspaceDir);
-    const agentsFile1 = result1.find((f) => f.name === DEFAULT_AGENTS_FILENAME);
-    expect(agentsFile1?.content).toBe(content);
-    expect(agentsFile1?.missing).toBe(false);
+    const agentsFile1 = await loadAgentsFile(workspaceDir);
+    expectAgentsContent(agentsFile1, content);
 
     // Delete the file
     await fs.unlink(filePath);
@@ -101,8 +105,7 @@ describe("workspace bootstrap file caching", () => {
     // All results should be identical
     for (const result of results) {
       const agentsFile = result.find((f) => f.name === DEFAULT_AGENTS_FILENAME);
-      expect(agentsFile?.content).toBe(content);
-      expect(agentsFile?.missing).toBe(false);
+      expectAgentsContent(agentsFile, content);
     }
   });
 
@@ -127,4 +130,10 @@ describe("workspace bootstrap file caching", () => {
     expect(agentsFile1?.content).toBe(content1);
     expect(agentsFile2?.content).toBe(content2);
   });
+
+  it("returns missing=true when bootstrap file never existed", async () => {
+    const agentsFile = await loadAgentsFile(workspaceDir);
+    expect(agentsFile?.missing).toBe(true);
+    expect(agentsFile?.content).toBeUndefined();
+  });
 });