mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 03:45:46 -06:00
fix(claws): remove owner-qualified skills (#119606)
* fix(claws): remove qualified skill refs * test(claws): update uninstall plan fixtures
This commit is contained in:
committed by
GitHub
parent
0d7baffc19
commit
359d8d867b
@@ -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)}`,
|
||||
},
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
};
|
||||
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: {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -649,7 +649,7 @@ export async function performClawHubSkillInstall(
|
||||
markClawPackageIndependentlyOwned({
|
||||
kind: "skill",
|
||||
source: "clawhub",
|
||||
ref: params.slug,
|
||||
ref: formatClawHubSkillRef(params),
|
||||
version,
|
||||
workspace: params.workspaceDir,
|
||||
});
|
||||
|
||||
@@ -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<ReturnType<typeof fixture>>,
|
||||
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<string, { ownerHandle: string }>;
|
||||
};
|
||||
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<string, unknown>;
|
||||
};
|
||||
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<string, unknown>;
|
||||
};
|
||||
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");
|
||||
|
||||
@@ -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<ClawHubSkillUninstallPlanResult> {
|
||||
let slug: string;
|
||||
let requestedRef: ReturnType<typeof parseRequestedClawHubSkillRef>;
|
||||
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) {
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user