mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 05:16:23 -06:00
refactor(skills): prepare reversible workspace writes
This commit is contained in:
Vincent Koc
@@ -0,0 +1,87 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { createTrackedTempDirs } from "../../test-utils/tracked-temp-dirs.js";
|
||||
import {
|
||||
applyWorkspaceSkillMutation,
|
||||
prepareWorkspaceSkillMutation,
|
||||
restoreWorkspaceSkillMutation,
|
||||
} from "./workspace-skill-write.js";
|
||||
|
||||
const tempDirs = createTrackedTempDirs();
|
||||
const symlinkPolicy = { allowWrites: false, allowedTargetRealPaths: [] };
|
||||
|
||||
afterEach(async () => {
|
||||
await tempDirs.cleanup();
|
||||
});
|
||||
|
||||
describe("workspace skill mutations", () => {
|
||||
it("removes support files when the activating SKILL.md write fails", async () => {
|
||||
const workspaceDir = await tempDirs.make("openclaw-workspace-skill-write-failure-");
|
||||
const skillDir = path.join(workspaceDir, "skills", "partial-create");
|
||||
const skillFile = path.join(skillDir, "SKILL.md");
|
||||
const supportFile = path.join(skillDir, "references", "proof.md");
|
||||
const mutation = await prepareWorkspaceSkillMutation({
|
||||
workspaceDir,
|
||||
skillDir,
|
||||
skillFile,
|
||||
content: "# Partial Create\n",
|
||||
supportFiles: [{ path: "references/proof.md", content: "new support\n" }],
|
||||
mode: "create",
|
||||
symlinkPolicy,
|
||||
});
|
||||
await fs.mkdir(skillFile, { recursive: true });
|
||||
|
||||
await expect(applyWorkspaceSkillMutation(mutation)).rejects.toThrow();
|
||||
await expect(fs.access(supportFile)).rejects.toThrow();
|
||||
});
|
||||
|
||||
it("restores the complete previous update bundle", async () => {
|
||||
const workspaceDir = await tempDirs.make("openclaw-workspace-skill-write-update-");
|
||||
const skillDir = path.join(workspaceDir, "skills", "reversible-update");
|
||||
const skillFile = path.join(skillDir, "SKILL.md");
|
||||
const supportFile = path.join(skillDir, "references", "proof.md");
|
||||
await fs.mkdir(path.dirname(supportFile), { recursive: true });
|
||||
await fs.writeFile(skillFile, "# Before\n", "utf8");
|
||||
await fs.writeFile(supportFile, "before support\n", "utf8");
|
||||
const mutation = await prepareWorkspaceSkillMutation({
|
||||
workspaceDir,
|
||||
skillDir,
|
||||
skillFile,
|
||||
content: "# After\n",
|
||||
supportFiles: [{ path: "references/proof.md", content: "after support\n" }],
|
||||
mode: "update",
|
||||
symlinkPolicy,
|
||||
});
|
||||
|
||||
await applyWorkspaceSkillMutation(mutation);
|
||||
await expect(fs.readFile(skillFile, "utf8")).resolves.toBe("# After\n");
|
||||
await expect(fs.readFile(supportFile, "utf8")).resolves.toBe("after support\n");
|
||||
|
||||
await restoreWorkspaceSkillMutation(mutation);
|
||||
await expect(fs.readFile(skillFile, "utf8")).resolves.toBe("# Before\n");
|
||||
await expect(fs.readFile(supportFile, "utf8")).resolves.toBe("before support\n");
|
||||
});
|
||||
|
||||
it("removes every file from a restored create mutation", async () => {
|
||||
const workspaceDir = await tempDirs.make("openclaw-workspace-skill-write-create-");
|
||||
const skillDir = path.join(workspaceDir, "skills", "reversible-create");
|
||||
const skillFile = path.join(skillDir, "SKILL.md");
|
||||
const supportFile = path.join(skillDir, "references", "proof.md");
|
||||
const mutation = await prepareWorkspaceSkillMutation({
|
||||
workspaceDir,
|
||||
skillDir,
|
||||
skillFile,
|
||||
content: "# Created\n",
|
||||
supportFiles: [{ path: "references/proof.md", content: "created support\n" }],
|
||||
mode: "create",
|
||||
symlinkPolicy,
|
||||
});
|
||||
|
||||
await applyWorkspaceSkillMutation(mutation);
|
||||
await restoreWorkspaceSkillMutation(mutation);
|
||||
|
||||
await expect(fs.access(skillFile)).rejects.toThrow();
|
||||
await expect(fs.access(supportFile)).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
@@ -9,11 +9,11 @@ const ALLOWED_SUPPORT_FILE_ROOTS = new Set(
|
||||
);
|
||||
export const MAX_WORKSPACE_SKILL_SUPPORT_FILE_BYTES = 256 * 1024;
|
||||
|
||||
type WorkspaceSkillSymlinkWritePolicy = {
|
||||
export type WorkspaceSkillSymlinkWritePolicy = {
|
||||
allowWrites: boolean;
|
||||
allowedTargetRealPaths: readonly string[];
|
||||
};
|
||||
type WorkspaceSkillSupportFileWrite = { path: string; content: string };
|
||||
export type WorkspaceSkillSupportFileWrite = { path: string; content: string };
|
||||
|
||||
type WorkspaceSkillWriteTargetParams = {
|
||||
workspaceDir: string;
|
||||
@@ -21,7 +21,21 @@ type WorkspaceSkillWriteTargetParams = {
|
||||
symlinkPolicy: WorkspaceSkillSymlinkWritePolicy;
|
||||
};
|
||||
|
||||
type PreviousSupportFile = { path: string; existed: boolean; previousContent?: string };
|
||||
type PreparedWorkspaceSkillFileMutation = {
|
||||
filePath: string;
|
||||
rootDir: string;
|
||||
relativePath: string;
|
||||
previousContent: string | null;
|
||||
content: string;
|
||||
};
|
||||
|
||||
export type PreparedWorkspaceSkillMutation = {
|
||||
mode: "create" | "update";
|
||||
workspaceDir: string;
|
||||
skillDir: string;
|
||||
skillFile: PreparedWorkspaceSkillFileMutation;
|
||||
supportFiles: Array<PreparedWorkspaceSkillFileMutation & { path: string }>;
|
||||
};
|
||||
|
||||
export function normalizeWorkspaceSkillSupportPath(input: string): string {
|
||||
const trimmed = input.trim();
|
||||
@@ -113,48 +127,111 @@ export async function writeWorkspaceSkill(params: {
|
||||
mode: "create" | "update";
|
||||
symlinkPolicy: WorkspaceSkillSymlinkWritePolicy;
|
||||
}): Promise<void> {
|
||||
const mutation = await prepareWorkspaceSkillMutation(params);
|
||||
await applyWorkspaceSkillMutation(mutation);
|
||||
}
|
||||
|
||||
export async function prepareWorkspaceSkillMutation(params: {
|
||||
workspaceDir: string;
|
||||
skillDir: string;
|
||||
skillFile: string;
|
||||
content: string;
|
||||
supportFiles?: readonly WorkspaceSkillSupportFileWrite[];
|
||||
mode: "create" | "update";
|
||||
symlinkPolicy: WorkspaceSkillSymlinkWritePolicy;
|
||||
}): Promise<PreparedWorkspaceSkillMutation> {
|
||||
assertInsideWorkspace(params.workspaceDir, params.skillDir, "skill directory");
|
||||
const supportFiles = normalizeSupportFiles(params.supportFiles ?? []);
|
||||
const previousSupportFiles = await prepareWorkspaceSkillWrite({
|
||||
const skillTarget = await resolveWorkspaceSkillWriteTarget({
|
||||
workspaceDir: params.workspaceDir,
|
||||
filePath: params.skillFile,
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
});
|
||||
const previousContent = await readWorkspaceSkillFile(params.skillFile);
|
||||
if (params.mode === "create" && previousContent !== null) {
|
||||
throw new Error(`Target skill already exists: ${params.skillFile}`);
|
||||
}
|
||||
if (params.mode === "update" && previousContent === null) {
|
||||
throw new Error(`Target skill is missing: ${params.skillFile}`);
|
||||
}
|
||||
|
||||
const preparedSupportFiles: PreparedWorkspaceSkillMutation["supportFiles"] = [];
|
||||
for (const file of supportFiles) {
|
||||
const filePath = path.join(params.skillDir, ...file.path.split("/"));
|
||||
const target = await resolveWorkspaceSkillWriteTarget({
|
||||
workspaceDir: params.workspaceDir,
|
||||
filePath,
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
});
|
||||
const previousSupportContent = await readWorkspaceSupportFile({
|
||||
skillDir: params.skillDir,
|
||||
relativePath: file.path,
|
||||
});
|
||||
if (params.mode === "create" && previousSupportContent !== null) {
|
||||
throw new Error(`Target support file already exists: ${filePath}`);
|
||||
}
|
||||
preparedSupportFiles.push({
|
||||
path: file.path,
|
||||
filePath,
|
||||
...target,
|
||||
previousContent: previousSupportContent,
|
||||
content: file.content,
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
mode: params.mode,
|
||||
workspaceDir: params.workspaceDir,
|
||||
skillDir: params.skillDir,
|
||||
skillFile: params.skillFile,
|
||||
supportFiles,
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
});
|
||||
skillFile: {
|
||||
filePath: params.skillFile,
|
||||
...skillTarget,
|
||||
previousContent,
|
||||
content: params.content,
|
||||
},
|
||||
supportFiles: preparedSupportFiles,
|
||||
};
|
||||
}
|
||||
|
||||
export async function applyWorkspaceSkillMutation(
|
||||
mutation: PreparedWorkspaceSkillMutation,
|
||||
): Promise<void> {
|
||||
const written: PreparedWorkspaceSkillFileMutation[] = [];
|
||||
const writtenSupportPaths: string[] = [];
|
||||
try {
|
||||
for (const file of supportFiles) {
|
||||
await writeWorkspaceFile({
|
||||
workspaceDir: params.workspaceDir,
|
||||
filePath: path.join(params.skillDir, ...file.path.split("/")),
|
||||
content: file.content,
|
||||
overwrite: params.mode === "update",
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
});
|
||||
for (const file of mutation.supportFiles) {
|
||||
await writePreparedWorkspaceFile(file, mutation.mode === "update");
|
||||
written.push(file);
|
||||
writtenSupportPaths.push(file.path);
|
||||
}
|
||||
await writeWorkspaceFile({
|
||||
workspaceDir: params.workspaceDir,
|
||||
filePath: params.skillFile,
|
||||
content: params.content,
|
||||
overwrite: params.mode === "update",
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
});
|
||||
await writePreparedWorkspaceFile(mutation.skillFile, mutation.mode === "update");
|
||||
} catch (error) {
|
||||
await restoreSupportFilesAfterFailedWrite({
|
||||
mode: params.mode,
|
||||
workspaceDir: params.workspaceDir,
|
||||
skillDir: params.skillDir,
|
||||
writtenSupportPaths,
|
||||
previousSupportFiles,
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
});
|
||||
try {
|
||||
await restorePreparedWorkspaceFiles(written.toReversed());
|
||||
} catch (restoreError) {
|
||||
const failure = new Error(
|
||||
`Skill write failed and ${writtenSupportPaths.length} support file restoration(s) failed.`,
|
||||
{ cause: error },
|
||||
);
|
||||
Object.assign(failure, { restoreError });
|
||||
throw failure;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
export async function restoreWorkspaceSkillMutation(
|
||||
mutation: PreparedWorkspaceSkillMutation,
|
||||
): Promise<void> {
|
||||
// SKILL.md is the activation marker: restore support first for updates, but
|
||||
// remove it first for failed creates so a partial new skill is not discoverable.
|
||||
const files =
|
||||
mutation.mode === "create"
|
||||
? [mutation.skillFile, ...mutation.supportFiles.toReversed()]
|
||||
: [...mutation.supportFiles.toReversed(), mutation.skillFile];
|
||||
await restorePreparedWorkspaceFiles(files);
|
||||
}
|
||||
|
||||
function normalizeSupportFiles(
|
||||
supportFiles: readonly WorkspaceSkillSupportFileWrite[],
|
||||
): WorkspaceSkillSupportFileWrite[] {
|
||||
@@ -166,110 +243,45 @@ function normalizeSupportFiles(
|
||||
return normalized;
|
||||
}
|
||||
|
||||
async function prepareWorkspaceSkillWrite(params: {
|
||||
mode: "create" | "update";
|
||||
workspaceDir: string;
|
||||
skillDir: string;
|
||||
skillFile: string;
|
||||
supportFiles: readonly WorkspaceSkillSupportFileWrite[];
|
||||
symlinkPolicy: WorkspaceSkillSymlinkWritePolicy;
|
||||
}): Promise<PreviousSupportFile[]> {
|
||||
await resolveWorkspaceSkillWriteTarget({
|
||||
workspaceDir: params.workspaceDir,
|
||||
filePath: params.skillFile,
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
});
|
||||
const previousContent = await readWorkspaceSkillFile(params.skillFile);
|
||||
if (params.mode === "create" && previousContent !== null) {
|
||||
throw new Error(`Target skill already exists: ${params.skillFile}`);
|
||||
}
|
||||
if (params.mode === "update" && previousContent === null) {
|
||||
throw new Error(`Target skill is missing: ${params.skillFile}`);
|
||||
}
|
||||
|
||||
const previousSupportFiles: PreviousSupportFile[] = [];
|
||||
for (const file of params.supportFiles) {
|
||||
const filePath = path.join(params.skillDir, ...file.path.split("/"));
|
||||
await resolveWorkspaceSkillWriteTarget({
|
||||
workspaceDir: params.workspaceDir,
|
||||
filePath,
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
});
|
||||
if (params.mode === "update") {
|
||||
const previousSupportContent = await readWorkspaceSupportFile({
|
||||
skillDir: params.skillDir,
|
||||
relativePath: file.path,
|
||||
});
|
||||
previousSupportFiles.push(
|
||||
previousSupportContent === null
|
||||
? { path: file.path, existed: false }
|
||||
: { path: file.path, existed: true, previousContent: previousSupportContent },
|
||||
);
|
||||
}
|
||||
}
|
||||
return previousSupportFiles;
|
||||
}
|
||||
|
||||
async function writeWorkspaceFile(params: {
|
||||
workspaceDir: string;
|
||||
filePath: string;
|
||||
content: string;
|
||||
overwrite: boolean;
|
||||
symlinkPolicy: WorkspaceSkillSymlinkWritePolicy;
|
||||
}): Promise<void> {
|
||||
const target = await resolveWorkspaceSkillWriteTarget(params);
|
||||
const targetRoot = await root(target.rootDir);
|
||||
await targetRoot.write(target.relativePath, params.content, {
|
||||
async function writePreparedWorkspaceFile(
|
||||
file: PreparedWorkspaceSkillFileMutation,
|
||||
overwrite: boolean,
|
||||
): Promise<void> {
|
||||
const targetRoot = await root(file.rootDir);
|
||||
await targetRoot.write(file.relativePath, file.content, {
|
||||
encoding: "utf8",
|
||||
mkdir: true,
|
||||
overwrite: params.overwrite,
|
||||
overwrite,
|
||||
});
|
||||
}
|
||||
|
||||
async function removeWorkspaceFile(params: {
|
||||
workspaceDir: string;
|
||||
filePath: string;
|
||||
symlinkPolicy: WorkspaceSkillSymlinkWritePolicy;
|
||||
}): Promise<void> {
|
||||
const target = await resolveWorkspaceSkillWriteTarget(params);
|
||||
const targetRoot = await root(target.rootDir);
|
||||
await targetRoot.remove(target.relativePath).catch((error: unknown) => {
|
||||
if ((error as { code?: string })?.code !== "ENOENT") {
|
||||
throw error;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
async function restoreSupportFilesAfterFailedWrite(params: {
|
||||
mode: "create" | "update";
|
||||
workspaceDir: string;
|
||||
skillDir: string;
|
||||
writtenSupportPaths: readonly string[];
|
||||
previousSupportFiles: readonly PreviousSupportFile[];
|
||||
symlinkPolicy: WorkspaceSkillSymlinkWritePolicy;
|
||||
}): Promise<void> {
|
||||
const previousByPath = new Map(params.previousSupportFiles.map((file) => [file.path, file]));
|
||||
await Promise.allSettled(
|
||||
params.writtenSupportPaths.toReversed().map(async (relativePath) => {
|
||||
const filePath = path.join(params.skillDir, ...relativePath.split("/"));
|
||||
const previous = previousByPath.get(relativePath);
|
||||
if (params.mode === "update" && previous?.existed) {
|
||||
await writeWorkspaceFile({
|
||||
workspaceDir: params.workspaceDir,
|
||||
filePath,
|
||||
content: previous.previousContent ?? "",
|
||||
overwrite: true,
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
async function restorePreparedWorkspaceFiles(
|
||||
files: readonly PreparedWorkspaceSkillFileMutation[],
|
||||
): Promise<void> {
|
||||
const errors: unknown[] = [];
|
||||
for (const file of files) {
|
||||
try {
|
||||
const targetRoot = await root(file.rootDir);
|
||||
if (file.previousContent === null) {
|
||||
await targetRoot.remove(file.relativePath).catch((error: unknown) => {
|
||||
if ((error as { code?: string })?.code !== "ENOENT") {
|
||||
throw error;
|
||||
}
|
||||
});
|
||||
} else {
|
||||
await removeWorkspaceFile({
|
||||
workspaceDir: params.workspaceDir,
|
||||
filePath,
|
||||
symlinkPolicy: params.symlinkPolicy,
|
||||
await targetRoot.write(file.relativePath, file.previousContent, {
|
||||
encoding: "utf8",
|
||||
mkdir: true,
|
||||
overwrite: true,
|
||||
});
|
||||
}
|
||||
}),
|
||||
);
|
||||
} catch (error) {
|
||||
errors.push(error);
|
||||
}
|
||||
}
|
||||
if (errors.length > 0) {
|
||||
throw new AggregateError(errors, "Failed to restore the previous workspace skill state.");
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveWorkspaceSkillWriteTarget(
|
||||
|
||||
Reference in New Issue
Block a user