diff --git a/CHANGELOG.md b/CHANGELOG.md index f960107dcb6..2849515dc8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -103,6 +103,7 @@ Docs: https://docs.openclaw.ai - Diffs/viewer: re-read remote viewer access policy from live runtime config on each request, so toggling `plugins.entries.diffs.config.security.allowRemoteViewer` closes proxied viewer access immediately instead of waiting for a restart. Thanks @vincentkoc. - Diffs/tooling: re-read `viewerBaseUrl`, presentation defaults, and viewer access policy from live runtime config, and fail closed when the live `diffs` plugin entry disappears instead of reviving startup viewer settings. Thanks @vincentkoc. - Memory/LanceDB: stop resurrecting removed live `memory-lancedb` hook config from startup snapshots, so deleting or disabling the plugin entry shuts off auto-recall and auto-capture without a restart. Thanks @vincentkoc. +- Skill Workshop: keep the tool plus `before_prompt_build` / `agent_end` hooks wired while the plugin is disabled at startup, so turning the plugin back on in live config starts guidance and capture without waiting for a restart. Thanks @vincentkoc. - Active Memory: stop reviving removed live `active-memory` config from startup snapshots, so removing the plugin entry turns the hook off immediately instead of waiting for a restart. Thanks @vincentkoc. - Agents/subagents: drop bare `NO_REPLY` from the parent turn when the session still has pending spawned children, so direct-conversation surfaces such as Telegram DMs no longer rewrite the sentinel into visible fallback chatter while waiting for the child completion event. (#69942) Thanks @neeravmakwana. - Plugins/install: keep bundled plugin dependencies off npm install while repairing them when plugins activate from a packaged install, including Feishu/Lark, Browser, and direct bundled channel setup-entry loads. diff --git a/extensions/skill-workshop/index.test.ts b/extensions/skill-workshop/index.test.ts index d6074f539b0..09eb6c378f8 100644 --- a/extensions/skill-workshop/index.test.ts +++ b/extensions/skill-workshop/index.test.ts @@ -51,19 +51,23 @@ function createProposal( } describe("skill-workshop", () => { - it("does not register hooks or tools when disabled", () => { + it("registers inert hooks and a null tool when disabled", () => { const on = vi.fn(); - const registerTool = vi.fn(); + let tool: AnyAgentTool | null | undefined; const api = createTestPluginApi({ pluginConfig: { enabled: false }, on, - registerTool, + registerTool(registered) { + const resolved = typeof registered === "function" ? registered({}) : registered; + tool = Array.isArray(resolved) ? resolved[0] : resolved; + }, }); plugin.register(api); - expect(registerTool).not.toHaveBeenCalled(); - expect(on).not.toHaveBeenCalled(); + expect(tool).toBeNull(); + expect(on).toHaveBeenCalledWith("before_prompt_build", expect.any(Function)); + expect(on).toHaveBeenCalledWith("agent_end", expect.any(Function)); }); it("detects user corrections and creates an animated GIF proposal", async () => { @@ -371,17 +375,93 @@ describe("skill-workshop", () => { ).rejects.toMatchObject({ code: "ENOENT" }); }); - it("skips agent_end hook wiring when auto-capture is disabled", () => { + it("uses live runtime config to enable prompt guidance and capture after startup disable", async () => { + const workspaceDir = await makeTempDir(); + const stateDir = await makeTempDir(); + let configFile: Record = { + plugins: { + entries: { + "skill-workshop": { + config: { + enabled: false, + autoCapture: false, + reviewMode: "off", + }, + }, + }, + }, + }; const on = vi.fn(); + let toolFactory: + | ((ctx: { workspaceDir?: string }) => AnyAgentTool | AnyAgentTool[] | null | undefined) + | undefined; const api = createTestPluginApi({ - pluginConfig: { autoCapture: false }, + pluginConfig: { enabled: false, autoCapture: false, reviewMode: "off" }, + runtime: { + agent: { + resolveAgentWorkspaceDir: () => workspaceDir, + }, + state: { + resolveStateDir: () => stateDir, + }, + config: { + loadConfig: () => configFile, + }, + } as never, on, + registerTool(registered) { + toolFactory = typeof registered === "function" ? registered : undefined; + }, }); plugin.register(api); - expect(on).toHaveBeenCalledWith("before_prompt_build", expect.any(Function)); - expect(on).not.toHaveBeenCalledWith("agent_end", expect.any(Function)); + const beforePromptBuild = on.mock.calls.find((call) => call[0] === "before_prompt_build")?.[1]; + const agentEnd = on.mock.calls.find((call) => call[0] === "agent_end")?.[1]; + expect(beforePromptBuild).toBeTypeOf("function"); + expect(agentEnd).toBeTypeOf("function"); + expect(toolFactory?.({ workspaceDir }) ?? null).toBeNull(); + await expect(beforePromptBuild?.({}, {})).resolves.toBeUndefined(); + + configFile = { + plugins: { + entries: { + "skill-workshop": { + config: { + enabled: true, + autoCapture: true, + approvalPolicy: "auto", + reviewMode: "heuristic", + }, + }, + }, + }, + }; + + const refreshedTool = toolFactory?.({ workspaceDir }); + const tool = Array.isArray(refreshedTool) ? refreshedTool[0] : refreshedTool; + expect(tool?.name).toBe("skill_workshop"); + await expect(beforePromptBuild?.({}, {})).resolves.toEqual({ + prependSystemContext: expect.stringContaining(""), + }); + + await agentEnd?.( + { + success: true, + messages: [ + { + role: "user", + content: + "From now on when asked for animated GIFs, verify the file is actually animated.", + }, + ], + }, + { workspaceDir }, + ); + + await expect( + fs.access(path.join(workspaceDir, "skills", "animated-gif-workflow", "SKILL.md")), + ).resolves.toBeUndefined(); }); it("uses live runtime config to skip capture when review mode turns off", async () => { @@ -455,7 +535,29 @@ describe("skill-workshop", () => { expect(logger.info).not.toHaveBeenCalledWith("skill-workshop: applied animated-gif-workflow"); }); - it("skips agent_end hook wiring when review mode is off", () => { + it("keeps agent_end registered but inert when auto-capture is disabled", async () => { + const on = vi.fn(); + const api = createTestPluginApi({ + pluginConfig: { autoCapture: false }, + on, + }); + + plugin.register(api); + + const handler = on.mock.calls.find((call) => call[0] === "agent_end")?.[1]; + expect(handler).toBeTypeOf("function"); + await expect( + handler?.( + { + success: true, + messages: [{ role: "user", content: "remember this animation workflow" }], + }, + {}, + ), + ).resolves.toBeUndefined(); + }); + + it("keeps agent_end registered but inert when review mode is off", async () => { const on = vi.fn(); const api = createTestPluginApi({ pluginConfig: { reviewMode: "off" }, @@ -464,8 +566,17 @@ describe("skill-workshop", () => { plugin.register(api); - expect(on).toHaveBeenCalledWith("before_prompt_build", expect.any(Function)); - expect(on).not.toHaveBeenCalledWith("agent_end", expect.any(Function)); + const handler = on.mock.calls.find((call) => call[0] === "agent_end")?.[1]; + expect(handler).toBeTypeOf("function"); + await expect( + handler?.( + { + success: true, + messages: [{ role: "user", content: "remember this animation workflow" }], + }, + {}, + ), + ).resolves.toBeUndefined(); }); it("lets explicit tool suggestions stay pending in auto mode", async () => { diff --git a/extensions/skill-workshop/index.ts b/extensions/skill-workshop/index.ts index 6578ab912c5..097c323734c 100644 --- a/extensions/skill-workshop/index.ts +++ b/extensions/skill-workshop/index.ts @@ -13,10 +13,6 @@ export default definePluginEntry({ description: "Captures repeatable workflows as workspace skills, with pending review and safe writes.", register(api) { - const startupConfig = resolveConfig(api.pluginConfig); - if (!startupConfig.enabled) { - return; - } const resolveCurrentConfig = () => { const runtimePluginConfig = resolveLivePluginConfigObject( api.runtime.config?.loadConfig, @@ -49,97 +45,94 @@ export default definePluginEntry({ }; }); - if (startupConfig.autoCapture && startupConfig.reviewMode !== "off") { - api.on("agent_end", async (event, ctx) => { - const config = resolveCurrentConfig(); - if (!config.enabled || !config.autoCapture || config.reviewMode === "off") { - return; - } - if (!event.success) { - return; - } - if (ctx.sessionId?.startsWith("skill-workshop-review-")) { - return; - } - const agentId = ctx.agentId ?? resolveDefaultAgentId(api.config); - const workspaceDir = - ctx.workspaceDir || api.runtime.agent.resolveAgentWorkspaceDir(api.config, agentId); - const store = createStoreForContext({ api, ctx: { ...ctx, workspaceDir }, config }); - const heuristicProposal = createProposalFromMessages({ - messages: event.messages, - workspaceDir, - agentId, - sessionId: ctx.sessionId, - }); - const heuristicEnabled = - config.reviewMode === "heuristic" || config.reviewMode === "hybrid"; - if (heuristicEnabled && heuristicProposal) { - try { - const result = await applyOrStoreProposal({ - proposal: heuristicProposal, - store, - config, - workspaceDir, - }); - if (result.status === "applied") { - api.logger.info(`skill-workshop: applied ${heuristicProposal.skillName}`); - } else if (result.status === "quarantined") { - api.logger.warn(`skill-workshop: quarantined ${heuristicProposal.skillName}`); - } else { - api.logger.info(`skill-workshop: queued ${heuristicProposal.skillName}`); - } - } catch (error) { - api.logger.warn(`skill-workshop: heuristic capture skipped: ${String(error)}`); - } - } - - const llmEnabled = config.reviewMode === "llm" || config.reviewMode === "hybrid"; - if (!llmEnabled) { - return; - } - const reviewState = await store.recordReviewTurn(countToolCalls(event.messages)); - const thresholdMet = - reviewState.turnsSinceReview >= config.reviewInterval || - reviewState.toolCallsSinceReview >= config.reviewMinToolCalls; - const shouldReview = - thresholdMet || (config.reviewMode === "llm" && heuristicProposal !== undefined); - if (!shouldReview) { - return; - } - await store.markReviewed(); + api.on("agent_end", async (event, ctx) => { + const config = resolveCurrentConfig(); + if (!config.enabled || !config.autoCapture || config.reviewMode === "off") { + return; + } + if (!event.success) { + return; + } + if (ctx.sessionId?.startsWith("skill-workshop-review-")) { + return; + } + const agentId = ctx.agentId ?? resolveDefaultAgentId(api.config); + const workspaceDir = + ctx.workspaceDir || api.runtime.agent.resolveAgentWorkspaceDir(api.config, agentId); + const store = createStoreForContext({ api, ctx: { ...ctx, workspaceDir }, config }); + const heuristicProposal = createProposalFromMessages({ + messages: event.messages, + workspaceDir, + agentId, + sessionId: ctx.sessionId, + }); + const heuristicEnabled = config.reviewMode === "heuristic" || config.reviewMode === "hybrid"; + if (heuristicEnabled && heuristicProposal) { try { - const proposal = await reviewTranscriptForProposal({ - api, + const result = await applyOrStoreProposal({ + proposal: heuristicProposal, + store, config, - messages: event.messages, - ctx: { - agentId, - sessionId: ctx.sessionId, - sessionKey: ctx.sessionKey, - workspaceDir, - modelProviderId: ctx.modelProviderId, - modelId: ctx.modelId, - messageProvider: ctx.messageProvider, - channelId: ctx.channelId, - }, + workspaceDir, }); - if (!proposal) { - api.logger.debug?.("skill-workshop: reviewer found no update"); - return; - } - const result = await applyOrStoreProposal({ proposal, store, config, workspaceDir }); if (result.status === "applied") { - api.logger.info(`skill-workshop: applied ${proposal.skillName}`); + api.logger.info(`skill-workshop: applied ${heuristicProposal.skillName}`); } else if (result.status === "quarantined") { - api.logger.warn(`skill-workshop: quarantined ${proposal.skillName}`); + api.logger.warn(`skill-workshop: quarantined ${heuristicProposal.skillName}`); } else { - api.logger.info(`skill-workshop: queued ${proposal.skillName}`); + api.logger.info(`skill-workshop: queued ${heuristicProposal.skillName}`); } } catch (error) { - api.logger.warn(`skill-workshop: reviewer skipped: ${String(error)}`); + api.logger.warn(`skill-workshop: heuristic capture skipped: ${String(error)}`); } - }); - } + } + + const llmEnabled = config.reviewMode === "llm" || config.reviewMode === "hybrid"; + if (!llmEnabled) { + return; + } + const reviewState = await store.recordReviewTurn(countToolCalls(event.messages)); + const thresholdMet = + reviewState.turnsSinceReview >= config.reviewInterval || + reviewState.toolCallsSinceReview >= config.reviewMinToolCalls; + const shouldReview = + thresholdMet || (config.reviewMode === "llm" && heuristicProposal !== undefined); + if (!shouldReview) { + return; + } + await store.markReviewed(); + try { + const proposal = await reviewTranscriptForProposal({ + api, + config, + messages: event.messages, + ctx: { + agentId, + sessionId: ctx.sessionId, + sessionKey: ctx.sessionKey, + workspaceDir, + modelProviderId: ctx.modelProviderId, + modelId: ctx.modelId, + messageProvider: ctx.messageProvider, + channelId: ctx.channelId, + }, + }); + if (!proposal) { + api.logger.debug?.("skill-workshop: reviewer found no update"); + return; + } + const result = await applyOrStoreProposal({ proposal, store, config, workspaceDir }); + if (result.status === "applied") { + api.logger.info(`skill-workshop: applied ${proposal.skillName}`); + } else if (result.status === "quarantined") { + api.logger.warn(`skill-workshop: quarantined ${proposal.skillName}`); + } else { + api.logger.info(`skill-workshop: queued ${proposal.skillName}`); + } + } catch (error) { + api.logger.warn(`skill-workshop: reviewer skipped: ${String(error)}`); + } + }); }, });