diff --git a/src/claws/lifecycle-config-removal.ts b/src/claws/lifecycle-config-removal.ts index a7e4cc83c51f..eceb3989944c 100644 --- a/src/claws/lifecycle-config-removal.ts +++ b/src/claws/lifecycle-config-removal.ts @@ -1,5 +1,6 @@ import { createHash } from "node:crypto"; import { stableStringify } from "@openclaw/normalization-core"; +import { beginAgentDeletion } from "../agents/agent-lifecycle-registry.js"; import { listAgentEntries } from "../agents/agent-scope.js"; import { getRuntimeConfig } from "../config/config.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; @@ -7,7 +8,9 @@ import { AgentConfigPreconditionError, deleteAgentConfigEntry, } from "../gateway/server-methods/agents-config-mutations.js"; +import { withAgentExecApprovalsRemoved } from "../infra/exec-approvals.js"; import { normalizeAgentId } from "../routing/session-key.js"; +import { readAgentDeletionJournal } from "../state/agent-deletion-journal.js"; import { digestClawAgentConfig } from "./agent-config-digest.js"; import { deletionEffects, @@ -17,6 +20,25 @@ import { export type ConfigCommit = (transform: (config: OpenClawConfig) => OpenClawConfig) => Promise; +type ClawAgentConfigRemovalParams = { + agentId: string; + expectedDigest: string; + expectedRemovalSurfaceDigest: string; + expectedState: "present" | "missing"; + fallbackWorkspace: string; + config?: OpenClawConfig; + commitConfig?: ConfigCommit; + trashPath?: ClawTrashPath; + onModified: () => Error; +}; + +type ClawAgentConfigRemovalResult = { + agentRemoved: boolean; + cleanupTargets?: ClawCleanupTargets; + configBeforeDelete: OpenClawConfig; + nextConfig: OpenClawConfig; +}; + export { digestClawAgentConfig } from "./agent-config-digest.js"; export function digestClawAgentRemovalSurface(config: OpenClawConfig, agentId: string): string { @@ -32,31 +54,11 @@ export function digestClawAgentRemovalSurface(config: OpenClawConfig, agentId: s return `sha256:${createHash("sha256").update(stableStringify(surface)).digest("hex")}`; } -export async function claimClawAgentConfigRemoval(params: { - agentId: string; - expectedDigest: string; - expectedRemovalSurfaceDigest: string; - expectedState: "present" | "missing"; - fallbackWorkspace: string; - config?: OpenClawConfig; - commitConfig?: ConfigCommit; - trashPath?: ClawTrashPath; - onModified: () => Error; -}): Promise<{ - agentRemoved: boolean; - cleanupTargets?: ClawCleanupTargets; - configBeforeDelete: OpenClawConfig; - nextConfig: OpenClawConfig; -}> { +async function commitClawAgentConfigRemoval( + params: ClawAgentConfigRemovalParams, +): Promise { if (params.commitConfig) { - let result: - | { - agentRemoved: boolean; - cleanupTargets?: ClawCleanupTargets; - configBeforeDelete: OpenClawConfig; - nextConfig: OpenClawConfig; - } - | undefined; + let result: ClawAgentConfigRemovalResult | undefined; await params.commitConfig((config) => { const effects = deletionEffects(config, params.agentId, params.fallbackWorkspace); const agent = listAgentEntries(config).find((candidate) => candidate.id === params.agentId); @@ -149,3 +151,35 @@ export async function claimClawAgentConfigRemoval(params: { }; } } + +export async function claimClawAgentConfigRemoval(params: ClawAgentConfigRemovalParams) { + const config = params.config ?? getRuntimeConfig(); + const effects = deletionEffects(config, params.agentId, params.fallbackWorkspace); + // beginAgentDeletion takes over an existing journal row instead of refusing it, so rolling back + // a row this call did not open would erase another deletion's record. + const existingJournal = readAgentDeletionJournal(params.agentId); + const deletion = beginAgentDeletion({ + agentId: params.agentId, + workspaceDir: effects.workspace, + agentDir: effects.agentDir, + sessionsDir: effects.sessionsDir, + // Claw removal owns selective cleanup and may retain modified or untracked workspace entries, + // so the journal must not claim authority to trash them. + deleteFiles: existingJournal?.deleteFiles ?? false, + }); + try { + const result = await withAgentExecApprovalsRemoved(params.agentId, async () => + commitClawAgentConfigRemoval({ ...params, config }), + ); + deletion.commit(); + // The journal fences only the roster and approvals commit here; Claw's own filesystem cleanup + // runs afterwards and may legitimately end partial, so completion is recorded now. + deletion.finish(); + return result; + } catch (error) { + if (!existingJournal) { + deletion.rollback(); + } + throw error; + } +} diff --git a/src/claws/lifecycle-remove-approvals.test.ts b/src/claws/lifecycle-remove-approvals.test.ts new file mode 100644 index 000000000000..52d3d9c9b440 --- /dev/null +++ b/src/claws/lifecycle-remove-approvals.test.ts @@ -0,0 +1,156 @@ +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; +import { beginAgentDeletion } from "../agents/agent-lifecycle-registry.js"; +import { withTempHomeConfig, writeOpenClawConfig } from "../config/test-helpers.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { loadExecApprovals, saveExecApprovals } from "../infra/exec-approvals.js"; +import { readAgentDeletionJournal } from "../state/agent-deletion-journal.js"; +import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; +import { captureEnv, setTestEnvValue } from "../test-utils/env.js"; +import { applyClawAddPlan } from "./add.js"; +import { + claimClawAgentConfigRemoval, + digestClawAgentRemovalSurface, +} from "./lifecycle-config-removal.js"; +import { applyClawRemovePlan, buildClawRemovePlan } from "./lifecycle-state.js"; +import { buildClawAddPlan } from "./lifecycle.js"; +import { parseClawManifest } from "./schema.js"; +import type { ClawSourceIdentity } from "./types.js"; + +const tempDirs = useAutoCleanupTempDirTracker(afterEach); +const envSnapshot = captureEnv(["OPENCLAW_STATE_DIR"]); + +afterEach(() => { + closeOpenClawStateDatabaseForTest(); + envSnapshot.restore(); +}); + +async function buildApprovalFixture() { + const root = tempDirs.make("openclaw-claw-remove-approvals-"); + const parsed = parseClawManifest({ + schemaVersion: 1, + agent: { id: "worker", name: "Worker" }, + }); + if (!parsed.ok) { + throw new Error(JSON.stringify(parsed.diagnostics)); + } + const source: ClawSourceIdentity = { + kind: "package", + name: "@acme/worker", + version: "1.0.0", + packageRoot: root, + manifestPath: join(root, "openclaw.claw.json"), + integrityKind: "artifact", + integrity: "sha256:manifest", + byteLength: 100, + }; + return await buildClawAddPlan({ + manifest: parsed.manifest, + source, + context: { workspace: join(root, "workspace-worker") }, + }); +} + +describe("Claw exec approvals removal", () => { + it.each([ + { label: "config-file commit", useCommitConfig: false }, + { label: "commitConfig seam", useCommitConfig: true }, + ])("removes only the claw agent policy through the $label", async ({ useCommitConfig }) => { + const addPlan = await buildApprovalFixture(); + + await withTempHomeConfig({}, async ({ home }) => { + const env = { OPENCLAW_STATE_DIR: join(home, ".openclaw") }; + setTestEnvValue("OPENCLAW_STATE_DIR", env.OPENCLAW_STATE_DIR); + let config: OpenClawConfig = {}; + await applyClawAddPlan(addPlan, { + consentPlanIntegrity: addPlan.planIntegrity, + env, + commitConfig: async (transform) => { + config = transform(config); + }, + }); + await writeOpenClawConfig(home, config); + saveExecApprovals({ + version: 1, + agents: { + "*": { security: "deny" }, + worker: { + security: "allowlist", + allowlist: [{ pattern: "/usr/bin/rm" }], + }, + kept: { + security: "allowlist", + allowlist: [{ pattern: "/usr/bin/keep" }], + }, + }, + }); + const plan = useCommitConfig + ? await buildClawRemovePlan("worker", { env, config }) + : await buildClawRemovePlan("worker"); + const common = { + consentPlanIntegrity: plan.planIntegrity, + trashPath: async () => true, + }; + + const result = useCommitConfig + ? await applyClawRemovePlan(plan, { + ...common, + env, + config, + commitConfig: async (transform) => { + config = transform(config); + }, + }) + : await applyClawRemovePlan(plan, common); + + expect(result).toMatchObject({ status: "complete", agentRemoved: true }); + expect(loadExecApprovals().agents).toEqual({ + "*": { security: "deny" }, + kept: { + security: "allowlist", + allowlist: [expect.objectContaining({ pattern: "/usr/bin/keep" })], + }, + }); + expect(readAgentDeletionJournal("worker")).toMatchObject({ + cleanupCompleted: true, + deleteFiles: false, + }); + }); + }); + + // beginAgentDeletion takes over an existing journal row, so a failed Claw removal must not roll + // back a deletion another path started. + it.each([ + { label: "keeps a pre-existing journal", seedJournal: true }, + { label: "rolls back the journal it opened", seedJournal: false }, + ])("$label when the config commit fails", async ({ seedJournal }) => { + const root = tempDirs.make("openclaw-claw-remove-journal-"); + setTestEnvValue("OPENCLAW_STATE_DIR", join(root, "state")); + if (seedJournal) { + beginAgentDeletion({ + agentId: "worker", + agentDir: join(root, "agent"), + workspaceDir: join(root, "workspace"), + sessionsDir: join(root, "sessions"), + }); + } + + await expect( + claimClawAgentConfigRemoval({ + agentId: "worker", + expectedDigest: "sha256:unused", + expectedRemovalSurfaceDigest: digestClawAgentRemovalSurface({}, "worker"), + expectedState: "present", + fallbackWorkspace: join(root, "workspace"), + config: {}, + commitConfig: async () => { + throw new Error("claw commit failed"); + }, + onModified: () => new Error("claw agent modified"), + }), + ).rejects.toThrow("claw commit failed"); + + expect(readAgentDeletionJournal("worker") === undefined).toBe(!seedJournal); + }); +});