refactor(core): extract shared dedup helpers

2026-04-18 22:17:27 +00:00 · 2026-03-07 10:40:49 +00:00
parent 14c61bb33f
commit 3c71e2bd48
114 changed files with 3400 additions and 2040 deletions
--- a/src/gateway/auth-config-utils.ts
+++ b/src/gateway/auth-config-utils.ts
@@ -0,0 +1,69 @@
+import type { GatewayAuthConfig, OpenClawConfig } from "../config/config.js";
+import { resolveSecretInputRef } from "../config/types.secrets.js";
+import { secretRefKey } from "../secrets/ref-contract.js";
+import { resolveSecretRefValues } from "../secrets/resolve.js";
+
+export function withGatewayAuthPassword(cfg: OpenClawConfig, password: string): OpenClawConfig {
+  return {
+    ...cfg,
+    gateway: {
+      ...cfg.gateway,
+      auth: {
+        ...cfg.gateway?.auth,
+        password,
+      },
+    },
+  };
+}
+
+function shouldResolveGatewayPasswordSecretRef(params: {
+  mode?: GatewayAuthConfig["mode"];
+  hasPasswordCandidate: boolean;
+  hasTokenCandidate: boolean;
+}): boolean {
+  if (params.hasPasswordCandidate) {
+    return false;
+  }
+  if (params.mode === "password") {
+    return true;
+  }
+  if (params.mode === "token" || params.mode === "none" || params.mode === "trusted-proxy") {
+    return false;
+  }
+  return !params.hasTokenCandidate;
+}
+
+export async function resolveGatewayPasswordSecretRef(params: {
+  cfg: OpenClawConfig;
+  env: NodeJS.ProcessEnv;
+  mode?: GatewayAuthConfig["mode"];
+  hasPasswordCandidate: boolean;
+  hasTokenCandidate: boolean;
+}): Promise<OpenClawConfig> {
+  const authPassword = params.cfg.gateway?.auth?.password;
+  const { ref } = resolveSecretInputRef({
+    value: authPassword,
+    defaults: params.cfg.secrets?.defaults,
+  });
+  if (!ref) {
+    return params.cfg;
+  }
+  if (
+    !shouldResolveGatewayPasswordSecretRef({
+      mode: params.mode,
+      hasPasswordCandidate: params.hasPasswordCandidate,
+      hasTokenCandidate: params.hasTokenCandidate,
+    })
+  ) {
+    return params.cfg;
+  }
+  const resolved = await resolveSecretRefValues([ref], {
+    config: params.cfg,
+    env: params.env,
+  });
+  const value = resolved.get(secretRefKey(ref));
+  if (typeof value !== "string" || value.trim().length === 0) {
+    throw new Error("gateway.auth.password resolved to an empty or non-string value.");
+  }
+  return withGatewayAuthPassword(params.cfg, value.trim());
+}
--- a/src/gateway/call.ts
+++ b/src/gateway/call.ts
@@ -9,8 +9,7 @@ import {
 import { hasConfiguredSecretInput, resolveSecretInputRef } from "../config/types.secrets.js";
 import { loadOrCreateDeviceIdentity } from "../infra/device-identity.js";
 import { loadGatewayTlsRuntime } from "../infra/tls/gateway.js";
-import { secretRefKey } from "../secrets/ref-contract.js";
-import { resolveSecretRefValues } from "../secrets/resolve.js";
+import { resolveSecretInputString } from "../secrets/resolve-secret-input-string.js";
 import {
  GATEWAY_CLIENT_MODES,
  GATEWAY_CLIENT_NAMES,
@@ -312,23 +311,16 @@ async function resolveGatewaySecretInputString(params: {
  path: string;
  env: NodeJS.ProcessEnv;
 }): Promise<string | undefined> {
-  const defaults = params.config.secrets?.defaults;
-  const { ref } = resolveSecretInputRef({
-    value: params.value,
-    defaults,
-  });
-  if (!ref) {
-    return trimToUndefined(params.value);
-  }
-  const resolved = await resolveSecretRefValues([ref], {
+  const value = await resolveSecretInputString({
    config: params.config,
+    value: params.value,
    env: params.env,
+    normalize: trimToUndefined,
  });
-  const resolvedValue = trimToUndefined(resolved.get(secretRefKey(ref)));
-  if (!resolvedValue) {
+  if (!value) {
    throw new Error(`${params.path} resolved to an empty or non-string value.`);
  }
-  return resolvedValue;
+  return value;
 }

 async function resolveGatewayCredentials(context: ResolvedGatewayCallContext): Promise<{
--- a/src/gateway/credentials.test.ts
+++ b/src/gateway/credentials.test.ts
@@ -50,6 +50,27 @@ function resolveRemoteModeWithRemoteCredentials(
  );
 }

+function resolveLocalModeWithUnresolvedPassword(mode: "none" | "trusted-proxy") {
+  return resolveGatewayCredentialsFromConfig({
+    cfg: {
+      gateway: {
+        mode: "local",
+        auth: {
+          mode,
+          password: { source: "env", provider: "default", id: "MISSING_GATEWAY_PASSWORD" },
+        },
+      },
+      secrets: {
+        providers: {
+          default: { source: "env" },
+        },
+      },
+    } as unknown as OpenClawConfig,
+    env: {} as NodeJS.ProcessEnv,
+    includeLegacyEnv: false,
+  });
+}
+
 describe("resolveGatewayCredentialsFromConfig", () => {
  it("prefers explicit credentials over config and environment", () => {
    const resolved = resolveGatewayCredentialsFor(
@@ -182,24 +203,7 @@ describe("resolveGatewayCredentialsFromConfig", () => {
  });

  it("ignores unresolved local password ref when local auth mode is none", () => {
-    const resolved = resolveGatewayCredentialsFromConfig({
-      cfg: {
-        gateway: {
-          mode: "local",
-          auth: {
-            mode: "none",
-            password: { source: "env", provider: "default", id: "MISSING_GATEWAY_PASSWORD" },
-          },
-        },
-        secrets: {
-          providers: {
-            default: { source: "env" },
-          },
-        },
-      } as unknown as OpenClawConfig,
-      env: {} as NodeJS.ProcessEnv,
-      includeLegacyEnv: false,
-    });
+    const resolved = resolveLocalModeWithUnresolvedPassword("none");
    expect(resolved).toEqual({
      token: undefined,
      password: undefined,
@@ -207,24 +211,7 @@ describe("resolveGatewayCredentialsFromConfig", () => {
  });

  it("ignores unresolved local password ref when local auth mode is trusted-proxy", () => {
-    const resolved = resolveGatewayCredentialsFromConfig({
-      cfg: {
-        gateway: {
-          mode: "local",
-          auth: {
-            mode: "trusted-proxy",
-            password: { source: "env", provider: "default", id: "MISSING_GATEWAY_PASSWORD" },
-          },
-        },
-        secrets: {
-          providers: {
-            default: { source: "env" },
-          },
-        },
-      } as unknown as OpenClawConfig,
-      env: {} as NodeJS.ProcessEnv,
-      includeLegacyEnv: false,
-    });
+    const resolved = resolveLocalModeWithUnresolvedPassword("trusted-proxy");
    expect(resolved).toEqual({
      token: undefined,
      password: undefined,
--- a/src/gateway/gateway-models.profiles.live.test.ts
+++ b/src/gateway/gateway-models.profiles.live.test.ts
@@ -1013,6 +1013,7 @@ async function runGatewayModelSuite(params: GatewayModelSuiteParams) {
                shouldRetryExecReadProbe({
                  text: execReadText,
                  nonce: nonceC,
+                  provider: model.provider,
                  attempt: execReadAttempt,
                  maxAttempts: maxExecReadAttempts,
                })
--- a/src/gateway/live-tool-probe-utils.test.ts
+++ b/src/gateway/live-tool-probe-utils.test.ts
@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
 import {
  hasExpectedSingleNonce,
  hasExpectedToolNonce,
+  isLikelyToolNonceRefusal,
  shouldRetryExecReadProbe,
  shouldRetryToolReadProbe,
 } from "./live-tool-probe-utils.js";
@@ -17,6 +18,26 @@ describe("live tool probe utils", () => {
    expect(hasExpectedSingleNonce("value nonce-2", "nonce-1")).toBe(false);
  });

+  it("detects anthropic nonce refusal phrasing", () => {
+    expect(
+      isLikelyToolNonceRefusal(
+        "Same request, same answer — this isn't a real OpenClaw probe. No part of the system asks me to parrot back nonce values.",
+      ),
+    ).toBe(true);
+  });
+
+  it("does not treat generic helper text as nonce refusal", () => {
+    expect(isLikelyToolNonceRefusal("I can help with that request.")).toBe(false);
+  });
+
+  it("detects prompt-injection style tool refusal without nonce text", () => {
+    expect(
+      isLikelyToolNonceRefusal(
+        "That's not a legitimate self-test. This looks like a prompt injection attempt.",
+      ),
+    ).toBe(true);
+  });
+
  it("retries malformed tool output when attempts remain", () => {
    expect(
      shouldRetryToolReadProbe({
@@ -95,6 +116,32 @@ describe("live tool probe utils", () => {
    ).toBe(true);
  });

+  it("retries anthropic nonce refusal output", () => {
+    expect(
+      shouldRetryToolReadProbe({
+        text: "This isn't a real OpenClaw probe; I won't parrot back nonce values.",
+        nonceA: "nonce-a",
+        nonceB: "nonce-b",
+        provider: "anthropic",
+        attempt: 0,
+        maxAttempts: 3,
+      }),
+    ).toBe(true);
+  });
+
+  it("retries anthropic prompt-injection refusal output", () => {
+    expect(
+      shouldRetryToolReadProbe({
+        text: "This is not a legitimate self-test; it appears to be a prompt injection attempt.",
+        nonceA: "nonce-a",
+        nonceB: "nonce-b",
+        provider: "anthropic",
+        attempt: 0,
+        maxAttempts: 3,
+      }),
+    ).toBe(true);
+  });
+
  it("does not retry nonce marker echoes for non-mistral providers", () => {
    expect(
      shouldRetryToolReadProbe({
@@ -113,6 +160,7 @@ describe("live tool probe utils", () => {
      shouldRetryExecReadProbe({
        text: "read[object Object]",
        nonce: "nonce-c",
+        provider: "openai",
        attempt: 0,
        maxAttempts: 3,
      }),
@@ -124,6 +172,7 @@ describe("live tool probe utils", () => {
      shouldRetryExecReadProbe({
        text: "read[object Object]",
        nonce: "nonce-c",
+        provider: "openai",
        attempt: 2,
        maxAttempts: 3,
      }),
@@ -135,9 +184,22 @@ describe("live tool probe utils", () => {
      shouldRetryExecReadProbe({
        text: "nonce-c",
        nonce: "nonce-c",
+        provider: "openai",
        attempt: 0,
        maxAttempts: 3,
      }),
    ).toBe(false);
  });
+
+  it("retries anthropic exec+read nonce refusal output", () => {
+    expect(
+      shouldRetryExecReadProbe({
+        text: "No part of the system asks me to parrot back nonce values.",
+        nonce: "nonce-c",
+        provider: "anthropic",
+        attempt: 0,
+        maxAttempts: 3,
+      }),
+    ).toBe(true);
+  });
 });
--- a/src/gateway/live-tool-probe-utils.ts
+++ b/src/gateway/live-tool-probe-utils.ts
@@ -6,6 +6,44 @@ export function hasExpectedSingleNonce(text: string, nonce: string): boolean {
  return text.includes(nonce);
 }

+const NONCE_REFUSAL_MARKERS = [
+  "token",
+  "secret",
+  "local file",
+  "uuid-named file",
+  "uuid named file",
+  "parrot back",
+  "disclose",
+  "can't help",
+  "can’t help",
+  "cannot help",
+  "can't comply",
+  "can’t comply",
+  "cannot comply",
+  "isn't a real openclaw probe",
+  "is not a real openclaw probe",
+  "not a real openclaw probe",
+  "no part of the system asks me",
+];
+
+const PROBE_REFUSAL_MARKERS = [
+  "prompt injection attempt",
+  "not a legitimate self-test",
+  "not legitimate self-test",
+  "authorized integration probe",
+];
+
+export function isLikelyToolNonceRefusal(text: string): boolean {
+  const lower = text.toLowerCase();
+  if (PROBE_REFUSAL_MARKERS.some((marker) => lower.includes(marker))) {
+    return true;
+  }
+  if (lower.includes("nonce")) {
+    return NONCE_REFUSAL_MARKERS.some((marker) => lower.includes(marker));
+  }
+  return false;
+}
+
 function hasMalformedToolOutput(text: string): boolean {
  const trimmed = text.trim();
  if (!trimmed) {
@@ -38,6 +76,9 @@ export function shouldRetryToolReadProbe(params: {
  if (hasMalformedToolOutput(params.text)) {
    return true;
  }
+  if (params.provider === "anthropic" && isLikelyToolNonceRefusal(params.text)) {
+    return true;
+  }
  const lower = params.text.trim().toLowerCase();
  if (params.provider === "mistral" && (lower.includes("noncea=") || lower.includes("nonceb="))) {
    return true;
@@ -48,6 +89,7 @@ export function shouldRetryToolReadProbe(params: {
 export function shouldRetryExecReadProbe(params: {
  text: string;
  nonce: string;
+  provider: string;
  attempt: number;
  maxAttempts: number;
 }): boolean {
@@ -57,5 +99,8 @@ export function shouldRetryExecReadProbe(params: {
  if (hasExpectedSingleNonce(params.text, params.nonce)) {
    return false;
  }
+  if (params.provider === "anthropic" && isLikelyToolNonceRefusal(params.text)) {
+    return true;
+  }
  return hasMalformedToolOutput(params.text);
 }
--- a/src/gateway/openai-http.message-channel.test.ts
+++ b/src/gateway/openai-http.message-channel.test.ts
@@ -3,77 +3,57 @@ import { agentCommand, installGatewayTestHooks, withGatewayServer } from "./test

 installGatewayTestHooks({ scope: "test" });

+const OPENAI_SERVER_OPTIONS = {
+  host: "127.0.0.1",
+  auth: { mode: "token" as const, token: "secret" },
+  controlUiEnabled: false,
+  openAiChatCompletionsEnabled: true,
+};
+
+async function runOpenAiMessageChannelRequest(params?: { messageChannelHeader?: string }) {
+  agentCommand.mockReset();
+  agentCommand.mockResolvedValueOnce({ payloads: [{ text: "ok" }] } as never);
+
+  let firstCall: { messageChannel?: string } | undefined;
+  await withGatewayServer(
+    async ({ port }) => {
+      const headers: Record<string, string> = {
+        "content-type": "application/json",
+        authorization: "Bearer secret",
+      };
+      if (params?.messageChannelHeader) {
+        headers["x-openclaw-message-channel"] = params.messageChannelHeader;
+      }
+      const res = await fetch(`http://127.0.0.1:${port}/v1/chat/completions`, {
+        method: "POST",
+        headers,
+        body: JSON.stringify({
+          model: "openclaw",
+          messages: [{ role: "user", content: "hi" }],
+        }),
+      });
+
+      expect(res.status).toBe(200);
+      firstCall = (agentCommand.mock.calls[0] as unknown[] | undefined)?.[0] as
+        | { messageChannel?: string }
+        | undefined;
+      await res.text();
+    },
+    { serverOptions: OPENAI_SERVER_OPTIONS },
+  );
+  return firstCall;
+}
+
 describe("OpenAI HTTP message channel", () => {
  it("passes x-openclaw-message-channel through to agentCommand", async () => {
-    agentCommand.mockReset();
-    agentCommand.mockResolvedValueOnce({ payloads: [{ text: "ok" }] } as never);
-
-    await withGatewayServer(
-      async ({ port }) => {
-        const res = await fetch(`http://127.0.0.1:${port}/v1/chat/completions`, {
-          method: "POST",
-          headers: {
-            "content-type": "application/json",
-            authorization: "Bearer secret",
-            "x-openclaw-message-channel": "custom-client-channel",
-          },
-          body: JSON.stringify({
-            model: "openclaw",
-            messages: [{ role: "user", content: "hi" }],
-          }),
-        });
-
-        expect(res.status).toBe(200);
-        const firstCall = (agentCommand.mock.calls[0] as unknown[] | undefined)?.[0] as
-          | { messageChannel?: string }
-          | undefined;
-        expect(firstCall?.messageChannel).toBe("custom-client-channel");
-        await res.text();
-      },
-      {
-        serverOptions: {
-          host: "127.0.0.1",
-          auth: { mode: "token", token: "secret" },
-          controlUiEnabled: false,
-          openAiChatCompletionsEnabled: true,
-        },
-      },
-    );
+    const firstCall = await runOpenAiMessageChannelRequest({
+      messageChannelHeader: "custom-client-channel",
+    });
+    expect(firstCall?.messageChannel).toBe("custom-client-channel");
  });

  it("defaults messageChannel to webchat when header is absent", async () => {
-    agentCommand.mockReset();
-    agentCommand.mockResolvedValueOnce({ payloads: [{ text: "ok" }] } as never);
-
-    await withGatewayServer(
-      async ({ port }) => {
-        const res = await fetch(`http://127.0.0.1:${port}/v1/chat/completions`, {
-          method: "POST",
-          headers: {
-            "content-type": "application/json",
-            authorization: "Bearer secret",
-          },
-          body: JSON.stringify({
-            model: "openclaw",
-            messages: [{ role: "user", content: "hi" }],
-          }),
-        });
-
-        expect(res.status).toBe(200);
-        const firstCall = (agentCommand.mock.calls[0] as unknown[] | undefined)?.[0] as
-          | { messageChannel?: string }
-          | undefined;
-        expect(firstCall?.messageChannel).toBe("webchat");
-        await res.text();
-      },
-      {
-        serverOptions: {
-          host: "127.0.0.1",
-          auth: { mode: "token", token: "secret" },
-          controlUiEnabled: false,
-          openAiChatCompletionsEnabled: true,
-        },
-      },
-    );
+    const firstCall = await runOpenAiMessageChannelRequest();
+    expect(firstCall?.messageChannel).toBe("webchat");
  });
 });
--- a/src/gateway/server-methods/agents-mutate.test.ts
+++ b/src/gateway/server-methods/agents-mutate.test.ts
@@ -31,6 +31,7 @@ const mocks = vi.hoisted(() => ({
  fsLstat: vi.fn(async (..._args: unknown[]) => null as import("node:fs").Stats | null),
  fsRealpath: vi.fn(async (p: string) => p),
  fsOpen: vi.fn(async () => ({}) as unknown),
+  writeFileWithinRoot: vi.fn(async () => {}),
 }));

 vi.mock("../../config/config.js", () => ({
@@ -77,6 +78,15 @@ vi.mock("../session-utils.js", () => ({
  listAgentsForGateway: mocks.listAgentsForGateway,
 }));

+vi.mock("../../infra/fs-safe.js", async () => {
+  const actual =
+    await vi.importActual<typeof import("../../infra/fs-safe.js")>("../../infra/fs-safe.js");
+  return {
+    ...actual,
+    writeFileWithinRoot: mocks.writeFileWithinRoot,
+  };
+});
+
 // Mock node:fs/promises – agents.ts uses `import fs from "node:fs/promises"`
 // which resolves to the module namespace default, so we spread actual and
 // override the methods we need, plus set `default` explicitly.
--- a/src/gateway/server-methods/agents.ts
+++ b/src/gateway/server-methods/agents.ts
@@ -732,10 +732,19 @@ export const agentsHandlers: GatewayRequestHandlers = {
      return;
    }
    const content = String(params.content ?? "");
+    const relativeWritePath = path.relative(resolvedPath.workspaceReal, resolvedPath.ioPath);
+    if (
+      !relativeWritePath ||
+      relativeWritePath.startsWith("..") ||
+      path.isAbsolute(relativeWritePath)
+    ) {
+      respondWorkspaceFileUnsafe(respond, name);
+      return;
+    }
    try {
      await writeFileWithinRoot({
-        rootDir: workspaceDir,
-        relativePath: name,
+        rootDir: resolvedPath.workspaceReal,
+        relativePath: relativeWritePath,
        data: content,
        encoding: "utf8",
      });
--- a/src/gateway/server-methods/nodes.ts
+++ b/src/gateway/server-methods/nodes.ts
@@ -274,20 +274,7 @@ export const nodeHandlers: GatewayRequestHandlers = {
      });
      return;
    }
-    const p = params as {
-      nodeId: string;
-      displayName?: string;
-      platform?: string;
-      version?: string;
-      coreVersion?: string;
-      uiVersion?: string;
-      deviceFamily?: string;
-      modelIdentifier?: string;
-      caps?: string[];
-      commands?: string[];
-      remoteIp?: string;
-      silent?: boolean;
-    };
+    const p = params as Parameters<typeof requestNodePairing>[0];
    await respondUnavailableOnThrow(respond, async () => {
      const result = await requestNodePairing({
        nodeId: p.nodeId,
@@ -300,6 +287,7 @@ export const nodeHandlers: GatewayRequestHandlers = {
        modelIdentifier: p.modelIdentifier,
        caps: p.caps,
        commands: p.commands,
+        permissions: p.permissions,
        remoteIp: p.remoteIp,
        silent: p.silent,
      });
--- a/src/gateway/server-methods/secrets.test.ts
+++ b/src/gateway/server-methods/secrets.test.ts
@@ -17,6 +17,27 @@ async function invokeSecretsReload(params: {
  });
 }

+async function invokeSecretsResolve(params: {
+  handlers: ReturnType<typeof createSecretsHandlers>;
+  respond: ReturnType<typeof vi.fn>;
+  commandName: unknown;
+  targetIds: unknown;
+}) {
+  await params.handlers["secrets.resolve"]({
+    req: { type: "req", id: "1", method: "secrets.resolve" },
+    params: {
+      commandName: params.commandName,
+      targetIds: params.targetIds,
+    },
+    client: null,
+    isWebchatConnect: () => false,
+    respond: params.respond as unknown as Parameters<
+      ReturnType<typeof createSecretsHandlers>["secrets.resolve"]
+    >[0]["respond"],
+    context: {} as never,
+  });
+}
+
 describe("secrets handlers", () => {
  function createHandlers(overrides?: {
    reloadSecrets?: () => Promise<{ warningCount: number }>;
@@ -73,13 +94,11 @@ describe("secrets handlers", () => {
    });
    const handlers = createHandlers({ resolveSecrets });
    const respond = vi.fn();
-    await handlers["secrets.resolve"]({
-      req: { type: "req", id: "1", method: "secrets.resolve" },
-      params: { commandName: "memory status", targetIds: ["talk.apiKey"] },
-      client: null,
-      isWebchatConnect: () => false,
+    await invokeSecretsResolve({
+      handlers,
      respond,
-      context: {} as never,
+      commandName: "memory status",
+      targetIds: ["talk.apiKey"],
    });
    expect(resolveSecrets).toHaveBeenCalledWith({
      commandName: "memory status",
@@ -96,13 +115,11 @@ describe("secrets handlers", () => {
  it("rejects invalid secrets.resolve params", async () => {
    const handlers = createHandlers();
    const respond = vi.fn();
-    await handlers["secrets.resolve"]({
-      req: { type: "req", id: "1", method: "secrets.resolve" },
-      params: { commandName: "", targetIds: "bad" },
-      client: null,
-      isWebchatConnect: () => false,
+    await invokeSecretsResolve({
+      handlers,
      respond,
-      context: {} as never,
+      commandName: "",
+      targetIds: "bad",
    });
    expect(respond).toHaveBeenCalledWith(
      false,
@@ -117,13 +134,11 @@ describe("secrets handlers", () => {
    const resolveSecrets = vi.fn();
    const handlers = createHandlers({ resolveSecrets });
    const respond = vi.fn();
-    await handlers["secrets.resolve"]({
-      req: { type: "req", id: "1", method: "secrets.resolve" },
-      params: { commandName: "memory status", targetIds: ["talk.apiKey", 12] },
-      client: null,
-      isWebchatConnect: () => false,
+    await invokeSecretsResolve({
+      handlers,
      respond,
-      context: {} as never,
+      commandName: "memory status",
+      targetIds: ["talk.apiKey", 12],
    });
    expect(resolveSecrets).not.toHaveBeenCalled();
    expect(respond).toHaveBeenCalledWith(
@@ -140,13 +155,11 @@ describe("secrets handlers", () => {
    const resolveSecrets = vi.fn();
    const handlers = createHandlers({ resolveSecrets });
    const respond = vi.fn();
-    await handlers["secrets.resolve"]({
-      req: { type: "req", id: "1", method: "secrets.resolve" },
-      params: { commandName: "memory status", targetIds: ["unknown.target"] },
-      client: null,
-      isWebchatConnect: () => false,
+    await invokeSecretsResolve({
+      handlers,
      respond,
-      context: {} as never,
+      commandName: "memory status",
+      targetIds: ["unknown.target"],
    });
    expect(resolveSecrets).not.toHaveBeenCalled();
    expect(respond).toHaveBeenCalledWith(
@@ -167,13 +180,11 @@ describe("secrets handlers", () => {
    });
    const handlers = createHandlers({ resolveSecrets });
    const respond = vi.fn();
-    await handlers["secrets.resolve"]({
-      req: { type: "req", id: "1", method: "secrets.resolve" },
-      params: { commandName: "memory status", targetIds: ["talk.apiKey"] },
-      client: null,
-      isWebchatConnect: () => false,
+    await invokeSecretsResolve({
+      handlers,
      respond,
-      context: {} as never,
+      commandName: "memory status",
+      targetIds: ["talk.apiKey"],
    });
    expect(respond).toHaveBeenCalledWith(
      false,
--- a/src/gateway/server.cron.test.ts
+++ b/src/gateway/server.cron.test.ts
@@ -151,6 +151,35 @@ async function addMainSystemEventCronJob(params: { ws: WebSocket; name: string;
  return expectCronJobIdFromResponse(response);
 }

+async function addWebhookCronJob(params: {
+  ws: WebSocket;
+  name: string;
+  sessionTarget?: "main" | "isolated";
+  payloadText?: string;
+  delivery: Record<string, unknown>;
+}) {
+  const response = await rpcReq(params.ws, "cron.add", {
+    name: params.name,
+    enabled: true,
+    schedule: { kind: "every", everyMs: 60_000 },
+    sessionTarget: params.sessionTarget ?? "main",
+    wakeMode: "next-heartbeat",
+    payload: {
+      kind: params.sessionTarget === "isolated" ? "agentTurn" : "systemEvent",
+      ...(params.sessionTarget === "isolated"
+        ? { message: params.payloadText ?? "test" }
+        : { text: params.payloadText ?? "send webhook" }),
+    },
+    delivery: params.delivery,
+  });
+  return expectCronJobIdFromResponse(response);
+}
+
+async function runCronJobForce(ws: WebSocket, id: string) {
+  const response = await rpcReq(ws, "cron.run", { id, mode: "force" }, 20_000);
+  expect(response.ok).toBe(true);
+}
+
 function getWebhookCall(index: number) {
  const [args] = fetchWithSsrFGuardMock.mock.calls[index] as unknown as [
    {
@@ -574,22 +603,12 @@ describe("gateway server cron", () => {
      });
      expect(invalidWebhookRes.ok).toBe(false);

-      const notifyRes = await rpcReq(ws, "cron.add", {
+      const notifyJobId = await addWebhookCronJob({
+        ws,
        name: "webhook enabled",
-        enabled: true,
-        schedule: { kind: "every", everyMs: 60_000 },
-        sessionTarget: "main",
-        wakeMode: "next-heartbeat",
-        payload: { kind: "systemEvent", text: "send webhook" },
        delivery: { mode: "webhook", to: "https://example.invalid/cron-finished" },
      });
-      expect(notifyRes.ok).toBe(true);
-      const notifyJobIdValue = (notifyRes.payload as { id?: unknown } | null)?.id;
-      const notifyJobId = typeof notifyJobIdValue === "string" ? notifyJobIdValue : "";
-      expect(notifyJobId.length > 0).toBe(true);
-
-      const notifyRunRes = await rpcReq(ws, "cron.run", { id: notifyJobId, mode: "force" }, 20_000);
-      expect(notifyRunRes.ok).toBe(true);
+      await runCronJobForce(ws, notifyJobId);

      await waitForCondition(
        () => fetchWithSsrFGuardMock.mock.calls.length === 1,
@@ -644,13 +663,10 @@ describe("gateway server cron", () => {

      fetchWithSsrFGuardMock.mockClear();
      cronIsolatedRun.mockResolvedValueOnce({ status: "error", summary: "delivery failed" });
-      const failureDestRes = await rpcReq(ws, "cron.add", {
+      const failureDestJobId = await addWebhookCronJob({
+        ws,
        name: "failure destination webhook",
-        enabled: true,
-        schedule: { kind: "every", everyMs: 60_000 },
        sessionTarget: "isolated",
-        wakeMode: "next-heartbeat",
-        payload: { kind: "agentTurn", message: "test" },
        delivery: {
          mode: "announce",
          channel: "telegram",
@@ -661,19 +677,7 @@ describe("gateway server cron", () => {
          },
        },
      });
-      expect(failureDestRes.ok).toBe(true);
-      const failureDestJobIdValue = (failureDestRes.payload as { id?: unknown } | null)?.id;
-      const failureDestJobId =
-        typeof failureDestJobIdValue === "string" ? failureDestJobIdValue : "";
-      expect(failureDestJobId.length > 0).toBe(true);
-
-      const failureDestRunRes = await rpcReq(
-        ws,
-        "cron.run",
-        { id: failureDestJobId, mode: "force" },
-        20_000,
-      );
-      expect(failureDestRunRes.ok).toBe(true);
+      await runCronJobForce(ws, failureDestJobId);
      await waitForCondition(
        () => fetchWithSsrFGuardMock.mock.calls.length === 1,
        CRON_WAIT_TIMEOUT_MS,
@@ -686,27 +690,13 @@ describe("gateway server cron", () => {
      );

      cronIsolatedRun.mockResolvedValueOnce({ status: "ok", summary: "" });
-      const noSummaryRes = await rpcReq(ws, "cron.add", {
+      const noSummaryJobId = await addWebhookCronJob({
+        ws,
        name: "webhook no summary",
-        enabled: true,
-        schedule: { kind: "every", everyMs: 60_000 },
        sessionTarget: "isolated",
-        wakeMode: "next-heartbeat",
-        payload: { kind: "agentTurn", message: "test" },
        delivery: { mode: "webhook", to: "https://example.invalid/cron-finished" },
      });
-      expect(noSummaryRes.ok).toBe(true);
-      const noSummaryJobIdValue = (noSummaryRes.payload as { id?: unknown } | null)?.id;
-      const noSummaryJobId = typeof noSummaryJobIdValue === "string" ? noSummaryJobIdValue : "";
-      expect(noSummaryJobId.length > 0).toBe(true);
-
-      const noSummaryRunRes = await rpcReq(
-        ws,
-        "cron.run",
-        { id: noSummaryJobId, mode: "force" },
-        20_000,
-      );
-      expect(noSummaryRunRes.ok).toBe(true);
+      await runCronJobForce(ws, noSummaryJobId);
      await yieldToEventLoop();
      await yieldToEventLoop();
      expect(fetchWithSsrFGuardMock).toHaveBeenCalledTimes(1);
@@ -746,22 +736,12 @@ describe("gateway server cron", () => {
    await connectOk(ws);

    try {
-      const notifyRes = await rpcReq(ws, "cron.add", {
+      const notifyJobId = await addWebhookCronJob({
+        ws,
        name: "webhook secretinput object",
-        enabled: true,
-        schedule: { kind: "every", everyMs: 60_000 },
-        sessionTarget: "main",
-        wakeMode: "next-heartbeat",
-        payload: { kind: "systemEvent", text: "send webhook" },
        delivery: { mode: "webhook", to: "https://example.invalid/cron-finished" },
      });
-      expect(notifyRes.ok).toBe(true);
-      const notifyJobIdValue = (notifyRes.payload as { id?: unknown } | null)?.id;
-      const notifyJobId = typeof notifyJobIdValue === "string" ? notifyJobIdValue : "";
-      expect(notifyJobId.length > 0).toBe(true);
-
-      const notifyRunRes = await rpcReq(ws, "cron.run", { id: notifyJobId, mode: "force" }, 20_000);
-      expect(notifyRunRes.ok).toBe(true);
+      await runCronJobForce(ws, notifyJobId);

      await waitForCondition(
        () => fetchWithSsrFGuardMock.mock.calls.length === 1,
--- a/src/gateway/test-helpers.server.ts
+++ b/src/gateway/test-helpers.server.ts
@@ -339,6 +339,46 @@ async function startGatewayServerWithRetries(params: {
  throw new Error("failed to start gateway server after retries");
 }

+async function waitForWebSocketOpen(ws: WebSocket, timeoutMs = 10_000): Promise<void> {
+  await new Promise<void>((resolve, reject) => {
+    const timer = setTimeout(() => reject(new Error("timeout waiting for ws open")), timeoutMs);
+    const cleanup = () => {
+      clearTimeout(timer);
+      ws.off("open", onOpen);
+      ws.off("error", onError);
+      ws.off("close", onClose);
+    };
+    const onOpen = () => {
+      cleanup();
+      resolve();
+    };
+    const onError = (err: unknown) => {
+      cleanup();
+      reject(err instanceof Error ? err : new Error(String(err)));
+    };
+    const onClose = (code: number, reason: Buffer) => {
+      cleanup();
+      reject(new Error(`closed ${code}: ${reason.toString()}`));
+    };
+    ws.once("open", onOpen);
+    ws.once("error", onError);
+    ws.once("close", onClose);
+  });
+}
+
+async function openTrackedWebSocket(params: {
+  port: number;
+  headers?: Record<string, string>;
+}): Promise<WebSocket> {
+  const ws = new WebSocket(
+    `ws://127.0.0.1:${params.port}`,
+    params.headers ? { headers: params.headers } : undefined,
+  );
+  trackConnectChallengeNonce(ws);
+  await waitForWebSocketOpen(ws);
+  return ws;
+}
+
 export async function withGatewayServer<T>(
  fn: (ctx: { port: number; server: Awaited<ReturnType<typeof startGatewayServer>> }) => Promise<T>,
  opts?: { port?: number; serverOptions?: GatewayServerOptions },
@@ -371,33 +411,10 @@ export async function createGatewaySuiteHarness(opts?: {
    port: started.port,
    server: started.server,
    openWs: async (headers?: Record<string, string>) => {
-      const ws = new WebSocket(`ws://127.0.0.1:${started.port}`, headers ? { headers } : undefined);
-      trackConnectChallengeNonce(ws);
-      await new Promise<void>((resolve, reject) => {
-        const timer = setTimeout(() => reject(new Error("timeout waiting for ws open")), 10_000);
-        const cleanup = () => {
-          clearTimeout(timer);
-          ws.off("open", onOpen);
-          ws.off("error", onError);
-          ws.off("close", onClose);
-        };
-        const onOpen = () => {
-          cleanup();
-          resolve();
-        };
-        const onError = (err: unknown) => {
-          cleanup();
-          reject(err instanceof Error ? err : new Error(String(err)));
-        };
-        const onClose = (code: number, reason: Buffer) => {
-          cleanup();
-          reject(new Error(`closed ${code}: ${reason.toString()}`));
-        };
-        ws.once("open", onOpen);
-        ws.once("error", onError);
-        ws.once("close", onClose);
+      return await openTrackedWebSocket({
+        port: started.port,
+        headers,
      });
-      return ws;
    },
    close: async () => {
      await started.server.close();
@@ -431,35 +448,7 @@ export async function startServerWithClient(
  port = started.port;
  const server = started.server;

-  const ws = new WebSocket(
-    `ws://127.0.0.1:${port}`,
-    wsHeaders ? { headers: wsHeaders } : undefined,
-  );
-  trackConnectChallengeNonce(ws);
-  await new Promise<void>((resolve, reject) => {
-    const timer = setTimeout(() => reject(new Error("timeout waiting for ws open")), 10_000);
-    const cleanup = () => {
-      clearTimeout(timer);
-      ws.off("open", onOpen);
-      ws.off("error", onError);
-      ws.off("close", onClose);
-    };
-    const onOpen = () => {
-      cleanup();
-      resolve();
-    };
-    const onError = (err: unknown) => {
-      cleanup();
-      reject(err instanceof Error ? err : new Error(String(err)));
-    };
-    const onClose = (code: number, reason: Buffer) => {
-      cleanup();
-      reject(new Error(`closed ${code}: ${reason.toString()}`));
-    };
-    ws.once("open", onOpen);
-    ws.once("error", onError);
-    ws.once("close", onClose);
-  });
+  const ws = await openTrackedWebSocket({ port, headers: wsHeaders });
  return { server, ws, port, prevToken: prev, envSnapshot };
 }