diff --git a/ui/src/e2e/skill-workshop-applied-diff.e2e.test.ts b/ui/src/e2e/skill-workshop-applied-diff.e2e.test.ts new file mode 100644 index 000000000000..3a593aa52ab2 --- /dev/null +++ b/ui/src/e2e/skill-workshop-applied-diff.e2e.test.ts @@ -0,0 +1,148 @@ +import { mkdir, rm } from "node:fs/promises"; +import path from "node:path"; +import { chromium, type Browser } from "playwright"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + canRunPlaywrightChromium, + installMockGateway, + resolvePlaywrightChromiumExecutablePath, + startControlUiE2eServer, + type ControlUiE2eServer, + type MockGatewayRequest, +} from "../test-helpers/control-ui-e2e.ts"; + +const chromiumExecutablePath = resolvePlaywrightChromiumExecutablePath(chromium.executablePath()); +const chromiumAvailable = canRunPlaywrightChromium(chromiumExecutablePath); +const allowMissingChromium = process.env.OPENCLAW_UI_E2E_ALLOW_MISSING_CHROMIUM === "1"; +const describeControlUiE2e = chromiumAvailable || !allowMissingChromium ? describe : describe.skip; +const artifactDir = path.resolve(process.cwd(), ".artifacts/control-ui-e2e/applied-revision-diff"); + +let browser: Browser; +let server: ControlUiE2eServer; + +function appliedProposal(id: string, updatedAt: string) { + return { + createdAt: updatedAt, + description: "Keep the release procedure accurate.", + id, + kind: "update", + scanState: "clean", + skillKey: "deploy-review", + skillName: "Deploy Review", + status: "applied", + title: "Deploy review", + updatedAt, + }; +} + +function inspectResponse( + proposal: ReturnType, + content: string, + version: string, +) { + return { + content, + record: { + ...proposal, + proposedVersion: version, + target: { skillKey: proposal.skillKey, skillName: proposal.skillName }, + }, + supportFiles: [], + }; +} + +function inspectedProposalId(request: MockGatewayRequest): unknown { + const params = request.params; + if (!params || typeof params !== "object" || !("proposalId" in params)) { + throw new Error("Expected skills.proposals.inspect params"); + } + return params.proposalId; +} + +describeControlUiE2e("Skill Workshop applied revision diff mocked Gateway E2E", () => { + beforeAll(async () => { + if (!chromiumAvailable) { + throw new Error(`Playwright Chromium is unavailable at ${chromiumExecutablePath}`); + } + await rm(artifactDir, { force: true, recursive: true }); + await mkdir(artifactDir, { recursive: true }); + server = await startControlUiE2eServer(); + browser = await chromium.launch({ executablePath: chromiumExecutablePath }); + }); + + afterAll(async () => { + await browser?.close(); + await server?.close(); + }); + + it("shows a compact revision diff and switches to the full body", async () => { + const previous = appliedProposal("proposal-v1", "2026-08-16T10:00:00.000Z"); + const latest = appliedProposal("proposal-v2", "2026-08-17T10:00:00.000Z"); + const previousBody = "# Deploy review\n\n## Steps\n1. Verify package.\n2. Publish release."; + const latestBody = "# Deploy review\n\n## Steps\n1. Verify package.\n2. Publish package."; + const context = await browser.newContext({ + locale: "en-US", + recordVideo: { dir: artifactDir, size: { height: 900, width: 1280 } }, + serviceWorkers: "block", + viewport: { height: 900, width: 1280 }, + }); + const page = await context.newPage(); + const gateway = await installMockGateway(page, { + methodResponses: { + "skills.proposals.inspect": { + cases: [ + { + match: { proposalId: latest.id }, + response: inspectResponse(latest, latestBody, "v2"), + }, + { + match: { proposalId: previous.id }, + response: inspectResponse(previous, previousBody, "v1"), + }, + ], + }, + "skills.proposals.list": { + proposals: [latest, previous], + schema: "openclaw.skill-workshop.proposals-manifest.v1", + updatedAt: latest.updatedAt, + }, + }, + }); + + try { + const response = await page.goto(`${server.baseUrl}skills/workshop`); + expect(response?.status()).toBe(200); + await gateway.waitForRequest("skills.proposals.list"); + await page.locator("#skill-workshop-mode-tab-board").click(); + await page.locator(".sw-lifecycle-tab", { hasText: "Applied" }).click(); + + const changes = page.getByRole("button", { name: "Changes", exact: true }); + await expect.poll(() => changes.getAttribute("aria-pressed")).toBe("true"); + await page.locator(".sw-diff__row--add", { hasText: "Publish package." }).waitFor(); + await page.locator(".sw-diff__row--del", { hasText: "Publish release." }).waitFor(); + const inspectRequests = await gateway.getRequests("skills.proposals.inspect"); + expect(new Set(inspectRequests.map(inspectedProposalId))).toEqual( + new Set([latest.id, previous.id]), + ); + await page.screenshot({ + animations: "disabled", + fullPage: true, + path: path.join(artifactDir, "01-changes.png"), + }); + + await page.getByRole("button", { name: "Full body", exact: true }).click(); + await page.getByText("Publish package.", { exact: true }).waitFor(); + expect(await page.locator(".sw-diff").count()).toBe(0); + expect(await gateway.getRequests("skills.proposals.inspect")).toHaveLength( + inspectRequests.length, + ); + await page.screenshot({ + animations: "disabled", + fullPage: true, + path: path.join(artifactDir, "02-full-body.png"), + }); + } finally { + await context.close(); + } + }); +}); diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index 43ef5b26ed10..98038f269c87 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -3479,6 +3479,15 @@ export const en: TranslationMap = { yesterday: "Yesterday", earlier: "Earlier this week", }, + diff: { + changes: "Changes", + fullBody: "Full body", + viewLabel: "Revision view", + unchanged: "This revision left the skill body unchanged.", + loadingPrevious: "Loading the previous revision\u2026", + previousUnavailable: "The previous revision is unavailable, so this is the full body.", + tooLarge: "This comparison is too large to show here. Switch to Full body to read it.", + }, applied: { history: "History", revision: "{count} revision", diff --git a/ui/src/lib/chat/tool-call-diff.test.ts b/ui/src/lib/chat/tool-call-diff.test.ts index 0d10092e512d..93b58cfc6f27 100644 --- a/ui/src/lib/chat/tool-call-diff.test.ts +++ b/ui/src/lib/chat/tool-call-diff.test.ts @@ -106,6 +106,35 @@ describe("computeLineDiff", () => { { kind: "skip", text: "" }, ]); }); + + it("collapses unchanged runs to three context lines when asked", () => { + const oldLines = Array.from({ length: 40 }, (_, index) => `line ${index}`); + const newLines = [...oldLines]; + newLines[20] = "changed"; + + const full = computeLineDiff(oldLines.join("\n"), newLines.join("\n")); + const compact = computeLineDiff(oldLines.join("\n"), newLines.join("\n"), { + compactUnchanged: true, + }); + + expect(full).toHaveLength(41); + expect(compact.filter((line) => line.kind === "ctx").map((line) => line.text)).toEqual([ + "line 17", + "line 18", + "line 19", + "line 21", + "line 22", + "line 23", + ]); + expect(compact.filter((line) => line.kind === "skip")).toHaveLength(2); + }); + + it("compacts an identical pair to nothing so callers can say unchanged", () => { + const text = "alpha\nbeta\ngamma"; + + expect(computeLineDiff(text, text, { compactUnchanged: true })).toEqual([]); + expect(computeLineDiff(text, text)).toHaveLength(3); + }); }); describe("buildWriteDiffLines", () => { diff --git a/ui/src/lib/chat/tool-call-diff.ts b/ui/src/lib/chat/tool-call-diff.ts index 2339c6bb327d..bb45583b5723 100644 --- a/ui/src/lib/chat/tool-call-diff.ts +++ b/ui/src/lib/chat/tool-call-diff.ts @@ -100,12 +100,19 @@ function splitDiffLines(text: string): string[] { return lines; } -function compactLineDiff(lines: DiffLine[], inputTruncated: boolean): DiffLine[] { - if (lines.length <= MAX_DIFF_RENDER_LINES && !inputTruncated) { +function compactLineDiff( + lines: DiffLine[], + inputTruncated: boolean, + compactUnchanged: boolean, +): DiffLine[] { + if (!compactUnchanged && lines.length <= MAX_DIFF_RENDER_LINES && !inputTruncated) { return lines; } const hasChange = lines.some((line) => line.kind === "add" || line.kind === "del"); if (!hasChange) { + if (compactUnchanged && !inputTruncated) { + return []; + } return inputTruncated ? [{ kind: "skip", text: "" }] : [...lines.slice(0, MAX_DIFF_RENDER_LINES), { kind: "skip", text: "" }]; @@ -151,8 +158,14 @@ function compactLineDiff(lines: DiffLine[], inputTruncated: boolean): DiffLine[] /** * Compute a line diff between two snippets (no file line numbers available). * Standard LCS table; inputs are bounded so the quadratic cost stays small. + * + * `compactUnchanged` collapses unchanged runs to three lines of context. */ -export function computeLineDiff(oldText: string, newText: string): DiffLine[] { +export function computeLineDiff( + oldText: string, + newText: string, + options?: { compactUnchanged?: boolean }, +): DiffLine[] { const allOldLines = splitDiffLines(oldText); const allNewLines = splitDiffLines(newText); const inputTruncated = @@ -213,7 +226,7 @@ export function computeLineDiff(oldText: string, newText: string): DiffLine[] { } j++; } - return compactLineDiff(lines, inputTruncated); + return compactLineDiff(lines, inputTruncated, options?.compactUnchanged === true); } /** All-added preview for freshly written files, numbered from line 1. */ diff --git a/ui/src/lib/skill-workshop/index.test.ts b/ui/src/lib/skill-workshop/index.test.ts index b9b40a0d1cd3..cba7ea770f5e 100644 --- a/ui/src/lib/skill-workshop/index.test.ts +++ b/ui/src/lib/skill-workshop/index.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { filterSkillWorkshopAppliedSkills, filterSkillWorkshopProposals, + findSkillWorkshopAppliedPredecessor, type SkillWorkshopProposal, type SkillWorkshopProposalStatus, } from "./index.ts"; @@ -30,6 +31,7 @@ function proposal(options: { recencyGroup: "today", ageLabel: "now", supportFiles: [], + bodyLoaded: true, isNew: false, }; } @@ -98,4 +100,26 @@ describe("Skill Workshop proposal filtering", () => { } expect(filterSkillWorkshopProposals(proposals, "all", "")).toEqual(proposals); }); + + it("points every applied revision at the one it replaced", () => { + const proposals = [ + proposal({ key: "v3", updatedAt: 3 }), + proposal({ key: "v1", updatedAt: 1 }), + proposal({ key: "v2", updatedAt: 2 }), + proposal({ key: "other", slug: "other-skill", updatedAt: 9 }), + proposal({ key: "pending", status: "pending", updatedAt: 4 }), + ]; + + const [skill] = filterSkillWorkshopAppliedSkills(proposals, "release-sanity"); + expect( + skill?.revisions.map(({ proposal: item, previous }) => [item.key, previous?.key]), + ).toEqual([ + ["v3", "v2"], + ["v2", "v1"], + ["v1", undefined], + ]); + expect(findSkillWorkshopAppliedPredecessor(proposals, "v3")?.key).toBe("v2"); + expect(findSkillWorkshopAppliedPredecessor(proposals, "v1")).toBeNull(); + expect(findSkillWorkshopAppliedPredecessor(proposals, "pending")).toBeNull(); + }); }); diff --git a/ui/src/lib/skill-workshop/index.ts b/ui/src/lib/skill-workshop/index.ts index dbbc427e6819..12bb1922b947 100644 --- a/ui/src/lib/skill-workshop/index.ts +++ b/ui/src/lib/skill-workshop/index.ts @@ -57,6 +57,12 @@ export type SkillWorkshopProposal = { name: string; oneLine: string; body: string; + /** + * A proposal inspected through the gateway may legitimately have an empty + * body, so emptiness alone cannot mean "not fetched yet". Cold entries from + * the manifest carry `false`. + */ + bodyLoaded: boolean; status: SkillWorkshopProposalStatus; origin?: { agentId?: string; @@ -78,6 +84,7 @@ export type SkillWorkshopProposal = { export type SkillWorkshopStatusFilter = "all" | SkillWorkshopProposalStatus; export type SkillWorkshopAction = "apply" | "evaluate" | "revise" | "reject"; export type SkillWorkshopMode = "board" | "today"; +export type SkillWorkshopAppliedDiffMode = "changes" | "full"; export type SkillWorkshopActionBusy = { key: string; @@ -94,6 +101,7 @@ type SkillWorkshopAppliedRevision = { proposal: SkillWorkshopProposal; version: number; operation: SkillWorkshopProposal["kind"]; + previous: SkillWorkshopProposal | null; }; export type SkillWorkshopAppliedSkill = { @@ -140,11 +148,29 @@ function groupSkillWorkshopAppliedSkills( latest: proposalsForSkill[0], revisions: proposalsForSkill.map((proposal, index) => { const version = proposalsForSkill.length - index; - return { proposal, version, operation: proposal.kind }; + return { + proposal, + version, + operation: proposal.kind, + previous: proposalsForSkill[index + 1] ?? null, + }; }), })); } +export function findSkillWorkshopAppliedPredecessor( + proposals: SkillWorkshopProposal[], + key: string, +): SkillWorkshopProposal | null { + for (const skill of groupSkillWorkshopAppliedSkills(proposals)) { + const revision = skill.revisions.find(({ proposal }) => proposal.key === key); + if (revision) { + return revision.previous; + } + } + return null; +} + export function filterSkillWorkshopAppliedSkills( proposals: SkillWorkshopProposal[], query: string, diff --git a/ui/src/pages/skill-workshop/applied-history.runtime.ts b/ui/src/pages/skill-workshop/applied-history.runtime.ts index 754d01969fe2..8fc67976b9cb 100644 --- a/ui/src/pages/skill-workshop/applied-history.runtime.ts +++ b/ui/src/pages/skill-workshop/applied-history.runtime.ts @@ -1,8 +1,43 @@ import { html } from "lit"; import { t } from "../../i18n/index.ts"; +import { computeLineDiff, type DiffLine, diffStat } from "../../lib/chat/tool-call-diff.ts"; import type { SkillWorkshopAppliedSkill } from "../../lib/skill-workshop/index.ts"; import type { SkillWorkshopProps } from "./view-types.ts"; +const DIFF_SIGN: Record = { + add: "+", + del: "-", + ctx: " ", + file: " ", + skip: "\u22ef", +}; + +function renderDiffRow(line: DiffLine) { + return html` +
+ + ${line.kind === "skip" ? "" : line.text} +
+ `; +} + +export function renderAppliedRevisionDiff(previousBody: string, body: string) { + const lines = computeLineDiff(previousBody, body, { compactUnchanged: true }); + if (lines.length === 0) { + return html`

${t("skillWorkshop.diff.unchanged")}

`; + } + const { added, removed } = diffStat(lines); + return html` +
+

+ +${added} + -${removed} +

+
${lines.map(renderDiffRow)}
+
+ `; +} + export function renderAppliedHistory(props: SkillWorkshopProps, skill: SkillWorkshopAppliedSkill) { return html`
diff --git a/ui/src/pages/skill-workshop/applied-history.ts b/ui/src/pages/skill-workshop/applied-history.ts index 5b8b97ea6670..176d387d9f5b 100644 --- a/ui/src/pages/skill-workshop/applied-history.ts +++ b/ui/src/pages/skill-workshop/applied-history.ts @@ -8,18 +8,26 @@ import { } from "../../lib/skill-workshop/index.ts"; import type { SkillWorkshopProps } from "./view-types.ts"; -type AppliedHistoryRenderer = typeof import("./applied-history.runtime.ts").renderAppliedHistory; +type AppliedHistoryRuntime = typeof import("./applied-history.runtime.ts"); -let appliedHistoryRenderer: AppliedHistoryRenderer | undefined; -let appliedHistoryRuntime: Promise | undefined; +const MAX_APPLIED_DIFF_INPUT_CHARS = 120_000; -function loadAppliedHistoryRenderer(): Promise { - return (appliedHistoryRuntime ??= import("./applied-history.runtime.ts").then((runtime) => { - appliedHistoryRenderer = runtime.renderAppliedHistory; - return appliedHistoryRenderer; +let appliedHistoryRuntime: AppliedHistoryRuntime | undefined; +let appliedHistoryLoad: Promise | undefined; + +// Diff computation and its markup stay behind this one dynamic import so the +// startup bundle never carries them; both lazy entries share the same load. +function loadAppliedHistoryRuntime(): Promise { + return (appliedHistoryLoad ??= import("./applied-history.runtime.ts").then((runtime) => { + appliedHistoryRuntime = runtime; + return runtime; })); } +function pendingRuntime() { + return html`

${t("common.loading")}

`; +} + export function resolveAppliedHistory( proposals: SkillWorkshopProposal[], query: string, @@ -35,15 +43,58 @@ export function resolveAppliedHistory( return { skills, selectedSkill, selectedProposal }; } +/** + * What the body card should show for a revision. `previousUnavailable` keeps + * the full body but says so, because a predecessor inspect can fail and a + * silent full body would read as "nothing changed". + */ +export type SkillWorkshopBodyView = + | { kind: "full" } + | { kind: "loadingPrevious" } + | { kind: "previousUnavailable" } + | { kind: "tooLarge" } + | { kind: "diff"; previous: SkillWorkshopProposal }; + +export function resolveAppliedBodyView( + props: SkillWorkshopProps, + proposal: SkillWorkshopProposal, + previous: SkillWorkshopProposal | null, +): SkillWorkshopBodyView { + if (!previous || props.appliedDiffMode === "full") { + return { kind: "full" }; + } + if (previous.bodyLoaded) { + if (previous.body.length + proposal.body.length > MAX_APPLIED_DIFF_INPUT_CHARS) { + return { kind: "tooLarge" }; + } + return { kind: "diff", previous }; + } + return props.inspectingKey === previous.key + ? { kind: "loadingPrevious" } + : { kind: "previousUnavailable" }; +} + export function renderLazyAppliedHistory( props: SkillWorkshopProps, skill: SkillWorkshopAppliedSkill, ) { - if (appliedHistoryRenderer) { - return appliedHistoryRenderer(props, skill); + if (appliedHistoryRuntime) { + return appliedHistoryRuntime.renderAppliedHistory(props, skill); } return until( - loadAppliedHistoryRenderer().then((renderer) => renderer(props, skill)), - html`

${t("common.loading")}

`, + loadAppliedHistoryRuntime().then((runtime) => runtime.renderAppliedHistory(props, skill)), + pendingRuntime(), + ); +} + +export function renderLazyAppliedRevisionDiff(previousBody: string, body: string) { + if (appliedHistoryRuntime) { + return appliedHistoryRuntime.renderAppliedRevisionDiff(previousBody, body); + } + return until( + loadAppliedHistoryRuntime().then((runtime) => + runtime.renderAppliedRevisionDiff(previousBody, body), + ), + pendingRuntime(), ); } diff --git a/ui/src/pages/skill-workshop/evaluation.browser.test.ts b/ui/src/pages/skill-workshop/evaluation.browser.test.ts index 8c199e9cbc26..0c7fbc6a04f0 100644 --- a/ui/src/pages/skill-workshop/evaluation.browser.test.ts +++ b/ui/src/pages/skill-workshop/evaluation.browser.test.ts @@ -84,6 +84,7 @@ const proposal: SkillWorkshopProposal = { recencyGroup: "today", ageLabel: "now", supportFiles: [], + bodyLoaded: true, isNew: false, }; @@ -101,6 +102,7 @@ function propsFor(mode: SkillWorkshopMode): SkillWorkshopProps { inspectingKey: null, proposals: [proposal], selectedKey: proposal.key, + appliedDiffMode: "changes", statusFilter: "pending", query: "", filePreviewKey: null, @@ -123,6 +125,7 @@ function propsFor(mode: SkillWorkshopMode): SkillWorkshopProps { onQueueWidthChange: vi.fn(), onModeChange: vi.fn(), onSelect: vi.fn(), + onAppliedDiffModeChange: vi.fn(), onPrev: vi.fn(), onNext: vi.fn(), onApply: vi.fn(), diff --git a/ui/src/pages/skill-workshop/proposal-records.ts b/ui/src/pages/skill-workshop/proposal-records.ts new file mode 100644 index 000000000000..d4e6d4ce6845 --- /dev/null +++ b/ui/src/pages/skill-workshop/proposal-records.ts @@ -0,0 +1,244 @@ +import { parseDateStringTimestampMs } from "@openclaw/normalization-core/number-coercion"; +import { formatBytes } from "../../lib/agents/display.ts"; +import type { + SkillWorkshopEvaluation, + SkillWorkshopProposal, + SkillWorkshopProposalStatus, +} from "../../lib/skill-workshop/index.ts"; + +type SkillProposalStatus = SkillWorkshopProposalStatus; +type SkillProposalKind = SkillWorkshopProposal["kind"]; +type SkillProposalScanState = "pending" | "clean" | "failed" | "quarantined"; + +type SkillProposalManifestEntry = { + id: string; + kind: SkillProposalKind; + status: SkillProposalStatus; + title: string; + description: string; + skillName: string; + skillKey: string; + createdAt: string; + updatedAt: string; + scanState: SkillProposalScanState; +}; + +export type SkillProposalManifest = { + schema: "openclaw.skill-workshop.proposals-manifest.v1"; + updatedAt: string; + proposals: SkillProposalManifestEntry[]; +}; + +type SkillProposalSupportFileRecord = { + path: string; + sizeBytes: number; +}; + +type SkillProposalOrigin = { + agentId?: string; + sessionKey?: string; + runId?: string; + messageId?: string; +}; + +type SkillProposalRecord = { + id: string; + kind: SkillProposalKind; + status: SkillProposalStatus; + title: string; + description: string; + createdAt: string; + updatedAt: string; + proposedVersion: string; + draftHash: string; + evaluation?: SkillWorkshopEvaluation; + origin?: SkillProposalOrigin; + supportFiles?: SkillProposalSupportFileRecord[]; + target: { + skillName: string; + skillKey: string; + }; +}; + +type SkillProposalSupportFile = { + path: string; + content: string; +}; + +export type SkillProposalInspectResult = { + record: SkillProposalRecord; + revisionHash?: string; + content: string; + supportFiles?: SkillProposalSupportFile[]; +}; + +export type SkillProposalEvaluateResult = { + record: SkillProposalRecord; + evaluation: SkillWorkshopEvaluation; +}; + +export function parseDateMs(value: string | undefined): number { + return parseDateStringTimestampMs(value) ?? Date.now(); +} + +function startOfLocalDay(ms: number): number { + const date = new Date(ms); + return new Date(date.getFullYear(), date.getMonth(), date.getDate()).getTime(); +} + +function recencyGroup(ms: number): SkillWorkshopProposal["recencyGroup"] { + const today = startOfLocalDay(Date.now()); + const day = startOfLocalDay(ms); + if (day === today) { + return "today"; + } + if (day === today - 24 * 60 * 60 * 1000) { + return "yesterday"; + } + return "earlier"; +} + +function compactAgeLabel(ms: number): string { + const diff = Math.max(0, Date.now() - ms); + const min = Math.floor(diff / 60_000); + if (min < 1) { + return "now"; + } + if (min < 60) { + return `${min}m`; + } + const hr = Math.floor(min / 60); + if (hr < 24) { + return `${hr}h`; + } + const day = Math.floor(hr / 24); + return `${day}d`; +} + +function proposedVersionNumber(value: string | undefined): number { + const parsed = Number.parseInt((value ?? "").replace(/^v/i, ""), 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : 1; +} + +function byteLength(value: string): number { + return new TextEncoder().encode(value).length; +} + +function stripProposalFrontmatter(content: string): string { + return content.replace(/^---\r?\n[\s\S]*?\r?\n---\r?\n?/, "").trim(); +} + +function supportFilesFromInspect( + result: SkillProposalInspectResult, +): SkillWorkshopProposal["supportFiles"] { + const sizes = new Map( + (result.record.supportFiles ?? []).map((file) => [file.path, file.sizeBytes]), + ); + return (result.supportFiles ?? []).map((file) => ({ + path: file.path, + size: formatBytes(Math.max(0, sizes.get(file.path) ?? byteLength(file.content)), { + fallback: "0 B", + maxUnit: "kilo", + fractionDigits: (_value, unit) => (unit === "byte" ? null : 1), + }), + contents: file.content, + })); +} + +export function proposalFromManifest( + entry: SkillProposalManifestEntry, + previous: SkillWorkshopProposal | undefined, +): SkillWorkshopProposal { + const updatedAt = parseDateMs(entry.updatedAt); + const createdAt = parseDateMs(entry.createdAt); + const previousIsCurrent = previous?.updatedAt === updatedAt; + return { + key: entry.id, + kind: entry.kind, + slug: entry.skillKey, + name: entry.title || entry.skillName, + oneLine: entry.description, + body: previousIsCurrent ? previous.body : "", + bodyLoaded: previousIsCurrent ? previous.bodyLoaded : false, + status: entry.status, + ...(previousIsCurrent && previous.origin ? { origin: previous.origin } : {}), + version: previousIsCurrent ? previous.version : 1, + revisionHash: previousIsCurrent ? previous.revisionHash : null, + ...(previousIsCurrent && previous.evaluation ? { evaluation: previous.evaluation } : {}), + createdAt, + updatedAt, + recencyGroup: recencyGroup(updatedAt || createdAt), + ageLabel: compactAgeLabel(updatedAt || createdAt), + supportFiles: previousIsCurrent ? previous.supportFiles : [], + isNew: previous?.isNew ?? false, + }; +} + +export function proposalFromInspect( + result: SkillProposalInspectResult, + previous: SkillWorkshopProposal | undefined, +): SkillWorkshopProposal { + const record = result.record; + const updatedAt = parseDateMs(record.updatedAt); + const createdAt = parseDateMs(record.createdAt); + const revisionHash = result.revisionHash?.trim() || null; + const evaluation = + record.evaluation?.revisionHash === revisionHash + ? record.evaluation + : previous?.evaluation?.revisionHash === revisionHash + ? previous.evaluation + : undefined; + return { + key: record.id, + kind: record.kind, + slug: record.target.skillKey, + name: record.title || record.target.skillName, + oneLine: record.description, + body: stripProposalFrontmatter(result.content), + bodyLoaded: true, + status: record.status, + ...(record.origin ? { origin: record.origin } : {}), + version: proposedVersionNumber(record.proposedVersion), + revisionHash, + ...(evaluation ? { evaluation } : {}), + createdAt, + updatedAt, + recencyGroup: recencyGroup(updatedAt || createdAt), + ageLabel: compactAgeLabel(updatedAt || createdAt), + supportFiles: supportFilesFromInspect(result), + isNew: previous?.isNew ?? false, + }; +} + +export function proposalFromEvaluation( + result: SkillProposalEvaluateResult, + previous: SkillWorkshopProposal, +): SkillWorkshopProposal { + const record = result.record; + const updatedAt = parseDateMs(record.updatedAt); + const createdAt = parseDateMs(record.createdAt); + return { + key: record.id, + kind: record.kind, + slug: record.target.skillKey, + name: record.title || record.target.skillName, + oneLine: record.description, + body: previous.body, + bodyLoaded: previous.bodyLoaded, + status: record.status, + ...(record.origin + ? { origin: record.origin } + : previous.origin + ? { origin: previous.origin } + : {}), + version: proposedVersionNumber(record.proposedVersion), + revisionHash: result.evaluation.revisionHash, + evaluation: result.evaluation, + createdAt, + updatedAt, + recencyGroup: recencyGroup(updatedAt || createdAt), + ageLabel: compactAgeLabel(updatedAt || createdAt), + supportFiles: previous.supportFiles, + isNew: previous.isNew, + }; +} diff --git a/ui/src/pages/skill-workshop/proposals.test.ts b/ui/src/pages/skill-workshop/proposals.test.ts index 861a39b3bf3d..d96ed5a8dfc0 100644 --- a/ui/src/pages/skill-workshop/proposals.test.ts +++ b/ui/src/pages/skill-workshop/proposals.test.ts @@ -129,6 +129,7 @@ function proposal(overrides: Partial = {}): SkillWorkshop recencyGroup: "today", ageLabel: "now", supportFiles: [], + bodyLoaded: true, isNew: false, ...overrides, }; @@ -189,6 +190,37 @@ describe("Skill Workshop proposal RPCs", () => { expect(state.skillWorkshopProposals[0]?.kind).toBe("create"); }); + it("reports a failed inspect for a selection retained across refresh", async () => { + const appliedManifest = manifest("applied"); + const latest = appliedManifest.proposals[0]; + if (!latest) { + throw new Error("Expected proposal fixture"); + } + const previous = { + ...latest, + id: "proposal-0", + updatedAt: "2026-06-15T12:00:00.000Z", + }; + const { state, context, request } = createFixture({ + skillWorkshopAgentId: "research", + skillWorkshopSelectedKey: "proposal-1", + }); + request.mockImplementation(async (method: string) => { + if (method === "skills.proposals.list") { + return { ...appliedManifest, proposals: [latest, previous] }; + } + throw new Error("inspect failed"); + }); + + await loadSkillWorkshopProposals(state, context, { force: true }); + + expect(state.skillWorkshopSelectedKey).toBe("proposal-1"); + expect(state.skillWorkshopError).toContain("inspect failed"); + expect(request.mock.calls.filter(([method]) => method === "skills.proposals.inspect")).toEqual([ + ["skills.proposals.inspect", { agentId: "research", proposalId: "proposal-1" }], + ]); + }); + it("preserves capped support-file size formatting through the shared helper", async () => { const { state, context, request } = createFixture(); const baseInspect = inspectResult(); @@ -217,7 +249,7 @@ describe("Skill Workshop proposal RPCs", () => { it("inspects a selected proposal with the agent from the current session", async () => { const { state, context, request } = createFixture( - { skillWorkshopProposals: [proposal({ body: "" })] }, + { skillWorkshopProposals: [proposal({ body: "", bodyLoaded: false })] }, { sessionKey: "agent:ops-team:main" }, ["skills.proposals.inspect"], ); @@ -363,7 +395,7 @@ describe("Skill Workshop proposal RPCs", () => { const { state, context, request } = createFixture( { skillWorkshopAgentId: "research", - skillWorkshopProposals: [proposal({ body: "" })], + skillWorkshopProposals: [proposal({ body: "", bodyLoaded: false })], }, {}, ["skills.proposals.inspect", "skills.proposals.evaluate"], @@ -385,7 +417,7 @@ describe("Skill Workshop proposal RPCs", () => { it("drops an inspected evaluation that belongs to a different revision", async () => { const baseInspect = inspectResult(); const { state, context, request } = createFixture( - { skillWorkshopProposals: [proposal({ body: "" })] }, + { skillWorkshopProposals: [proposal({ body: "", bodyLoaded: false })] }, {}, ["skills.proposals.inspect"], ); @@ -535,7 +567,7 @@ describe("Skill Workshop proposal RPCs", () => { const { state, context, request } = createFixture( { skillWorkshopAgentId: "research", - skillWorkshopProposals: [proposal({ body: "" })], + skillWorkshopProposals: [proposal({ body: "", bodyLoaded: false })], }, {}, ["skills.proposals.inspect"], @@ -584,7 +616,7 @@ describe("Skill Workshop proposal RPCs", () => { const { state, context, request } = createFixture( { skillWorkshopAgentId: "research", - skillWorkshopProposals: [proposal({ body: "" })], + skillWorkshopProposals: [proposal({ body: "", bodyLoaded: false })], skillWorkshopRevisionDraft: "Tighten the trigger.", }, {}, @@ -611,7 +643,7 @@ describe("Skill Workshop proposal RPCs", () => { const { state, context, request } = createFixture( { skillWorkshopAgentId: "research", - skillWorkshopProposals: [proposal({ body: "" })], + skillWorkshopProposals: [proposal({ body: "", bodyLoaded: false })], skillWorkshopRevisionDraft: "Tighten the trigger.", }, {}, @@ -634,4 +666,62 @@ describe("Skill Workshop proposal RPCs", () => { await expect(revision).resolves.toBe(false); expect(sendRevisionRequest).not.toHaveBeenCalled(); }); + + it("ignores a superseded selection and keeps its error out of the pane", async () => { + const first = createDeferred>(); + const second = createDeferred>(); + const { state, context, request } = createFixture( + { + skillWorkshopAgentId: "research", + skillWorkshopProposals: [ + proposal({ key: "proposal-1", body: "", bodyLoaded: false }), + proposal({ key: "proposal-2", body: "", bodyLoaded: false }), + ], + }, + {}, + ["skills.proposals.inspect"], + ); + request.mockImplementation(async (_method, payload) => + (payload as { proposalId: string }).proposalId === "proposal-1" + ? first.promise + : second.promise, + ); + + const stale = selectSkillWorkshopProposal(state, context, "proposal-1"); + const latest = selectSkillWorkshopProposal(state, context, "proposal-2"); + const base = inspectResult(); + second.resolve({ ...base, record: { ...base.record, id: "proposal-2" } }); + await latest; + first.reject(new Error("inspect failed")); + await stale; + + expect(state.skillWorkshopSelectedKey).toBe("proposal-2"); + expect(state.skillWorkshopError).toBeNull(); + }); + + it("inspects a revision once even when its body is legitimately empty", async () => { + const { state, context, request } = createFixture( + { + skillWorkshopAgentId: "research", + skillWorkshopProposals: [proposal({ body: "", bodyLoaded: false })], + }, + {}, + ["skills.proposals.inspect"], + ); + const base = inspectResult(); + request.mockResolvedValue({ ...base, content: "" }); + + await Promise.all([ + selectSkillWorkshopProposal(state, context, "proposal-1"), + selectSkillWorkshopProposal(state, context, "proposal-1"), + ]); + await selectSkillWorkshopProposal(state, context, "proposal-1"); + + expect(state.skillWorkshopProposals[0]?.body).toBe(""); + expect(state.skillWorkshopProposals[0]?.bodyLoaded).toBe(true); + expect( + request.mock.calls.filter(([method]) => method === "skills.proposals.inspect"), + ).toHaveLength(1); + expect(state.skillWorkshopSelectedKey).toBe("proposal-1"); + }); }); diff --git a/ui/src/pages/skill-workshop/proposals.ts b/ui/src/pages/skill-workshop/proposals.ts index 59fe23211f63..460dcd1a0079 100644 --- a/ui/src/pages/skill-workshop/proposals.ts +++ b/ui/src/pages/skill-workshop/proposals.ts @@ -1,9 +1,7 @@ // Control UI controller manages skill workshop gateway state. -import { parseDateStringTimestampMs } from "@openclaw/normalization-core/number-coercion"; import type { AgentSelectionCapability } from "../../app/agent-selection.ts"; import type { ApplicationGateway } from "../../app/context.ts"; import { t } from "../../i18n/index.ts"; -import { formatBytes } from "../../lib/agents/display.ts"; import { formatUiError } from "../../lib/format-error.ts"; import { canCallGatewayMethod } from "../../lib/gateway-methods.ts"; import { @@ -11,12 +9,21 @@ import { parseAgentSessionKey, resolveUiSelectedGlobalAgentId, } from "../../lib/sessions/session-key.ts"; -import type { - SkillWorkshopAction, - SkillWorkshopEvaluation, - SkillWorkshopProposal, - SkillWorkshopProposalStatus, +import { + findSkillWorkshopAppliedPredecessor, + type SkillWorkshopAction, + type SkillWorkshopProposal, + type SkillWorkshopProposalStatus, } from "../../lib/skill-workshop/index.ts"; +import { + parseDateMs, + proposalFromEvaluation, + proposalFromInspect, + proposalFromManifest, + type SkillProposalEvaluateResult, + type SkillProposalInspectResult, + type SkillProposalManifest, +} from "./proposal-records.ts"; import { createSkillWorkshopHistoryScanState, type SkillWorkshopState } from "./state.ts"; export { createSkillWorkshopState, @@ -27,77 +34,6 @@ export { const SKILL_WORKSHOP_NOTICE_MS = 2800; -type SkillProposalStatus = SkillWorkshopProposalStatus; -type SkillProposalKind = SkillWorkshopProposal["kind"]; -type SkillProposalScanState = "pending" | "clean" | "failed" | "quarantined"; - -type SkillProposalManifestEntry = { - id: string; - kind: SkillProposalKind; - status: SkillProposalStatus; - title: string; - description: string; - skillName: string; - skillKey: string; - createdAt: string; - updatedAt: string; - scanState: SkillProposalScanState; -}; - -type SkillProposalManifest = { - schema: "openclaw.skill-workshop.proposals-manifest.v1"; - updatedAt: string; - proposals: SkillProposalManifestEntry[]; -}; - -type SkillProposalSupportFileRecord = { - path: string; - sizeBytes: number; -}; - -type SkillProposalOrigin = { - agentId?: string; - sessionKey?: string; - runId?: string; - messageId?: string; -}; - -type SkillProposalRecord = { - id: string; - kind: SkillProposalKind; - status: SkillProposalStatus; - title: string; - description: string; - createdAt: string; - updatedAt: string; - proposedVersion: string; - draftHash: string; - evaluation?: SkillWorkshopEvaluation; - origin?: SkillProposalOrigin; - supportFiles?: SkillProposalSupportFileRecord[]; - target: { - skillName: string; - skillKey: string; - }; -}; - -type SkillProposalSupportFile = { - path: string; - content: string; -}; - -type SkillProposalInspectResult = { - record: SkillProposalRecord; - revisionHash?: string; - content: string; - supportFiles?: SkillProposalSupportFile[]; -}; - -type SkillProposalEvaluateResult = { - record: SkillProposalRecord; - evaluation: SkillWorkshopEvaluation; -}; - export type SkillWorkshopContext = { gateway: ApplicationGateway; agentSelection: Pick; @@ -139,170 +75,10 @@ function resetSkillWorkshopAgentScope(state: SkillWorkshopState, agentId: string state.skillWorkshopRevisionDraft = ""; state.skillWorkshopFilePreviewKey = null; state.skillWorkshopFilePreviewQuery = ""; + state.skillWorkshopAppliedDiffMode = "changes"; state.skillWorkshopHistoryScan = createSkillWorkshopHistoryScanState(); -} - -function parseDateMs(value: string | undefined): number { - return parseDateStringTimestampMs(value) ?? Date.now(); -} - -function startOfLocalDay(ms: number): number { - const date = new Date(ms); - return new Date(date.getFullYear(), date.getMonth(), date.getDate()).getTime(); -} - -function recencyGroup(ms: number): SkillWorkshopProposal["recencyGroup"] { - const today = startOfLocalDay(Date.now()); - const day = startOfLocalDay(ms); - if (day === today) { - return "today"; - } - if (day === today - 24 * 60 * 60 * 1000) { - return "yesterday"; - } - return "earlier"; -} - -function compactAgeLabel(ms: number): string { - const diff = Math.max(0, Date.now() - ms); - const min = Math.floor(diff / 60_000); - if (min < 1) { - return "now"; - } - if (min < 60) { - return `${min}m`; - } - const hr = Math.floor(min / 60); - if (hr < 24) { - return `${hr}h`; - } - const day = Math.floor(hr / 24); - return `${day}d`; -} - -function proposedVersionNumber(value: string | undefined): number { - const parsed = Number.parseInt((value ?? "").replace(/^v/i, ""), 10); - return Number.isFinite(parsed) && parsed > 0 ? parsed : 1; -} - -function byteLength(value: string): number { - return new TextEncoder().encode(value).length; -} - -function stripProposalFrontmatter(content: string): string { - return content.replace(/^---\r?\n[\s\S]*?\r?\n---\r?\n?/, "").trim(); -} - -function supportFilesFromInspect( - result: SkillProposalInspectResult, -): SkillWorkshopProposal["supportFiles"] { - const sizes = new Map( - (result.record.supportFiles ?? []).map((file) => [file.path, file.sizeBytes]), - ); - return (result.supportFiles ?? []).map((file) => ({ - path: file.path, - size: formatBytes(Math.max(0, sizes.get(file.path) ?? byteLength(file.content)), { - fallback: "0 B", - maxUnit: "kilo", - fractionDigits: (_value, unit) => (unit === "byte" ? null : 1), - }), - contents: file.content, - })); -} - -function proposalFromManifest( - entry: SkillProposalManifestEntry, - previous: SkillWorkshopProposal | undefined, -): SkillWorkshopProposal { - const updatedAt = parseDateMs(entry.updatedAt); - const createdAt = parseDateMs(entry.createdAt); - const previousIsCurrent = previous?.updatedAt === updatedAt; - return { - key: entry.id, - kind: entry.kind, - slug: entry.skillKey, - name: entry.title || entry.skillName, - oneLine: entry.description, - body: previousIsCurrent ? previous.body : "", - status: entry.status, - ...(previousIsCurrent && previous.origin ? { origin: previous.origin } : {}), - version: previousIsCurrent ? previous.version : 1, - revisionHash: previousIsCurrent ? previous.revisionHash : null, - ...(previousIsCurrent && previous.evaluation ? { evaluation: previous.evaluation } : {}), - createdAt, - updatedAt, - recencyGroup: recencyGroup(updatedAt || createdAt), - ageLabel: compactAgeLabel(updatedAt || createdAt), - supportFiles: previousIsCurrent ? previous.supportFiles : [], - isNew: previous?.isNew ?? false, - }; -} - -function proposalFromInspect( - result: SkillProposalInspectResult, - previous: SkillWorkshopProposal | undefined, -): SkillWorkshopProposal { - const record = result.record; - const updatedAt = parseDateMs(record.updatedAt); - const createdAt = parseDateMs(record.createdAt); - const revisionHash = result.revisionHash?.trim() || null; - const evaluation = - record.evaluation?.revisionHash === revisionHash - ? record.evaluation - : previous?.evaluation?.revisionHash === revisionHash - ? previous.evaluation - : undefined; - return { - key: record.id, - kind: record.kind, - slug: record.target.skillKey, - name: record.title || record.target.skillName, - oneLine: record.description, - body: stripProposalFrontmatter(result.content), - status: record.status, - ...(record.origin ? { origin: record.origin } : {}), - version: proposedVersionNumber(record.proposedVersion), - revisionHash, - ...(evaluation ? { evaluation } : {}), - createdAt, - updatedAt, - recencyGroup: recencyGroup(updatedAt || createdAt), - ageLabel: compactAgeLabel(updatedAt || createdAt), - supportFiles: supportFilesFromInspect(result), - isNew: previous?.isNew ?? false, - }; -} - -function proposalFromEvaluation( - result: SkillProposalEvaluateResult, - previous: SkillWorkshopProposal, -): SkillWorkshopProposal { - const record = result.record; - const updatedAt = parseDateMs(record.updatedAt); - const createdAt = parseDateMs(record.createdAt); - return { - key: record.id, - kind: record.kind, - slug: record.target.skillKey, - name: record.title || record.target.skillName, - oneLine: record.description, - body: previous.body, - status: record.status, - ...(record.origin - ? { origin: record.origin } - : previous.origin - ? { origin: previous.origin } - : {}), - version: proposedVersionNumber(record.proposedVersion), - revisionHash: result.evaluation.revisionHash, - evaluation: result.evaluation, - createdAt, - updatedAt, - recencyGroup: recencyGroup(updatedAt || createdAt), - ageLabel: compactAgeLabel(updatedAt || createdAt), - supportFiles: previous.supportFiles, - isNew: previous.isNew, - }; + inspectRequestsByState.delete(state); + selectionRequestByState.delete(state); } function mergeProposal(state: SkillWorkshopState, proposal: SkillWorkshopProposal): void { @@ -350,7 +126,7 @@ function showActionNotice( export function countSkillWorkshopProposals( proposals: SkillWorkshopProposal[], -): Record<"all" | SkillProposalStatus, number> { +): Record<"all" | SkillWorkshopProposalStatus, number> { // Applied renders one row per skill, so its tab count is grouped skills; // every other status stays a per-proposal count. const appliedSkills = new Set(); @@ -409,9 +185,24 @@ export async function loadSkillWorkshopProposals( state.skillWorkshopLoaded = true; if (!proposals.some((proposal) => proposal.key === state.skillWorkshopSelectedKey)) { state.skillWorkshopSelectedKey = proposals[0]?.key ?? null; + // Only a refresh that actually reassigns the pane owns the selection + // fence; otherwise a background reload would silence an in-flight click. + if (state.skillWorkshopSelectedKey) { + markSkillWorkshopSelectionRequest(state, state.skillWorkshopSelectedKey); + } } - if (state.skillWorkshopSelectedKey) { - await loadSkillWorkshopProposalDetail(state, context, state.skillWorkshopSelectedKey); + const selectedKey = state.skillWorkshopSelectedKey; + if (selectedKey) { + // Route data retains the selection but not its ephemeral request fence. + if (!selectionRequestByState.has(state)) { + markSkillWorkshopSelectionRequest(state, selectedKey); + } + const selectedLoaded = await loadSkillWorkshopProposalDetail(state, context, selectedKey); + if (selectedLoaded) { + // The Applied tab can be opened without a fresh click, so the predecessor + // has to be warmed here too or the diff never has a baseline. + await loadSkillWorkshopPredecessorBody(state, context, selectedKey); + } } } catch (err) { state.skillWorkshopError = formatUiError(err); @@ -423,25 +214,40 @@ export async function loadSkillWorkshopProposals( } } -async function loadSkillWorkshopProposalDetail( +type SkillWorkshopGatewayClient = NonNullable; + +// Rapid history clicks overlap: each inspect awaits the Gateway, so a slower +// earlier request must neither re-issue the same call nor publish its selection +// or error after a newer click won the pane. Both fences are keyed on the live +// state object so nothing reaches the persisted route data. +const inspectRequestsByState = new WeakMap>>(); +const selectionRequestByState = new WeakMap(); + +function inspectRequests(state: SkillWorkshopState): Map> { + const existing = inspectRequestsByState.get(state); + if (existing) { + return existing; + } + const requests = new Map>(); + inspectRequestsByState.set(state, requests); + return requests; +} + +function markSkillWorkshopSelectionRequest(state: SkillWorkshopState, proposalId: string): void { + selectionRequestByState.set(state, proposalId); +} + +function isLatestSkillWorkshopSelection(state: SkillWorkshopState, proposalId: string): boolean { + return selectionRequestByState.get(state) === proposalId; +} + +async function inspectSkillWorkshopProposal( state: SkillWorkshopState, context: SkillWorkshopContext, + client: SkillWorkshopGatewayClient, proposalId: string, - options?: { force?: boolean }, + existing: SkillWorkshopProposal | undefined, ): Promise { - const snapshot = context.gateway.snapshot; - const client = snapshot.client; - if ( - !client || - snapshot.phase !== "connected" || - state.skillWorkshopInspectingKey === proposalId - ) { - return false; - } - const existing = state.skillWorkshopProposals.find((proposal) => proposal.key === proposalId); - if (existing?.body && !options?.force) { - return true; - } const requestAgentId = loadedSkillWorkshopAgentParams(state, context).agentId; if (state.skillWorkshopAgentId === null) { state.skillWorkshopAgentId = requestAgentId; @@ -454,16 +260,18 @@ async function loadSkillWorkshopProposalDetail( "skills.proposals.inspect", requestParams, ); - if ( - state.skillWorkshopAgentId !== requestAgentId || - state.skillWorkshopInspectingKey !== proposalId - ) { + if (state.skillWorkshopAgentId !== requestAgentId) { return false; } mergeProposal(state, proposalFromInspect(result, existing)); return true; } catch (err) { - if (state.skillWorkshopAgentId === requestAgentId) { + // Only the revision the operator is waiting on may publish an error; a + // superseded click or a background predecessor fetch stays quiet. + if ( + state.skillWorkshopAgentId === requestAgentId && + isLatestSkillWorkshopSelection(state, proposalId) + ) { state.skillWorkshopError = formatUiError(err); } return false; @@ -477,19 +285,68 @@ async function loadSkillWorkshopProposalDetail( } } +function loadSkillWorkshopProposalDetail( + state: SkillWorkshopState, + context: SkillWorkshopContext, + proposalId: string, + options?: { force?: boolean }, +): Promise { + const snapshot = context.gateway.snapshot; + const client = snapshot.client; + if (!client || snapshot.phase !== "connected") { + return Promise.resolve(false); + } + const existing = state.skillWorkshopProposals.find((proposal) => proposal.key === proposalId); + if (existing?.bodyLoaded && !options?.force) { + return Promise.resolve(true); + } + const requests = inspectRequests(state); + const inFlight = requests.get(proposalId); + if (inFlight) { + return inFlight; + } + const request = inspectSkillWorkshopProposal( + state, + context, + client, + proposalId, + existing, + ).finally(() => { + if (requests.get(proposalId) === request) { + requests.delete(proposalId); + } + }); + requests.set(proposalId, request); + return request; +} + +function loadSkillWorkshopPredecessorBody( + state: SkillWorkshopState, + context: SkillWorkshopContext, + proposalId: string, +): Promise { + const previous = findSkillWorkshopAppliedPredecessor(state.skillWorkshopProposals, proposalId); + return previous && !previous.bodyLoaded + ? loadSkillWorkshopProposalDetail(state, context, previous.key) + : Promise.resolve(false); +} + export async function selectSkillWorkshopProposal( state: SkillWorkshopState, context: SkillWorkshopContext, proposalId: string, ): Promise { + markSkillWorkshopSelectionRequest(state, proposalId); const current = state.skillWorkshopProposals.find((proposal) => proposal.key === proposalId); - if (!current?.body) { + if (!current?.bodyLoaded) { const loaded = await loadSkillWorkshopProposalDetail(state, context, proposalId); - if (!loaded) { + if (!loaded || !isLatestSkillWorkshopSelection(state, proposalId)) { return; } } state.skillWorkshopSelectedKey = proposalId; + state.skillWorkshopAppliedDiffMode = "changes"; + await loadSkillWorkshopPredecessorBody(state, context, proposalId); } async function refreshAfterMutation( diff --git a/ui/src/pages/skill-workshop/skill-workshop-page.applied.test.ts b/ui/src/pages/skill-workshop/skill-workshop-page.applied.test.ts index be952f6870f0..915852516642 100644 --- a/ui/src/pages/skill-workshop/skill-workshop-page.applied.test.ts +++ b/ui/src/pages/skill-workshop/skill-workshop-page.applied.test.ts @@ -60,75 +60,98 @@ function createContext(request: ReturnType): ApplicationContext { } as unknown as ApplicationContext; } +function appliedProposals(): SkillWorkshopProposal[] { + return [1, 2, 3, 4].map((updatedAt) => ({ + key: `proposal-${updatedAt}`, + kind: updatedAt === 2 ? "create" : "update", + slug: "release-sanity", + name: `${updatedAt === 1 ? "Create" : "Update"} release-sanity`, + oneLine: `Revision ${updatedAt} description`, + body: updatedAt === 1 ? "" : `## Workflow\n- Revision ${updatedAt}`, + status: "applied", + version: 1, + revisionHash: null, + createdAt: updatedAt, + updatedAt, + recencyGroup: "today", + ageLabel: `${updatedAt}h`, + supportFiles: [], + bodyLoaded: updatedAt !== 1, + isNew: false, + })); +} + +function inspectRequest() { + return vi.fn(async (method: string, params?: unknown) => { + if (method !== "skills.proposals.inspect") { + return {}; + } + expect(params).toEqual({ agentId: "research", proposalId: "proposal-1" }); + return { + record: { + id: "proposal-1", + kind: "update", + status: "applied", + title: "Create release-sanity", + description: "Revision 1 description", + createdAt: new Date(1).toISOString(), + updatedAt: new Date(1).toISOString(), + proposedVersion: "v1", + draftHash: "a".repeat(64), + target: { skillName: "Release sanity", skillKey: "release-sanity" }, + }, + revisionHash: "b".repeat(64), + content: "## Workflow\n- Revision 1 inspected", + supportFiles: [], + }; + }); +} + +async function mountAppliedPage( + request: ReturnType, + proposals: SkillWorkshopProposal[], +): Promise { + const loadedState = createSkillWorkshopState(); + loadedState.skillWorkshopAgentId = "research"; + loadedState.skillWorkshopLoaded = true; + loadedState.skillWorkshopProposals = proposals; + loadedState.skillWorkshopSelectedKey = "proposal-4"; + // SAFETY: the registered custom element exposes the tested reactive page fields. + const page = document.createElement( + "openclaw-skill-workshop-page", + ) as SkillWorkshopPageTestElement; + page.data = skillWorkshopRouteData(loadedState); + page.context = createContext(request); + document.body.append(page); + await page.updateComplete; + if (!page.state) { + throw new Error("Expected Skill Workshop state"); + } + page.state.skillWorkshopMode = "board"; + page.state.skillWorkshopStatusFilter = "applied"; + page.requestUpdate(); + await page.updateComplete; + return page; +} + +function historyItems(page: SkillWorkshopPageTestElement) { + return page.querySelectorAll(".sw-applied-history__item"); +} + +function modeButton(page: SkillWorkshopPageTestElement, label: string) { + return [...page.querySelectorAll(".sw-body-mode__button")].find( + (button) => button.textContent?.trim() === label, + ); +} + afterEach(() => { document.body.replaceChildren(); }); describe("Skill Workshop applied history", () => { it("renders one skill row and inspects a selected revision", async () => { - const timestamps = [1, 2, 3, 4]; - const proposals = timestamps.map( - (updatedAt): SkillWorkshopProposal => ({ - key: `proposal-${updatedAt}`, - kind: updatedAt === 2 ? "create" : "update", - slug: "release-sanity", - name: `${updatedAt === 1 ? "Create" : "Update"} release-sanity`, - oneLine: `Revision ${updatedAt} description`, - body: updatedAt === 1 ? "" : `## Workflow\n- Revision ${updatedAt}`, - status: "applied", - version: 1, - revisionHash: null, - createdAt: updatedAt, - updatedAt, - recencyGroup: "today", - ageLabel: `${updatedAt}h`, - supportFiles: [], - isNew: false, - }), - ); - const request = vi.fn(async (method: string, params?: unknown) => { - if (method !== "skills.proposals.inspect") { - return {}; - } - expect(params).toEqual({ agentId: "research", proposalId: "proposal-1" }); - return { - record: { - id: "proposal-1", - kind: "update", - status: "applied", - title: "Create release-sanity", - description: "Revision 1 description", - createdAt: new Date(1).toISOString(), - updatedAt: new Date(1).toISOString(), - proposedVersion: "v1", - draftHash: "a".repeat(64), - target: { skillName: "Release sanity", skillKey: "release-sanity" }, - }, - revisionHash: "b".repeat(64), - content: "## Workflow\n- Revision 1 inspected", - supportFiles: [], - }; - }); - const loadedState = createSkillWorkshopState(); - loadedState.skillWorkshopAgentId = "research"; - loadedState.skillWorkshopLoaded = true; - loadedState.skillWorkshopProposals = proposals; - loadedState.skillWorkshopSelectedKey = "proposal-4"; - // SAFETY: the registered custom element exposes the tested reactive page fields. - const page = document.createElement( - "openclaw-skill-workshop-page", - ) as SkillWorkshopPageTestElement; - page.data = skillWorkshopRouteData(loadedState); - page.context = createContext(request); - document.body.append(page); - await page.updateComplete; - if (!page.state) { - throw new Error("Expected Skill Workshop state"); - } - page.state.skillWorkshopMode = "board"; - page.state.skillWorkshopStatusFilter = "applied"; - page.requestUpdate(); - await page.updateComplete; + const request = inspectRequest(); + const page = await mountAppliedPage(request, appliedProposals()); expect(page.querySelectorAll(".sw-row")).toHaveLength(1); expect(page.querySelector(".sw-row")?.textContent).toContain("4 revisions"); @@ -137,11 +160,8 @@ describe("Skill Workshop applied history", () => { button.textContent?.includes("Applied"), ); expect(appliedFilter?.querySelector(".settings-count")?.textContent).toBe("1"); - await vi.waitFor( - () => expect(page.querySelectorAll(".sw-applied-history__item")).toHaveLength(4), - { interval: 1 }, - ); - const history = page.querySelectorAll(".sw-applied-history__item"); + await vi.waitFor(() => expect(historyItems(page)).toHaveLength(4), { interval: 1 }); + const history = historyItems(page); expect(history[0]?.textContent).toContain("Update"); expect(history[0]?.textContent).toContain("v4"); expect(history[2]?.textContent).toContain("Create"); @@ -163,4 +183,69 @@ describe("Skill Workshop applied history", () => { { interval: 1 }, ); }); + + it("diffs a revision against its predecessor and toggles back to the full body", async () => { + const page = await mountAppliedPage(inspectRequest(), appliedProposals()); + await vi.waitFor(() => expect(historyItems(page)).toHaveLength(4), { interval: 1 }); + + await vi.waitFor(() => expect(page.querySelector(".sw-diff")).not.toBeNull(), { interval: 1 }); + const added = [...page.querySelectorAll(".sw-diff__row--add")].map((row) => row.textContent); + const removed = [...page.querySelectorAll(".sw-diff__row--del")].map((row) => row.textContent); + expect(added.join("")).toContain("Revision 4"); + expect(removed.join("")).toContain("Revision 3"); + expect(page.querySelector(".sw-diff__stat")?.textContent?.replace(/\s+/gu, "")).toBe("+1-1"); + + modeButton(page, "Full body")?.click(); + await vi.waitFor( + () => { + expect(page.querySelector(".sw-diff")).toBeNull(); + expect(page.querySelector(".sw-body-card")?.textContent).toContain("Revision 4"); + }, + { interval: 1 }, + ); + expect(modeButton(page, "Full body")?.getAttribute("aria-pressed")).toBe("true"); + expect(modeButton(page, "Changes")?.getAttribute("aria-pressed")).toBe("false"); + }); + + it("shows the oldest revision as a full body with no diff toggle", async () => { + const page = await mountAppliedPage(inspectRequest(), appliedProposals()); + await vi.waitFor(() => expect(historyItems(page)).toHaveLength(4), { interval: 1 }); + + historyItems(page)[3]?.click(); + await vi.waitFor( + () => { + expect(page.querySelector(".sw-detail__body")?.textContent).toContain( + "Revision 1 inspected", + ); + expect(page.querySelector(".sw-body-mode")).toBeNull(); + expect(page.querySelector(".sw-diff")).toBeNull(); + }, + { interval: 1 }, + ); + }); + + it("bounds large comparisons and keeps the full body available", async () => { + const largeBody = "x".repeat(60_001); + const proposals = appliedProposals(); + for (const proposal of proposals.slice(2)) { + proposal.body = largeBody; + } + const page = await mountAppliedPage(inspectRequest(), proposals); + + await vi.waitFor( + () => { + expect(page.querySelector(".sw-diff")).toBeNull(); + expect(page.querySelector(".sw-body-card")?.textContent).toContain( + "This comparison is too large to show here.", + ); + }, + { interval: 1 }, + ); + + modeButton(page, "Full body")?.click(); + await vi.waitFor( + () => expect(page.querySelector(".sw-body-card")?.textContent).toContain(largeBody), + { interval: 1 }, + ); + }); }); diff --git a/ui/src/pages/skill-workshop/skill-workshop-page.test.ts b/ui/src/pages/skill-workshop/skill-workshop-page.test.ts index 3a8c4e3f0adc..912118528ef8 100644 --- a/ui/src/pages/skill-workshop/skill-workshop-page.test.ts +++ b/ui/src/pages/skill-workshop/skill-workshop-page.test.ts @@ -127,6 +127,7 @@ function createProposal(overrides: Partial): SkillWorksho recencyGroup: "today", ageLabel: "now", supportFiles: [], + bodyLoaded: true, isNew: false, ...overrides, }; diff --git a/ui/src/pages/skill-workshop/skill-workshop-page.ts b/ui/src/pages/skill-workshop/skill-workshop-page.ts index e870c788fae4..65d7048a4633 100644 --- a/ui/src/pages/skill-workshop/skill-workshop-page.ts +++ b/ui/src/pages/skill-workshop/skill-workshop-page.ts @@ -8,7 +8,10 @@ import "../../components/tooltip.ts"; import { t } from "../../i18n/index.ts"; import { sessionNavigationTarget } from "../../lib/sessions/route-navigation.ts"; import { normalizeAgentId } from "../../lib/sessions/session-key.ts"; -import { filterSkillWorkshopProposals } from "../../lib/skill-workshop/index.ts"; +import { + filterSkillWorkshopProposals, + type SkillWorkshopAppliedDiffMode, +} from "../../lib/skill-workshop/index.ts"; import { OpenClawLightDomElement } from "../../lit/openclaw-element.ts"; import { SubscriptionsController } from "../../lit/subscriptions-controller.ts"; import { PLUGINS_HUB_PANEL_ID, pluginsHubTabs } from "../plugins/plugins-hub.ts"; @@ -145,6 +148,7 @@ function renderSkillWorkshopPage( inspectingKey: state.skillWorkshopInspectingKey, proposals: state.skillWorkshopProposals, selectedKey: state.skillWorkshopSelectedKey, + appliedDiffMode: state.skillWorkshopAppliedDiffMode, statusFilter: state.skillWorkshopStatusFilter, query: state.skillWorkshopQuery, filePreviewKey: state.skillWorkshopFilePreviewKey, @@ -195,6 +199,10 @@ function renderSkillWorkshopPage( }, onModeChange: (mode) => setSkillWorkshopMode(state, mode, requestUpdate), onSelect: selectProposal, + onAppliedDiffModeChange: (mode: SkillWorkshopAppliedDiffMode) => { + state.skillWorkshopAppliedDiffMode = mode; + requestUpdate(); + }, onPrev: () => selectRelativeProposal(-1), onNext: () => selectRelativeProposal(1), onApply: (key) => { diff --git a/ui/src/pages/skill-workshop/state.ts b/ui/src/pages/skill-workshop/state.ts index 83662e6a4d0a..f7f7fc1c6350 100644 --- a/ui/src/pages/skill-workshop/state.ts +++ b/ui/src/pages/skill-workshop/state.ts @@ -1,6 +1,7 @@ import type { SkillWorkshopAction, SkillWorkshopActionNotice, + SkillWorkshopAppliedDiffMode, SkillWorkshopMode, SkillWorkshopProposal, SkillWorkshopStatusFilter, @@ -45,6 +46,7 @@ export type SkillWorkshopState = { skillWorkshopInspectingKey: string | null; skillWorkshopProposals: SkillWorkshopProposal[]; skillWorkshopSelectedKey: string | null; + skillWorkshopAppliedDiffMode: SkillWorkshopAppliedDiffMode; skillWorkshopActionBusy: { key: string; action: SkillWorkshopAction } | null; skillWorkshopActionNotice: SkillWorkshopActionNotice | null; skillWorkshopActionNoticeTimer?: ReturnType | number | null; @@ -85,6 +87,7 @@ export function createSkillWorkshopState(data?: SkillWorkshopRouteData): SkillWo skillWorkshopInspectingKey: data?.skillWorkshopInspectingKey ?? null, skillWorkshopProposals: data?.skillWorkshopProposals ?? [], skillWorkshopSelectedKey: data?.skillWorkshopSelectedKey ?? null, + skillWorkshopAppliedDiffMode: "changes", skillWorkshopActionBusy: data?.skillWorkshopActionBusy ?? null, skillWorkshopActionNotice: data?.skillWorkshopActionNotice ?? null, skillWorkshopActionNoticeTimer: null, diff --git a/ui/src/pages/skill-workshop/view-types.ts b/ui/src/pages/skill-workshop/view-types.ts index 6743d57a7b3c..3fae479bdd11 100644 --- a/ui/src/pages/skill-workshop/view-types.ts +++ b/ui/src/pages/skill-workshop/view-types.ts @@ -1,6 +1,7 @@ import type { SkillWorkshopActionBusy, SkillWorkshopActionNotice, + SkillWorkshopAppliedDiffMode, SkillWorkshopMode, SkillWorkshopProposal, SkillWorkshopStatusFilter, @@ -16,6 +17,7 @@ export type SkillWorkshopProps = { inspectingKey: string | null; proposals: SkillWorkshopProposal[]; selectedKey: string | null; + appliedDiffMode: SkillWorkshopAppliedDiffMode; statusFilter: SkillWorkshopStatusFilter; query: string; filePreviewKey: string | null; @@ -38,6 +40,7 @@ export type SkillWorkshopProps = { onQueueWidthChange: (width: number) => void; onModeChange: (mode: SkillWorkshopMode) => void; onSelect: (key: string) => void; + onAppliedDiffModeChange: (mode: SkillWorkshopAppliedDiffMode) => void; onPrev: () => void; onNext: () => void; onApply: (key: string) => void; diff --git a/ui/src/pages/skill-workshop/view.ts b/ui/src/pages/skill-workshop/view.ts index 92624caabc36..d9e48351abbd 100644 --- a/ui/src/pages/skill-workshop/view.ts +++ b/ui/src/pages/skill-workshop/view.ts @@ -14,6 +14,7 @@ import "../../styles/skill-workshop.css"; import { filterSkillWorkshopProposals, type SkillWorkshopActionNotice, + type SkillWorkshopAppliedDiffMode, type SkillWorkshopAppliedSkill, type SkillWorkshopEvaluation, type SkillWorkshopEvaluationFinding, @@ -21,7 +22,13 @@ import { type SkillWorkshopProposal, type SkillWorkshopStatusFilter, } from "../../lib/skill-workshop/index.ts"; -import { renderLazyAppliedHistory, resolveAppliedHistory } from "./applied-history.ts"; +import { + renderLazyAppliedHistory, + renderLazyAppliedRevisionDiff, + resolveAppliedBodyView, + resolveAppliedHistory, + type SkillWorkshopBodyView, +} from "./applied-history.ts"; import { renderBoardEmptyDetail, renderWorkshopEmptyState } from "./empty-states.ts"; import { renderSkillWorkshopHistoryScan } from "./history-scan.ts"; import { renderSkillWorkshopProposalList } from "./proposal-list.ts"; @@ -309,6 +316,53 @@ function renderLifecycleTabs(props: SkillWorkshopProps) { `; } +function renderBodyModeButton( + props: SkillWorkshopProps, + mode: SkillWorkshopAppliedDiffMode, + label: string, +) { + const active = props.appliedDiffMode === mode; + return html` + + `; +} + +function renderBodyModeToggle(props: SkillWorkshopProps) { + return html` +
+ ${renderBodyModeButton(props, "changes", t("skillWorkshop.diff.changes"))} + ${renderBodyModeButton(props, "full", t("skillWorkshop.diff.fullBody"))} +
+ `; +} + +function renderRevisionBody(view: SkillWorkshopBodyView, proposal: SkillWorkshopProposal) { + if (view.kind === "diff") { + return renderLazyAppliedRevisionDiff(view.previous.body, proposal.body); + } + if (view.kind === "loadingPrevious") { + return html`

+ ${t("skillWorkshop.diff.loadingPrevious")} +

`; + } + if (view.kind === "previousUnavailable") { + return html` +

${t("skillWorkshop.diff.previousUnavailable")}

+ ${renderProposalBody(proposal.body)} + `; + } + if (view.kind === "tooLarge") { + return html`

${t("skillWorkshop.diff.tooLarge")}

`; + } + return renderProposalBody(proposal.body); +} + function renderDetail( props: SkillWorkshopProps, proposal: SkillWorkshopProposal, @@ -319,8 +373,12 @@ function renderDetail( const createdLabel = editedAt ? t("skillWorkshop.detail.edited", { time: formatRelative(editedAt) }) : t("skillWorkshop.detail.created", { time: formatRelative(proposal.createdAt) }); - const detailLoading = props.inspectingKey === proposal.key && !proposal.body; + const detailLoading = props.inspectingKey === proposal.key && !proposal.bodyLoaded; const firstSupportFile = proposal.supportFiles[0]; + const previousRevision = + appliedSkill?.revisions.find(({ proposal: revision }) => revision.key === proposal.key) + ?.previous ?? null; + const bodyView = resolveAppliedBodyView(props, proposal, previousRevision); return html`
@@ -359,10 +417,13 @@ function renderDetail(
-

${proposal.slug}

+
+

${proposal.slug}

+ ${previousRevision ? renderBodyModeToggle(props) : nothing} +
${detailLoading ? html`

${t("skillWorkshop.detail.loading")}

` - : renderProposalBody(proposal.body)} + : renderRevisionBody(bodyView, proposal)}
${appliedSkill ? renderLazyAppliedHistory(props, appliedSkill) : nothing} diff --git a/ui/src/styles/skill-workshop.css b/ui/src/styles/skill-workshop.css index 9155e7cec0b4..37d594566a88 100644 --- a/ui/src/styles/skill-workshop.css +++ b/ui/src/styles/skill-workshop.css @@ -603,8 +603,110 @@ line-height: 1.65; } +.sw-body-card__head { + display: flex; + align-items: baseline; + justify-content: space-between; + gap: 12px; + flex-wrap: wrap; + margin-bottom: 14px; +} + +.sw-body-mode { + display: inline-flex; + border: 1px solid var(--border); + border-radius: var(--radius-md); + overflow: hidden; +} + +.sw-body-mode__button { + border: 0; + background: transparent; + color: var(--muted); + font-size: 11.5px; + font-weight: 600; + padding: 4px 10px; + cursor: var(--cursor-action); +} + +.sw-body-mode__button:hover { + background: var(--bg-hover); + color: var(--text); +} + +.sw-body-mode__button.is-active { + background: color-mix(in srgb, var(--accent) 12%, var(--bg) 88%); + color: var(--text-strong); +} + +.sw-diff__stat { + display: flex; + gap: 10px; + margin: 0 0 8px; + font-family: var(--mono); + font-size: 11.5px; + font-weight: 600; +} + +.sw-diff__stat-add { + color: var(--ok); +} + +.sw-diff__stat-del { + color: var(--danger); +} + +.sw-diff__rows { + border: 1px solid var(--border); + border-radius: var(--radius-md); + overflow: auto; + font-family: var(--mono); + font-size: 11.5px; + line-height: 1.55; +} + +.sw-diff__row { + display: flex; + gap: 8px; + padding: 0 10px; + white-space: pre-wrap; + word-break: break-word; +} + +.sw-diff__row--add { + background: var(--ok-subtle); + color: var(--text); +} + +.sw-diff__row--del { + background: var(--danger-subtle); + color: var(--text); +} + +.sw-diff__row--ctx { + color: var(--muted); +} + +.sw-diff__row--skip { + border-top: 1px solid var(--border); + border-bottom: 1px solid var(--border); + background: var(--bg-elevated); + color: var(--muted); + padding: 2px 10px; +} + +.sw-diff__sign { + flex: 0 0 auto; + width: 8px; + color: var(--muted); +} + +.sw-diff__text { + flex: 1 1 auto; +} + .sw-body-card h1 { - margin: 0 0 14px; + margin: 0; font-size: 20px; color: var(--text-strong); font-weight: 700;