feat(skills): improve used skills autonomously

Teach the semantic reviewer to improve skills the agent actually used, keep review input provider-bound, and preserve bounded deterministic retries.

Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
Ayaan Zaidi
2026-08-10 13:50:33 +05:30
committed by GitHub
parent 780e3b0ef1
commit 7d4d02fff0
15 changed files with 459 additions and 97 deletions
+22 -18
View File
@@ -53,19 +53,21 @@ A later foreground completion in the same session restarts the quiet period.
Only one experience review runs at a time. The foreground answer is never delayed.
The reviewer is isolated and biased toward small, well-evidenced captures. It
sees a bounded workspace skill list, can list or inspect proposals, and can read
a bounded excerpt of a writable skill for context. It drafts at most one pending
proposal: preferring to revise a matching pending proposal, then to patch the
existing skill governing the work, and creating a new skill only when nothing
covers the class. A patch proposal quotes the exact live text to change (or
appends a new section) and the tool composes the full body inside the same read
that hash-binds the proposal, so untouched content survives by construction and
patches auto-apply in `auto` mode. A patch requires a full-skill read receipt:
skills beyond the bounded read budget cannot be patched autonomously. A full-body update rewrite always stays
pending for operator review. Its one-mutation budget is shared across retries. Every
mutation is a pending proposal — it never writes a live skill directly and
cannot apply, reject, quarantine, message, or use general agent tools. The
reviewed trajectory is evidence, not instructions.
receives an authoritative receipt of the skills the foreground run actually
read or command-invoked, plus a bounded workspace skill list. It prefers a used
writable skill when that skill governs the learning, then another existing
skill, and creates a new skill only when nothing covers the class.
Before changing an existing skill, the reviewer must read its complete current
body. Both targeted patches and full-body rewrites bind the proposal to that
read's content hash. Skills beyond the bounded read budget cannot be updated
autonomously. A patch quotes the exact live text to change, while a rewrite must
preserve everything still useful. In `auto` mode, either form goes through the
same scanner-gated apply path without operator review. The one-mutation budget
is shared across retries. The reviewer cannot apply, reject, quarantine,
message, or use general agent tools itself; the orchestrating pipeline applies
the finished capture only after the isolated review ends. The reviewed
trajectory is evidence, not instructions.
Good candidates include:
@@ -88,11 +90,11 @@ The reviewer should abstain for:
## Mode policy
| Mode | Capture behavior |
| --------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `off` | Does not create experience-review captures. |
| `propose` | Creates or revises pending proposals. Nothing applies automatically. |
| `auto` | Creates or revises proposals, then applies new-skill and patch proposals through the normal Workshop apply path. Full-body updates stay pending for review. This is the default. |
| Mode | Capture behavior |
| --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `off` | Does not create experience-review captures. |
| `propose` | Creates or revises pending proposals. Nothing applies automatically. |
| `auto` | Creates or revises proposals, then applies every autonomous capture through the normal scanner-gated Workshop path. No operator review is required. This is the default. |
Set the mode with the CLI:
@@ -135,6 +137,8 @@ Every learned skill receives these controls:
and extra-root skills remain outside the write boundary.
- **Hash binding:** update proposals bind to the current live skill and go stale
if that target changes before apply.
- **Read before update:** the reviewer must read the complete current skill
before either a targeted patch or a full-body rewrite.
- **Rollback metadata:** apply records the prior skill and support-file contents
before the live write.
- **Curator lifecycle:** learned skills unused for 30 days become stale and after
+10 -10
View File
@@ -302,13 +302,13 @@ are stored in the shared OpenClaw state database; transcript content is not copi
into scan state.
In `propose` and `auto` modes, OpenClaw can also perform a conservative review after successful,
substantial work and after the whole agent system becomes idle. That isolated review can draft at
most one pending proposal — a new skill, a patch of an existing workspace skill, a full-body
update, or a revision of a pending proposal. It never writes a live skill directly and cannot
apply, reject, or quarantine a proposal. Patch proposals quote the exact live text to change; the
tool composes the full body from the live skill. In `auto` mode, the orchestrating capture
pipeline applies new-skill and patch results afterward through the normal scanner-gated service;
full-body update proposals always stay pending for operator review.
substantial work and after the whole agent system becomes idle. The review receives an
authoritative receipt of skills the foreground run actually used. It can draft at most one pending
proposal: a new skill, a patch or full-body rewrite of an existing workspace skill, or a revision
of a pending proposal. Existing skills must be read before either update form, and the proposal is
bound to that exact content hash. The reviewer never writes a live skill directly and cannot
apply, reject, or quarantine a proposal. In `auto` mode, the orchestrating pipeline applies every
autonomous result afterward through the normal scanner-gated service, without operator review.
See [Self-learning](/tools/self-learning) for enablement, eligibility, privacy and cost details,
the proposal threshold, and troubleshooting.
@@ -343,9 +343,9 @@ In `propose` and `auto` modes, an isolated run of the selected model decides whe
completed trajectory clears the evidence-gated proposal bar. The foreground model is not prompted
to learn before it replies. The background reviewer preserves the foreground run as proposal
provenance, cannot access general agent tools, and cannot make lifecycle decisions. In `auto`
mode, the capture pipeline applies resulting new-skill and patch proposals only after the
isolated run completes; full-body update proposals always stay pending for operator review,
because the reviewer authors them without a mechanical preservation guarantee. The review starts
mode, the capture pipeline applies every autonomous proposal only after the isolated run
completes. Existing-skill changes require a complete read receipt and content-hash binding before
they are eligible for that apply step. The review starts
only when the foreground runtime reports its resolved model
and that `skill_workshop` was actually available. Restrictive or unknown tool policy therefore
fails closed and creates no proposal.
@@ -39,6 +39,7 @@ import { createHookRunner, type HookRunner } from "../plugins/hooks.js";
import { createEmptyPluginRegistry } from "../plugins/registry-empty.js";
import { setActivePluginRegistry } from "../plugins/runtime.js";
import { setPluginToolMeta } from "../plugins/tools.js";
import { consumeRunSkillUsage } from "../skills/runtime/run-usage.js";
import { createCanonicalFixtureSkill } from "../skills/test-support/test-helpers.js";
import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js";
import {
@@ -1495,6 +1496,10 @@ describe("before_tool_call loop detection behavior", () => {
expect(JSON.stringify(emitted)).not.toContain("SKILL.md");
expect(JSON.stringify(emitted)).not.toContain(skillBaseDir);
expect(privateData[0]?.skillUsage?.skillFile).toBe(skillFilePath);
expect(consumeRunSkillUsage("run-1")).toEqual([
{ name: "demo-skill", source: "workspace", activation: "read" },
]);
expect(consumeRunSkillUsage("run-1")).toEqual([]);
});
});
@@ -14,6 +14,7 @@ import {
} from "../infra/diagnostic-trace-context.js";
import { pruneMapToMaxSize } from "../infra/map-size.js";
import { copyPluginToolMeta, getPluginToolMeta } from "../plugins/tools.js";
import { recordRunSkillUsage } from "../skills/runtime/run-usage.js";
import {
buildToolContentPrivateData,
emitSkillUsedDiagnostic,
@@ -545,6 +546,14 @@ export function wrapToolWithBeforeToolCallHook(
toolParams: executeParams,
ctx,
});
if (skillMatch) {
recordRunSkillUsage({
runId: ctx?.runId,
name: skillMatch.skillName,
source: skillMatch.skillSource,
activation: skillMatch.activation,
});
}
if (hookOptions.emitDiagnostics) {
if (skillMatch) {
emitSkillUsedDiagnostic({
@@ -1,5 +1,6 @@
// Verifies agent-end side effects keep plugin hooks independent from experience review.
import { beforeEach, describe, expect, it, vi } from "vitest";
import { recordRunSkillUsage } from "../../skills/runtime/run-usage.js";
import { scheduleSkillExperienceReview } from "../../skills/workshop/experience-review-default.js";
import { awaitAgentEndSideEffects, runAgentEndSideEffects } from "./agent-end-side-effects.js";
import {
@@ -28,6 +29,12 @@ describe("agent end side effects", () => {
});
it("fires plugin agent_end hooks alongside experience review scheduling", async () => {
recordRunSkillUsage({
runId: "run-1",
name: "release-runbook",
source: "workspace",
activation: "read",
});
runAgentEndSideEffects({
event: {
messages: [],
@@ -52,6 +59,11 @@ describe("agent end side effects", () => {
expect(mockRunAgentEndHook).toHaveBeenCalledTimes(1);
await vi.waitFor(() => expect(mockExperienceReview).toHaveBeenCalledTimes(1));
expect(mockExperienceReview).toHaveBeenCalledWith(
expect.objectContaining({
usedSkills: [{ name: "release-runbook", source: "workspace", activation: "read" }],
}),
);
});
it("still runs agent_end hooks when experience review scheduling fails", async () => {
@@ -6,6 +6,7 @@ import type { ChatType } from "../../channels/chat-type.js";
* either fire-and-forget or awaited during tests/shutdown.
*/
import { createSubsystemLogger } from "../../logging/subsystem.js";
import { consumeRunSkillUsage } from "../../skills/runtime/run-usage.js";
import {
awaitAgentHarnessAgentEndHook,
runAgentHarnessAgentEndHook,
@@ -36,12 +37,14 @@ type AgentEndSideEffectsParams = Omit<BaseAgentEndSideEffectsParams, "ctx"> & {
};
async function runCoreAgentEndSideEffects(params: AgentEndSideEffectsParams): Promise<void> {
const usedSkills = consumeRunSkillUsage(params.ctx.runId);
try {
const { scheduleSkillExperienceReview } =
await import("../../skills/workshop/experience-review-default.js");
scheduleSkillExperienceReview({
event: params.event,
ctx: params.ctx,
usedSkills,
...(params.ctx.config ? { config: params.ctx.config } : {}),
});
} catch (error) {
@@ -1,5 +1,6 @@
// skill_workshop review-mode tests cover the proposal-only reviewer surface:
// mutation budgets, read receipts, and patch/update drafting for live skills.
import { writeFileSync } from "node:fs";
import fs from "node:fs/promises";
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
@@ -115,6 +116,14 @@ describe("skill_workshop review mode", () => {
updateProposals: true,
proposalMutationBudget,
});
await expect(
reviewTool.execute("update-without-read", {
action: "update",
skill_name: "weather-planner",
proposal_content: "# Weather Planner\n\nCheck alerts and timing.\n",
}),
).rejects.toThrow("read the live skill first");
await reviewTool.execute("review-read", { action: "read", skill_name: "weather-planner" });
const update = await reviewTool.execute("review-update", {
action: "update",
skill_name: "weather-planner",
@@ -183,7 +192,6 @@ describe("skill_workshop review mode", () => {
kind: "update",
skillKey: "weather-planner",
});
expect(proposalMutationBudget.patchProposalIds?.size).toBe(1);
expect((extended.details as { description?: string }).description ?? "").not.toContain(
"Replacement",
);
@@ -234,6 +242,66 @@ describe("skill_workshop review mode", () => {
expect(proposalMutationBudget.remaining).toBe(1);
});
it("refunds a stale update race so the reviewer can re-read and retry", async () => {
const workspaceDir = await tempDirs.make("openclaw-skill-workshop-stale-update-race-");
await seedLiveSkill(
workspaceDir,
"weather-planner",
"Plan around the weather forecast",
"# Weather Planner\n\nCheck weather before outdoor recommendations.\n",
);
const liveSkillFile = path.join(workspaceDir, "skills", "weather-planner", "SKILL.md");
const operatorEditedSkill = (await fs.readFile(liveSkillFile, "utf8")).replace(
"Check weather before outdoor recommendations.",
"Operator-edited steps during proposal creation.",
);
let remaining = 1;
let mutateOnReserve = false;
const proposalMutationBudget: SkillWorkshopProposalMutationBudget = {
get remaining() {
return remaining;
},
set remaining(value) {
remaining = value;
if (mutateOnReserve && value === 0) {
mutateOnReserve = false;
writeFileSync(liveSkillFile, operatorEditedSkill, "utf8");
}
},
};
const reviewTool = createSkillWorkshopTool({
workspaceDir,
proposalOnly: true,
updateProposals: true,
proposalMutationBudget,
});
await reviewTool.execute("review-read", { action: "read", skill_name: "weather-planner" });
mutateOnReserve = true;
await expect(
reviewTool.execute("stale-update-race", {
action: "update",
skill_name: "weather-planner",
proposal_content: "# Weather Planner\n\nCheck alerts and timing.\n",
}),
).rejects.toThrow("Skill changed since the reviewer's read");
expect(proposalMutationBudget.remaining).toBe(1);
await reviewTool.execute("review-read-again", {
action: "read",
skill_name: "weather-planner",
});
const update = await reviewTool.execute("review-update-retry", {
action: "update",
skill_name: "weather-planner",
proposal_content:
"# Weather Planner\n\nOperator-edited steps during proposal creation.\nCheck alerts and timing.\n",
});
expect(update.details).toMatchObject({ status: "pending", kind: "update" });
expect(proposalMutationBudget.remaining).toBe(0);
});
it("caps reviewer live-skill reads at the read budget", async () => {
const workspaceDir = await tempDirs.make("openclaw-skill-workshop-review-read-cap-");
await seedLiveSkill(
@@ -265,7 +333,7 @@ describe("skill_workshop review mode", () => {
old_string: "A detailed operational line.",
new_string: "A rewritten operational line.",
}),
).rejects.toThrow("cannot be patched autonomously");
).rejects.toThrow("cannot be updated autonomously");
});
it("does not refund the review mutation budget after a failed mutation", async () => {
+32 -40
View File
@@ -19,6 +19,7 @@ import {
rejectSkillProposal,
resolvePendingSkillProposal,
reviseSkillProposal,
SkillProposalStaleTargetError,
} from "../../skills/workshop/service.js";
import { SKILL_AUTHORING_STANDARDS_PROMPT } from "../../skills/workshop/skill-authoring-standards.js";
import type {
@@ -113,7 +114,7 @@ function buildSkillWorkshopToolSchema(
{
action: stringEnum(proposalOnly ? proposalActions : [...SKILL_WORKSHOP_ACTIONS], {
description: proposalOnly
? `create = new skill;${updateProposals ? " patch = targeted find-and-replace on an existing live skill (quote the exact current text in old_string, replacement in new_string; empty old_string appends new_string at the end); read = bounded excerpt of an existing live skill (read before patching so you can quote it); update = full-body update proposal (stays pending for operator review);" : ""} revise = existing pending proposal; list/inspect discover pending proposals (not filesystem search).${supportsCompletion ? " complete = durably finish this review after all proposal work." : ""} Nothing writes a live skill directly; lifecycle actions are unavailable.`
? `create = new skill;${updateProposals ? " patch = targeted find-and-replace on an existing live skill (quote the exact current text in old_string, replacement in new_string; empty old_string appends new_string at the end); read = bounded excerpt of an existing live skill (required before patch or update); update = full-body rewrite of an existing live skill after reading it;" : ""} revise = existing pending proposal; list/inspect discover pending proposals (not filesystem search).${supportsCompletion ? " complete = durably finish this review after all proposal work." : ""} Nothing writes a live skill directly; lifecycle actions are unavailable.`
: "create = new skill; update = existing live skill; revise = existing pending proposal; list/inspect discover pending proposals (not filesystem search); evaluate runs plugin evaluators for the exact draft; apply/reject/quarantine are explicit lifecycle actions.",
}),
proposal_id: Type.Optional(
@@ -436,48 +437,47 @@ export function createSkillWorkshopTool(options: SkillWorkshopToolOptions): AnyA
const goal = readStringParam(params, "goal");
const evidence = readStringParam(params, "evidence");
let resolvedPatchSkillKey: string | undefined;
if (action === "patch") {
if (options.updateProposals !== true) {
throw new ToolInputError("this Skill Workshop session cannot patch live skills");
}
// Pre-validate before spending the mutation budget so a mismatched quote or a
// stale read costs a retry, not the whole review. The read receipt proves the
// reviewer itself saw the current body — a quoted span alone could have been
// injected through the untrusted trajectory. The service still composes
// authoritatively from its own hash-binding read.
if (action === "patch" && options.updateProposals !== true) {
throw new ToolInputError("this Skill Workshop session cannot patch live skills");
}
let reviewerUpdateContentHash: string | undefined;
if (options.updateProposals === true && (action === "patch" || action === "update")) {
// The reviewer must see the entire current skill before either a targeted
// patch or a rewrite. The service binds the resulting proposal to that read.
const target = await readWritableWorkspaceSkill(
options.workspaceDir,
readStringParam(params, "skill_name", { required: true, label: "skill_name" }),
{ config: options.config, agentId: options.agentId },
);
resolvedPatchSkillKey = target.skillKey;
const readHash = options.proposalMutationBudget?.readSkillHashes?.get(target.skillKey);
if (!readHash) {
throw new ToolInputError(
target.content.length > REVIEWER_SKILL_READ_MAX_CHARS
? `skill "${target.skillKey}" exceeds the reviewer read budget and cannot be patched autonomously; draft a full-body update instead (it stays pending for the operator)`
: `read the live skill first: call action=read with skill_name "${target.skillKey}", then quote its current text in the patch`,
? `skill "${target.skillKey}" exceeds the reviewer read budget and cannot be updated autonomously`
: `read the live skill first: call action=read with skill_name "${target.skillKey}", then ${action === "patch" ? "quote its current text in the patch" : "rewrite it from the returned content"}`,
);
}
if (readHash !== sha256Hex(target.content)) {
options.proposalMutationBudget?.readSkillHashes?.delete(target.skillKey);
throw new ToolInputError(
`skill "${target.skillKey}" changed since it was read: call action=read again and redraft the patch from the current content`,
`skill "${target.skillKey}" changed since it was read: call action=read again and redraft the ${action} from the current content`,
);
}
try {
composeSkillBodyPatch(stripProposalFrontmatterForSkill(target.content), {
oldString:
readStringParam(params, "old_string", { label: "old_string", trim: false }) ?? "",
newString: readStringParam(params, "new_string", {
required: true,
label: "new_string",
trim: false,
}),
});
} catch (error) {
throw new ToolInputError(error instanceof Error ? error.message : String(error));
reviewerUpdateContentHash = readHash;
if (action === "patch") {
try {
composeSkillBodyPatch(stripProposalFrontmatterForSkill(target.content), {
oldString:
readStringParam(params, "old_string", { label: "old_string", trim: false }) ?? "",
newString: readStringParam(params, "new_string", {
required: true,
label: "new_string",
trim: false,
}),
});
} catch (error) {
throw new ToolInputError(error instanceof Error ? error.message : String(error));
}
}
}
@@ -530,6 +530,7 @@ export function createSkillWorkshopTool(options: SkillWorkshopToolOptions): AnyA
required: true,
label: "skill_name",
}),
expectedCurrentContentHash: reviewerUpdateContentHash,
description: readStringParam(params, "description"),
content: requireProposalContent(proposalContent),
supportFiles,
@@ -553,9 +554,7 @@ export function createSkillWorkshopTool(options: SkillWorkshopToolOptions): AnyA
required: true,
label: "skill_name",
}),
expectedCurrentContentHash: options.proposalMutationBudget?.readSkillHashes?.get(
resolvedPatchSkillKey ?? "",
),
expectedCurrentContentHash: reviewerUpdateContentHash,
composePatch: {
oldString:
readStringParam(params, "old_string", { label: "old_string", trim: false }) ?? "",
@@ -609,12 +608,6 @@ export function createSkillWorkshopTool(options: SkillWorkshopToolOptions): AnyA
options.proposalMutationBudget.mutatedProposalIds ?? new Set<string>();
mutatedProposalIds.add(proposal.record.id);
options.proposalMutationBudget.mutatedProposalIds = mutatedProposalIds;
if (action === "patch") {
const patchProposalIds =
options.proposalMutationBudget.patchProposalIds ?? new Set<string>();
patchProposalIds.add(proposal.record.id);
options.proposalMutationBudget.patchProposalIds = patchProposalIds;
}
options.proposalMutationBudget.completed = mutatedProposalIds.size;
options.proposalMutationBudget.successfulMutations =
(options.proposalMutationBudget.successfulMutations ?? 0) + 1;
@@ -628,10 +621,9 @@ export function createSkillWorkshopTool(options: SkillWorkshopToolOptions): AnyA
return proposalResult(proposal, { contentText });
} catch (error) {
if (reservesMutation && options.proposalMutationBudget) {
// A service-side patch composition failure means the target changed in the
// instant between prevalidation and the service read — not a model error.
// Refund so the reviewer can re-read and retry within its budget.
if (action === "patch" && error instanceof Error && error.message.startsWith("Patch ")) {
// A concurrent live edit is not a reviewer mutation. Preserve the budget
// so the reviewer can re-read the new body and redraft either update form.
if (error instanceof SkillProposalStaleTargetError) {
options.proposalMutationBudget.remaining += 1;
}
options.proposalMutationBudget.failedMutations =
+35
View File
@@ -0,0 +1,35 @@
import { pruneMapToMaxSize } from "../../infra/map-size.js";
import type { SkillTelemetrySource } from "../types.js";
const MAX_TRACKED_SKILL_USAGE_RUNS = 1024;
export type RunSkillUsage = Readonly<{
name: string;
source: SkillTelemetrySource;
activation: "command" | "read";
}>;
const skillUsageByRun = new Map<string, Map<string, RunSkillUsage>>();
/** Records the skills the foreground run demonstrably invoked or read. */
export function recordRunSkillUsage(params: RunSkillUsage & { runId?: string }): void {
const runId = params.runId;
if (!runId) {
return;
}
const usage = skillUsageByRun.get(runId) ?? new Map<string, RunSkillUsage>();
const record = { name: params.name, source: params.source, activation: params.activation };
usage.set(`${record.source}\u0000${record.name}\u0000${record.activation}`, record);
skillUsageByRun.set(runId, usage);
pruneMapToMaxSize(skillUsageByRun, MAX_TRACKED_SKILL_USAGE_RUNS);
}
/** Transfers one completed run's usage receipt to its terminal side effects. */
export function consumeRunSkillUsage(runId: string | undefined): RunSkillUsage[] {
if (!runId) {
return [];
}
const usage = skillUsageByRun.get(runId);
skillUsageByRun.delete(runId);
return usage ? [...usage.values()] : [];
}
@@ -91,7 +91,7 @@ describe("experience review auto apply", () => {
);
});
it("leaves reviewer update proposals pending instead of auto-applying them", async () => {
it("auto-applies reviewer full-body updates after an authoritative read", async () => {
const workspaceDir = await tempDirs.make("openclaw-experience-auto-apply-update-");
const seedTool = createSkillWorkshopTool({
workspaceDir,
@@ -121,6 +121,10 @@ describe("experience review auto apply", () => {
autonomousCapture: params.skillWorkshopAutonomousCapture,
proposalMutationBudget: params.skillWorkshopProposalMutationBudget,
});
await tool.execute("review-read", {
action: "read",
skill_name: "deployment-preflight",
});
await tool.execute("review-update", {
action: "update",
skill_name: "deployment-preflight",
@@ -150,11 +154,11 @@ describe("experience review auto apply", () => {
const updateEntry = manifest.proposals.find((entry) => entry.kind === "update");
expect(updateEntry).toMatchObject({
skillKey: "deployment-preflight",
status: "pending",
status: "applied",
});
await expect(
fs.readFile(`${workspaceDir}/skills/deployment-preflight/SKILL.md`, "utf8"),
).resolves.toContain("Operator-authored preflight steps.");
).resolves.toContain("Reviewer-rewritten steps.");
});
it("auto-applies reviewer patch proposals composed from the live body", async () => {
@@ -1,16 +1,19 @@
import { isRecord } from "@openclaw/normalization-core/record-coerce";
import { sliceUtf16Safe, truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice";
import type { RunSkillUsage } from "../runtime/run-usage.js";
import { SKILL_AUTHORING_STANDARDS_PROMPT } from "./skill-authoring-standards.js";
const EXPERIENCE_REVIEW_MAX_TRANSCRIPT_CHARS = 60_000;
const EXPERIENCE_REVIEW_MAX_SKILL_ENTRIES = 50;
const EXPERIENCE_REVIEW_MAX_SKILL_LINE_CHARS = 200;
const EXPERIENCE_REVIEW_MAX_USED_SKILLS_CHARS = 2_000;
type ExperienceReviewPromptCandidate = {
ctx: { runId?: string };
transcript: string;
modelIterations: number;
turnAborted?: boolean;
usedSkills?: readonly RunSkillUsage[];
existingSkills?: readonly { name: string; description?: string }[];
};
@@ -110,6 +113,52 @@ function renderExistingSkillsSection(
];
}
function compareRunSkillUsage(left: RunSkillUsage, right: RunSkillUsage): number {
for (const field of ["name", "source", "activation"] as const) {
if (left[field] !== right[field]) {
return left[field] < right[field] ? -1 : 1;
}
}
return 0;
}
function renderUsedSkillsSection(
usedSkills: ExperienceReviewPromptCandidate["usedSkills"],
): string[] {
if (!usedSkills?.length) {
return [];
}
const shown = usedSkills
.toSorted(compareRunSkillUsage)
.slice(0, EXPERIENCE_REVIEW_MAX_SKILL_ENTRIES);
const header = "Skills actually used in this trajectory (authoritative runtime receipt):";
const preference =
"Prefer improving a used writable workspace skill when it governs the learning.";
const reservedOmission = `(+${usedSkills.length} more used skills omitted)`;
const entries: string[] = [];
for (const skill of shown) {
const line = truncateUtf16Safe(
`- ${skill.name} (${skill.source}, ${skill.activation})`,
EXPERIENCE_REVIEW_MAX_SKILL_LINE_CHARS,
);
if (
["", header, ...entries, line, reservedOmission, preference].join("\n").length >
EXPERIENCE_REVIEW_MAX_USED_SKILLS_CHARS
) {
break;
}
entries.push(line);
}
const omitted = usedSkills.length - entries.length;
return [
"",
header,
...entries,
...(omitted > 0 ? [`(+${omitted} more used skills omitted)`] : []),
preference,
];
}
export function buildSkillExperienceReviewPrompt(
candidate: ExperienceReviewPromptCandidate,
): string {
@@ -127,7 +176,7 @@ export function buildSkillExperienceReviewPrompt(
"",
SKILL_AUTHORING_STANDARDS_PROMPT,
"",
"Choose the smallest mutation, in order: (1) revise a pending proposal on the same topic — use list/inspect to check; (2) patch the existing workspace skill that governs this work — read it first, then quote the exact text to change in old_string with your replacement in new_string, or use an empty old_string to append a new section; place the learning where it belongs and match the skill's style; (3) update with a full replacement body only when the whole skill needs restructuring — those stay pending for the operator; (4) create one new class-level skill only when no existing skill covers this class of work. Make at most one create/patch/update/revise call. Every mutation is a pending proposal; nothing writes a live skill directly, and the tool cannot apply, reject, or quarantine. If nothing genuinely clears the bar, answer NOTHING_TO_LEARN.",
"Choose the smallest mutation, in order: (1) revise a pending proposal on the same topic — use list/inspect to check; (2) patch a used writable workspace skill that governs this work, otherwise the best existing workspace skill — read it first, then quote the exact text to change in old_string with your replacement in new_string, or use an empty old_string to append a new section; place the learning where it belongs and match the skill's style; (3) update with a full replacement body only when the whole skill needs restructuring — read it first and preserve everything still useful; (4) create one new class-level skill only when no existing skill covers this class of work. Make at most one create/patch/update/revise call. Every mutation starts as a pending proposal; nothing writes a live skill during this review, and the configured pipeline decides whether to apply it afterward. If nothing genuinely clears the bar, answer NOTHING_TO_LEARN.",
"",
candidate.turnAborted === true
? `Interrupted run (stopped before completion): ${candidate.ctx.runId ?? "unknown"}`
@@ -137,6 +186,7 @@ export function buildSkillExperienceReviewPrompt(
"The trajectory may end mid-task. Only capture procedures that visibly worked before the interruption.",
]
: []),
...renderUsedSkillsSection(candidate.usedSkills),
...renderExistingSkillsSection(candidate.existingSkills),
`Model iterations in turn: ${candidate.modelIterations}`,
"",
+166 -4
View File
@@ -25,6 +25,9 @@ function completedRun(
senderId?: string;
senderName?: string;
chatType?: "direct" | "group";
modelProviderId?: string;
authProfileId?: string;
usedSkills?: SkillExperienceReviewParams["usedSkills"];
} = {},
): SkillExperienceReviewParams {
const iterations = options.iterations ?? 10;
@@ -55,9 +58,9 @@ function completedRun(
...(options.modelMetadata === false
? {}
: {
modelProviderId: "openai",
modelProviderId: options.modelProviderId ?? "openai",
modelId: "gpt-test",
authProfileId: "openai:work",
authProfileId: options.authProfileId ?? "openai:work",
}),
skillWorkshopAvailable: options.skillWorkshopAvailable ?? true,
...(options.modelIterations === undefined
@@ -76,6 +79,7 @@ function completedRun(
},
},
},
...(options.usedSkills ? { usedSkills: options.usedSkills } : {}),
};
}
@@ -112,6 +116,38 @@ describe("skill experience review scheduler", () => {
scheduler.clear();
});
it("scopes deep direct and group reviews to the current user turn", async () => {
vi.useFakeTimers();
const runReview = vi.fn().mockResolvedValue(undefined);
const scheduler = createSkillExperienceReviewScheduler({
isSystemActive: () => false,
runReview,
});
const direct = completedRun({ sessionKey: "agent:main:direct" });
direct.event.messages.unshift(
{ role: "user", content: "Earlier correction from this direct session." },
{ role: "assistant", content: "Earlier response." },
);
scheduler.schedule(direct);
await vi.advanceTimersByTimeAsync(30_000);
expect(runReview.mock.calls[0]?.[0].transcript).not.toContain(
"Earlier correction from this direct session.",
);
runReview.mockClear();
const group = completedRun({ sessionKey: "agent:main:group", chatType: "group" });
group.event.messages.unshift(
{ role: "user", content: "Earlier message from another group participant." },
{ role: "assistant", content: "Earlier group response." },
);
scheduler.schedule(group);
await vi.advanceTimersByTimeAsync(30_000);
expect(runReview.mock.calls[0]?.[0].transcript).not.toContain(
"Earlier message from another group participant.",
);
scheduler.clear();
});
it("uses exact harness iterations for a Codex-style projected trajectory", async () => {
vi.useFakeTimers();
const runReview = vi.fn().mockResolvedValue(undefined);
@@ -146,6 +182,89 @@ describe("skill experience review scheduler", () => {
scheduler.clear();
});
it("carries skills actually used across accumulated shallow turns", async () => {
vi.useFakeTimers();
const runReview = vi.fn().mockResolvedValue(undefined);
const scheduler = createSkillExperienceReviewScheduler({
isSystemActive: () => false,
runReview,
});
scheduler.schedule(
completedRun({
modelIterations: 5,
runId: "run-a",
usedSkills: [{ name: "release-runbook", source: "workspace", activation: "read" }],
}),
);
scheduler.schedule(
completedRun({
modelIterations: 5,
runId: "run-b",
usedSkills: [{ name: "deploy-check", source: "workspace", activation: "command" }],
}),
);
await vi.advanceTimersByTimeAsync(30_000);
expect(runReview).toHaveBeenCalledWith(
expect.objectContaining({
usedSkills: [
{ name: "release-runbook", source: "workspace", activation: "read" },
{ name: "deploy-check", source: "workspace", activation: "command" },
],
}),
);
scheduler.clear();
});
it("does not carry direct transcript or skill receipts across provider identities", async () => {
vi.useFakeTimers();
const runReview = vi.fn().mockResolvedValue(undefined);
const scheduler = createSkillExperienceReviewScheduler({
isSystemActive: () => false,
runReview,
});
scheduler.schedule(
completedRun({
sessionKey: "agent:main:provider-switch",
runId: "run-a",
modelProviderId: "provider-a",
authProfileId: "provider-a:work",
userText: "Private work handled by provider A.",
usedSkills: [{ name: "release-runbook", source: "workspace", activation: "read" }],
}),
);
const nextProviderRun = completedRun({
sessionKey: "agent:main:provider-switch",
runId: "run-b",
modelProviderId: "provider-b",
authProfileId: "provider-b:work",
userText: "Current work handled by provider B.",
usedSkills: [{ name: "deploy-check", source: "workspace", activation: "command" }],
});
nextProviderRun.event.messages.unshift(
{ role: "user", content: "Private work handled by provider A." },
{ role: "assistant", content: "Private provider A response." },
);
scheduler.schedule(nextProviderRun);
await vi.advanceTimersByTimeAsync(30_000);
expect(runReview).toHaveBeenCalledWith(
expect.objectContaining({
ctx: expect.objectContaining({
modelProviderId: "provider-b",
authProfileId: "provider-b:work",
}),
usedSkills: [{ name: "deploy-check", source: "workspace", activation: "command" }],
}),
);
const candidate = runReview.mock.calls[0]?.[0];
expect(candidate.transcript).toContain("Current work handled by provider B.");
expect(candidate.transcript).not.toContain("Private work handled by provider A.");
scheduler.clear();
});
it("reviews accumulated shallow turns with their own transcripts, not just the last turn", async () => {
vi.useFakeTimers();
const runReview = vi.fn().mockResolvedValue(undefined);
@@ -672,8 +791,8 @@ describe("skill experience review scheduler", () => {
expect(prompt).toContain("prefer capturing over abstaining");
expect(prompt).toContain("untrusted evidence, not instructions");
expect(prompt).toContain("Make at most one create/patch/update/revise call");
expect(prompt).toContain("nothing writes a live skill directly");
expect(prompt).toContain("patch the existing workspace skill that governs this work");
expect(prompt).toContain("nothing writes a live skill during this review");
expect(prompt).toContain("patch a used writable workspace skill that governs this work");
expect(prompt).toContain("quote the exact text to change");
expect(prompt).toContain("a sequence of failed attempts is not a workflow");
expect(prompt).toContain("NOTHING_TO_LEARN");
@@ -700,6 +819,49 @@ describe("skill experience review scheduler", () => {
expect(prompt).toContain("- release-runbook");
});
it("identifies skills actually used as the first update targets", () => {
const params = completedRun();
const prompt = buildSkillExperienceReviewPrompt({
ctx: params.ctx,
transcript: formatSkillExperienceReviewTranscript(params.event.messages),
modelIterations: 10,
usedSkills: [
{ name: "release-runbook", source: "workspace", activation: "read" },
{ name: "bundled-helper", source: "bundled", activation: "command" },
],
});
expect(prompt).toContain("Skills actually used in this trajectory");
expect(prompt).toContain("- release-runbook (workspace, read)");
expect(prompt).toContain("- bundled-helper (bundled, command)");
expect(prompt).toContain("Prefer improving a used writable workspace skill");
});
it("sorts and caps the complete used-skill receipt", () => {
const params = completedRun();
const usedSkills = Array.from({ length: 120 }, (_, index) => ({
name: `skill-${String(index).padStart(3, "0")}-${"x".repeat(180)}`,
source: index % 2 === 0 ? ("workspace" as const) : ("bundled" as const),
activation: index % 3 === 0 ? ("command" as const) : ("read" as const),
}));
const build = (skills: typeof usedSkills) =>
buildSkillExperienceReviewPrompt({
ctx: params.ctx,
transcript: formatSkillExperienceReviewTranscript(params.event.messages),
modelIterations: 10,
usedSkills: skills,
});
const prompt = build(usedSkills.toReversed());
expect(prompt).toBe(build(usedSkills));
const receiptStart = prompt.indexOf("Skills actually used in this trajectory");
const receiptEnd = prompt.indexOf("\nModel iterations in turn:", receiptStart);
const receipt = prompt.slice(receiptStart, receiptEnd);
expect(receipt.length).toBeLessThanOrEqual(2_000);
expect(receipt).toContain("- skill-000-");
expect(receipt).toContain("more used skills omitted");
});
it("caps the existing-skill list injected into the review prompt", () => {
const params = completedRun();
const prompt = buildSkillExperienceReviewPrompt({
+32 -14
View File
@@ -5,6 +5,7 @@ import type { OpenClawConfig } from "../../config/types.openclaw.js";
import { createSubsystemLogger } from "../../logging/subsystem.js";
import { runWithGatewayIndependentRootWorkAdmission } from "../../process/gateway-work-admission.js";
import { CommandLane } from "../../process/lanes.js";
import type { RunSkillUsage } from "../runtime/run-usage.js";
import { autoApplySkillProposal } from "./auto-apply.js";
import { resolveSkillWorkshopConfig } from "./config.js";
import {
@@ -71,6 +72,7 @@ type ExperienceReviewAgentContext = {
export type SkillExperienceReviewParams = {
event: ExperienceReviewAgentEndEvent;
ctx: ExperienceReviewAgentContext;
usedSkills?: readonly RunSkillUsage[];
config?: OpenClawConfig;
};
@@ -79,6 +81,7 @@ export type ExperienceReviewCandidate = {
config?: OpenClawConfig;
transcript: string;
modelIterations: number;
usedSkills?: readonly RunSkillUsage[];
turnAborted?: boolean;
};
@@ -104,6 +107,18 @@ type PendingExperienceReview = {
timer?: ExperienceReviewTimer;
};
function mergeRunSkillUsage(
...groups: Array<readonly RunSkillUsage[] | undefined>
): RunSkillUsage[] {
const merged = new Map<string, RunSkillUsage>();
for (const group of groups) {
for (const usage of group ?? []) {
merged.set(`${usage.source}\u0000${usage.name}\u0000${usage.activation}`, usage);
}
}
return [...merged.values()];
}
function isAuthProfileMigrationRequiredError(
error: unknown,
): error is { code: "AUTH_PROFILE_MIGRATION_REQUIRED" } {
@@ -216,6 +231,7 @@ export function createSkillExperienceReviewScheduler(deps: ExperienceReviewSched
senderScope: string;
iterations: number;
messages: unknown[];
usedSkills: RunSkillUsage[];
aborted: boolean;
lastRunId?: string;
}
@@ -347,6 +363,12 @@ export function createSkillExperienceReviewScheduler(deps: ExperienceReviewSched
let reviewIterations = modelIterations;
let reviewMessages = turnMessages;
let reviewAborted = !params.event.success;
let reviewUsedSkills = mergeRunSkillUsage(
existing && existing.candidate.ctx.runId === params.ctx.runId
? existing.candidate.usedSkills
: undefined,
params.usedSkills,
);
if (modelIterations >= EXPERIENCE_REVIEW_MIN_MODEL_ITERATIONS) {
shallowBySession.delete(sessionKey);
} else {
@@ -393,7 +415,13 @@ export function createSkillExperienceReviewScheduler(deps: ExperienceReviewSched
shallowBySession.delete(oldestKey);
}
}
accumulator = { senderScope, iterations: 0, messages: [], aborted: false };
accumulator = {
senderScope,
iterations: 0,
messages: [],
usedSkills: [],
aborted: false,
};
shallowBySession.set(sessionKey, accumulator);
}
const runId = params.ctx.runId?.trim();
@@ -405,6 +433,7 @@ export function createSkillExperienceReviewScheduler(deps: ExperienceReviewSched
accumulator.lastRunId = runId;
accumulator.iterations += modelIterations;
accumulator.aborted = accumulator.aborted || !params.event.success;
accumulator.usedSkills = mergeRunSkillUsage(accumulator.usedSkills, params.usedSkills);
accumulator.messages = [...accumulator.messages, ...turnMessages].slice(
-EXPERIENCE_REVIEW_MAX_SHALLOW_MESSAGES,
);
@@ -418,6 +447,7 @@ export function createSkillExperienceReviewScheduler(deps: ExperienceReviewSched
reviewIterations = accumulator.iterations;
reviewMessages = accumulator.messages;
reviewAborted = accumulator.aborted;
reviewUsedSkills = accumulator.usedSkills;
}
{
if (!existing && pendingBySession.size >= EXPERIENCE_REVIEW_MAX_PENDING) {
@@ -462,6 +492,7 @@ export function createSkillExperienceReviewScheduler(deps: ExperienceReviewSched
...(params.config ? { config: params.config } : {}),
transcript: formatSkillExperienceReviewTranscript(reviewMessages),
modelIterations: reviewIterations,
usedSkills: reviewUsedSkills,
turnAborted: reviewAborted,
};
const pending = existing ?? { candidate, generation: 0 };
@@ -514,7 +545,6 @@ async function runSkillExperienceReviewInner(
const sessionId = randomUUID();
const proposalMutationBudget: SkillWorkshopProposalMutationBudget = {
remaining: 1,
patchProposalIds: new Set(),
readSkillHashes: new Map(),
};
const reviewSessionKey = `agent:${candidate.ctx.agentId ?? "main"}:${EXPERIENCE_REVIEW_SESSION_SEGMENT}:incognito-${sessionId}`;
@@ -606,18 +636,6 @@ async function runSkillExperienceReviewInner(
) {
continue;
}
// Patch proposals auto-apply: the service composed them by replacing only the span
// the reviewer quoted from the live body (or appending), so untouched content
// survives by construction. Full-body update proposals stay pending for review.
if (
proposal.record.kind === "update" &&
proposalMutationBudget.patchProposalIds?.has(proposalId) !== true
) {
log.info(
`skill experience review left full-body update proposal ${proposalId} pending for operator review`,
);
continue;
}
await autoApplySkillProposal({
workspaceDir,
...(candidate.ctx.agentId ? { agentId: candidate.ctx.agentId } : {}),
+4 -2
View File
@@ -69,6 +69,8 @@ type SkillWorkshopWorkspaceOptions = {
agentId?: string;
};
export class SkillProposalStaleTargetError extends Error {}
function proposalStoreOptions(env?: NodeJS.ProcessEnv) {
return env ? { env } : {};
}
@@ -269,8 +271,8 @@ export async function proposeUpdateSkill(
input.expectedCurrentContentHash !== undefined &&
sha256Hex(currentContent) !== input.expectedCurrentContentHash
) {
throw new Error(
"Patch target changed since the reviewer's read: read the skill again and redraft the patch.",
throw new SkillProposalStaleTargetError(
"Skill changed since the reviewer's read: read it again and redraft the update.",
);
}
// Composition uses the same read that currentContentHash binds the proposal to, so a
+1 -3
View File
@@ -73,9 +73,7 @@ export type SkillWorkshopProposalMutationBudget = {
failedMutations?: number;
/** Run-local identity set used to keep idea counts distinct. */
mutatedProposalIds?: Set<string>;
/** Proposals composed mechanically by patching the live body with a reviewer edit. */
patchProposalIds?: Set<string>;
/** Content hash per live skill read this run; patches require a matching receipt. */
/** Content hash per live skill read this run; autonomous updates require a matching receipt. */
readSkillHashes?: Map<string, string>;
};