refactor(shared): dedupe requirements evaluation

src/agents/skills-status.ts

@@ -1,5 +1,12 @@
 import path from "node:path";
 import type { OpenClawConfig } from "../config/config.js";
+import {
+  buildConfigChecks,
+  resolveMissingAnyBins,
+  resolveMissingBins,
+  resolveMissingEnv,
+  resolveMissingOs,
+} from "../shared/requirements.js";
 import { CONFIG_DIR } from "../utils.js";
 import {
   hasBinary,
@@ -202,48 +209,36 @@ function buildSkillStatus(
   const requiredConfig = entry.metadata?.requires?.config ?? [];
   const requiredOs = entry.metadata?.os ?? [];
 
-  const missingBins = requiredBins.filter((bin) => {
-    if (hasBinary(bin)) {
-      return false;
-    }
-    if (eligibility?.remote?.hasBin?.(bin)) {
-      return false;
-    }
-    return true;
+  const missingBins = resolveMissingBins({
+    required: requiredBins,
+    hasLocalBin: hasBinary,
+    hasRemoteBin: eligibility?.remote?.hasBin,
+  });
+  const missingAnyBins = resolveMissingAnyBins({
+    required: requiredAnyBins,
+    hasLocalBin: hasBinary,
+    hasRemoteAnyBin: eligibility?.remote?.hasAnyBin,
+  });
+  const missingOs = resolveMissingOs({
+    required: requiredOs,
+    localPlatform: process.platform,
+    remotePlatforms: eligibility?.remote?.platforms,
   });
-  const missingAnyBins =
-    requiredAnyBins.length > 0 &&
-    !(
-      requiredAnyBins.some((bin) => hasBinary(bin)) ||
-      eligibility?.remote?.hasAnyBin?.(requiredAnyBins)
-    )
-      ? requiredAnyBins
-      : [];
-  const missingOs =
-    requiredOs.length > 0 &&
-    !requiredOs.includes(process.platform) &&
-    !eligibility?.remote?.platforms?.some((platform) => requiredOs.includes(platform))
-      ? requiredOs
-      : [];
 
-  const missingEnv: string[] = [];
-  for (const envName of requiredEnv) {
-    if (process.env[envName]) {
-      continue;
-    }
-    if (skillConfig?.env?.[envName]) {
-      continue;
-    }
-    if (skillConfig?.apiKey && entry.metadata?.primaryEnv === envName) {
-      continue;
-    }
-    missingEnv.push(envName);
-  }
+  const missingEnv = resolveMissingEnv({
+    required: requiredEnv,
+    isSatisfied: (envName) =>
+      Boolean(
+        process.env[envName] ||
+        skillConfig?.env?.[envName] ||
+        (skillConfig?.apiKey && entry.metadata?.primaryEnv === envName),
+      ),
+  });
 
-  const configChecks: SkillStatusConfigCheck[] = requiredConfig.map((pathStr) => {
-    const value = resolveConfigPath(config, pathStr);
-    const satisfied = isConfigPathTruthy(config, pathStr);
-    return { path: pathStr, value, satisfied };
+  const configChecks: SkillStatusConfigCheck[] = buildConfigChecks({
+    required: requiredConfig,
+    resolveValue: (pathStr) => resolveConfigPath(config, pathStr),
+    isSatisfied: (pathStr) => isConfigPathTruthy(config, pathStr),
   });
   const missingConfig = configChecks.filter((check) => !check.satisfied).map((check) => check.path);
 

src/hooks/hooks-status.ts

@@ -1,6 +1,13 @@
 import path from "node:path";
 import type { OpenClawConfig } from "../config/config.js";
 import type { HookEligibilityContext, HookEntry, HookInstallSpec } from "./types.js";
+import {
+  buildConfigChecks,
+  resolveMissingAnyBins,
+  resolveMissingBins,
+  resolveMissingEnv,
+  resolveMissingOs,
+} from "../shared/requirements.js";
 import { CONFIG_DIR } from "../utils.js";
 import { hasBinary, isConfigPathTruthy, resolveConfigPath, resolveHookConfig } from "./config.js";
 import { loadWorkspaceHookEntries } from "./workspace.js";
@@ -115,49 +122,31 @@ function buildHookStatus(
   const requiredConfig = entry.metadata?.requires?.config ?? [];
   const requiredOs = entry.metadata?.os ?? [];
 
-  const missingBins = requiredBins.filter((bin) => {
-    if (hasBinary(bin)) {
-      return false;
-    }
-    if (eligibility?.remote?.hasBin?.(bin)) {
-      return false;
-    }
-    return true;
+  const missingBins = resolveMissingBins({
+    required: requiredBins,
+    hasLocalBin: hasBinary,
+    hasRemoteBin: eligibility?.remote?.hasBin,
+  });
+  const missingAnyBins = resolveMissingAnyBins({
+    required: requiredAnyBins,
+    hasLocalBin: hasBinary,
+    hasRemoteAnyBin: eligibility?.remote?.hasAnyBin,
+  });
+  const missingOs = resolveMissingOs({
+    required: requiredOs,
+    localPlatform: process.platform,
+    remotePlatforms: eligibility?.remote?.platforms,
+  });
+  const missingEnv = resolveMissingEnv({
+    required: requiredEnv,
+    isSatisfied: (envName) => Boolean(process.env[envName] || hookConfig?.env?.[envName]),
   });
 
-  const missingAnyBins =
-    requiredAnyBins.length > 0 &&
-    !(
-      requiredAnyBins.some((bin) => hasBinary(bin)) ||
-      eligibility?.remote?.hasAnyBin?.(requiredAnyBins)
-    )
-      ? requiredAnyBins
-      : [];
-
-  const missingOs =
-    requiredOs.length > 0 &&
-    !requiredOs.includes(process.platform) &&
-    !eligibility?.remote?.platforms?.some((platform) => requiredOs.includes(platform))
-      ? requiredOs
-      : [];
-
-  const missingEnv: string[] = [];
-  for (const envName of requiredEnv) {
-    if (process.env[envName]) {
-      continue;
-    }
-    if (hookConfig?.env?.[envName]) {
-      continue;
-    }
-    missingEnv.push(envName);
-  }
-
-  const configChecks: HookStatusConfigCheck[] = requiredConfig.map((pathStr) => {
-    const value = resolveConfigPath(config, pathStr);
-    const satisfied = isConfigPathTruthy(config, pathStr);
-    return { path: pathStr, value, satisfied };
+  const configChecks: HookStatusConfigCheck[] = buildConfigChecks({
+    required: requiredConfig,
+    resolveValue: (pathStr) => resolveConfigPath(config, pathStr),
+    isSatisfied: (pathStr) => isConfigPathTruthy(config, pathStr),
   });
-
   const missingConfig = configChecks.filter((check) => !check.satisfied).map((check) => check.path);
 
   const missing = always

src/shared/requirements.test.ts

@@ -0,0 +1,63 @@
+import { describe, expect, it } from "vitest";
+import {
+  buildConfigChecks,
+  resolveMissingAnyBins,
+  resolveMissingBins,
+  resolveMissingEnv,
+  resolveMissingOs,
+} from "./requirements.js";
+
+describe("requirements helpers", () => {
+  it("resolveMissingBins respects local+remote", () => {
+    expect(
+      resolveMissingBins({
+        required: ["a", "b", "c"],
+        hasLocalBin: (bin) => bin === "a",
+        hasRemoteBin: (bin) => bin === "b",
+      }),
+    ).toEqual(["c"]);
+  });
+
+  it("resolveMissingAnyBins requires at least one", () => {
+    expect(
+      resolveMissingAnyBins({
+        required: ["a", "b"],
+        hasLocalBin: () => false,
+        hasRemoteAnyBin: () => false,
+      }),
+    ).toEqual(["a", "b"]);
+    expect(
+      resolveMissingAnyBins({
+        required: ["a", "b"],
+        hasLocalBin: (bin) => bin === "b",
+      }),
+    ).toEqual([]);
+  });
+
+  it("resolveMissingOs allows remote platform", () => {
+    expect(
+      resolveMissingOs({
+        required: ["darwin"],
+        localPlatform: "linux",
+        remotePlatforms: ["darwin"],
+      }),
+    ).toEqual([]);
+    expect(resolveMissingOs({ required: ["darwin"], localPlatform: "linux" })).toEqual(["darwin"]);
+  });
+
+  it("resolveMissingEnv uses predicate", () => {
+    expect(
+      resolveMissingEnv({ required: ["A", "B"], isSatisfied: (name) => name === "B" }),
+    ).toEqual(["A"]);
+  });
+
+  it("buildConfigChecks includes value+status", () => {
+    expect(
+      buildConfigChecks({
+        required: ["a.b"],
+        resolveValue: (p) => (p === "a.b" ? 1 : null),
+        isSatisfied: (p) => p === "a.b",
+      }),
+    ).toEqual([{ path: "a.b", value: 1, satisfied: true }]);
+  });
+});

src/shared/requirements.ts

@@ -0,0 +1,90 @@
+export type Requirements = {
+  bins: string[];
+  anyBins: string[];
+  env: string[];
+  config: string[];
+  os: string[];
+};
+
+export type RequirementConfigCheck = {
+  path: string;
+  value: unknown;
+  satisfied: boolean;
+};
+
+export function resolveMissingBins(params: {
+  required: string[];
+  hasLocalBin: (bin: string) => boolean;
+  hasRemoteBin?: (bin: string) => boolean;
+}): string[] {
+  const remote = params.hasRemoteBin;
+  return params.required.filter((bin) => {
+    if (params.hasLocalBin(bin)) {
+      return false;
+    }
+    if (remote?.(bin)) {
+      return false;
+    }
+    return true;
+  });
+}
+
+export function resolveMissingAnyBins(params: {
+  required: string[];
+  hasLocalBin: (bin: string) => boolean;
+  hasRemoteAnyBin?: (bins: string[]) => boolean;
+}): string[] {
+  if (params.required.length === 0) {
+    return [];
+  }
+  if (params.required.some((bin) => params.hasLocalBin(bin))) {
+    return [];
+  }
+  if (params.hasRemoteAnyBin?.(params.required)) {
+    return [];
+  }
+  return params.required;
+}
+
+export function resolveMissingOs(params: {
+  required: string[];
+  localPlatform: string;
+  remotePlatforms?: string[];
+}): string[] {
+  if (params.required.length === 0) {
+    return [];
+  }
+  if (params.required.includes(params.localPlatform)) {
+    return [];
+  }
+  if (params.remotePlatforms?.some((platform) => params.required.includes(platform))) {
+    return [];
+  }
+  return params.required;
+}
+
+export function resolveMissingEnv(params: {
+  required: string[];
+  isSatisfied: (envName: string) => boolean;
+}): string[] {
+  const missing: string[] = [];
+  for (const envName of params.required) {
+    if (params.isSatisfied(envName)) {
+      continue;
+    }
+    missing.push(envName);
+  }
+  return missing;
+}
+
+export function buildConfigChecks(params: {
+  required: string[];
+  resolveValue: (pathStr: string) => unknown;
+  isSatisfied: (pathStr: string) => boolean;
+}): RequirementConfigCheck[] {
+  return params.required.map((pathStr) => {
+    const value = params.resolveValue(pathStr);
+    const satisfied = params.isSatisfied(pathStr);
+    return { path: pathStr, value, satisfied };
+  });
+}