fix(skills): skill workshop apply hangs despite auto approval (#102530)

* fix: honor skill workshop auto approval without hook config

* test(skills): cover workshop config read failures

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Wynne668
2026-07-11 00:10:23 +08:00
committed by GitHub
parent afd0c60f15
commit dd265e2c2a
3 changed files with 115 additions and 5 deletions

View File

@@ -4,6 +4,7 @@
* plugin hook decisions.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { clearRuntimeConfigSnapshot, setRuntimeConfigSnapshot } from "../config/config.js";
import { setEmbeddedMode } from "../infra/embedded-mode.js";
import {
EmbeddedPluginApprovalBroker,
@@ -93,6 +94,7 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => {
});
afterEach(() => {
clearRuntimeConfigSnapshot();
setEmbeddedPluginApprovalBroker(null);
setEmbeddedMode(false);
setActivePluginRegistry(createEmptyPluginRegistry());
@@ -670,6 +672,30 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => {
expect(runBeforeToolCallMock).not.toHaveBeenCalled();
});
it("uses runtime config for skill_workshop auto mode when hook context config is absent", async () => {
(hookRunner.hasHooks as ReturnType<typeof vi.fn>).mockReturnValue(false);
setRuntimeConfigSnapshot({
skills: {
workshop: {
approvalPolicy: "auto",
},
},
});
const result = await runBeforeToolCallHook({
toolName: "skill_workshop",
params: { action: "apply", proposal_id: "weather-20260530-a1b2c3d4e5" },
ctx: { agentId: "main", sessionKey: "main" },
});
expect(result).toEqual({
blocked: false,
params: { action: "apply", proposal_id: "weather-20260530-a1b2c3d4e5" },
});
expect(mockCallGatewayTool).not.toHaveBeenCalled();
expect(runBeforeToolCallMock).not.toHaveBeenCalled();
});
it("preserves trusted policy params when before_tool_call hooks leave params unchanged", async () => {
const registry = createEmptyPluginRegistry();
registry.trustedToolPolicies = [

View File

@@ -1,4 +1,9 @@
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import {
clearRuntimeConfigSnapshot,
getRuntimeConfig,
setRuntimeConfigSnapshot,
} from "../../config/config.js";
import { PLUGIN_APPROVAL_DESCRIPTION_MAX_LENGTH } from "../../infra/plugin-approvals.js";
import {
createOpenClawTestState,
@@ -19,6 +24,7 @@ beforeEach(async () => {
});
afterEach(async () => {
clearRuntimeConfigSnapshot();
await testState.cleanup();
await tempDirs.cleanup();
});
@@ -102,9 +108,7 @@ describe("resolveSkillWorkshopToolApproval", () => {
});
const approvalDescription = result?.requireApproval?.description ?? "";
expect(approvalDescription.length).toBeLessThanOrEqual(
PLUGIN_APPROVAL_DESCRIPTION_MAX_LENGTH,
);
expect(approvalDescription.length).toBeLessThanOrEqual(PLUGIN_APPROVAL_DESCRIPTION_MAX_LENGTH);
expect(approvalDescription).toContain(`Proposal ID: ${proposal.record.id}`);
expect(approvalDescription).toContain(`Description: ${description}`);
expect(approvalDescription).toContain("Support files: 0");
@@ -169,4 +173,70 @@ describe("resolveSkillWorkshopToolApproval", () => {
"Apply a pending workspace skill proposal into live workspace skills.",
);
});
it("uses runtime config when lifecycle hook config is absent", async () => {
setRuntimeConfigSnapshot({
skills: {
workshop: {
approvalPolicy: "auto",
},
},
});
await expect(
resolveSkillWorkshopToolApproval({
toolName: "skill_workshop",
toolParams: { action: "apply", proposal_id: "weather-20260530-a1b2c3d4e5" },
}),
).resolves.toBeUndefined();
});
it("keeps approval pending when runtime config loading throws", async () => {
const sharedAgentDir = testState.agentDir("shared");
await testState.writeConfig({
agents: {
list: [
{ id: "alpha", agentDir: sharedAgentDir },
{ id: "beta", agentDir: sharedAgentDir },
],
},
});
const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
try {
expect(() => getRuntimeConfig()).toThrow(/duplicate agentDir/i);
const result = await resolveSkillWorkshopToolApproval({
toolName: "skill_workshop",
toolParams: { action: "quarantine", proposal_id: "weather-20260530-a1b2c3d4e5" },
});
expect(result?.requireApproval?.title).toBe("Quarantine workspace skill proposal");
} finally {
consoleError.mockRestore();
}
});
it("keeps explicit lifecycle hook config ahead of runtime config", async () => {
setRuntimeConfigSnapshot({
skills: {
workshop: {
approvalPolicy: "auto",
},
},
});
const result = await resolveSkillWorkshopToolApproval({
toolName: "skill_workshop",
toolParams: { action: "reject", proposal_id: "weather-20260530-a1b2c3d4e5" },
config: {
skills: {
workshop: {
approvalPolicy: "pending",
},
},
},
});
expect(result?.requireApproval?.title).toBe("Reject workspace skill proposal");
});
});

View File

@@ -1,6 +1,7 @@
// Workshop policy helpers validate generated skill drafts against workspace policy.
import { asNullableRecord } from "@openclaw/normalization-core/record-coerce";
import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice";
import { getRuntimeConfig } from "../../config/config.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import { PLUGIN_APPROVAL_DESCRIPTION_MAX_LENGTH } from "../../infra/plugin-approvals.js";
import type { PluginHookBeforeToolCallResult } from "../../plugins/hook-before-tool-call-result.js";
@@ -141,6 +142,19 @@ function lifecycleApprovalTimeoutReason(proposalId?: string): string {
].join(" ");
}
function resolveApprovalConfig(config?: OpenClawConfig): OpenClawConfig | undefined {
if (config) {
return config;
}
// Explicit hook config wins. Missing hook config may happen on agent paths;
// unreadable runtime config keeps the default pending approval gate.
try {
return getRuntimeConfig();
} catch {
return undefined;
}
}
/** Returns approval policy for skill workshop lifecycle tool calls. */
export async function resolveSkillWorkshopToolApproval(params: {
toolName: string;
@@ -155,7 +169,7 @@ export async function resolveSkillWorkshopToolApproval(params: {
if (!action) {
return undefined;
}
const config = resolveSkillWorkshopConfig(params.config);
const config = resolveSkillWorkshopConfig(resolveApprovalConfig(params.config));
if (config.approvalPolicy === "auto") {
return undefined;
}