Skip to content

Commit 2680949

Browse files
steipetesteipete
committed
fix: reduce brew noise in onboarding
1 parent 9a765c9 commit 2680949

4 files changed

Lines changed: 287 additions & 45 deletions

File tree

‎src/agents/skills-status.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { describe, expect, it } from "vitest";
2+
import type { SkillEntry } from "./skills/types.js";
3+
import { buildWorkspaceSkillStatus } from "./skills-status.js";
4+
5+
describe("buildWorkspaceSkillStatus", () => {
6+
it("does not surface install options for OS-scoped skills on unsupported platforms", () => {
7+
if (process.platform === "win32") {
8+
// Keep this simple; win32 platform naming is already explicitly handled elsewhere.
9+
return;
10+
}
11+
12+
const mismatchedOs = process.platform === "darwin" ? "linux" : "darwin";
13+
14+
const entry: SkillEntry = {
15+
skill: {
16+
name: "os-scoped",
17+
description: "test",
18+
source: "test",
19+
filePath: "/tmp/os-scoped",
20+
baseDir: "/tmp",
21+
},
22+
frontmatter: {},
23+
metadata: {
24+
os: [mismatchedOs],
25+
requires: { bins: ["fakebin"] },
26+
install: [
27+
{
28+
id: "brew",
29+
kind: "brew",
30+
formula: "fake",
31+
bins: ["fakebin"],
32+
label: "Install fake (brew)",
33+
},
34+
],
35+
},
36+
};
37+
38+
const report = buildWorkspaceSkillStatus("/tmp/ws", { entries: [entry] });
39+
expect(report.skills).toHaveLength(1);
40+
expect(report.skills[0]?.install).toEqual([]);
41+
});
42+
});

‎src/agents/skills-status.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@ function normalizeInstallOptions(
111111
entry: SkillEntry,
112112
prefs: SkillsInstallPreferences,
113113
): SkillInstallOption[] {
114+
// If the skill is explicitly OS-scoped, don't surface install actions on unsupported platforms.
115+
// (Installers run locally; remote OS eligibility is handled separately.)
116+
const requiredOs = entry.metadata?.os ?? [];
117+
if (requiredOs.length > 0 && !requiredOs.includes(process.platform)) {
118+
return [];
119+
}
120+
114121
const install = entry.metadata?.install ?? [];
115122
if (install.length === 0) {
116123
return [];

‎src/commands/onboard-skills.test.ts‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import type { OpenClawConfig } from "../config/config.js";
3+
import type { RuntimeEnv } from "../runtime.js";
4+
import type { WizardPrompter } from "../wizard/prompts.js";
5+
6+
// Module under test imports these at module scope.
7+
vi.mock("../agents/skills-status.js", () => ({
8+
buildWorkspaceSkillStatus: vi.fn(),
9+
}));
10+
vi.mock("../agents/skills-install.js", () => ({
11+
installSkill: vi.fn(),
12+
}));
13+
vi.mock("./onboard-helpers.js", () => ({
14+
detectBinary: vi.fn(),
15+
resolveNodeManagerOptions: vi.fn(() => [
16+
{ value: "npm", label: "npm" },
17+
{ value: "pnpm", label: "pnpm" },
18+
{ value: "bun", label: "bun" },
19+
]),
20+
}));
21+
22+
import { installSkill } from "../agents/skills-install.js";
23+
import { buildWorkspaceSkillStatus } from "../agents/skills-status.js";
24+
import { detectBinary } from "./onboard-helpers.js";
25+
import { setupSkills } from "./onboard-skills.js";
26+
27+
function createPrompter(params: {
28+
configure?: boolean;
29+
showBrewInstall?: boolean;
30+
multiselect?: string[];
31+
}): { prompter: WizardPrompter; notes: Array<{ title?: string; message: string }> } {
32+
const notes: Array<{ title?: string; message: string }> = [];
33+
34+
const confirmAnswers: boolean[] = [];
35+
confirmAnswers.push(params.configure ?? true);
36+
37+
const prompter: WizardPrompter = {
38+
intro: vi.fn(async () => {}),
39+
outro: vi.fn(async () => {}),
40+
note: vi.fn(async (message: string, title?: string) => {
41+
notes.push({ title, message });
42+
}),
43+
select: vi.fn(async () => "npm"),
44+
multiselect: vi.fn(async () => params.multiselect ?? ["__skip__"]),
45+
text: vi.fn(async () => ""),
46+
confirm: vi.fn(async ({ message }) => {
47+
if (message === "Show Homebrew install command?") {
48+
return params.showBrewInstall ?? false;
49+
}
50+
return confirmAnswers.shift() ?? false;
51+
}),
52+
progress: vi.fn(() => ({ update: vi.fn(), stop: vi.fn() })),
53+
};
54+
55+
return { prompter, notes };
56+
}
57+
58+
const runtime: RuntimeEnv = {
59+
log: vi.fn(),
60+
error: vi.fn(),
61+
exit: ((code: number) => {
62+
throw new Error(`unexpected exit ${code}`);
63+
}) as RuntimeEnv["exit"],
64+
};
65+
66+
describe("setupSkills", () => {
67+
it("does not recommend Homebrew when user skips installing brew-backed deps", async () => {
68+
if (process.platform === "win32") {
69+
return;
70+
}
71+
72+
vi.mocked(detectBinary).mockResolvedValue(false);
73+
vi.mocked(installSkill).mockResolvedValue({
74+
ok: true,
75+
message: "Installed",
76+
stdout: "",
77+
stderr: "",
78+
code: 0,
79+
});
80+
vi.mocked(buildWorkspaceSkillStatus).mockReturnValue({
81+
workspaceDir: "/tmp/ws",
82+
managedSkillsDir: "/tmp/managed",
83+
skills: [
84+
{
85+
name: "apple-reminders",
86+
description: "macOS-only",
87+
source: "openclaw-bundled",
88+
bundled: true,
89+
filePath: "/tmp/skills/apple-reminders",
90+
baseDir: "/tmp/skills/apple-reminders",
91+
skillKey: "apple-reminders",
92+
always: false,
93+
disabled: false,
94+
blockedByAllowlist: false,
95+
eligible: false,
96+
requirements: { bins: ["remindctl"], anyBins: [], env: [], config: [], os: ["darwin"] },
97+
missing: { bins: ["remindctl"], anyBins: [], env: [], config: [], os: ["darwin"] },
98+
configChecks: [],
99+
install: [
100+
{ id: "brew", kind: "brew", label: "Install remindctl (brew)", bins: ["remindctl"] },
101+
],
102+
},
103+
{
104+
name: "video-frames",
105+
description: "ffmpeg",
106+
source: "openclaw-bundled",
107+
bundled: true,
108+
filePath: "/tmp/skills/video-frames",
109+
baseDir: "/tmp/skills/video-frames",
110+
skillKey: "video-frames",
111+
always: false,
112+
disabled: false,
113+
blockedByAllowlist: false,
114+
eligible: false,
115+
requirements: { bins: ["ffmpeg"], anyBins: [], env: [], config: [], os: [] },
116+
missing: { bins: ["ffmpeg"], anyBins: [], env: [], config: [], os: [] },
117+
configChecks: [],
118+
install: [{ id: "brew", kind: "brew", label: "Install ffmpeg (brew)", bins: ["ffmpeg"] }],
119+
},
120+
],
121+
});
122+
123+
const { prompter, notes } = createPrompter({ multiselect: ["__skip__"] });
124+
await setupSkills({} as OpenClawConfig, "/tmp/ws", runtime, prompter);
125+
126+
// OS-mismatched skill should be counted as unsupported, not installable/missing.
127+
const status = notes.find((n) => n.title === "Skills status")?.message ?? "";
128+
expect(status).toContain("Unsupported on this OS: 1");
129+
130+
const brewNote = notes.find((n) => n.title === "Homebrew recommended");
131+
expect(brewNote).toBeUndefined();
132+
});
133+
134+
it("recommends Homebrew when user selects a brew-backed install and brew is missing", async () => {
135+
if (process.platform === "win32") {
136+
return;
137+
}
138+
139+
vi.mocked(detectBinary).mockResolvedValue(false);
140+
vi.mocked(installSkill).mockResolvedValue({
141+
ok: true,
142+
message: "Installed",
143+
stdout: "",
144+
stderr: "",
145+
code: 0,
146+
});
147+
vi.mocked(buildWorkspaceSkillStatus).mockReturnValue({
148+
workspaceDir: "/tmp/ws",
149+
managedSkillsDir: "/tmp/managed",
150+
skills: [
151+
{
152+
name: "video-frames",
153+
description: "ffmpeg",
154+
source: "openclaw-bundled",
155+
bundled: true,
156+
filePath: "/tmp/skills/video-frames",
157+
baseDir: "/tmp/skills/video-frames",
158+
skillKey: "video-frames",
159+
always: false,
160+
disabled: false,
161+
blockedByAllowlist: false,
162+
eligible: false,
163+
requirements: { bins: ["ffmpeg"], anyBins: [], env: [], config: [], os: [] },
164+
missing: { bins: ["ffmpeg"], anyBins: [], env: [], config: [], os: [] },
165+
configChecks: [],
166+
install: [{ id: "brew", kind: "brew", label: "Install ffmpeg (brew)", bins: ["ffmpeg"] }],
167+
},
168+
],
169+
});
170+
171+
const { prompter, notes } = createPrompter({ multiselect: ["video-frames"] });
172+
await setupSkills({} as OpenClawConfig, "/tmp/ws", runtime, prompter);
173+
174+
const brewNote = notes.find((n) => n.title === "Homebrew recommended");
175+
expect(brewNote).toBeDefined();
176+
});
177+
});

‎src/commands/onboard-skills.ts‎

Lines changed: 61 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,19 @@ export async function setupSkills(
5555
): Promise<OpenClawConfig> {
5656
const report = buildWorkspaceSkillStatus(workspaceDir, { config: cfg });
5757
const eligible = report.skills.filter((s) => s.eligible);
58-
const missing = report.skills.filter((s) => !s.eligible && !s.disabled && !s.blockedByAllowlist);
58+
const unsupportedOs = report.skills.filter(
59+
(s) => !s.disabled && !s.blockedByAllowlist && s.missing.os.length > 0,
60+
);
61+
const missing = report.skills.filter(
62+
(s) => !s.eligible && !s.disabled && !s.blockedByAllowlist && s.missing.os.length === 0,
63+
);
5964
const blocked = report.skills.filter((s) => s.blockedByAllowlist);
6065

61-
const needsBrewPrompt =
62-
process.platform !== "win32" &&
63-
report.skills.some((skill) => skill.install.some((option) => option.kind === "brew")) &&
64-
!(await detectBinary("brew"));
65-
6666
await prompter.note(
6767
[
6868
`Eligible: ${eligible.length}`,
6969
`Missing requirements: ${missing.length}`,
70+
`Unsupported on this OS: ${unsupportedOs.length}`,
7071
`Blocked by allowlist: ${blocked.length}`,
7172
].join("\n"),
7273
"Skills status",
@@ -80,48 +81,10 @@ export async function setupSkills(
8081
return cfg;
8182
}
8283

83-
if (needsBrewPrompt) {
84-
await prompter.note(
85-
[
86-
"Many skill dependencies are shipped via Homebrew.",
87-
"Without brew, you'll need to build from source or download releases manually.",
88-
].join("\n"),
89-
"Homebrew recommended",
90-
);
91-
const showBrewInstall = await prompter.confirm({
92-
message: "Show Homebrew install command?",
93-
initialValue: true,
94-
});
95-
if (showBrewInstall) {
96-
await prompter.note(
97-
[
98-
"Run:",
99-
'/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"',
100-
].join("\n"),
101-
"Homebrew install",
102-
);
103-
}
104-
}
105-
106-
const nodeManager = (await prompter.select({
107-
message: "Preferred node manager for skill installs",
108-
options: resolveNodeManagerOptions(),
109-
})) as "npm" | "pnpm" | "bun";
110-
111-
let next: OpenClawConfig = {
112-
...cfg,
113-
skills: {
114-
...cfg.skills,
115-
install: {
116-
...cfg.skills?.install,
117-
nodeManager,
118-
},
119-
},
120-
};
121-
12284
const installable = missing.filter(
12385
(skill) => skill.install.length > 0 && skill.missing.bins.length > 0,
12486
);
87+
let next: OpenClawConfig = cfg;
12588
if (installable.length > 0) {
12689
const toInstall = await prompter.multiselect({
12790
message: "Install missing skill dependencies",
@@ -140,6 +103,59 @@ export async function setupSkills(
140103
});
141104

142105
const selected = toInstall.filter((name) => name !== "__skip__");
106+
107+
const selectedSkills = selected
108+
.map((name) => installable.find((s) => s.name === name))
109+
.filter((item): item is (typeof installable)[number] => Boolean(item));
110+
111+
const needsBrewPrompt =
112+
process.platform !== "win32" &&
113+
selectedSkills.some((skill) => skill.install.some((option) => option.kind === "brew")) &&
114+
!(await detectBinary("brew"));
115+
116+
if (needsBrewPrompt) {
117+
await prompter.note(
118+
[
119+
"Many skill dependencies are shipped via Homebrew.",
120+
"Without brew, you'll need to build from source or download releases manually.",
121+
].join("\n"),
122+
"Homebrew recommended",
123+
);
124+
const showBrewInstall = await prompter.confirm({
125+
message: "Show Homebrew install command?",
126+
initialValue: true,
127+
});
128+
if (showBrewInstall) {
129+
await prompter.note(
130+
[
131+
"Run:",
132+
'/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"',
133+
].join("\n"),
134+
"Homebrew install",
135+
);
136+
}
137+
}
138+
139+
const needsNodeManagerPrompt = selectedSkills.some((skill) =>
140+
skill.install.some((option) => option.kind === "node"),
141+
);
142+
if (needsNodeManagerPrompt) {
143+
const nodeManager = (await prompter.select({
144+
message: "Preferred node manager for skill installs",
145+
options: resolveNodeManagerOptions(),
146+
})) as "npm" | "pnpm" | "bun";
147+
next = {
148+
...next,
149+
skills: {
150+
...next.skills,
151+
install: {
152+
...next.skills?.install,
153+
nodeManager,
154+
},
155+
},
156+
};
157+
}
158+
143159
for (const name of selected) {
144160
const target = installable.find((s) => s.name === name);
145161
if (!target || target.install.length === 0) {

0 commit comments

Comments
 (0)