diff --git a/src/commands/doctor-skill-workshop-sqlite.test.ts b/src/commands/doctor-skill-workshop-sqlite.test.ts index df4e101ba600..e8e146cf0c69 100644 --- a/src/commands/doctor-skill-workshop-sqlite.test.ts +++ b/src/commands/doctor-skill-workshop-sqlite.test.ts @@ -2,7 +2,12 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { renderProposalMarkdown } from "../skills/workshop/frontmatter.js"; -import { inspectSkillProposal, listSkillProposals } from "../skills/workshop/service.js"; +import { + applySkillProposal, + inspectSkillProposal, + listSkillProposals, + reviseSkillProposal, +} from "../skills/workshop/service.js"; import { hashSkillProposalContent, readSkillProposalRollback } from "../skills/workshop/store.js"; import { SKILL_WORKSHOP_ROLLBACK_SCHEMA, @@ -37,6 +42,105 @@ afterEach(async () => { }); describe("doctor Skill Workshop SQLite migration", () => { + it("preserves shipped v1 proposals through migration, revision, and apply", async () => { + const workspaceDir = await tempDirs.make("openclaw-workshop-shipped-upgrade-"); + const proposalId = "shipped-workshop-20260729-1234567890"; + const proposalDir = path.join(testState.stateDir, "skill-workshop", "proposals", proposalId); + const targetDir = path.join(workspaceDir, "skills", "shipped-workshop"); + const now = "2026-07-29T00:00:00.000Z"; + const supportContent = "Shipped support artifact.\n"; + const content = renderProposalMarkdown({ + name: "shipped-workshop", + description: "Proposal written by a shipped Workshop release", + content: "# Shipped Workshop\n\nOriginal proposal body.\n", + date: now, + }); + const record: SkillProposalRecord = { + schema: SKILL_WORKSHOP_SCHEMA, + id: proposalId, + kind: "create", + status: "pending", + title: "Create Shipped Workshop", + description: "Proposal written by a shipped Workshop release", + createdAt: now, + updatedAt: now, + createdBy: "skill-workshop", + origin: { + agentId: "main", + sessionKey: "agent:main:shipped-workshop", + runId: "shipped-run", + }, + originRunIds: ["shipped-run"], + originRunMutationCounts: { "shipped-run": 1 }, + proposedVersion: "v1", + draftFile: "PROPOSAL.md", + draftHash: hashSkillProposalContent(content), + supportFiles: [ + { + path: "references/proof.md", + sizeBytes: Buffer.byteLength(supportContent, "utf8"), + hash: hashSkillProposalContent(supportContent), + }, + ], + target: { + skillName: "Shipped Workshop", + skillKey: "shipped-workshop", + skillDir: targetDir, + skillFile: path.join(targetDir, "SKILL.md"), + source: "openclaw-workspace", + }, + scan: { + state: "clean", + scannedAt: now, + critical: 0, + warn: 0, + info: 0, + findings: [], + }, + }; + await fs.mkdir(path.join(proposalDir, "references"), { recursive: true }); + await fs.writeFile(path.join(proposalDir, "proposal.json"), JSON.stringify(record), "utf8"); + await fs.writeFile(path.join(proposalDir, "PROPOSAL.md"), content, "utf8"); + await fs.writeFile(path.join(proposalDir, "references", "proof.md"), supportContent, "utf8"); + + await expect( + migrateLegacySkillWorkshopProposals({ + config: { + agents: { + entries: { + main: { default: true, workspace: workspaceDir }, + }, + }, + }, + }), + ).resolves.toMatchObject({ detected: 1, migrated: 1, warnings: [] }); + + const revised = await reviseSkillProposal({ + workspaceDir, + agentId: "main", + proposalId, + content: "# Shipped Workshop\n\nRevised after upgrade.\n", + }); + expect(revised.record).toMatchObject({ + id: proposalId, + proposedVersion: "v2", + status: "pending", + supportFiles: [expect.objectContaining({ path: "references/proof.md" })], + }); + + await expect( + applySkillProposal({ workspaceDir, agentId: "main", proposalId }), + ).resolves.toMatchObject({ + record: { id: proposalId, status: "applied" }, + }); + await expect(fs.readFile(record.target.skillFile, "utf8")).resolves.toContain( + "Revised after upgrade.", + ); + await expect( + fs.readFile(path.join(record.target.skillDir, "references", "proof.md"), "utf8"), + ).resolves.toBe(supportContent); + }); + it("imports verified sidecars, preserves review artifacts, and removes legacy JSON", async () => { const oldWorkspace = await tempDirs.make("openclaw-workshop-old-workspace-"); const currentWorkspace = await tempDirs.make("openclaw-workshop-current-workspace-"); diff --git a/src/commands/doctor-skill-workshop-sqlite.ts b/src/commands/doctor-skill-workshop-sqlite.ts index abc5df95c482..abe341924f39 100644 --- a/src/commands/doctor-skill-workshop-sqlite.ts +++ b/src/commands/doctor-skill-workshop-sqlite.ts @@ -13,10 +13,10 @@ import { normalizeAgentId, resolveAgentIdFromSessionKey } from "../routing/sessi import { hashSkillProposalContent, importLegacySkillProposal, - parseSkillProposalRecord, - parseSkillProposalRollback, readSkillProposalRecord, readSkillProposalRollback, + validateSkillProposalRecord, + validateSkillProposalRollback, } from "../skills/workshop/store.js"; import type { SkillProposalRecord, SkillProposalRollback } from "../skills/workshop/types.js"; @@ -96,13 +96,16 @@ async function readLegacyRollback( proposalId: string, ): Promise { try { - const rollback = parseSkillProposalRollback( + const rollback = validateSkillProposalRollback( await readJson(stateRoot, `${PROPOSALS_DIR}/${proposalId}/rollback.json`, MAX_ROLLBACK_BYTES), ); - if (!rollback || rollback.proposalId !== proposalId) { + if (!rollback.ok) { + throw new Error(rollback.error.message); + } + if (rollback.value.proposalId !== proposalId) { throw new Error("invalid rollback metadata"); } - return rollback; + return rollback.value; } catch (error) { if (isNotFoundError(error)) { return undefined; @@ -139,10 +142,13 @@ async function migrateProposal(params: { stateRoot: Root; }): Promise<"imported" | "already-imported"> { const proposalDir = `${PROPOSALS_DIR}/${params.proposalId}`; - const record = parseSkillProposalRecord( + const record = validateSkillProposalRecord( await readJson(params.stateRoot, `${proposalDir}/proposal.json`, MAX_RECORD_BYTES), ); - if (!record || record.id !== params.proposalId) { + if (!record.ok) { + throw new Error(record.error.message); + } + if (record.value.id !== params.proposalId) { throw new Error("invalid proposal metadata"); } const draft = await params.stateRoot.read(`${proposalDir}/PROPOSAL.md`, { @@ -150,15 +156,15 @@ async function migrateProposal(params: { maxBytes: MAX_RECORD_BYTES, symlinks: "reject", }); - if (hashSkillProposalContent(draft.buffer.toString("utf8")) !== record.draftHash) { + if (hashSkillProposalContent(draft.buffer.toString("utf8")) !== record.value.draftHash) { throw new Error("proposal draft hash does not match proposal metadata"); } const rollback = await readLegacyRollback(params.stateRoot, params.proposalId); - const workspaceDir = proposalWorkspace(record); + const workspaceDir = proposalWorkspace(record.value); const ownerAgentId = inferOwnerAgentId({ config: params.config, env: params.env, - record, + record: record.value, workspaceDir, }); if (!ownerAgentId) { @@ -167,13 +173,13 @@ async function migrateProposal(params: { ); } const result = importLegacySkillProposal({ - record, + record: record.value, rollback, ownerAgentId, workspaceDir, store: { env: params.env }, }); - await verifyImportedProposal({ env: params.env, record, rollback }); + await verifyImportedProposal({ env: params.env, record: record.value, rollback }); if (rollback) { await params.stateRoot.remove(`${proposalDir}/rollback.json`); } diff --git a/src/skills/workshop/proposal-draft.test.ts b/src/skills/workshop/proposal-draft.test.ts new file mode 100644 index 000000000000..b4180cba5269 --- /dev/null +++ b/src/skills/workshop/proposal-draft.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from "vitest"; +import { + nextProposalVersion, + prepareSkillProposalDraft, + resolveUpdateProposalDescription, +} from "./proposal-draft.js"; + +describe("Skill Workshop proposal draft preparation", () => { + it("normalizes one canonical draft result for create and revise callers", () => { + const prepared = prepareSkillProposalDraft({ + name: "release-check", + description: "Check a release", + content: "# Release Check\n", + date: "2026-07-29T00:00:00.000Z", + maxSkillBytes: 1024, + supportFiles: [{ path: "references/checklist.md", content: "Verify artifacts.\n" }], + goal: " Preserve release quality. ", + evidence: " Existing operator checklist. ", + }); + + expect(prepared).toMatchObject({ + ok: true, + value: { + description: "Check a release", + goal: "Preserve release quality.", + evidence: "Existing operator checklist.", + scan: { state: "clean", critical: 0 }, + supportFiles: [ + expect.objectContaining({ + path: "references/checklist.md", + content: "Verify artifacts.\n", + }), + ], + }, + }); + if (!prepared.ok) { + throw prepared.error.cause; + } + expect(prepared.value.content).toContain('name: "release-check"'); + expect(prepared.value.content).toContain('date: "2026-07-29T00:00:00.000Z"'); + expect(prepared.value.draftHash).toMatch(/^[a-f0-9]{64}$/); + }); + + it("returns the existing validation messages without persisting partial output", () => { + const oversized = prepareSkillProposalDraft({ + name: "release-check", + description: "Check a release", + content: "x".repeat(5), + date: "2026-07-29T00:00:00.000Z", + maxSkillBytes: 4, + }); + expect(oversized).toMatchObject({ + ok: false, + error: { + message: "Skill proposal content is too large (5 bytes, max 4).", + }, + }); + + const secret = prepareSkillProposalDraft({ + name: "release-check", + description: "Check a release", + content: "# Release Check\n", + date: "2026-07-29T00:00:00.000Z", + maxSkillBytes: 1024, + secretScanMetadata: [ + { + file: "skill-name", + content: "ghp_1234567890abcdefghijklmnopqrstuvwxyz", + }, + ], + }); + expect(secret).toMatchObject({ + ok: false, + error: { + message: expect.stringContaining("recognized literal credential in skill-name"), + }, + }); + }); + + it("preserves version and UTF-8 description behavior", () => { + expect(nextProposalVersion("v1")).toBe("v2"); + expect(nextProposalVersion("invalid")).toBe("v2"); + expect(resolveUpdateProposalDescription(undefined, ` ${"é".repeat(100)} `)).toBe( + "é".repeat(80), + ); + }); +}); diff --git a/src/skills/workshop/proposal-draft.ts b/src/skills/workshop/proposal-draft.ts new file mode 100644 index 000000000000..3ac10a54fba7 --- /dev/null +++ b/src/skills/workshop/proposal-draft.ts @@ -0,0 +1,212 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { err, ok, type Result } from "@openclaw/normalization-core/result"; +import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; +import { readLocalFileSafely, root, walkDirectory } from "../../infra/fs-safe.js"; +import { + MAX_WORKSPACE_SKILL_SUPPORT_FILE_BYTES, + normalizeWorkspaceSkillSupportPath, +} from "../lifecycle/workspace-skill-write.js"; +import { renderProposalMarkdown } from "./frontmatter.js"; +import { assertProposalContainsNoLiteralSecrets, scanProposalBundle } from "./proposal-scan.js"; +import { + hashSkillProposalContent, + MAX_PROPOSAL_SUPPORT_FILES, + prepareSkillProposalSupportFiles, + type PreparedSkillProposalSupportFile, +} from "./store.js"; +import type { SkillProposalScan, SkillProposalSupportFileInput } from "./types.js"; + +const MAX_PROPOSAL_DRAFT_BYTES = 1024 * 1024; +const MAX_PROPOSAL_DIRECTORY_ENTRIES = MAX_PROPOSAL_SUPPORT_FILES * 4; +const MAX_SKILL_PROPOSAL_DESCRIPTION_BYTES = 160; + +type SkillProposalDraftValidationError = { + cause: Error; + message: string; +}; + +type PreparedSkillProposalDraft = { + content: string; + description: string; + draftHash: string; + evidence?: string; + goal?: string; + scan: SkillProposalScan; + supportFiles: PreparedSkillProposalSupportFile[]; +}; + +export function prepareSkillProposalDraft(input: { + name: string; + description: string; + content: string; + fallbackFrontmatterContent?: string; + version?: string; + date: string; + maxSkillBytes: number; + supportFiles?: readonly SkillProposalSupportFileInput[]; + secretScanMetadata?: readonly { file: string; content: string | undefined }[]; + goal?: string; + evidence?: string; +}): Result { + try { + assertProposalDescriptionWithinLimit(input.description); + assertProposalContentWithinLimit(input.content, input.maxSkillBytes); + const supportFiles = prepareSkillProposalSupportFiles(input.supportFiles); + const content = renderProposalMarkdown({ + name: input.name, + description: input.description, + content: input.content, + fallbackFrontmatterContent: input.fallbackFrontmatterContent, + version: input.version, + date: input.date, + }); + const goal = normalizeOptionalString(input.goal); + const evidence = normalizeOptionalString(input.evidence); + const scan = scanProposalBundle(content, supportFiles, [ + ...(input.secretScanMetadata ?? []), + { file: "description", content: input.description }, + { file: "goal", content: goal }, + { file: "evidence", content: evidence }, + ]); + assertProposalContainsNoLiteralSecrets(scan); + return ok({ + content, + description: input.description, + draftHash: hashSkillProposalContent(content), + scan, + supportFiles, + ...(goal ? { goal } : {}), + ...(evidence ? { evidence } : {}), + }); + } catch (cause) { + const error = cause instanceof Error ? cause : new Error(String(cause)); + return err({ cause: error, message: error.message }); + } +} + +export function resolveUpdateProposalDescription( + inputDescription: string | undefined, + currentDescription: string, +): string { + const supplied = normalizeOptionalString(inputDescription); + if (supplied) { + return supplied; + } + return truncateUtf8(currentDescription.trim(), MAX_SKILL_PROPOSAL_DESCRIPTION_BYTES); +} + +export function nextProposalVersion(version: string): string { + const match = /^v(\d+)$/.exec(version.trim()); + if (!match) { + return "v2"; + } + const current = Number.parseInt(match[1] ?? "1", 10); + return `v${Number.isSafeInteger(current) && current > 0 ? current + 1 : 2}`; +} + +export async function readSkillProposalDraftFile(filePath: string): Promise { + const read = await readLocalFileSafely({ + filePath, + maxBytes: MAX_PROPOSAL_DRAFT_BYTES, + }); + return decodeProposalTextFile(read.buffer, filePath); +} + +export async function readSkillProposalDraftDirectory(dirPath: string): Promise<{ + content: string; + supportFiles: SkillProposalSupportFileInput[]; +}> { + const absoluteDir = path.resolve(dirPath); + const draftRoot = await root(absoluteDir); + const proposal = await draftRoot.read("PROPOSAL.md", { + hardlinks: "reject", + maxBytes: MAX_PROPOSAL_DRAFT_BYTES, + symlinks: "reject", + }); + const scanned = await walkDirectory(absoluteDir, { + maxDepth: 8, + maxEntries: MAX_PROPOSAL_DIRECTORY_ENTRIES, + symlinks: "include", + }); + if (scanned.truncated) { + throw new Error("Proposal directory has too many entries."); + } + const supportFiles: SkillProposalSupportFileInput[] = []; + for (const entry of scanned.entries.toSorted((a, b) => + a.relativePath.localeCompare(b.relativePath), + )) { + const relativePath = toPortableRelativePath(entry.relativePath); + if (!relativePath || relativePath === "PROPOSAL.md") { + continue; + } + if (entry.kind === "directory") { + continue; + } + if (entry.kind !== "file") { + throw new Error(`Proposal support file must be a regular file: ${relativePath}`); + } + const supportPath = normalizeWorkspaceSkillSupportPath(relativePath); + const stats = await fs.stat(entry.path); + if ((stats.mode & 0o111) !== 0) { + throw new Error(`Proposal support files must not be executable: ${relativePath}`); + } + const read = await draftRoot.read(relativePath, { + hardlinks: "reject", + maxBytes: MAX_WORKSPACE_SKILL_SUPPORT_FILE_BYTES, + symlinks: "reject", + }); + supportFiles.push({ + path: supportPath, + content: decodeProposalTextFile(read.buffer, relativePath), + }); + } + return { + content: decodeProposalTextFile(proposal.buffer, "PROPOSAL.md"), + supportFiles, + }; +} + +function decodeProposalTextFile(buffer: Buffer, label: string): string { + const content = buffer.toString("utf8"); + if (!Buffer.from(content, "utf8").equals(buffer) || content.includes("\0")) { + throw new Error(`Proposal files must be UTF-8 text: ${label}`); + } + return content; +} + +function assertProposalDescriptionWithinLimit(description: string): void { + const sizeBytes = Buffer.byteLength(description, "utf8"); + if (sizeBytes > MAX_SKILL_PROPOSAL_DESCRIPTION_BYTES) { + throw new Error( + `Skill proposal description is too large (${sizeBytes} bytes, max ${MAX_SKILL_PROPOSAL_DESCRIPTION_BYTES}).`, + ); + } +} + +function assertProposalContentWithinLimit(content: string, maxSkillBytes: number): void { + const sizeBytes = Buffer.byteLength(content, "utf8"); + if (sizeBytes > maxSkillBytes) { + throw new Error( + `Skill proposal content is too large (${sizeBytes} bytes, max ${maxSkillBytes}).`, + ); + } +} + +function truncateUtf8(value: string, maxBytes: number): string { + let out = ""; + let sizeBytes = 0; + for (const char of value) { + const charBytes = Buffer.byteLength(char, "utf8"); + if (sizeBytes + charBytes > maxBytes) { + break; + } + out += char; + sizeBytes += charBytes; + } + return out.trimEnd(); +} + +function toPortableRelativePath(relativePath: string): string { + return relativePath.split(path.sep).join("/"); +} diff --git a/src/skills/workshop/service.ts b/src/skills/workshop/service.ts index c858497dbdc3..d30fba276f56 100644 --- a/src/skills/workshop/service.ts +++ b/src/skills/workshop/service.ts @@ -1,8 +1,6 @@ -import fs from "node:fs/promises"; import path from "node:path"; import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; -import { readLocalFileSafely, root, walkDirectory } from "../../infra/fs-safe.js"; import { buildWorkspaceSkillStatus, resolveSkillStatusEntry, @@ -16,8 +14,6 @@ import { import { assertInsideWorkspace, assertWorkspaceSkillWriteTarget, - MAX_WORKSPACE_SKILL_SUPPORT_FILE_BYTES, - normalizeWorkspaceSkillSupportPath, readWorkspaceSkillFile, readWorkspaceSupportFile, writeWorkspaceSkill, @@ -25,14 +21,16 @@ import { import { resolveAllowedSkillSymlinkTargetRealPaths } from "../loading/symlink-targets.js"; import { bumpSkillsSnapshotVersion } from "../runtime/refresh-state.js"; import { resolveSkillWorkshopConfig } from "./config.js"; -import { - readProposalFrontmatter, - renderProposalMarkdown, - stripProposalFrontmatterForSkill, -} from "./frontmatter.js"; +import { readProposalFrontmatter, stripProposalFrontmatterForSkill } from "./frontmatter.js"; import { createSkillProposalEvent, dispatchSkillProposalChanged } from "./plugin-hooks.js"; +import { + nextProposalVersion, + prepareSkillProposalDraft, + resolveUpdateProposalDescription, +} from "./proposal-draft.js"; +export { readSkillProposalDraftDirectory, readSkillProposalDraftFile } from "./proposal-draft.js"; import { readSkillProposalTargetTreeSha256 } from "./proposal-bundle.js"; -import { assertProposalContainsNoLiteralSecrets, scanProposalBundle } from "./proposal-scan.js"; +import { scanProposalBundle } from "./proposal-scan.js"; import { hashSkillProposalRevision } from "./revision-hash.js"; import { assertExpectedRevisionHash, @@ -44,8 +42,6 @@ import { createSkillProposalId, createSkillProposalRollback, hashSkillProposalContent, - MAX_PROPOSAL_SUPPORT_FILES, - prepareSkillProposalSupportFiles, readProposalSupportFiles, replaceSkillProposalDraft, resolveSkillProposalTarget, @@ -76,7 +72,6 @@ import { type SkillProposalReviseInput, type SkillProposalRollback, type SkillProposalSupportFile, - type SkillProposalSupportFileInput, type SkillProposalUpdateInput, } from "./types.js"; @@ -90,10 +85,6 @@ function proposalStoreOptions(env?: NodeJS.ProcessEnv) { } const WRITABLE_WORKSPACE_SOURCES = new Set(["openclaw-workspace", "agents-skills-project"]); -const MAX_PROPOSAL_DRAFT_BYTES = 1024 * 1024; -const MAX_PROPOSAL_DIRECTORY_ENTRIES = MAX_PROPOSAL_SUPPORT_FILES * 4; -const MAX_SKILL_PROPOSAL_DESCRIPTION_BYTES = 160; - class SkillProposalLifecycleError extends Error { constructor( message: string, @@ -109,76 +100,6 @@ type SkillProposalTransitionInput = Pick< "agentId" | "correlationId" | "env" | "eventActor" | "workspaceDir" >; -export async function readSkillProposalDraftFile(filePath: string): Promise { - const read = await readLocalFileSafely({ - filePath, - maxBytes: MAX_PROPOSAL_DRAFT_BYTES, - }); - return decodeProposalTextFile(read.buffer, filePath); -} - -export async function readSkillProposalDraftDirectory(dirPath: string): Promise<{ - content: string; - supportFiles: SkillProposalSupportFileInput[]; -}> { - const absoluteDir = path.resolve(dirPath); - const draftRoot = await root(absoluteDir); - const proposal = await draftRoot.read("PROPOSAL.md", { - hardlinks: "reject", - maxBytes: MAX_PROPOSAL_DRAFT_BYTES, - symlinks: "reject", - }); - const scanned = await walkDirectory(absoluteDir, { - maxDepth: 8, - maxEntries: MAX_PROPOSAL_DIRECTORY_ENTRIES, - symlinks: "include", - }); - if (scanned.truncated) { - throw new Error("Proposal directory has too many entries."); - } - const supportFiles: SkillProposalSupportFileInput[] = []; - for (const entry of scanned.entries.toSorted((a, b) => - a.relativePath.localeCompare(b.relativePath), - )) { - const relativePath = toPortableRelativePath(entry.relativePath); - if (!relativePath || relativePath === "PROPOSAL.md") { - continue; - } - if (entry.kind === "directory") { - continue; - } - if (entry.kind !== "file") { - throw new Error(`Proposal support file must be a regular file: ${relativePath}`); - } - const supportPath = normalizeWorkspaceSkillSupportPath(relativePath); - const stats = await fs.stat(entry.path); - if ((stats.mode & 0o111) !== 0) { - throw new Error(`Proposal support files must not be executable: ${relativePath}`); - } - const read = await draftRoot.read(relativePath, { - hardlinks: "reject", - maxBytes: MAX_WORKSPACE_SKILL_SUPPORT_FILE_BYTES, - symlinks: "reject", - }); - supportFiles.push({ - path: supportPath, - content: decodeProposalTextFile(read.buffer, relativePath), - }); - } - return { - content: decodeProposalTextFile(proposal.buffer, "PROPOSAL.md"), - supportFiles, - }; -} - -function decodeProposalTextFile(buffer: Buffer, label: string): string { - const content = buffer.toString("utf8"); - if (!Buffer.from(content, "utf8").equals(buffer) || content.includes("\0")) { - throw new Error(`Proposal files must be UTF-8 text: ${label}`); - } - return content; -} - function normalizeProposalOrigin( origin: SkillProposalOrigin | undefined, ): SkillProposalOrigin | undefined { @@ -230,31 +151,35 @@ export async function proposeCreateSkill( const name = normalizeRequired(input.name, "Skill name"); const description = normalizeRequired(input.description, "Skill description"); const config = resolveSkillWorkshopConfig(input.config); - assertProposalDescriptionWithinLimit(description); - assertProposalContentWithinLimit(input.content, config.maxSkillBytes); const target = resolveSkillProposalTarget({ workspaceDir: input.workspaceDir, skillName: name }); if ((await readWorkspaceSkillFile(target.skillFile)) !== null) { throw new Error(`Skill already exists at ${target.skillFile}.`); } - const supportFiles = prepareSkillProposalSupportFiles(input.supportFiles); const now = new Date().toISOString(); - const proposalContent = renderProposalMarkdown({ + const prepared = prepareSkillProposalDraft({ name: target.skillKey, description, content: input.content, date: now, + maxSkillBytes: config.maxSkillBytes, + supportFiles: input.supportFiles, + secretScanMetadata: [{ file: "skill-name", content: name }], + goal: input.goal, + evidence: input.evidence, }); + if (!prepared.ok) { + throw prepared.error.cause; + } + const { + content: proposalContent, + draftHash, + evidence, + goal, + scan, + supportFiles, + } = prepared.value; const id = createSkillProposalId(name); - const goal = normalizeOptionalString(input.goal); - const evidence = normalizeOptionalString(input.evidence); - const scan = scanProposalBundle(proposalContent, supportFiles, [ - { file: "skill-name", content: name }, - { file: "description", content: description }, - { file: "goal", content: goal }, - { file: "evidence", content: evidence }, - ]); - assertProposalContainsNoLiteralSecrets(scan); const origin = normalizeProposalOrigin({ ...input.origin, agentId: input.origin?.agentId ?? input.agentId, @@ -275,7 +200,7 @@ export async function proposeCreateSkill( ...originRunProvenance, proposedVersion: "v1", draftFile: "PROPOSAL.md", - draftHash: hashSkillProposalContent(proposalContent), + draftHash, target: { skillName: name, skillKey: target.skillKey, @@ -368,26 +293,31 @@ export async function proposeUpdateSkill( throw new Error(`Skill file is missing: ${targetSkill.filePath}`); } const description = resolveUpdateProposalDescription(input.description, targetSkill.description); - assertProposalContentWithinLimit(input.content, config.maxSkillBytes); - const supportFiles = prepareSkillProposalSupportFiles(input.supportFiles); const now = new Date().toISOString(); - const proposalContent = renderProposalMarkdown({ + const prepared = prepareSkillProposalDraft({ name: targetSkill.skillKey, description, content: input.content, fallbackFrontmatterContent: currentContent, date: now, + maxSkillBytes: config.maxSkillBytes, + supportFiles: input.supportFiles, + goal: input.goal, + evidence: input.evidence, }); + if (!prepared.ok) { + throw prepared.error.cause; + } + const { + content: proposalContent, + draftHash, + evidence, + goal, + scan, + supportFiles, + } = prepared.value; const id = createSkillProposalId(targetSkill.skillKey || targetSkill.name); - const goal = normalizeOptionalString(input.goal); - const evidence = normalizeOptionalString(input.evidence); - const scan = scanProposalBundle(proposalContent, supportFiles, [ - { file: "description", content: description }, - { file: "goal", content: goal }, - { file: "evidence", content: evidence }, - ]); - assertProposalContainsNoLiteralSecrets(scan); const origin = normalizeProposalOrigin({ ...input.origin, agentId: input.origin?.agentId ?? input.agentId, @@ -408,7 +338,7 @@ export async function proposeUpdateSkill( ...originRunProvenance, proposedVersion: "v1", draftFile: "PROPOSAL.md", - draftHash: hashSkillProposalContent(proposalContent), + draftHash, target: { skillName: targetSkill.name, skillKey: targetSkill.skillKey, @@ -499,57 +429,56 @@ export async function reviseSkillProposal( const supportFiles = input.supportFiles === undefined ? await readProposalSupportFiles(record, proposalStoreOptions(input.env)) - : prepareSkillProposalSupportFiles(input.supportFiles); + : input.supportFiles; const requestedContent = input.content ?? read.content; - assertProposalContentWithinLimit(requestedContent, config.maxSkillBytes); - const supportFileMetadata = - supportFiles.length > 0 - ? await buildSupportFileMetadata( - supportFiles, - record.kind === "update" ? record.target.skillDir : undefined, - ) - : []; const nextVersion = nextProposalVersion(record.proposedVersion); const description = normalizeOptionalString(input.description) ?? record.description; - assertProposalDescriptionWithinLimit(description); const now = new Date().toISOString(); - const proposalContent = renderProposalMarkdown({ + const prepared = prepareSkillProposalDraft({ name: record.target.skillKey, description, content: requestedContent, fallbackFrontmatterContent: read.content, version: nextVersion, date: now, + maxSkillBytes: config.maxSkillBytes, + supportFiles, + goal: input.goal === undefined ? record.goal : input.goal, + evidence: input.evidence === undefined ? record.evidence : input.evidence, }); - const goal = - input.goal === undefined - ? normalizeOptionalString(record.goal) - : normalizeOptionalString(input.goal); - const evidence = - input.evidence === undefined - ? normalizeOptionalString(record.evidence) - : normalizeOptionalString(input.evidence); + if (!prepared.ok) { + throw prepared.error.cause; + } + const { + content: proposalContent, + draftHash, + evidence, + goal, + scan, + supportFiles: preparedSupportFiles, + } = prepared.value; + const supportFileMetadata = + preparedSupportFiles.length > 0 + ? await buildSupportFileMetadata( + preparedSupportFiles, + record.kind === "update" ? record.target.skillDir : undefined, + ) + : []; const origin = normalizeProposalOrigin(input.origin); const originRunProvenance = mergeProposalOriginRunProvenance(record, origin); const previousSupportFiles = record.supportFiles; - const scan = scanProposalBundle(proposalContent, supportFiles, [ - { file: "description", content: description }, - { file: "goal", content: goal }, - { file: "evidence", content: evidence }, - ]); - assertProposalContainsNoLiteralSecrets(scan); const revised: SkillProposalRecord = { ...record, description, updatedAt: now, proposedVersion: nextVersion, - draftHash: hashSkillProposalContent(proposalContent), + draftHash, scan, ...(origin ? { origin } : {}), ...originRunProvenance, }; delete revised.evaluation; - if (supportFiles.length > 0) { + if (preparedSupportFiles.length > 0) { revised.supportFiles = supportFileMetadata; } else { delete revised.supportFiles; @@ -568,7 +497,7 @@ export async function reviseSkillProposal( record: revised, previousSupportFiles, content: proposalContent, - supportFiles, + supportFiles: preparedSupportFiles, event: createSkillProposalEvent({ record: revised, type: "revised", @@ -951,50 +880,6 @@ async function readApplyTargetState( return { previousContent, previousSupportFiles }; } -function assertProposalDescriptionWithinLimit(description: string): void { - const sizeBytes = Buffer.byteLength(description, "utf8"); - if (sizeBytes > MAX_SKILL_PROPOSAL_DESCRIPTION_BYTES) { - throw new Error( - `Skill proposal description is too large (${sizeBytes} bytes, max ${MAX_SKILL_PROPOSAL_DESCRIPTION_BYTES}).`, - ); - } -} - -function resolveUpdateProposalDescription( - inputDescription: string | undefined, - currentDescription: string, -): string { - const supplied = normalizeOptionalString(inputDescription); - if (supplied) { - assertProposalDescriptionWithinLimit(supplied); - return supplied; - } - return truncateUtf8(currentDescription.trim(), MAX_SKILL_PROPOSAL_DESCRIPTION_BYTES); -} - -function truncateUtf8(value: string, maxBytes: number): string { - let out = ""; - let sizeBytes = 0; - for (const char of value) { - const charBytes = Buffer.byteLength(char, "utf8"); - if (sizeBytes + charBytes > maxBytes) { - break; - } - out += char; - sizeBytes += charBytes; - } - return out.trimEnd(); -} - -function assertProposalContentWithinLimit(content: string, maxSkillBytes: number): void { - const sizeBytes = Buffer.byteLength(content, "utf8"); - if (sizeBytes > maxSkillBytes) { - throw new Error( - `Skill proposal content is too large (${sizeBytes} bytes, max ${maxSkillBytes}).`, - ); - } -} - async function buildSupportFileMetadata( files: readonly PreparedSkillProposalSupportFile[], targetSkillDir?: string, @@ -1021,15 +906,6 @@ async function buildSupportFileMetadata( return out; } -function nextProposalVersion(version: string): string { - const match = /^v(\d+)$/.exec(version.trim()); - if (!match) { - return "v2"; - } - const current = Number.parseInt(match[1] ?? "1", 10); - return `v${Number.isSafeInteger(current) && current > 0 ? current + 1 : 2}`; -} - async function markProposal( input: SkillProposalActionInput, status: "rejected", @@ -1222,7 +1098,4 @@ function normalizeRequired(value: string, label: string): string { return normalized; } -function toPortableRelativePath(relativePath: string): string { - return relativePath.split(path.sep).join("/"); -} /* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ diff --git a/src/skills/workshop/store-record.test.ts b/src/skills/workshop/store-record.test.ts new file mode 100644 index 000000000000..d0d140637fb7 --- /dev/null +++ b/src/skills/workshop/store-record.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it } from "vitest"; +import { + parseSkillProposalRecord, + parseSkillProposalRollback, + validateSkillProposalRecord, + validateSkillProposalRollback, +} from "./store-record.js"; +import { SKILL_WORKSHOP_ROLLBACK_SCHEMA, SKILL_WORKSHOP_SCHEMA } from "./types.js"; + +const shippedProposal = { + schema: SKILL_WORKSHOP_SCHEMA, + id: "shipped-workshop-20260729-1234567890", + kind: "update", + status: "pending", + title: "Update shipped-workshop", + description: "Proposal written by a shipped Workshop release", + createdAt: "2026-07-29T00:00:00.000Z", + updatedAt: "2026-07-29T00:00:00.000Z", + createdBy: "skill-workshop", + origin: { + agentId: "main", + sessionKey: "agent:main:workshop", + runId: "shipped-run", + }, + originRunIds: ["shipped-run"], + originRunMutationCounts: { "shipped-run": 1 }, + proposedVersion: "v1", + draftFile: "PROPOSAL.md", + draftHash: "a".repeat(64), + supportFiles: [ + { + path: "references/proof.md", + sizeBytes: 6, + hash: "b".repeat(64), + targetExisted: true, + targetContentHash: "c".repeat(64), + }, + ], + target: { + skillName: "shipped-workshop", + skillKey: "shipped-workshop", + skillDir: "/workspace/skills/shipped-workshop", + skillFile: "/workspace/skills/shipped-workshop/SKILL.md", + source: "openclaw-workspace", + currentContentHash: "d".repeat(64), + }, + scan: { + state: "clean", + scannedAt: "2026-07-29T00:00:00.000Z", + critical: 0, + warn: 0, + info: 0, + findings: [], + }, + goal: "Preserve existing Workshop behavior.", + evidence: "Record shape from v2026.7.2-beta.5.", +} as const; + +const shippedRollback = { + schema: SKILL_WORKSHOP_ROLLBACK_SCHEMA, + proposalId: shippedProposal.id, + writtenAt: "2026-07-29T00:00:00.000Z", + targetSkillFile: shippedProposal.target.skillFile, + action: "update", + previousContentHash: "e".repeat(64), + previousContent: "# Previous skill\n", + supportFiles: [ + { + path: "references/proof.md", + existed: true, + previousContentHash: "f".repeat(64), + previousContent: "proof\n", + }, + ], +} as const; + +describe("Skill Workshop persisted record validation", () => { + it("accepts the shipped v1 proposal and rollback shapes unchanged", () => { + expect(validateSkillProposalRecord(shippedProposal)).toEqual({ + ok: true, + value: shippedProposal, + }); + expect(validateSkillProposalRollback(shippedRollback)).toEqual({ + ok: true, + value: shippedRollback, + }); + expect(parseSkillProposalRecord(shippedProposal)).toBe(shippedProposal); + expect(parseSkillProposalRollback(shippedRollback)).toBe(shippedRollback); + }); + + it("maps invalid metadata to the existing migration errors", () => { + expect(validateSkillProposalRecord({ ...shippedProposal, schema: "invalid" })).toEqual({ + ok: false, + error: { + code: "invalid-proposal-metadata", + message: "invalid proposal metadata", + }, + }); + expect(validateSkillProposalRollback({ ...shippedRollback, action: "invalid" })).toEqual({ + ok: false, + error: { + code: "invalid-rollback-metadata", + message: "invalid rollback metadata", + }, + }); + }); +}); diff --git a/src/skills/workshop/store-record.ts b/src/skills/workshop/store-record.ts index 86f61e97fe91..19197863be6a 100644 --- a/src/skills/workshop/store-record.ts +++ b/src/skills/workshop/store-record.ts @@ -1,3 +1,4 @@ +import { err, ok, type Result } from "@openclaw/normalization-core/result"; import type { PluginHookSkillEvaluationFinding, PluginHookSkillProposalEvaluateResult, @@ -22,6 +23,11 @@ export const MAX_PROPOSAL_SUPPORT_FILES = 64; export const MAX_SKILL_PROPOSAL_EVALUATION_BYTES = 512 * 1024; const PROPOSAL_ID_PATTERN = /^[a-z0-9][a-z0-9-]{5,120}$/; +type SkillProposalRecordValidationError = { + code: "invalid-proposal-metadata" | "invalid-rollback-metadata"; + message: string; +}; + export function assertSkillProposalEvaluationWithinLimit( evaluation: SkillProposalEvaluation, ): void { @@ -39,9 +45,11 @@ export function assertProposalId(proposalId: string): void { } } -export function parseSkillProposalRecord(raw: unknown): SkillProposalRecord | null { +export function validateSkillProposalRecord( + raw: unknown, +): Result { if (!raw || typeof raw !== "object" || Array.isArray(raw)) { - return null; + return invalidProposalMetadata(); } const record = raw as SkillProposalRecord; if ( @@ -68,9 +76,14 @@ export function parseSkillProposalRecord(raw: unknown): SkillProposalRecord | nu !record.scan || typeof record.scan !== "object" ) { - return null; + return invalidProposalMetadata(); } - return record; + return ok(record); +} + +export function parseSkillProposalRecord(raw: unknown): SkillProposalRecord | null { + const result = validateSkillProposalRecord(raw); + return result.ok ? result.value : null; } export function parseSkillProposalEvaluation(raw: unknown): SkillProposalEvaluation | null { @@ -241,9 +254,11 @@ function isValidSupportFileList(value: unknown): boolean { return true; } -export function parseSkillProposalRollback(raw: unknown): SkillProposalRollback | null { +export function validateSkillProposalRollback( + raw: unknown, +): Result { if (!raw || typeof raw !== "object" || Array.isArray(raw)) { - return null; + return invalidRollbackMetadata(); } const rollback = raw as SkillProposalRollback; if ( @@ -258,7 +273,32 @@ export function parseSkillProposalRollback(raw: unknown): SkillProposalRollback (rollback.previousContent !== undefined && typeof rollback.previousContent !== "string") || (rollback.supportFiles !== undefined && !Array.isArray(rollback.supportFiles)) ) { - return null; + return invalidRollbackMetadata(); } - return rollback; + return ok(rollback); +} + +export function parseSkillProposalRollback(raw: unknown): SkillProposalRollback | null { + const result = validateSkillProposalRollback(raw); + return result.ok ? result.value : null; +} + +function invalidProposalMetadata(): Result< + SkillProposalRecord, + SkillProposalRecordValidationError +> { + return err({ + code: "invalid-proposal-metadata", + message: "invalid proposal metadata", + }); +} + +function invalidRollbackMetadata(): Result< + SkillProposalRollback, + SkillProposalRecordValidationError +> { + return err({ + code: "invalid-rollback-metadata", + message: "invalid rollback metadata", + }); } diff --git a/src/skills/workshop/store.ts b/src/skills/workshop/store.ts index da17d6e594e7..2f5bb60ca6a6 100644 --- a/src/skills/workshop/store.ts +++ b/src/skills/workshop/store.ts @@ -64,8 +64,8 @@ const TARGET_LEASE_MS = 60_000; const TARGET_LEASE_WAIT_MS = 5_000; export { MAX_PROPOSAL_SUPPORT_FILES, - parseSkillProposalRecord, - parseSkillProposalRollback, + validateSkillProposalRecord, + validateSkillProposalRollback, } from "./store-record.js"; export { readSkillProposalRollback, writeSkillProposalRollback } from "./store-sqlite-rollback.js";