fix(skill-workshop): honor pending approval for tool suggestions [AI] (#78516)

pgondhi987 · web-flow · commit 2d65ead914c7 · 2026-05-07T13:27:32.000+05:30
* fix: honor pending skill workshop approvals

* addressing review-skill

* addressing codex review

* addressing codex review

* fix: require approval before skill workshop apply

* docs: add changelog entry for PR merge
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -140,6 +140,7 @@ Docs: https://docs.openclaw.ai
 
 ### Fixes
 
+- fix(skill-workshop): honor pending approval for tool suggestions [AI]. (#78516) Thanks @pgondhi987.
 - Native chat: decode gateway-provided thinking metadata for the iOS/macOS picker so provider-specific levels such as `adaptive`, `xhigh`, and `max` appear without leaking unsupported default-model options. Thanks @BunsDev.
 - Agents/tools: fail `exec host=node` before `system.run` when the selected node is known to be disconnected, with an actionable reconnect message instead of a raw node invoke failure. Thanks @BunsDev.
 - Agents/models: accept legacy `anthropic-cli/*` model refs as Claude CLI runtime refs instead of failing model resolution with `Unknown model`. Thanks @BunsDev.
diff --git a/docs/plugins/skill-workshop.md b/docs/plugins/skill-workshop.md
@@ -357,7 +357,7 @@ Create a proposal. With `approvalPolicy: "pending"` (default), this queues inste
 ```
 
 <AccordionGroup>
-  <Accordion title="Force a safe write (apply: true)">
+  <Accordion title="Request immediate write in auto mode (apply: true)">
 
 ```json
 {
@@ -369,6 +369,9 @@ Create a proposal. With `approvalPolicy: "pending"` (default), this queues inste
 }
 ```
 
+With `approvalPolicy: "pending"`, `apply: true` still queues the proposal. Review it, then use
+the `apply` action after approval.
+
   </Accordion>
 
   <Accordion title="Force pending under auto policy (apply: false)">
@@ -417,6 +420,9 @@ Create a proposal. With `approvalPolicy: "pending"` (default), this queues inste
 
 Apply a pending proposal.
 
+With `approvalPolicy: "pending"`, this action asks for operator approval before writing the
+workspace skill.
+
 ```json
 {
   "action": "apply",
@@ -551,8 +557,8 @@ The guidance emphasizes:
 
 The write mode text changes with `approvalPolicy`:
 
-- pending mode: queue suggestions; apply only after explicit approval
-- auto mode: apply safe workspace-skill updates when clearly reusable
+- pending mode: queue suggestions; use `apply` after explicit approval
+- auto mode: apply safe workspace-skill updates unless `apply: false` queues instead
 
 ## Costs and runtime behavior
 
diff --git a/extensions/skill-workshop/index.test.ts b/extensions/skill-workshop/index.test.ts
@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import type { AnyAgentTool } from "openclaw/plugin-sdk/agent-runtime";
+import type { PluginTrustedToolPolicyRegistration } from "openclaw/plugin-sdk/core";
 import { createTestPluginApi } from "openclaw/plugin-sdk/plugin-test-api";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import plugin, {
@@ -617,6 +618,72 @@ describe("skill-workshop", () => {
     expect(await store.list("pending")).toHaveLength(1);
   });
 
+  it("queues apply true suggestions in pending mode before explicit apply", async () => {
+    const workspaceDir = await makeTempDir();
+    const stateDir = await makeTempDir();
+    let tool: AnyAgentTool | undefined;
+    const api = createTestPluginApi({
+      pluginConfig: { approvalPolicy: "pending" },
+      runtime: {
+        agent: {
+          resolveAgentWorkspaceDir: () => workspaceDir,
+        },
+        state: {
+          resolveStateDir: () => stateDir,
+        },
+      } as never,
+      registerTool(registered) {
+        const resolved =
+          typeof registered === "function" ? registered({ workspaceDir }) : registered;
+        tool = Array.isArray(resolved) ? resolved[0] : (resolved ?? undefined);
+      },
+    });
+
+    plugin.register(api);
+    const result = await tool?.execute?.("call-1", {
+      action: "suggest",
+      apply: true,
+      skillName: "screenshot-asset-workflow",
+      description: "Screenshot asset workflow",
+      body: "Verify dimensions, optimize the PNG, and run the relevant gate.",
+    });
+
+    expect(result?.details).toMatchObject({ status: "pending" });
+    const proposalId =
+      (result?.details as { proposal?: { id?: string } } | undefined)?.proposal?.id ?? "";
+    expect(proposalId).toBeTruthy();
+    await expect(
+      fs.access(path.join(workspaceDir, "skills", "screenshot-asset-workflow", "SKILL.md")),
+    ).rejects.toMatchObject({ code: "ENOENT" });
+    const store = new SkillWorkshopStore({ stateDir, workspaceDir });
+    expect(await store.list("pending")).toHaveLength(1);
+    expect(await store.list("applied")).toHaveLength(0);
+  });
+
+  it("requires operator approval before applying queued proposals in pending mode", async () => {
+    let trustedPolicy: PluginTrustedToolPolicyRegistration | undefined;
+    const api = createTestPluginApi({
+      pluginConfig: { approvalPolicy: "pending" },
+      registerTrustedToolPolicy(policy) {
+        trustedPolicy = policy;
+      },
+    });
+
+    plugin.register(api);
+
+    const result = await trustedPolicy?.evaluate(
+      { toolName: "skill_workshop", params: { action: "apply", id: "proposal-1" } },
+      { toolName: "skill_workshop" },
+    );
+
+    expect(result).toMatchObject({
+      requireApproval: {
+        title: "Apply workspace skill proposal",
+        allowedDecisions: ["allow-once", "deny"],
+      },
+    });
+  });
+
   it("uses the reviewer to propose existing skill repairs", async () => {
     const workspaceDir = await makeTempDir();
     const stateDir = await makeTempDir();
diff --git a/extensions/skill-workshop/index.ts b/extensions/skill-workshop/index.ts
@@ -38,6 +38,30 @@ export default definePluginEntry({
       },
     );
 
+    api.registerTrustedToolPolicy({
+      id: "skill-workshop-apply-approval",
+      description: "Require operator approval before applying queued workspace skill proposals.",
+      evaluate(event) {
+        const config = resolveCurrentConfig();
+        if (
+          !config.enabled ||
+          config.approvalPolicy === "auto" ||
+          event.toolName !== "skill_workshop" ||
+          event.params.action !== "apply"
+        ) {
+          return undefined;
+        }
+        return {
+          requireApproval: {
+            title: "Apply workspace skill proposal",
+            description: "Apply a queued workspace skill proposal.",
+            severity: "warning",
+            allowedDecisions: ["allow-once", "deny"],
+          },
+        };
+      },
+    });
+
     api.on("before_prompt_build", async () => {
       const config = resolveCurrentConfig();
       if (!config.enabled) {
diff --git a/extensions/skill-workshop/src/prompt.ts b/extensions/skill-workshop/src/prompt.ts
@@ -3,8 +3,8 @@ import type { SkillWorkshopConfig } from "./config.js";
 export function buildWorkshopGuidance(config: SkillWorkshopConfig): string {
   const writeMode =
     config.approvalPolicy === "auto"
-      ? "Auto mode: apply safe workspace-skill updates when clearly reusable."
-      : "Pending mode: queue suggestions; apply only after explicit approval.";
+      ? "Auto mode: apply safe workspace-skill updates; apply=false queues instead."
+      : "Pending mode: queue suggestions; use apply action after explicit approval.";
   return [
     "<skill_workshop>",
     "Use for durable procedural memory, not facts/preferences.",
diff --git a/extensions/skill-workshop/src/tool.ts b/extensions/skill-workshop/src/tool.ts
@@ -2,14 +2,9 @@ import { randomUUID } from "node:crypto";
 import { Type } from "typebox";
 import { jsonResult, type OpenClawPluginApi } from "../api.js";
 import type { SkillWorkshopConfig } from "./config.js";
-import {
-  applyProposalToWorkspace,
-  normalizeSkillName,
-  prepareProposalWrite,
-  writeSupportFile,
-} from "./skills.js";
+import { applyProposalToWorkspace, normalizeSkillName, writeSupportFile } from "./skills.js";
 import type { SkillChange, SkillProposal, SkillWorkshopStatus } from "./types.js";
-import { createStoreForContext, resolveWorkspaceDir } from "./workshop.js";
+import { applyOrStoreProposal, createStoreForContext, resolveWorkspaceDir } from "./workshop.js";
 
 type ToolParams = {
   action?: string;
@@ -150,65 +145,14 @@ export function createSkillWorkshopTool(params: {
       }
       if (action === "suggest") {
         const proposal = buildProposal({ workspaceDir, raw, source: "tool" });
-        const shouldApply =
-          raw.apply === true || (raw.apply !== false && params.config.approvalPolicy === "auto");
-        if (shouldApply) {
-          const prepared = await prepareProposalWrite({
-            proposal,
-            maxSkillBytes: params.config.maxSkillBytes,
-          });
-          const critical = prepared.findings.find((finding) => finding.severity === "critical");
-          if (critical) {
-            const stored = await store.add(
-              {
-                ...proposal,
-                status: "quarantined",
-                updatedAt: Date.now(),
-                scanFindings: prepared.findings,
-                quarantineReason: critical.message,
-              },
-              params.config.maxPending,
-            );
-            return jsonResult({ status: "quarantined", proposal: stored });
-          }
-          const applied = await applyProposalToWorkspace({
-            proposal,
-            maxSkillBytes: params.config.maxSkillBytes,
-          });
-          const stored = await store.add(
-            {
-              ...proposal,
-              status: "applied",
-              updatedAt: Date.now(),
-              scanFindings: applied.findings,
-            },
-            params.config.maxPending,
-          );
-          return jsonResult({ status: "applied", skillPath: applied.skillPath, proposal: stored });
-        }
-        const prepared = await prepareProposalWrite({
+        const result = await applyOrStoreProposal({
           proposal,
-          maxSkillBytes: params.config.maxSkillBytes,
+          store,
+          config: params.config,
+          workspaceDir,
+          skipAutoApply: raw.apply === false,
         });
-        const critical = prepared.findings.find((finding) => finding.severity === "critical");
-        if (critical) {
-          const stored = await store.add(
-            {
-              ...proposal,
-              status: "quarantined",
-              updatedAt: Date.now(),
-              scanFindings: prepared.findings,
-              quarantineReason: critical.message,
-            },
-            params.config.maxPending,
-          );
-          return jsonResult({ status: "quarantined", proposal: stored });
-        }
-        const stored = await store.add(
-          { ...proposal, scanFindings: prepared.findings },
-          params.config.maxPending,
-        );
-        return jsonResult({ status: "pending", proposal: stored });
+        return jsonResult(result);
       }
       if (action === "apply") {
         if (!raw.id) {
diff --git a/extensions/skill-workshop/src/workshop.ts b/extensions/skill-workshop/src/workshop.ts
@@ -37,6 +37,7 @@ export async function applyOrStoreProposal(params: {
   store: SkillWorkshopStore;
   config: SkillWorkshopConfig;
   workspaceDir: string;
+  skipAutoApply?: boolean;
 }): Promise<{
   status: "pending" | "applied" | "quarantined";
   skillPath?: string;
@@ -60,7 +61,7 @@ export async function applyOrStoreProposal(params: {
     );
     return { status: "quarantined", proposal: stored };
   }
-  if (params.config.approvalPolicy === "auto") {
+  if (params.config.approvalPolicy === "auto" && !params.skipAutoApply) {
     const applied = await applyProposalToWorkspace({
       proposal: params.proposal,
       maxSkillBytes: params.config.maxSkillBytes,
