From 43e1fb00969e4e8d8c8b0eaefb173795dc345d86 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 29 Jul 2026 19:02:46 +0800 Subject: [PATCH] fix(skills): avoid recursive workshop recovery locks --- src/skills/workshop/apply-transition.ts | 16 ++++++++++++---- src/skills/workshop/service-evaluation.ts | 2 ++ src/skills/workshop/service-query.ts | 9 +++++++-- src/skills/workshop/service.test.ts | 10 ++++++++++ src/skills/workshop/service.ts | 9 +++++++-- src/skills/workshop/store.ts | 11 +++++++++-- 6 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/skills/workshop/apply-transition.ts b/src/skills/workshop/apply-transition.ts index d12e1ec17d39..234696aced66 100644 --- a/src/skills/workshop/apply-transition.ts +++ b/src/skills/workshop/apply-transition.ts @@ -83,7 +83,10 @@ export type SkillProposalApplyTransitionDependencies = { workspaceDir?: string, env?: NodeJS.ProcessEnv, agentId?: string, - config?: OpenClawConfig, + readOptions?: { + config?: OpenClawConfig; + reconcile?: boolean; + }, ) => Promise; }; @@ -113,12 +116,17 @@ export async function applySkillProposalTransition( input: SkillProposalActionInput, dependencies: SkillProposalApplyTransitionDependencies, ): Promise { + const recoveryReadOptions = input.config ? { config: input.config } : undefined; + const lockedReadOptions = { + ...(input.config ? { config: input.config } : {}), + reconcile: false, + }; const initial = await dependencies.readRequiredProposal( input.proposalId, input.workspaceDir, input.env, input.agentId, - input.config, + recoveryReadOptions, ); if (initial.record.status !== "pending") { throw new Error( @@ -149,7 +157,7 @@ export async function applySkillProposalTransition( input.workspaceDir, input.env, input.agentId, - input.config, + lockedReadOptions, ); if ( current.record.status === "pending" && @@ -190,7 +198,7 @@ export async function applySkillProposalTransition( input.workspaceDir, input.env, input.agentId, - input.config, + lockedReadOptions, ); const { record, content } = read; if (record.status !== "pending") { diff --git a/src/skills/workshop/service-evaluation.ts b/src/skills/workshop/service-evaluation.ts index c859788c71a8..6d677f5ac379 100644 --- a/src/skills/workshop/service-evaluation.ts +++ b/src/skills/workshop/service-evaluation.ts @@ -57,6 +57,7 @@ export async function evaluateSkillProposal( input.workspaceDir, input.env, input.agentId, + { reconcile: false }, ); if (read.record.status !== "pending") { throw new Error( @@ -154,6 +155,7 @@ export async function evaluateSkillProposal( input.workspaceDir, input.env, input.agentId, + { reconcile: false }, ); if ( current.record.status !== "pending" || diff --git a/src/skills/workshop/service-query.ts b/src/skills/workshop/service-query.ts index 2c129bcd2968..f4c52c39dff0 100644 --- a/src/skills/workshop/service-query.ts +++ b/src/skills/workshop/service-query.ts @@ -16,6 +16,11 @@ type SkillProposalScopeOptions = { workspaceDir?: string; }; +type RequiredProposalReadOptions = { + config?: OpenClawConfig; + reconcile?: boolean; +}; + function storeOptions(env?: NodeJS.ProcessEnv) { return env ? { env } : {}; } @@ -123,7 +128,7 @@ export async function readRequiredProposal( workspaceDir?: string, env?: NodeJS.ProcessEnv, agentId?: string, - config?: OpenClawConfig, + readOptions: RequiredProposalReadOptions = {}, ): Promise { const read = await readSkillProposal( proposalId, @@ -132,7 +137,7 @@ export async function readRequiredProposal( ...(agentId ? { agentId } : {}), ...(workspaceDir ? { workspaceDir } : {}), }, - config, + readOptions, ); if (!read) { throw new Error(`Skill proposal not found: ${proposalId}`); diff --git a/src/skills/workshop/service.test.ts b/src/skills/workshop/service.test.ts index 453106c39abf..a8257e1badf0 100644 --- a/src/skills/workshop/service.test.ts +++ b/src/skills/workshop/service.test.ts @@ -1159,6 +1159,16 @@ describe("skill workshop proposals", () => { "# External change\n", ); await expect(readSkillProposalRollback(proposal.record.id)).resolves.not.toBeNull(); + + const startedAt = Date.now(); + await expect( + rejectSkillProposal({ + workspaceDir, + proposalId: proposal.record.id, + reason: "external target retained", + }), + ).resolves.toMatchObject({ status: "rejected" }); + expect(Date.now() - startedAt).toBeLessThan(2_000); }); it("reconciles an update apply interrupted after the live skill write", async () => { diff --git a/src/skills/workshop/service.ts b/src/skills/workshop/service.ts index 7f134d7e4e64..38901b0b3879 100644 --- a/src/skills/workshop/service.ts +++ b/src/skills/workshop/service.ts @@ -642,12 +642,17 @@ async function withPendingSkillProposalMutation( action: "applied" | "quarantined" | "rejected" | "revised", fn: (read: SkillProposalReadResult) => Promise, ): Promise { + const recoveryReadOptions = input.config ? { config: input.config } : undefined; + const lockedReadOptions = { + ...(input.config ? { config: input.config } : {}), + reconcile: false, + }; const initial = await readRequiredProposal( input.proposalId, input.workspaceDir, input.env, input.agentId, - input.config, + recoveryReadOptions, ); return await withSkillProposalTargetLock( initial.record, @@ -657,7 +662,7 @@ async function withPendingSkillProposalMutation( input.workspaceDir, input.env, input.agentId, - input.config, + lockedReadOptions, ); if (read.record.status !== "pending") { throw new Error( diff --git a/src/skills/workshop/store.ts b/src/skills/workshop/store.ts index da5a0622df8d..7360888ce539 100644 --- a/src/skills/workshop/store.ts +++ b/src/skills/workshop/store.ts @@ -71,6 +71,11 @@ type SkillProposalLookupScope = { workspaceDir?: string; }; +type SkillProposalReadOptions = { + config?: OpenClawConfig; + reconcile?: boolean; +}; + export type PreparedSkillProposalSupportFile = SkillProposalSupportFile & { content: string; }; @@ -178,13 +183,15 @@ export async function readSkillProposal( proposalId: string, options: SkillWorkshopStoreOptions = {}, scope: SkillProposalLookupScope = {}, - recoveryConfig?: OpenClawConfig, + readOptions: SkillProposalReadOptions = {}, ): Promise { let stored = readStoredProposal(proposalId, options); if (!stored || !isStoredProposalVisible(stored.row, scope)) { return null; } - await reconcileInterruptedApply(proposalId, options, recoveryConfig); + if (readOptions.reconcile !== false) { + await reconcileInterruptedApply(proposalId, options, readOptions.config); + } stored = readStoredProposal(proposalId, options); if (!stored || !isStoredProposalVisible(stored.row, scope)) { return null;