Skip to content

Commit bc66063

Browse files
committed
fix(skills): keep blank env requirements unsatisfied
1 parent 4bc84db commit bc66063

4 files changed

Lines changed: 107 additions & 10 deletions

File tree

src/skills/discovery/status.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,30 @@ type SkillStatus = ReturnType<typeof buildWorkspaceSkillStatus>["skills"][number
1313
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
1414

1515
describe("buildWorkspaceSkillStatus", () => {
16+
it("reports blank env requirements as missing", () => {
17+
const envName = "OPENCLAW_TEST_BLANK_SKILL_STATUS";
18+
const original = process.env[envName];
19+
process.env[envName] = " ";
20+
try {
21+
const report = buildWorkspaceSkillStatus("/tmp/ws", {
22+
entries: [
23+
createEntry("blank-env", {
24+
metadata: { primaryEnv: envName, requires: { env: [envName] } },
25+
}),
26+
],
27+
});
28+
29+
expect(report.skills[0]?.eligible).toBe(false);
30+
expect(report.skills[0]?.missing.env).toEqual([envName]);
31+
} finally {
32+
if (original === undefined) {
33+
delete process.env[envName];
34+
} else {
35+
process.env[envName] = original;
36+
}
37+
}
38+
});
39+
1640
it("surfaces valid ClawHub linkage and local Skill Card metadata", async () => {
1741
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skill-status-"));
1842
try {

src/skills/discovery/status.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { resolveBundledSkillsContext } from "../loading/bundled-context.js";
1616
import {
1717
hasBinary,
1818
isBundledSkillAllowed,
19+
isSkillEnvRequirementSatisfied,
1920
isSkillConfigPathTruthy,
2021
resolveBundledAllowlist,
2122
resolveSkillConfig,
@@ -269,11 +270,11 @@ function buildSkillStatus(
269270
const blockedByAgentFilter = agentSkillFilter !== undefined && !indexed.agentAllowed;
270271
const always = entry.metadata?.always === true;
271272
const isEnvSatisfied = (envName: string) =>
272-
Boolean(
273-
process.env[envName] ||
274-
skillConfig?.env?.[envName] ||
275-
(skillConfig?.apiKey && entry.metadata?.primaryEnv === envName),
276-
);
273+
isSkillEnvRequirementSatisfied({
274+
envName,
275+
skillConfig,
276+
primaryEnv: entry.metadata?.primaryEnv,
277+
});
277278
const isConfigSatisfied = (pathStr: string) => isSkillConfigPathTruthy(config, pathStr);
278279
const skillSource = indexed.source;
279280
const bundled = indexed.bundled;

src/skills/loading/config.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,22 @@ export function resolveSkillConfig(
5656
return entry;
5757
}
5858

59+
export function isSkillEnvRequirementSatisfied(params: {
60+
envName: string;
61+
skillConfig?: SkillConfig;
62+
primaryEnv?: string;
63+
}): boolean {
64+
const { envName, skillConfig, primaryEnv } = params;
65+
return (
66+
normalizeOptionalString(process.env[envName]) !== undefined ||
67+
normalizeOptionalString(skillConfig?.env?.[envName]) !== undefined ||
68+
(primaryEnv === envName &&
69+
(typeof skillConfig?.apiKey === "string"
70+
? normalizeOptionalString(skillConfig.apiKey) !== undefined
71+
: skillConfig?.apiKey !== undefined))
72+
);
73+
}
74+
5975
function normalizeAllowlist(input: unknown): ReadonlySet<string> | undefined {
6076
if (!input) {
6177
return undefined;
@@ -113,11 +129,11 @@ export function shouldIncludeSkill(params: {
113129
hasRemoteBin: eligibility?.remote?.hasBin,
114130
hasAnyRemoteBin: eligibility?.remote?.hasAnyBin,
115131
hasEnv: (envName) =>
116-
Boolean(
117-
process.env[envName] ||
118-
skillConfig?.env?.[envName] ||
119-
(skillConfig?.apiKey && entry.metadata?.primaryEnv === envName),
120-
),
132+
isSkillEnvRequirementSatisfied({
133+
envName,
134+
skillConfig,
135+
primaryEnv: entry.metadata?.primaryEnv,
136+
}),
121137
isConfigPathTruthy: (configPath) => isSkillConfigPathTruthy(config, configPath),
122138
});
123139
}

src/skills/loading/skills.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
type SkillsHomeEnvSnapshot,
2525
} from "../test-support/home-env.test-support.js";
2626
import type { SkillEntry, SkillSnapshot } from "../types.js";
27+
import { shouldIncludeSkill } from "./config.js";
2728
import { buildWorkspaceSkillsPrompt } from "./workspace.js";
2829

2930
vi.mock("./plugin-skills.js", () => ({
@@ -502,6 +503,61 @@ describe("buildWorkspaceSkillsPrompt", () => {
502503
});
503504
});
504505

506+
describe("shouldIncludeSkill", () => {
507+
const envName = "OPENCLAW_TEST_SKILL_REQUIREMENT";
508+
const entry = makeSkillEntry("env-skill", {
509+
primaryEnv: envName,
510+
requires: { env: [envName] },
511+
});
512+
513+
function shouldInclude(config?: OpenClawConfig): boolean {
514+
return shouldIncludeSkill({ entry, config, bundledAllowlist: undefined });
515+
}
516+
517+
it("requires non-blank host and configured env values", () => {
518+
withClearedEnv([envName], () => {
519+
expect(shouldInclude()).toBe(false);
520+
521+
process.env[envName] = " ";
522+
expect(shouldInclude()).toBe(false);
523+
524+
process.env[envName] = " example ";
525+
expect(shouldInclude()).toBe(true);
526+
527+
delete process.env[envName];
528+
expect(
529+
shouldInclude({ skills: { entries: { "env-skill": { env: { [envName]: " " } } } } }),
530+
).toBe(false);
531+
expect(
532+
shouldInclude({
533+
skills: { entries: { "env-skill": { env: { [envName]: " example " } } } },
534+
}),
535+
).toBe(true);
536+
});
537+
});
538+
539+
it("requires a non-blank primary apiKey", () => {
540+
withClearedEnv([envName], () => {
541+
expect(shouldInclude(resolvedSkillApiKeyConfig("env-skill", " "))).toBe(false);
542+
expect(shouldInclude(resolvedSkillApiKeyConfig("env-skill", " example "))).toBe(true);
543+
});
544+
});
545+
546+
it("keeps always-on skills eligible without credentials", () => {
547+
withClearedEnv([envName], () => {
548+
expect(
549+
shouldIncludeSkill({
550+
entry: makeSkillEntry("always-skill", {
551+
always: true,
552+
requires: { env: [envName] },
553+
}),
554+
bundledAllowlist: undefined,
555+
}),
556+
).toBe(true);
557+
});
558+
});
559+
});
560+
505561
describe("applySkillEnvOverrides", () => {
506562
it("sets and restores env vars", () => {
507563
const entries = envSkillEntries("env-skill", {

0 commit comments

Comments
 (0)