refactor: dedupe internal helper glue

2026-04-17 12:11:20 +00:00 · 2026-04-08 13:51:23 +01:00
parent 5f370149f3
commit 6e0957ca47
7 changed files with 165 additions and 205 deletions
--- a/src/agents/pi-embedded-runner/tool-schema-runtime.ts
+++ b/src/agents/pi-embedded-runner/tool-schema-runtime.ts
@@ -20,6 +20,22 @@ type ProviderToolSchemaParams<TSchemaType extends TSchema = TSchema, TResult = u
  model?: ProviderRuntimeModel;
 };

+function buildProviderToolSchemaContext<TSchemaType extends TSchema = TSchema, TResult = unknown>(
+  params: ProviderToolSchemaParams<TSchemaType, TResult>,
+  provider: string,
+) {
+  return {
+    config: params.config,
+    workspaceDir: params.workspaceDir,
+    env: params.env,
+    provider,
+    modelId: params.modelId,
+    modelApi: params.modelApi,
+    model: params.model,
+    tools: params.tools as unknown as AnyAgentTool[],
+  };
+}
+
 /**
 * Runs provider-owned tool-schema normalization without encoding provider
 * families in the embedded runner.
@@ -34,16 +50,7 @@ export function normalizeProviderToolSchemas<
    config: params.config,
    workspaceDir: params.workspaceDir,
    env: params.env,
-    context: {
-      config: params.config,
-      workspaceDir: params.workspaceDir,
-      env: params.env,
-      provider,
-      modelId: params.modelId,
-      modelApi: params.modelApi,
-      model: params.model,
-      tools: params.tools as unknown as AnyAgentTool[],
-    },
+    context: buildProviderToolSchemaContext(params, provider),
  });
  return Array.isArray(pluginNormalized)
    ? (pluginNormalized as AgentTool<TSchemaType, TResult>[])
@@ -60,16 +67,7 @@ export function logProviderToolSchemaDiagnostics(params: ProviderToolSchemaParam
    config: params.config,
    workspaceDir: params.workspaceDir,
    env: params.env,
-    context: {
-      config: params.config,
-      workspaceDir: params.workspaceDir,
-      env: params.env,
-      provider,
-      modelId: params.modelId,
-      modelApi: params.modelApi,
-      model: params.model,
-      tools: params.tools as unknown as AnyAgentTool[],
-    },
+    context: buildProviderToolSchemaContext(params, provider),
  });
  if (!Array.isArray(diagnostics)) {
    return;
--- a/src/agents/tools/pdf-tool.model-config.ts
+++ b/src/agents/tools/pdf-tool.model-config.ts
@@ -12,6 +12,29 @@ import {
 import { hasAuthForProvider, resolveDefaultModelRef } from "./model-config.helpers.js";
 import { coercePdfModelConfig } from "./pdf-tool.helpers.js";

+function resolveBundledImageCandidateRefs(params: {
+  cfg?: OpenClawConfig;
+  agentDir: string;
+  filter?: (providerId: string) => boolean;
+}): string[] {
+  return resolveBundledAutoMediaKeyProviders("image")
+    .filter((providerId) => !params.filter || params.filter(providerId))
+    .filter((providerId) => hasAuthForProvider({ provider: providerId, agentDir: params.agentDir }))
+    .map((providerId) => {
+      const modelId =
+        resolveProviderVisionModelFromConfig({
+          cfg: params.cfg,
+          provider: providerId,
+        })?.split("/")[1] ??
+        resolveBundledDefaultMediaModel({
+          providerId,
+          capability: "image",
+        });
+      return modelId ? `${providerId}/${modelId}` : null;
+    })
+    .filter((value): value is string => Boolean(value));
+}
+
 export function resolvePdfModelConfigForTool(params: {
  cfg?: OpenClawConfig;
  agentDir: string;
@@ -51,37 +74,15 @@ export function resolvePdfModelConfigForTool(params: {
      capability: "image",
    });
  const primarySupportsNativePdf = bundledProviderSupportsNativePdfDocument(primary.provider);
-  const nativePdfCandidates = resolveBundledAutoMediaKeyProviders("image")
-    .filter((providerId) => bundledProviderSupportsNativePdfDocument(providerId))
-    .filter((providerId) => hasAuthForProvider({ provider: providerId, agentDir: params.agentDir }))
-    .map((providerId) => {
-      const modelId =
-        resolveProviderVisionModelFromConfig({
-          cfg: params.cfg,
-          provider: providerId,
-        })?.split("/")[1] ??
-        resolveBundledDefaultMediaModel({
-          providerId,
-          capability: "image",
-        });
-      return modelId ? `${providerId}/${modelId}` : null;
-    })
-    .filter((value): value is string => Boolean(value));
-  const genericImageCandidates = resolveBundledAutoMediaKeyProviders("image")
-    .filter((providerId) => hasAuthForProvider({ provider: providerId, agentDir: params.agentDir }))
-    .map((providerId) => {
-      const modelId =
-        resolveProviderVisionModelFromConfig({
-          cfg: params.cfg,
-          provider: providerId,
-        })?.split("/")[1] ??
-        resolveBundledDefaultMediaModel({
-          providerId,
-          capability: "image",
-        });
-      return modelId ? `${providerId}/${modelId}` : null;
-    })
-    .filter((value): value is string => Boolean(value));
+  const nativePdfCandidates = resolveBundledImageCandidateRefs({
+    cfg: params.cfg,
+    agentDir: params.agentDir,
+    filter: bundledProviderSupportsNativePdfDocument,
+  });
+  const genericImageCandidates = resolveBundledImageCandidateRefs({
+    cfg: params.cfg,
+    agentDir: params.agentDir,
+  });

  if (params.cfg?.models?.providers && typeof params.cfg.models.providers === "object") {
    for (const [providerKey, providerCfg] of Object.entries(params.cfg.models.providers)) {
--- a/src/cli/acp-cli.ts
+++ b/src/cli/acp-cli.ts
@@ -1,37 +1,12 @@
 import type { Command } from "commander";
 import { runAcpClientInteractive } from "../acp/client.js";
-import { readSecretFromFile } from "../acp/secret-file.js";
 import { serveAcpGateway } from "../acp/server.js";
 import { normalizeAcpProvenanceMode } from "../acp/types.js";
 import { defaultRuntime } from "../runtime.js";
-import { normalizeOptionalString } from "../shared/string-coerce.js";
 import { formatDocsLink } from "../terminal/links.js";
 import { theme } from "../terminal/theme.js";
 import { inheritOptionFromParent } from "./command-options.js";
-
-function resolveSecretOption(params: {
-  direct?: string;
-  file?: string;
-  directFlag: string;
-  fileFlag: string;
-  label: string;
-}) {
-  const direct = normalizeOptionalString(params.direct);
-  const file = normalizeOptionalString(params.file);
-  if (direct && file) {
-    throw new Error(`Use either ${params.directFlag} or ${params.fileFlag} for ${params.label}.`);
-  }
-  if (file) {
-    return readSecretFromFile(file, params.label);
-  }
-  return direct || undefined;
-}
-
-function warnSecretCliFlag(flag: "--token" | "--password") {
-  defaultRuntime.error(
-    `Warning: ${flag} can be exposed via process listings. Prefer ${flag}-file or environment variables.`,
-  );
-}
+import { resolveGatewayAuthOptions } from "./gateway-secret-options.js";

 export function registerAcpCli(program: Command) {
  const acp = program.command("acp").description("Run an ACP bridge backed by the Gateway");
@@ -55,26 +30,7 @@ export function registerAcpCli(program: Command) {
    )
    .action(async (opts) => {
      try {
-        const gatewayToken = resolveSecretOption({
-          direct: opts.token as string | undefined,
-          file: opts.tokenFile as string | undefined,
-          directFlag: "--token",
-          fileFlag: "--token-file",
-          label: "Gateway token",
-        });
-        const gatewayPassword = resolveSecretOption({
-          direct: opts.password as string | undefined,
-          file: opts.passwordFile as string | undefined,
-          directFlag: "--password",
-          fileFlag: "--password-file",
-          label: "Gateway password",
-        });
-        if (opts.token) {
-          warnSecretCliFlag("--token");
-        }
-        if (opts.password) {
-          warnSecretCliFlag("--password");
-        }
+        const { gatewayToken, gatewayPassword } = resolveGatewayAuthOptions(opts);
        const provenanceMode = normalizeAcpProvenanceMode(opts.provenance as string | undefined);
        if (opts.provenance && !provenanceMode) {
          throw new Error("Invalid --provenance value. Use off, meta, or meta+receipt.");
--- a/src/cli/gateway-secret-options.ts
+++ b/src/cli/gateway-secret-options.ts
@@ -0,0 +1,59 @@
+import { readSecretFromFile } from "../acp/secret-file.js";
+import { defaultRuntime } from "../runtime.js";
+import { normalizeOptionalString } from "../shared/string-coerce.js";
+
+export function resolveGatewaySecretOption(params: {
+  direct?: unknown;
+  file?: unknown;
+  directFlag: string;
+  fileFlag: string;
+  label: string;
+}): string | undefined {
+  const direct = normalizeOptionalString(params.direct);
+  const file = normalizeOptionalString(params.file);
+  if (direct && file) {
+    throw new Error(`Use either ${params.directFlag} or ${params.fileFlag} for ${params.label}.`);
+  }
+  if (file) {
+    return readSecretFromFile(file, params.label);
+  }
+  return direct || undefined;
+}
+
+export function warnGatewaySecretCliFlag(flag: "--token" | "--password"): void {
+  defaultRuntime.error(
+    `Warning: ${flag} can be exposed via process listings. Prefer ${flag}-file or environment variables.`,
+  );
+}
+
+export function resolveGatewayAuthOptions(opts: {
+  token?: unknown;
+  tokenFile?: unknown;
+  password?: unknown;
+  passwordFile?: unknown;
+}): {
+  gatewayToken?: string;
+  gatewayPassword?: string;
+} {
+  const gatewayToken = resolveGatewaySecretOption({
+    direct: opts.token,
+    file: opts.tokenFile,
+    directFlag: "--token",
+    fileFlag: "--token-file",
+    label: "Gateway token",
+  });
+  const gatewayPassword = resolveGatewaySecretOption({
+    direct: opts.password,
+    file: opts.passwordFile,
+    directFlag: "--password",
+    fileFlag: "--password-file",
+    label: "Gateway password",
+  });
+  if (opts.token) {
+    warnGatewaySecretCliFlag("--token");
+  }
+  if (opts.password) {
+    warnGatewaySecretCliFlag("--password");
+  }
+  return { gatewayToken, gatewayPassword };
+}
--- a/src/cli/mcp-cli.ts
+++ b/src/cli/mcp-cli.ts
@@ -1,5 +1,4 @@
 import { Command } from "commander";
-import { readSecretFromFile } from "../acp/secret-file.js";
 import { parseConfigValue } from "../auto-reply/reply/config-value.js";
 import {
  listConfiguredMcpServers,
@@ -8,9 +7,11 @@ import {
 } from "../config/mcp-config.js";
 import { serveOpenClawChannelMcp } from "../mcp/channel-server.js";
 import { defaultRuntime } from "../runtime.js";
-import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
-import { normalizeOptionalString } from "../shared/string-coerce.js";
-import { normalizeStringifiedOptionalString } from "../shared/string-coerce.js";
+import {
+  normalizeLowercaseStringOrEmpty,
+  normalizeStringifiedOptionalString,
+} from "../shared/string-coerce.js";
+import { resolveGatewayAuthOptions } from "./gateway-secret-options.js";

 function fail(message: string): never {
  defaultRuntime.error(message);
@@ -22,30 +23,6 @@ function printJson(value: unknown): void {
  defaultRuntime.writeJson(value);
 }

-function resolveSecretOption(params: {
-  direct?: string;
-  file?: string;
-  directFlag: string;
-  fileFlag: string;
-  label: string;
-}) {
-  const direct = normalizeOptionalString(params.direct);
-  const file = normalizeOptionalString(params.file);
-  if (direct && file) {
-    throw new Error(`Use either ${params.directFlag} or ${params.fileFlag} for ${params.label}.`);
-  }
-  if (file) {
-    return readSecretFromFile(file, params.label);
-  }
-  return direct || undefined;
-}
-
-function warnSecretCliFlag(flag: "--token" | "--password") {
-  defaultRuntime.error(
-    `Warning: ${flag} can be exposed via process listings. Prefer ${flag}-file or environment variables.`,
-  );
-}
-
 export function registerMcpCli(program: Command) {
  const mcp = program.command("mcp").description("Manage OpenClaw MCP config and channel bridge");

@@ -65,26 +42,7 @@ export function registerMcpCli(program: Command) {
    .option("-v, --verbose", "Verbose logging to stderr", false)
    .action(async (opts) => {
      try {
-        const gatewayToken = resolveSecretOption({
-          direct: opts.token as string | undefined,
-          file: opts.tokenFile as string | undefined,
-          directFlag: "--token",
-          fileFlag: "--token-file",
-          label: "Gateway token",
-        });
-        const gatewayPassword = resolveSecretOption({
-          direct: opts.password as string | undefined,
-          file: opts.passwordFile as string | undefined,
-          directFlag: "--password",
-          fileFlag: "--password-file",
-          label: "Gateway password",
-        });
-        if (opts.token) {
-          warnSecretCliFlag("--token");
-        }
-        if (opts.password) {
-          warnSecretCliFlag("--password");
-        }
+        const { gatewayToken, gatewayPassword } = resolveGatewayAuthOptions(opts);
        const claudeChannelMode = normalizeLowercaseStringOrEmpty(
          normalizeStringifiedOptionalString(opts.claudeChannelMode) ?? "auto",
        );
--- a/src/media-understanding/shared.ts
+++ b/src/media-understanding/shared.ts
@@ -111,6 +111,30 @@ export async function fetchWithTimeoutGuarded(
  });
 }

+type GuardedPostRequestOptions = NonNullable<Parameters<typeof fetchWithTimeoutGuarded>[4]>;
+
+function resolveGuardedPostRequestOptions(params: {
+  pinDns?: boolean;
+  allowPrivateNetwork?: boolean;
+  dispatcherPolicy?: PinnedDispatcherPolicy;
+  auditContext?: string;
+}): GuardedPostRequestOptions | undefined {
+  if (
+    !params.allowPrivateNetwork &&
+    !params.dispatcherPolicy &&
+    params.pinDns === undefined &&
+    !params.auditContext
+  ) {
+    return undefined;
+  }
+  return {
+    ...(params.allowPrivateNetwork ? { ssrfPolicy: { allowPrivateNetwork: true } } : {}),
+    ...(params.pinDns !== undefined ? { pinDns: params.pinDns } : {}),
+    ...(params.dispatcherPolicy ? { dispatcherPolicy: params.dispatcherPolicy } : {}),
+    ...(params.auditContext ? { auditContext: params.auditContext } : {}),
+  };
+}
+
 export async function postTranscriptionRequest(params: {
  url: string;
  headers: Headers;
@@ -131,17 +155,7 @@ export async function postTranscriptionRequest(params: {
    },
    params.timeoutMs,
    params.fetchFn,
-    params.allowPrivateNetwork ||
-      params.dispatcherPolicy ||
-      params.pinDns !== undefined ||
-      params.auditContext
-      ? {
-          ...(params.allowPrivateNetwork ? { ssrfPolicy: { allowPrivateNetwork: true } } : {}),
-          ...(params.pinDns !== undefined ? { pinDns: params.pinDns } : {}),
-          ...(params.dispatcherPolicy ? { dispatcherPolicy: params.dispatcherPolicy } : {}),
-          ...(params.auditContext ? { auditContext: params.auditContext } : {}),
-        }
-      : undefined,
+    resolveGuardedPostRequestOptions(params),
  );
 }

@@ -165,17 +179,7 @@ export async function postJsonRequest(params: {
    },
    params.timeoutMs,
    params.fetchFn,
-    params.allowPrivateNetwork ||
-      params.dispatcherPolicy ||
-      params.pinDns !== undefined ||
-      params.auditContext
-      ? {
-          ...(params.allowPrivateNetwork ? { ssrfPolicy: { allowPrivateNetwork: true } } : {}),
-          ...(params.pinDns !== undefined ? { pinDns: params.pinDns } : {}),
-          ...(params.dispatcherPolicy ? { dispatcherPolicy: params.dispatcherPolicy } : {}),
-          ...(params.auditContext ? { auditContext: params.auditContext } : {}),
-        }
-      : undefined,
+    resolveGuardedPostRequestOptions(params),
  );
 }

--- a/src/tasks/task-flow-registry.ts
+++ b/src/tasks/task-flow-registry.ts
@@ -58,6 +58,26 @@ type FlowRecordPatch = Omit<
  endedAt?: number | null;
 };

+export type CreateFlowRecordParams = {
+  syncMode?: TaskFlowSyncMode;
+  ownerKey: string;
+  requesterOrigin?: TaskFlowRecord["requesterOrigin"];
+  controllerId?: string | null;
+  revision?: number;
+  status?: TaskFlowStatus;
+  notifyPolicy?: TaskNotifyPolicy;
+  goal: string;
+  currentStep?: string | null;
+  blockedTaskId?: string | null;
+  blockedSummary?: string | null;
+  stateJson?: JsonValue | null;
+  waitJson?: JsonValue | null;
+  cancelRequestedAt?: number | null;
+  createdAt?: number;
+  updatedAt?: number;
+  endedAt?: number | null;
+};
+
 export type TaskFlowUpdateResult =
  | {
      applied: true;
@@ -241,25 +261,7 @@ function persistFlowDelete(flowId: string) {
  persistFlowRegistry();
 }

-function buildFlowRecord(params: {
-  syncMode?: TaskFlowSyncMode;
-  ownerKey: string;
-  requesterOrigin?: TaskFlowRecord["requesterOrigin"];
-  controllerId?: string | null;
-  revision?: number;
-  status?: TaskFlowStatus;
-  notifyPolicy?: TaskNotifyPolicy;
-  goal: string;
-  currentStep?: string | null;
-  blockedTaskId?: string | null;
-  blockedSummary?: string | null;
-  stateJson?: JsonValue | null;
-  waitJson?: JsonValue | null;
-  cancelRequestedAt?: number | null;
-  createdAt?: number;
-  updatedAt?: number;
-  endedAt?: number | null;
-}): TaskFlowRecord {
+function buildFlowRecord(params: CreateFlowRecordParams): TaskFlowRecord {
  const now = params.createdAt ?? Date.now();
  const syncMode = params.syncMode ?? "managed";
  const controllerId = syncMode === "managed" ? assertControllerId(params.controllerId) : undefined;
@@ -341,25 +343,7 @@ function writeFlowRecord(next: TaskFlowRecord, previous?: TaskFlowRecord): TaskF
  return cloneFlowRecord(next);
 }

-export function createFlowRecord(params: {
-  syncMode?: TaskFlowSyncMode;
-  ownerKey: string;
-  requesterOrigin?: TaskFlowRecord["requesterOrigin"];
-  controllerId?: string | null;
-  revision?: number;
-  status?: TaskFlowStatus;
-  notifyPolicy?: TaskNotifyPolicy;
-  goal: string;
-  currentStep?: string | null;
-  blockedTaskId?: string | null;
-  blockedSummary?: string | null;
-  stateJson?: JsonValue | null;
-  waitJson?: JsonValue | null;
-  cancelRequestedAt?: number | null;
-  createdAt?: number;
-  updatedAt?: number;
-  endedAt?: number | null;
-}): TaskFlowRecord {
+export function createFlowRecord(params: CreateFlowRecordParams): TaskFlowRecord {
  ensureFlowRegistryReady();
  const record = buildFlowRecord(params);
  return writeFlowRecord(record);