diff --git a/src/claws/export.test.ts b/src/claws/export.test.ts index f915b9fbe503..a10b0c423dfb 100644 --- a/src/claws/export.test.ts +++ b/src/claws/export.test.ts @@ -152,11 +152,12 @@ async function installedFixture( ok: true as const, plan: { workspaceDir: plan.agent.workspace, - slug: "@acme/triage", + requestedRef: "@acme/triage", + slug: "triage", version: "2.0.0", installedAt: 0, - targetDir: join(plan.agent.workspace, "skills", "@acme", "triage"), - skillFilePath: join(plan.agent.workspace, "skills", "@acme", "triage", "SKILL.md"), + targetDir: join(plan.agent.workspace, "skills", "triage"), + skillFilePath: join(plan.agent.workspace, "skills", "triage", "SKILL.md"), skillFileSha256: "a".repeat(64), fileTreeSha256: `sha256:${"a".repeat(64)}`, }, diff --git a/src/claws/package-remove.test.ts b/src/claws/package-remove.test.ts index fb0d7152e790..11996f109d6e 100644 --- a/src/claws/package-remove.test.ts +++ b/src/claws/package-remove.test.ts @@ -1,7 +1,14 @@ -import { describe, expect, it, vi } from "vitest"; +import { createHash } from "node:crypto"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; +import { digestClawHubSkillTree } from "../skills/lifecycle/skill-tree-digest.js"; import { applyClawPackageRemovals, planClawPackageRemovals } from "./package-remove.js"; import type { PersistedClawInstall, PersistedClawPackageRef } from "./provenance.js"; +const tempDirs = useAutoCleanupTempDirTracker(afterEach); + const install = { workspace: "/tmp/claw-workspace", } as PersistedClawInstall; @@ -50,6 +57,51 @@ function packageRefStore(...initial: PersistedClawPackageRef[]) { }; } +async function trackedQualifiedSkillFixture() { + const workspaceDir = tempDirs.make("openclaw-claw-skill-remove-"); + const slug = "triage"; + const skillDir = join(workspaceDir, "skills", slug); + const content = "---\nname: triage\ndescription: Triage incidents\n---\n"; + const sha256 = createHash("sha256").update(content).digest("hex"); + const installedAt = 1; + const registry = "https://clawhub.ai"; + const ownerHandle = "owner"; + await mkdir(join(skillDir, ".clawhub"), { recursive: true }); + await mkdir(join(workspaceDir, ".clawhub"), { recursive: true }); + await writeFile(join(skillDir, "SKILL.md"), content); + const fileTreeSha256 = await digestClawHubSkillTree(skillDir); + const trackedMetadata = { + registry, + ownerHandle, + installedAt, + skillFile: { path: "SKILL.md", sha256 }, + fileTreeSha256, + }; + await writeFile( + join(skillDir, ".clawhub", "origin.json"), + JSON.stringify({ + version: 1, + slug, + installedVersion: "1.0.0", + ...trackedMetadata, + }), + ); + const lockPath = join(workspaceDir, ".clawhub", "lock.json"); + await writeFile( + lockPath, + JSON.stringify({ + version: 1, + skills: { + [slug]: { + version: "1.0.0", + ...trackedMetadata, + }, + }, + }), + ); + return { workspaceDir, slug, skillDir, lockPath }; +} + describe("Claw package removal", () => { it("retains referenced plugins by default while releasing the Claw reference", async () => { const ref = packageRef(); @@ -306,6 +358,40 @@ describe("Claw package removal", () => { ]); }); + it("removes a persisted owner-qualified skill through its local install identity", async () => { + const current = await trackedQualifiedSkillFixture(); + const currentInstall = { ...install, workspace: current.workspaceDir }; + const ref = packageRef({ + kind: "skill", + ref: "@owner/triage", + relationship: "managed", + }); + const store = packageRefStore(ref); + + const decisions = await planClawPackageRemovals(currentInstall, [ref], { + deps: store, + }); + + expect(decisions).toMatchObject([ + { + action: "uninstall", + skillPlan: { + requestedRef: "@owner/triage", + slug: "triage", + targetDir: current.skillDir, + }, + }, + ]); + await expect(applyClawPackageRemovals(decisions, { deps: store })).resolves.toMatchObject([ + { action: "uninstalled" }, + ]); + await expect(readFile(join(current.skillDir, "SKILL.md"), "utf8")).rejects.toThrow(); + const lock = JSON.parse(await readFile(current.lockPath, "utf8")) as { + skills: Record; + }; + expect(lock.skills).toEqual({}); + }); + it("treats equal skill refs in separate agent workspaces as separate artifacts", async () => { const ref = packageRef({ kind: "skill", ref: "triage", relationship: "managed" }); const other = packageRef({ @@ -316,12 +402,14 @@ describe("Claw package removal", () => { }); const skillPlan = { workspaceDir: install.workspace, + requestedRef: "triage", slug: "triage", version: "1.0.0", installedAt: 1, targetDir: "/tmp/claw-workspace/skills/triage", skillFilePath: "SKILL.md", skillFileSha256: "abc", + fileTreeSha256: "def", }; const decisions = await planClawPackageRemovals(install, [ref], { deps: { diff --git a/src/claws/update-plan.test.ts b/src/claws/update-plan.test.ts index 4023cd4fd701..f691c253031d 100644 --- a/src/claws/update-plan.test.ts +++ b/src/claws/update-plan.test.ts @@ -728,6 +728,7 @@ describe("buildClawUpdatePlan", () => { ok: true as const, plan: { workspaceDir: current.addPlan.agent.workspace, + requestedRef: "triage", slug: "triage", version: "1.0.0", installedAt: 0, diff --git a/src/skills/lifecycle/clawhub-install-core.ts b/src/skills/lifecycle/clawhub-install-core.ts index 04366c6f6c70..3e11c675091f 100644 --- a/src/skills/lifecycle/clawhub-install-core.ts +++ b/src/skills/lifecycle/clawhub-install-core.ts @@ -649,7 +649,7 @@ export async function performClawHubSkillInstall( markClawPackageIndependentlyOwned({ kind: "skill", source: "clawhub", - ref: params.slug, + ref: formatClawHubSkillRef(params), version, workspace: params.workspaceDir, }); diff --git a/src/skills/lifecycle/clawhub-uninstall.test.ts b/src/skills/lifecycle/clawhub-uninstall.test.ts index 96bd6e87497b..fbd7a95b5716 100644 --- a/src/skills/lifecycle/clawhub-uninstall.test.ts +++ b/src/skills/lifecycle/clawhub-uninstall.test.ts @@ -22,6 +22,7 @@ async function fixture() { const sha256 = createHash("sha256").update(content).digest("hex"); const installedAt = 123; const registry = "https://clawhub.ai"; + const ownerHandle = "owner"; await mkdir(join(skillDir, ".clawhub"), { recursive: true }); await mkdir(join(workspaceDir, ".clawhub"), { recursive: true }); await writeFile(join(skillDir, "SKILL.md"), content); @@ -34,6 +35,7 @@ async function fixture() { slug, installedVersion: "1.0.0", installedAt, + ownerHandle, skillFile: { path: "SKILL.md", sha256 }, fileTreeSha256, }), @@ -47,13 +49,37 @@ async function fixture() { version: "1.0.0", registry, installedAt, + ownerHandle, skillFile: { path: "SKILL.md", sha256 }, fileTreeSha256, }, }, }), ); - return { workspaceDir, slug, skillDir }; + return { + workspaceDir, + slug, + skillDir, + originPath: join(skillDir, ".clawhub", "origin.json"), + lockPath: join(workspaceDir, ".clawhub", "lock.json"), + }; +} + +async function replaceTrackedOwner( + current: Awaited>, + ownerHandle: string, +) { + const origin = JSON.parse(await readFile(current.originPath, "utf8")) as { + ownerHandle: string; + }; + origin.ownerHandle = ownerHandle; + await writeFile(current.originPath, JSON.stringify(origin)); + + const lock = JSON.parse(await readFile(current.lockPath, "utf8")) as { + skills: Record; + }; + lock.skills[current.slug]!.ownerHandle = ownerHandle; + await writeFile(current.lockPath, JSON.stringify(lock)); } describe("ClawHub skill uninstall lifecycle", () => { @@ -66,7 +92,10 @@ describe("ClawHub skill uninstall lifecycle", () => { slug: current.slug, expectedVersion: "1.0.0", }); - expect(planned).toMatchObject({ ok: true, plan: { slug: "triage", version: "1.0.0" } }); + expect(planned).toMatchObject({ + ok: true, + plan: { requestedRef: "triage", slug: "triage", version: "1.0.0" }, + }); if (!planned.ok) { throw new Error(planned.error); } @@ -95,6 +124,73 @@ describe("ClawHub skill uninstall lifecycle", () => { }); }); + it("removes an owner-qualified tracked skill by its local slug", async () => { + const current = await fixture(); + const planned = await planClawHubSkillUninstall({ + workspaceDir: current.workspaceDir, + slug: "@owner/triage", + expectedVersion: "1.0.0", + }); + + expect(planned).toMatchObject({ + ok: true, + plan: { requestedRef: "@owner/triage", slug: "triage", version: "1.0.0" }, + }); + if (!planned.ok) { + throw new Error(planned.error); + } + await expect(applyClawHubSkillUninstall(planned.plan)).resolves.toEqual({ ok: true }); + await expect(readFile(join(current.skillDir, "SKILL.md"), "utf8")).rejects.toThrow(); + const lock = JSON.parse(await readFile(current.lockPath, "utf8")) as { + skills: Record; + }; + expect(lock.skills).toEqual({}); + }); + + it("rejects an owner-qualified ref for a different tracked publisher", async () => { + const current = await fixture(); + + await expect( + planClawHubSkillUninstall({ + workspaceDir: current.workspaceDir, + slug: "@other/triage", + expectedVersion: "1.0.0", + }), + ).resolves.toEqual({ + ok: false, + code: "ambiguous", + error: 'Skill "triage" is tracked as @owner/triage, not @other/triage.', + }); + await expect(readFile(join(current.skillDir, "SKILL.md"), "utf8")).resolves.toContain( + "name: triage", + ); + const lock = JSON.parse(await readFile(current.lockPath, "utf8")) as { + skills: Record; + }; + expect(lock.skills.triage).toBeDefined(); + }); + + it("revalidates the requested publisher before applying removal", async () => { + const current = await fixture(); + const planned = await planClawHubSkillUninstall({ + workspaceDir: current.workspaceDir, + slug: "@owner/triage", + expectedVersion: "1.0.0", + }); + if (!planned.ok) { + throw new Error(planned.error); + } + await replaceTrackedOwner(current, "other"); + + await expect(applyClawHubSkillUninstall(planned.plan)).resolves.toEqual({ + ok: false, + error: 'Skill "triage" is tracked as @other/triage, not @owner/triage.', + }); + await expect(readFile(join(current.skillDir, "SKILL.md"), "utf8")).resolves.toContain( + "name: triage", + ); + }); + it("retains a locally modified skill", async () => { const current = await fixture(); await writeFile(join(current.skillDir, "SKILL.md"), "operator edit\n"); diff --git a/src/skills/lifecycle/clawhub-uninstall.ts b/src/skills/lifecycle/clawhub-uninstall.ts index c9b57d80e83d..dd288d7e76cb 100644 --- a/src/skills/lifecycle/clawhub-uninstall.ts +++ b/src/skills/lifecycle/clawhub-uninstall.ts @@ -2,7 +2,8 @@ import { randomUUID } from "node:crypto"; import fs from "node:fs/promises"; import path from "node:path"; import { sha256Hex } from "../../infra/crypto-digest.js"; -import { normalizeTrackedSkillSlug, resolveWorkspaceSkillInstallDir } from "./archive-install.js"; +import { resolveWorkspaceSkillInstallDir } from "./archive-install.js"; +import { formatClawHubSkillRef, parseRequestedClawHubSkillRef } from "./clawhub-store.js"; import { resolveClawHubSkillStatusLinkSync, untrackClawHubSkill } from "./clawhub.js"; import { dispatchCommittedSkillChangeBestEffort, @@ -13,6 +14,8 @@ import { digestClawHubSkillTree } from "./skill-tree-digest.js"; export type ClawHubSkillUninstallPlan = { workspaceDir: string; + // Replan from the registry identity so publisher/source changes cannot retarget deletion. + requestedRef: string; slug: string; version: string; installedAt: number; @@ -35,12 +38,13 @@ export async function planClawHubSkillUninstall(params: { slug: string; expectedVersion: string; }): Promise { - let slug: string; + let requestedRef: ReturnType; try { - slug = normalizeTrackedSkillSlug(params.slug); + requestedRef = parseRequestedClawHubSkillRef(params.slug); } catch (error) { return { ok: false, code: "ambiguous", error: String(error) }; } + const slug = requestedRef.slug; const targetDir = resolveWorkspaceSkillInstallDir(params.workspaceDir, slug); const link = resolveClawHubSkillStatusLinkSync({ workspaceDir: params.workspaceDir, @@ -63,6 +67,24 @@ export async function planClawHubSkillUninstall(params: { : link.reason, }; } + if (requestedRef.ownerHandle && link.ownerHandle !== requestedRef.ownerHandle) { + const trackedRef = link.ownerHandle ? `@${link.ownerHandle}/${slug}` : slug; + return { + ok: false, + code: "ambiguous", + error: `Skill ${JSON.stringify(slug)} is tracked as ${trackedRef}, not @${requestedRef.ownerHandle}/${slug}.`, + }; + } + if ( + requestedRef.requestedReference && + link.requestedReference !== requestedRef.requestedReference + ) { + return { + ok: false, + code: "ambiguous", + error: `Skill ${JSON.stringify(slug)} is not tracked from ${requestedRef.requestedReference}.`, + }; + } if (link.installedVersion !== params.expectedVersion) { return { ok: false, @@ -109,6 +131,7 @@ export async function planClawHubSkillUninstall(params: { ok: true, plan: { workspaceDir: params.workspaceDir, + requestedRef: requestedRef.requestedReference ?? formatClawHubSkillRef(requestedRef), slug, version: link.installedVersion, installedAt: link.installedAt, @@ -131,7 +154,7 @@ export async function applyClawHubSkillUninstall( ): Promise<{ ok: true } | { ok: false; error: string }> { const current = await planClawHubSkillUninstall({ workspaceDir: plan.workspaceDir, - slug: plan.slug, + slug: plan.requestedRef, expectedVersion: plan.version, }); if (!current.ok) { diff --git a/src/skills/lifecycle/clawhub.test.ts b/src/skills/lifecycle/clawhub.test.ts index e8afc5e4c9e1..c07342b556d3 100644 --- a/src/skills/lifecycle/clawhub.test.ts +++ b/src/skills/lifecycle/clawhub.test.ts @@ -25,6 +25,7 @@ const installPackageDirMock = vi.fn(); const evaluateSkillInstallPolicyMock = vi.fn(); const pathExistsMock = vi.fn(); const digestClawHubSkillTreeMock = vi.fn(async () => `sha256:${"a".repeat(64)}`); +const markClawPackageIndependentlyOwnedMock = vi.fn(); const tempDirs = createTrackedTempDirs(); vi.mock("../../infra/clawhub.js", async (importOriginal) => ({ @@ -66,6 +67,10 @@ vi.mock("./skill-tree-digest.js", () => ({ digestClawHubSkillTree: digestClawHubSkillTreeMock, })); +vi.mock("../../state/claw-package-adoption.js", () => ({ + markClawPackageIndependentlyOwned: markClawPackageIndependentlyOwnedMock, +})); + const { installSkillFromClawHub, preflightSkillFromClawHub, @@ -211,6 +216,7 @@ describe("skills-clawhub", () => { installPackageDirMock.mockReset(); evaluateSkillInstallPolicyMock.mockReset(); pathExistsMock.mockReset(); + markClawPackageIndependentlyOwnedMock.mockReset(); resolveClawHubBaseUrlMock.mockImplementation((baseUrl?: string) => (baseUrl ?? "https://clawhub.ai").replace(/\/+$/, ""), @@ -1137,6 +1143,13 @@ describe("skills-clawhub", () => { ownerHandle: "demo-owner", installedVersion: "1.0.0", }); + expect(markClawPackageIndependentlyOwnedMock).toHaveBeenCalledWith({ + kind: "skill", + source: "clawhub", + ref: "@demo-owner/weather", + version: "1.0.0", + workspace: workspaceDir, + }); expect(reportClawHubSkillInstallTelemetryMock).toHaveBeenCalledWith({ baseUrl: undefined, slug: "weather",