From 4f61a3f527354783251292e493a86f16eed60cc2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 14 Feb 2026 14:32:59 +0000 Subject: [PATCH] refactor(shared): centralize requirements evaluation --- src/agents/skills-status.ts | 65 ++++++++++--------------------------- src/hooks/hooks-status.ts | 63 ++++++++++------------------------- src/shared/requirements.ts | 59 +++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 92 deletions(-) diff --git a/src/agents/skills-status.ts b/src/agents/skills-status.ts index e1e5d29625d..ddca02f02b2 100644 --- a/src/agents/skills-status.ts +++ b/src/agents/skills-status.ts @@ -1,12 +1,6 @@ import path from "node:path"; import type { OpenClawConfig } from "../config/config.js"; -import { - buildConfigChecks, - resolveMissingAnyBins, - resolveMissingBins, - resolveMissingEnv, - resolveMissingOs, -} from "../shared/requirements.js"; +import { evaluateRequirements } from "../shared/requirements.js"; import { CONFIG_DIR } from "../utils.js"; import { hasBinary, @@ -209,57 +203,34 @@ function buildSkillStatus( const requiredConfig = entry.metadata?.requires?.config ?? []; const requiredOs = entry.metadata?.os ?? []; - const missingBins = resolveMissingBins({ - required: requiredBins, + const { + missing, + eligible: requirementsSatisfied, + configChecks, + } = evaluateRequirements({ + always, + required: { + bins: requiredBins, + anyBins: requiredAnyBins, + env: requiredEnv, + config: requiredConfig, + os: requiredOs, + }, 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) => + isEnvSatisfied: (envName) => Boolean( process.env[envName] || skillConfig?.env?.[envName] || (skillConfig?.apiKey && entry.metadata?.primaryEnv === envName), ), + resolveConfigValue: (pathStr) => resolveConfigPath(config, pathStr), + isConfigSatisfied: (pathStr) => isConfigPathTruthy(config, pathStr), }); - - 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); - - const missing = always - ? { bins: [], anyBins: [], env: [], config: [], os: [] } - : { - bins: missingBins, - anyBins: missingAnyBins, - env: missingEnv, - config: missingConfig, - os: missingOs, - }; - const eligible = - !disabled && - !blockedByAllowlist && - (always || - (missing.bins.length === 0 && - missing.anyBins.length === 0 && - missing.env.length === 0 && - missing.config.length === 0 && - missing.os.length === 0)); + const eligible = !disabled && !blockedByAllowlist && requirementsSatisfied; return { name: entry.skill.name, diff --git a/src/hooks/hooks-status.ts b/src/hooks/hooks-status.ts index 95d06b26862..b680ca668d5 100644 --- a/src/hooks/hooks-status.ts +++ b/src/hooks/hooks-status.ts @@ -1,13 +1,7 @@ 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 { evaluateRequirements } from "../shared/requirements.js"; import { CONFIG_DIR } from "../utils.js"; import { hasBinary, isConfigPathTruthy, resolveConfigPath, resolveHookConfig } from "./config.js"; import { loadWorkspaceHookEntries } from "./workspace.js"; @@ -122,51 +116,30 @@ function buildHookStatus( const requiredConfig = entry.metadata?.requires?.config ?? []; const requiredOs = entry.metadata?.os ?? []; - const missingBins = resolveMissingBins({ - required: requiredBins, + const { + missing, + eligible: requirementsSatisfied, + configChecks, + } = evaluateRequirements({ + always, + required: { + bins: requiredBins, + anyBins: requiredAnyBins, + env: requiredEnv, + config: requiredConfig, + os: requiredOs, + }, 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]), + isEnvSatisfied: (envName) => Boolean(process.env[envName] || hookConfig?.env?.[envName]), + resolveConfigValue: (pathStr) => resolveConfigPath(config, pathStr), + isConfigSatisfied: (pathStr) => isConfigPathTruthy(config, pathStr), }); - 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 - ? { bins: [], anyBins: [], env: [], config: [], os: [] } - : { - bins: missingBins, - anyBins: missingAnyBins, - env: missingEnv, - config: missingConfig, - os: missingOs, - }; - - const eligible = - !disabled && - (always || - (missing.bins.length === 0 && - missing.anyBins.length === 0 && - missing.env.length === 0 && - missing.config.length === 0 && - missing.os.length === 0)); + const eligible = !disabled && requirementsSatisfied; return { name: entry.hook.name, diff --git a/src/shared/requirements.ts b/src/shared/requirements.ts index 0de4946ea9a..6ecb511b170 100644 --- a/src/shared/requirements.ts +++ b/src/shared/requirements.ts @@ -88,3 +88,62 @@ export function buildConfigChecks(params: { return { path: pathStr, value, satisfied }; }); } + +export function evaluateRequirements(params: { + always: boolean; + required: Requirements; + hasLocalBin: (bin: string) => boolean; + hasRemoteBin?: (bin: string) => boolean; + hasRemoteAnyBin?: (bins: string[]) => boolean; + localPlatform: string; + remotePlatforms?: string[]; + isEnvSatisfied: (envName: string) => boolean; + resolveConfigValue: (pathStr: string) => unknown; + isConfigSatisfied: (pathStr: string) => boolean; +}): { missing: Requirements; eligible: boolean; configChecks: RequirementConfigCheck[] } { + const missingBins = resolveMissingBins({ + required: params.required.bins, + hasLocalBin: params.hasLocalBin, + hasRemoteBin: params.hasRemoteBin, + }); + const missingAnyBins = resolveMissingAnyBins({ + required: params.required.anyBins, + hasLocalBin: params.hasLocalBin, + hasRemoteAnyBin: params.hasRemoteAnyBin, + }); + const missingOs = resolveMissingOs({ + required: params.required.os, + localPlatform: params.localPlatform, + remotePlatforms: params.remotePlatforms, + }); + const missingEnv = resolveMissingEnv({ + required: params.required.env, + isSatisfied: params.isEnvSatisfied, + }); + const configChecks = buildConfigChecks({ + required: params.required.config, + resolveValue: params.resolveConfigValue, + isSatisfied: params.isConfigSatisfied, + }); + const missingConfig = configChecks.filter((check) => !check.satisfied).map((check) => check.path); + + const missing = params.always + ? { bins: [], anyBins: [], env: [], config: [], os: [] } + : { + bins: missingBins, + anyBins: missingAnyBins, + env: missingEnv, + config: missingConfig, + os: missingOs, + }; + + const eligible = + params.always || + (missing.bins.length === 0 && + missing.anyBins.length === 0 && + missing.env.length === 0 && + missing.config.length === 0 && + missing.os.length === 0); + + return { missing, eligible, configChecks }; +}