From 6f0f7574f01988f7671dd90f856a55756f38ce48 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Wed, 1 Jul 2026 10:54:01 -0500 Subject: [PATCH] fix(skills): install all default sandbox skills (build-os ref + resilient install) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs left task sandboxes with only 2 of 6 default skills: - DEFAULT_GLOBAL_SKILL_REFS listed `recoup-platform-build-workspace`, which was renamed to `recoup-platform-build-os` → `npx skills add` 404s on it. - installGlobalSkills threw on the first failed skill, so that bad ref aborted the loop and every skill after it (all 3 recoup-roster-*) never installed. Fix the ref and make installGlobalSkills best-effort per skill (log + continue), so one bad ref can't block the rest. Verified on the Apache→OneRPM run (recoupable/chat#1833): loading recoup-roster-list-artists returned "Skill not found. Available: recoup-platform-api-access, recoup-platform-email-helper". Co-Authored-By: Claude Opus 4.8 (1M context) --- .../__tests__/defaultGlobalSkillRefs.test.ts | 2 +- lib/skills/__tests__/installGlobalSkills.test.ts | 15 +++++++++++---- lib/skills/defaultGlobalSkillRefs.ts | 2 +- lib/skills/installGlobalSkills.ts | 9 +++++---- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/skills/__tests__/defaultGlobalSkillRefs.test.ts b/lib/skills/__tests__/defaultGlobalSkillRefs.test.ts index b4b481b70..361aa54f0 100644 --- a/lib/skills/__tests__/defaultGlobalSkillRefs.test.ts +++ b/lib/skills/__tests__/defaultGlobalSkillRefs.test.ts @@ -6,7 +6,7 @@ describe("DEFAULT_GLOBAL_SKILL_REFS", () => { const sourceNames = DEFAULT_GLOBAL_SKILL_REFS.map(r => `${r.source}::${r.skillName}`); expect(sourceNames).toContain("recoupable/skills::recoup-platform-api-access"); expect(sourceNames).toContain("recoupable/skills::recoup-platform-email-helper"); - expect(sourceNames).toContain("recoupable/skills::recoup-platform-build-workspace"); + expect(sourceNames).toContain("recoupable/skills::recoup-platform-build-os"); expect(sourceNames).toContain("recoupable/skills::recoup-roster-add-artist"); expect(sourceNames).toContain("recoupable/skills::recoup-roster-list-artists"); expect(sourceNames).toContain("recoupable/skills::recoup-roster-manage-artist"); diff --git a/lib/skills/__tests__/installGlobalSkills.test.ts b/lib/skills/__tests__/installGlobalSkills.test.ts index a532d2792..374c83924 100644 --- a/lib/skills/__tests__/installGlobalSkills.test.ts +++ b/lib/skills/__tests__/installGlobalSkills.test.ts @@ -52,7 +52,9 @@ describe("installGlobalSkills", () => { expect(exec).toHaveBeenCalledTimes(1); }); - it("throws when any install command fails", async () => { + it("continues past a failed skill and still installs the rest (best-effort, never throws)", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + // First skill fails; subsequent calls use the beforeEach success default. exec.mockResolvedValueOnce({ success: false, exitCode: 1, @@ -61,8 +63,13 @@ describe("installGlobalSkills", () => { truncated: false, }); - await expect(installGlobalSkills({ sandbox, globalSkillRefs: [REF_API] })).rejects.toThrow( - /package not found/, - ); + await expect( + installGlobalSkills({ sandbox, globalSkillRefs: [REF_API, REF_WORKSPACE] }), + ).resolves.toBeUndefined(); + + // A single bad skill must NOT block the rest — both are attempted. + expect(exec).toHaveBeenCalledTimes(2); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("package not found")); + errorSpy.mockRestore(); }); }); diff --git a/lib/skills/defaultGlobalSkillRefs.ts b/lib/skills/defaultGlobalSkillRefs.ts index b2cc1d6c1..2a1cab03b 100644 --- a/lib/skills/defaultGlobalSkillRefs.ts +++ b/lib/skills/defaultGlobalSkillRefs.ts @@ -17,7 +17,7 @@ export const DEFAULT_GLOBAL_SKILL_REFS: readonly GlobalSkillRef[] = [ // Reliable email send (Node helper) — pairs with the api-access skill. { source: "recoupable/skills", skillName: "recoup-platform-email-helper" }, // Folder-tree / RECOUP.md scaffolding + roster ops (was `artist-workspace`). - { source: "recoupable/skills", skillName: "recoup-platform-build-workspace" }, + { source: "recoupable/skills", skillName: "recoup-platform-build-os" }, { source: "recoupable/skills", skillName: "recoup-roster-add-artist" }, { source: "recoupable/skills", skillName: "recoup-roster-list-artists" }, { source: "recoupable/skills", skillName: "recoup-roster-manage-artist" }, diff --git a/lib/skills/installGlobalSkills.ts b/lib/skills/installGlobalSkills.ts index 4b0e825a4..7e3f3300a 100644 --- a/lib/skills/installGlobalSkills.ts +++ b/lib/skills/installGlobalSkills.ts @@ -8,9 +8,9 @@ const GLOBAL_SKILLS_INSTALL_TIMEOUT_MS = 120_000; /** * Installs the supplied skill refs into the sandbox by running * `npx skills add ...` for each one. Refs are validated and deduped - * via `globalSkillRefsSchema` before any command runs. Throws on the - * first failure — caller is expected to handle the error - * (typically best-effort: log and continue). + * via `globalSkillRefsSchema` before any command runs. Best-effort per + * skill: a single failed install is logged and skipped so it never blocks + * the remaining skills (a bad/renamed ref used to abort the whole batch). * * @param params.sandbox - The connected sandbox handle. * @param params.globalSkillRefs - Refs to install (defaults + user prefs). @@ -32,9 +32,10 @@ export async function installGlobalSkills(params: { ); if (!result.success) { - throw new Error( + console.error( `Failed to install global skill ${ref.skillName} from ${ref.source}: ${result.stderr || result.stdout || "unknown error"}`, ); + continue; } } }