refactor: unify channel plugin resolution, family ordering, and changelog entry tooling

2026-05-01 21:30:32 +00:00 · 2026-02-24 15:15:11 +00:00
parent 878b4e0ed7
commit d18ae2256f
7 changed files with 310 additions and 156 deletions
--- a/src/config/plugin-auto-enable.ts
+++ b/src/config/plugin-auto-enable.ts
@@ -49,7 +49,7 @@ function recordHasKeys(value: unknown): boolean {
  return isRecord(value) && Object.keys(value).length > 0;
 }

-function accountsHaveKeys(value: unknown, keys: string[]): boolean {
+function accountsHaveKeys(value: unknown, keys: readonly string[]): boolean {
  if (!isRecord(value)) {
    return false;
  }
@@ -75,108 +75,95 @@ function resolveChannelConfig(
  return isRecord(entry) ? entry : null;
 }

-function isTelegramConfigured(cfg: OpenClawConfig, env: NodeJS.ProcessEnv): boolean {
-  if (hasNonEmptyString(env.TELEGRAM_BOT_TOKEN)) {
-    return true;
+type StructuredChannelConfigSpec = {
+  envAny?: readonly string[];
+  envAll?: readonly string[];
+  stringKeys?: readonly string[];
+  numberKeys?: readonly string[];
+  accountStringKeys?: readonly string[];
+};
+
+const STRUCTURED_CHANNEL_CONFIG_SPECS: Record<string, StructuredChannelConfigSpec> = {
+  telegram: {
+    envAny: ["TELEGRAM_BOT_TOKEN"],
+    stringKeys: ["botToken", "tokenFile"],
+    accountStringKeys: ["botToken", "tokenFile"],
+  },
+  discord: {
+    envAny: ["DISCORD_BOT_TOKEN"],
+    stringKeys: ["token"],
+    accountStringKeys: ["token"],
+  },
+  irc: {
+    envAll: ["IRC_HOST", "IRC_NICK"],
+    stringKeys: ["host", "nick"],
+    accountStringKeys: ["host", "nick"],
+  },
+  slack: {
+    envAny: ["SLACK_BOT_TOKEN", "SLACK_APP_TOKEN", "SLACK_USER_TOKEN"],
+    stringKeys: ["botToken", "appToken", "userToken"],
+    accountStringKeys: ["botToken", "appToken", "userToken"],
+  },
+  signal: {
+    stringKeys: ["account", "httpUrl", "httpHost", "cliPath"],
+    numberKeys: ["httpPort"],
+    accountStringKeys: ["account", "httpUrl", "httpHost", "cliPath"],
+  },
+  imessage: {
+    stringKeys: ["cliPath"],
+  },
+};
+
+function envHasAnyKeys(env: NodeJS.ProcessEnv, keys: readonly string[]): boolean {
+  for (const key of keys) {
+    if (hasNonEmptyString(env[key])) {
+      return true;
+    }
  }
-  const entry = resolveChannelConfig(cfg, "telegram");
-  if (!entry) {
-    return false;
-  }
-  if (hasNonEmptyString(entry.botToken) || hasNonEmptyString(entry.tokenFile)) {
-    return true;
-  }
-  if (accountsHaveKeys(entry.accounts, ["botToken", "tokenFile"])) {
-    return true;
-  }
-  return recordHasKeys(entry);
+  return false;
 }

-function isDiscordConfigured(cfg: OpenClawConfig, env: NodeJS.ProcessEnv): boolean {
-  if (hasNonEmptyString(env.DISCORD_BOT_TOKEN)) {
-    return true;
+function envHasAllKeys(env: NodeJS.ProcessEnv, keys: readonly string[]): boolean {
+  for (const key of keys) {
+    if (!hasNonEmptyString(env[key])) {
+      return false;
+    }
  }
-  const entry = resolveChannelConfig(cfg, "discord");
-  if (!entry) {
-    return false;
-  }
-  if (hasNonEmptyString(entry.token)) {
-    return true;
-  }
-  if (accountsHaveKeys(entry.accounts, ["token"])) {
-    return true;
-  }
-  return recordHasKeys(entry);
+  return keys.length > 0;
 }

-function isIrcConfigured(cfg: OpenClawConfig, env: NodeJS.ProcessEnv): boolean {
-  if (hasNonEmptyString(env.IRC_HOST) && hasNonEmptyString(env.IRC_NICK)) {
-    return true;
+function hasAnyNumberKeys(entry: Record<string, unknown>, keys: readonly string[]): boolean {
+  for (const key of keys) {
+    if (typeof entry[key] === "number") {
+      return true;
+    }
  }
-  const entry = resolveChannelConfig(cfg, "irc");
-  if (!entry) {
-    return false;
-  }
-  if (hasNonEmptyString(entry.host) || hasNonEmptyString(entry.nick)) {
-    return true;
-  }
-  if (accountsHaveKeys(entry.accounts, ["host", "nick"])) {
-    return true;
-  }
-  return recordHasKeys(entry);
+  return false;
 }

-function isSlackConfigured(cfg: OpenClawConfig, env: NodeJS.ProcessEnv): boolean {
-  if (
-    hasNonEmptyString(env.SLACK_BOT_TOKEN) ||
-    hasNonEmptyString(env.SLACK_APP_TOKEN) ||
-    hasNonEmptyString(env.SLACK_USER_TOKEN)
-  ) {
+function isStructuredChannelConfigured(
+  cfg: OpenClawConfig,
+  channelId: string,
+  env: NodeJS.ProcessEnv,
+  spec: StructuredChannelConfigSpec,
+): boolean {
+  if (spec.envAny && envHasAnyKeys(env, spec.envAny)) {
    return true;
  }
-  const entry = resolveChannelConfig(cfg, "slack");
+  if (spec.envAll && envHasAllKeys(env, spec.envAll)) {
+    return true;
+  }
+  const entry = resolveChannelConfig(cfg, channelId);
  if (!entry) {
    return false;
  }
-  if (
-    hasNonEmptyString(entry.botToken) ||
-    hasNonEmptyString(entry.appToken) ||
-    hasNonEmptyString(entry.userToken)
-  ) {
+  if (spec.stringKeys && spec.stringKeys.some((key) => hasNonEmptyString(entry[key]))) {
    return true;
  }
-  if (accountsHaveKeys(entry.accounts, ["botToken", "appToken", "userToken"])) {
+  if (spec.numberKeys && hasAnyNumberKeys(entry, spec.numberKeys)) {
    return true;
  }
-  return recordHasKeys(entry);
-}
-
-function isSignalConfigured(cfg: OpenClawConfig): boolean {
-  const entry = resolveChannelConfig(cfg, "signal");
-  if (!entry) {
-    return false;
-  }
-  if (
-    hasNonEmptyString(entry.account) ||
-    hasNonEmptyString(entry.httpUrl) ||
-    hasNonEmptyString(entry.httpHost) ||
-    typeof entry.httpPort === "number" ||
-    hasNonEmptyString(entry.cliPath)
-  ) {
-    return true;
-  }
-  if (accountsHaveKeys(entry.accounts, ["account", "httpUrl", "httpHost", "cliPath"])) {
-    return true;
-  }
-  return recordHasKeys(entry);
-}
-
-function isIMessageConfigured(cfg: OpenClawConfig): boolean {
-  const entry = resolveChannelConfig(cfg, "imessage");
-  if (!entry) {
-    return false;
-  }
-  if (hasNonEmptyString(entry.cliPath)) {
+  if (spec.accountStringKeys && accountsHaveKeys(entry.accounts, spec.accountStringKeys)) {
    return true;
  }
  return recordHasKeys(entry);
@@ -203,24 +190,14 @@ export function isChannelConfigured(
  channelId: string,
  env: NodeJS.ProcessEnv = process.env,
 ): boolean {
-  switch (channelId) {
-    case "whatsapp":
-      return isWhatsAppConfigured(cfg);
-    case "telegram":
-      return isTelegramConfigured(cfg, env);
-    case "discord":
-      return isDiscordConfigured(cfg, env);
-    case "irc":
-      return isIrcConfigured(cfg, env);
-    case "slack":
-      return isSlackConfigured(cfg, env);
-    case "signal":
-      return isSignalConfigured(cfg);
-    case "imessage":
-      return isIMessageConfigured(cfg);
-    default:
-      return isGenericChannelConfigured(cfg, channelId);
+  if (channelId === "whatsapp") {
+    return isWhatsAppConfigured(cfg);
  }
+  const spec = STRUCTURED_CHANNEL_CONFIG_SPECS[channelId];
+  if (spec) {
+    return isStructuredChannelConfigured(cfg, channelId, env, spec);
+  }
+  return isGenericChannelConfigured(cfg, channelId);
 }

 function collectModelRefs(cfg: OpenClawConfig): string[] {
@@ -325,10 +302,34 @@ function buildChannelToPluginIdMap(registry: PluginManifestRegistry): Map<string
  return map;
 }

-type ChannelPluginPair = {
-  channelId: string;
-  pluginId: string;
-};
+function resolvePluginIdForChannel(
+  channelId: string,
+  channelToPluginId: ReadonlyMap<string, string>,
+): string {
+  // Third-party plugins can expose a channel id that differs from their
+  // manifest id; plugins.entries must always be keyed by manifest id.
+  const builtInId = normalizeChatChannelId(channelId);
+  if (builtInId) {
+    return builtInId;
+  }
+  return channelToPluginId.get(channelId) ?? channelId;
+}
+
+function collectCandidateChannelIds(cfg: OpenClawConfig): string[] {
+  const channelIds = new Set<string>(CHANNEL_PLUGIN_IDS);
+  const configuredChannels = cfg.channels as Record<string, unknown> | undefined;
+  if (!configuredChannels || typeof configuredChannels !== "object") {
+    return Array.from(channelIds);
+  }
+  for (const key of Object.keys(configuredChannels)) {
+    if (key === "defaults" || key === "modelByChannel") {
+      continue;
+    }
+    const normalizedBuiltIn = normalizeChatChannelId(key);
+    channelIds.add(normalizedBuiltIn ?? key);
+  }
+  return Array.from(channelIds);
+}

 function resolveConfiguredPlugins(
  cfg: OpenClawConfig,
@@ -337,45 +338,9 @@ function resolveConfiguredPlugins(
 ): PluginEnableChange[] {
  const changes: PluginEnableChange[] = [];
  // Build reverse map: channel ID → plugin ID from installed plugin manifests.
-  // This is needed when a third-party plugin declares a channel ID that differs
-  // from the plugin's own ID (e.g. plugin id="apn-channel", channels=["apn"]).
  const channelToPluginId = buildChannelToPluginIdMap(registry);
-
-  // For built-in and catalog entries: channelId === pluginId (they are the same).
-  const pairs: ChannelPluginPair[] = CHANNEL_PLUGIN_IDS.map((id) => ({
-    channelId: id,
-    pluginId: id,
-  }));
-
-  const configuredChannels = cfg.channels as Record<string, unknown> | undefined;
-  if (configuredChannels && typeof configuredChannels === "object") {
-    for (const key of Object.keys(configuredChannels)) {
-      if (key === "defaults" || key === "modelByChannel") {
-        continue;
-      }
-      const builtInId = normalizeChatChannelId(key);
-      if (builtInId) {
-        // Built-in channel: channelId and pluginId are the same.
-        pairs.push({ channelId: builtInId, pluginId: builtInId });
-      } else {
-        // Third-party channel plugin: look up the actual plugin ID from the
-        // manifest registry. If the plugin declares channels=["apn"] but its
-        // id is "apn-channel", we must use "apn-channel" as the pluginId so
-        // that plugins.entries is keyed correctly. Fall back to the channel key
-        // when no installed manifest declares this channel.
-        const pluginId = channelToPluginId.get(key) ?? key;
-        pairs.push({ channelId: key, pluginId });
-      }
-    }
-  }
-
-  // Deduplicate by channelId, preserving first occurrence.
-  const seenChannelIds = new Set<string>();
-  for (const { channelId, pluginId } of pairs) {
-    if (!channelId || !pluginId || seenChannelIds.has(channelId)) {
-      continue;
-    }
-    seenChannelIds.add(channelId);
+  for (const channelId of collectCandidateChannelIds(cfg)) {
+    const pluginId = resolvePluginIdForChannel(channelId, channelToPluginId);
    if (isChannelConfigured(cfg, channelId, env)) {
      changes.push({ pluginId, reason: `${channelId} configured` });
    }
--- a/src/infra/net/ssrf.pinning.test.ts
+++ b/src/infra/net/ssrf.pinning.test.ts
@@ -172,6 +172,16 @@ describe("ssrf pinning", () => {
    ]);
  });

+  it("uses DNS family metadata for ordering (not address string heuristics)", async () => {
+    const lookup = vi.fn(async () => [
+      { address: "2606:2800:220:1:248:1893:25c8:1946", family: 4 },
+      { address: "93.184.216.34", family: 6 },
+    ]) as unknown as LookupFn;
+
+    const pinned = await resolvePinnedHostname("example.com", lookup);
+    expect(pinned.addresses).toEqual(["2606:2800:220:1:248:1893:25c8:1946", "93.184.216.34"]);
+  });
+
  it("allows ISATAP embedded private IPv4 when private network is explicitly enabled", async () => {
    const lookup = vi.fn(async () => [
      { address: "2001:db8:1234::5efe:127.0.0.1", family: 6 },
--- a/src/infra/net/ssrf.ts
+++ b/src/infra/net/ssrf.ts
@@ -255,6 +255,24 @@ export type PinnedHostname = {
  lookup: typeof dnsLookupCb;
 };

+function dedupeAndPreferIpv4(results: readonly LookupAddress[]): string[] {
+  const seen = new Set<string>();
+  const ipv4: string[] = [];
+  const otherFamilies: string[] = [];
+  for (const entry of results) {
+    if (seen.has(entry.address)) {
+      continue;
+    }
+    seen.add(entry.address);
+    if (entry.family === 4) {
+      ipv4.push(entry.address);
+      continue;
+    }
+    otherFamilies.push(entry.address);
+  }
+  return [...ipv4, ...otherFamilies];
+}
+
 export async function resolvePinnedHostnameWithPolicy(
  hostname: string,
  params: { lookupFn?: LookupFn; policy?: SsrFPolicy } = {},
@@ -290,18 +308,9 @@ export async function resolvePinnedHostnameWithPolicy(
    assertAllowedResolvedAddressesOrThrow(results, params.policy);
  }

-  // Sort IPv4 addresses before IPv6 so that Happy Eyeballs (autoSelectFamily) and
-  // round-robin pinned lookups try IPv4 first.  This avoids connection failures on
-  // hosts where IPv6 is configured but not routed (common on cloud VMs and WSL2).
-  // See: https://github.com/openclaw/openclaw/issues/23975
-  const addresses = Array.from(new Set(results.map((entry) => entry.address))).toSorted((a, b) => {
-    const aIsV6 = a.includes(":");
-    const bIsV6 = b.includes(":");
-    if (aIsV6 === bIsV6) {
-      return 0;
-    }
-    return aIsV6 ? 1 : -1;
-  });
+  // Prefer addresses returned as IPv4 by DNS family metadata before other
+  // families so Happy Eyeballs and pinned round-robin both attempt IPv4 first.
+  const addresses = dedupeAndPreferIpv4(results);
  if (addresses.length === 0) {
    throw new Error(`Unable to resolve hostname: ${hostname}`);
  }