fix(worktrees): preserve active removal authority (#126174)

packages/gateway-protocol/src/schema/worktrees.ts

@@ -60,7 +60,7 @@ export const WorktreesRemoveParamsSchema = closedObject({
 export const WorktreesRemoveResultSchema = closedObject({
   removed: Type.Boolean(),
   snapshotRef: Type.Optional(NonEmptyString),
-  /** Why the pre-removal snapshot failed; present only on forced removals that continued without one. */
+  /** Why the pre-removal snapshot failed; removal may have stopped or continued without one. */
   snapshotError: Type.Optional(NonEmptyString),
 });
 

src/agents/worktrees/registry.ts

@@ -599,7 +599,6 @@ export function claimWorktreeRemovalRow(
   params: {
     worktreeId: string;
     token: string;
-    force: boolean;
     pid: number;
     startTime: number | null;
     now: number;
@@ -625,7 +624,7 @@ export function claimWorktreeRemovalRow(
         );
       }
       const { livePids, removingToken } = collectLiveRunLeases(db, k, scope, params.checks ?? {});
-      if (!params.force && livePids.length > 0) {
+      if (livePids.length > 0) {
         throw new WorktreeRemovalContentionError(
           "busy",
           `worktree is busy: locked by live pid ${livePids[0]}`,

src/agents/worktrees/run-lease.test.ts

@@ -136,9 +136,9 @@ describe("worktree run lease", () => {
     expect(resolved).toBe(created.id);
 
     const lease = await acquireWorktreeRunLease(created.id, { env });
-    expect(() =>
-      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover", force: false }),
-    ).toThrow("worktree is busy");
+    expect(() => claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover" })).toThrow(
+      "worktree is busy",
+    );
     await lease.release();
   });
 
@@ -155,7 +155,7 @@ describe("worktree run lease", () => {
 
     expect(hasLiveWorktreeRunLease(env, created.id)).toBe(false);
     expect(() =>
-      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover", force: false }),
+      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover" }),
     ).not.toThrow();
   });
 
@@ -179,22 +179,19 @@ describe("worktree run lease", () => {
     expect(hasLiveWorktreeRunLease(env, created.id)).toBe(false);
   });
 
-  it("rejects removal of a live lease unless forced", async () => {
+  it("rejects removal while a live lease exists", async () => {
     const created = await createSessionWorktree();
     const lease = await acquireWorktreeRunLease(created.id, { env });
 
-    expect(() =>
-      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover", force: false }),
-    ).toThrow("worktree is busy");
-    expect(() =>
-      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover", force: true }),
-    ).not.toThrow();
+    expect(() => claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover" })).toThrow(
+      "worktree is busy",
+    );
     await lease.release();
   });
 
   it("fails admission once a removal claim is held", async () => {
     const created = await createSessionWorktree();
-    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover", force: true });
+    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover" });
 
     await expect(acquireWorktreeRunLease(created.id, { env })).rejects.toThrow(
       `managed worktree was removed: ${created.path}`,
@@ -203,7 +200,7 @@ describe("worktree run lease", () => {
 
   it("recovers admission when the remover died before finalizing the removal", async () => {
     const created = await createSessionWorktree();
-    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover", force: true });
+    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover" });
     runLeaseTesting.setDeadPidResolverForTest((pid) => pid === process.pid);
 
     const lease = await acquireWorktreeRunLease(created.id, { env });
@@ -211,20 +208,17 @@ describe("worktree run lease", () => {
     await lease.release();
   });
 
-  it("rejects a second live remover until the first releases, even with force", async () => {
+  it("rejects a second live remover until the first releases", async () => {
     const created = await createSessionWorktree();
-    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-a", force: false });
+    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-a" });
 
-    expect(() =>
-      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-b", force: false }),
-    ).toThrow("worktree removal is already in progress");
-    expect(() =>
-      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-b", force: true }),
-    ).toThrow("worktree removal is already in progress");
+    expect(() => claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-b" })).toThrow(
+      "worktree removal is already in progress",
+    );
 
     abortWorktreeRemoval(env, created.id, "remover-a");
     expect(() =>
-      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-b", force: false }),
+      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-b" }),
     ).not.toThrow();
   });
 
@@ -264,16 +258,16 @@ describe("worktree run lease", () => {
     await lease.release();
     expect(hasLiveWorktreeRunLease(env, created.id)).toBe(true);
     expect(await lockState(record)).toEqual({ kind: "live", pid: process.pid });
-    expect(() =>
-      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover", force: false }),
-    ).toThrow("worktree is busy");
+    expect(() => claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover" })).toThrow(
+      "worktree is busy",
+    );
 
     fail = false;
     await runLeaseTesting.drainPendingCleanupsForTest();
     expect(hasLiveWorktreeRunLease(env, created.id)).toBe(false);
     expect(await lockState(record)).toEqual({ kind: "none" });
     expect(() =>
-      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover", force: false }),
+      claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover" }),
     ).not.toThrow();
   });
 
@@ -342,7 +336,11 @@ describe("worktree run lease", () => {
 
   it("fails closed when a session's authoritative worktree binding is removed", async () => {
     const created = await createSessionWorktree();
-    await service.remove({ id: created.id, reason: "manual-delete", force: true });
+    await service.remove({
+      id: created.id,
+      reason: "manual-delete",
+      allowSnapshotLoss: true,
+    });
 
     await expect(
       resolveWorktreeIdForPath({
@@ -355,10 +353,10 @@ describe("worktree run lease", () => {
 
   it("does not let a superseded remover clear a newer removal claim", async () => {
     const created = await createSessionWorktree();
-    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-a", force: false });
+    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-a" });
 
     runLeaseTesting.setDeadPidResolverForTest((pid) => pid === process.pid);
-    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-b", force: false });
+    claimWorktreeRemoval(env, { worktreeId: created.id, token: "remover-b" });
     runLeaseTesting.setDeadPidResolverForTest(null);
 
     abortWorktreeRemoval(env, created.id, "remover-a");

src/agents/worktrees/run-lease.ts

@@ -304,7 +304,7 @@ export async function acquireWorktreeRunLease(
 
 export function claimWorktreeRemoval(
   env: NodeJS.ProcessEnv,
-  params: { worktreeId: string; token: string; force: boolean },
+  params: { worktreeId: string; token: string },
 ): void {
   const pid = process.pid;
   claimWorktreeRemovalRow(env, {

src/agents/worktrees/service.remove-lease.test.ts

@@ -6,7 +6,7 @@ import { promisify } from "node:util";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
 import { closeOpenClawStateDatabaseForTest } from "../../state/openclaw-state-db.js";
 import { getRegistryWorktree } from "./registry.js";
-import { acquireWorktreeRunLease } from "./run-lease.js";
+import { acquireWorktreeRunLease, hasLiveWorktreeRunLease } from "./run-lease.js";
 import { testing as runLeaseTesting } from "./run-lease.test-support.js";
 import { IDLE_GC_MS, ManagedWorktreeService } from "./service.js";
 
@@ -78,6 +78,74 @@ describe("ManagedWorktreeService removal against a live run lease", () => {
     await expect(fs.stat(created.path)).rejects.toMatchObject({ code: "ENOENT" });
   });
 
+  it("does not let snapshot-loss permission bypass a live run lease", async () => {
+    const created = await createSessionWorktree();
+    const lease = await acquireWorktreeRunLease(created.id, { env });
+
+    await expect(
+      service.remove({ id: created.id, reason: "manual-delete", allowSnapshotLoss: true }),
+    ).rejects.toThrow("worktree is busy");
+    expect(hasLiveWorktreeRunLease(env, created.id)).toBe(true);
+    expect(getRegistryWorktree(env, created.id)?.removedAt).toBeUndefined();
+    expect(await fs.stat(created.path)).toBeTruthy();
+
+    await lease.release();
+  });
+
+  it("does not let snapshot-loss permission bypass a foreign Git lock", async () => {
+    const created = await createSessionWorktree();
+    await git(repo, "worktree", "lock", "--reason", "other-tool", created.path);
+
+    await expect(
+      service.remove({ id: created.id, reason: "manual-delete", allowSnapshotLoss: true }),
+    ).rejects.toThrow("worktree has a foreign lock: other-tool");
+    expect(getRegistryWorktree(env, created.id)?.removedAt).toBeUndefined();
+    expect(await fs.stat(created.path)).toBeTruthy();
+  });
+
+  it("rejects a snapshot-loss retry when a run starts after snapshot failure", async () => {
+    const created = await createSessionWorktree();
+    const nested = path.join(created.path, "nested");
+    await fs.mkdir(nested);
+    await git(nested, "init", "-b", "main");
+
+    await expect(service.remove({ id: created.id, reason: "manual-delete" })).rejects.toThrow(
+      "nested git repositories cannot be snapshotted losslessly",
+    );
+    const lease = await acquireWorktreeRunLease(created.id, { env });
+
+    await expect(
+      service.remove({ id: created.id, reason: "manual-delete", allowSnapshotLoss: true }),
+    ).rejects.toThrow("worktree is busy");
+    expect(hasLiveWorktreeRunLease(env, created.id)).toBe(true);
+    expect(getRegistryWorktree(env, created.id)?.removedAt).toBeUndefined();
+    expect(await fs.stat(created.path)).toBeTruthy();
+
+    await lease.release();
+  });
+
+  it("continues after snapshot failure when snapshot loss is allowed", async () => {
+    const created = await createSessionWorktree();
+    const nested = path.join(created.path, "nested");
+    await fs.mkdir(nested);
+    await git(nested, "init", "-b", "main");
+
+    const result = await service.remove({
+      id: created.id,
+      reason: "manual-delete",
+      allowSnapshotLoss: true,
+    });
+
+    expect(result).toEqual({
+      removed: true,
+      snapshotError: expect.stringContaining(
+        "nested git repositories cannot be snapshotted losslessly",
+      ),
+    });
+    expect(getRegistryWorktree(env, created.id)?.removedAt).toBe(now);
+    await expect(fs.stat(created.path)).rejects.toMatchObject({ code: "ENOENT" });
+  });
+
   it("rejects a concurrent second remover while the first holds the claim", async () => {
     const created = await createSessionWorktree();
     const first = service.remove({ id: created.id, reason: "manual-delete" });

src/agents/worktrees/service.run-end-cleanup.test.ts

@@ -106,7 +106,6 @@ describe("ManagedWorktreeService run-end cleanup outcomes", () => {
       claimWorktreeRemoval(env, {
         worktreeId: staleRecord.id,
         token: "late-remover",
-        force: false,
       });
     } catch (error) {
       contention = error;

src/agents/worktrees/service.ts

@@ -75,7 +75,7 @@ const WORKTREE_OWNER_LEASE_SCOPE = "core:managed-worktrees:owner";
 const WORKTREE_CREATE_LEASE_MS = 60_000;
 const WORKTREE_CREATE_LEASE_WAIT_MS = 5 * 60_000;
 
-/** Non-forced removal aborted because the safety snapshot failed. */
+/** Removal aborted because snapshot loss was not permitted. */
 export class WorktreeSnapshotError extends Error {
   readonly snapshotError: string;
   constructor(snapshotError: string, options?: ErrorOptions) {
@@ -925,21 +925,20 @@ export class ManagedWorktreeService {
   async remove(params: {
     id: string;
     reason: string;
-    force?: boolean;
+    allowSnapshotLoss?: boolean;
     claimToken?: string;
     runEndCleanup?: ManagedWorktreeRunEndCleanup;
   }): Promise<RemoveManagedWorktreeResult> {
     const record = this.requireLiveRecord(params.id);
-    const force = params.force ?? false;
     // Claim removal before any cleanliness or snapshot work so a live run lease
     // rejects it and an admitted run cannot start once the claim is held. The
     // opaque token makes the claim exclusive against competing removers; a caller
     // that already claimed (removeIfLossless) passes its token to keep one claim.
     const claimToken = params.claimToken ?? randomUUID();
-    claimWorktreeRemoval(this.env, { worktreeId: record.id, token: claimToken, force });
+    claimWorktreeRemoval(this.env, { worktreeId: record.id, token: claimToken });
     try {
       const state = await lockState(record);
-      if ((state.kind === "live" || state.kind === "foreign") && !force) {
+      if (state.kind === "live" || state.kind === "foreign") {
         throw new Error(
           state.kind === "live"
             ? `worktree is locked by live OpenClaw pid ${state.pid}`
@@ -977,7 +976,7 @@ export class ManagedWorktreeService {
             { cause: cleanupError },
           );
         }
-        if (!force) {
+        if (!params.allowSnapshotLoss) {
           throw new WorktreeSnapshotError(snapshotError, { cause: error });
         }
       }
@@ -1105,7 +1104,7 @@ export class ManagedWorktreeService {
     // Run-end cleanup must leave a durable outcome even when safety retains the checkout.
     // QA and operators observe this product-boundary fact through worktrees.list.
     try {
-      claimWorktreeRemoval(this.env, { worktreeId: id, token: claimToken, force: false });
+      claimWorktreeRemoval(this.env, { worktreeId: id, token: claimToken });
     } catch (error) {
       if (error instanceof WorktreeRemovalContentionError) {
         if (error.kind === "finalized") {

src/cli/worktrees-cli.test.ts

@@ -11,6 +11,23 @@ afterEach(() => {
 });
 
 describe("worktrees cli", () => {
+  it("maps --force only to snapshot-loss permission", async () => {
+    const remove = vi.spyOn(managedWorktrees, "remove").mockResolvedValue({ removed: true });
+    vi.spyOn(defaultRuntime, "log").mockImplementation(() => undefined);
+    const program = new Command().name("openclaw");
+    registerWorktreesCli(program);
+
+    await program.parseAsync(["worktrees", "remove", "worktree-id", "--force"], {
+      from: "user",
+    });
+
+    expect(remove).toHaveBeenCalledWith({
+      id: "worktree-id",
+      reason: "manual-delete",
+      allowSnapshotLoss: true,
+    });
+  });
+
   it("passes session owner activity and built-in limits to gc", async () => {
     setRuntimeConfigSnapshot({}, {});
     const gc = vi.spyOn(managedWorktrees, "gc").mockResolvedValue({

src/cli/worktrees-cli.ts

@@ -90,7 +90,7 @@ export function registerWorktreesCli(program: Command): void {
       const result = await managedWorktrees.remove({
         id,
         reason: "manual-delete",
-        force: opts.force,
+        allowSnapshotLoss: opts.force,
       });
       if (opts.json) {
         printJson(result);

src/commands/agent.worktree-race.test.ts

@@ -173,7 +173,11 @@ describe("agent command worktree admission", () => {
         branch: created.branch,
         repoRoot: created.repoRoot,
       });
-      await managedWorktrees.remove({ id: created.id, reason: "manual-delete", force: true });
+      await managedWorktrees.remove({
+        id: created.id,
+        reason: "manual-delete",
+        allowSnapshotLoss: true,
+      });
       expect(getRegistryWorktree(process.env, created.id)?.removedAt).toBeDefined();
 
       let preparationResult: string;

src/gateway/server-methods/sessions-create.ts

@@ -478,7 +478,7 @@ export const sessionCreateHandlers: GatewayRequestHandlers = {
                     await managedWorktrees.remove({
                       id: preparedWorktree.id,
                       reason: "session-create-failed",
-                      force: true,
+                      allowSnapshotLoss: true,
                     });
                   },
                 }

src/gateway/server-methods/worktrees.test.ts

@@ -116,7 +116,7 @@ describe("worktrees gateway methods", () => {
     expect(service.remove).toHaveBeenCalledWith({
       id: record.id,
       reason: "manual-delete",
-      force: true,
+      allowSnapshotLoss: true,
     });
   });
 

src/gateway/server-methods/worktrees.ts

@@ -100,7 +100,7 @@ export function createWorktreesHandlers(service: WorktreeService): GatewayReques
         const result = await service.remove({
           id: params.id,
           reason: "manual-delete",
-          force: params.force,
+          allowSnapshotLoss: params.force,
         });
         respond(
           true,

src/gateway/server.sessions.create.projects.test.ts

@@ -169,7 +169,11 @@ test("sessions.create provisions a managed worktree from a registered project at
     );
   } finally {
     if (worktreeId) {
-      await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
+      await managedWorktrees.remove({
+        id: worktreeId,
+        reason: "test-cleanup",
+        allowSnapshotLoss: true,
+      });
     }
   }
 });

src/gateway/server.sessions.create.test.ts

@@ -1431,7 +1431,7 @@ test("sessions.create rolls back failed provisioning before a same-key creator p
       await managedWorktrees.remove({
         id: successorWorktreeId,
         reason: "test-cleanup",
-        force: true,
+        allowSnapshotLoss: true,
       });
     }
     closeOpenClawStateDatabaseForTest();
@@ -1525,7 +1525,11 @@ test("sessions.create provisions and reuses a session worktree for later runs",
   } finally {
     createSpy.mockRestore();
     if (worktreeId) {
-      await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
+      await managedWorktrees.remove({
+        id: worktreeId,
+        reason: "test-cleanup",
+        allowSnapshotLoss: true,
+      });
     }
     closeOpenClawStateDatabaseForTest();
     testState.agentConfig = undefined;
@@ -1664,7 +1668,11 @@ test("sessions.create runs an existing managed worktree cwd for initial and foll
   } finally {
     ws.close();
     getAcpSessionManager.mockRestore();
-    await managedWorktrees.remove({ id: worktree.id, reason: "test-cleanup", force: true });
+    await managedWorktrees.remove({
+      id: worktree.id,
+      reason: "test-cleanup",
+      allowSnapshotLoss: true,
+    });
     closeOpenClawStateDatabaseForTest();
     testState.agentsConfig = undefined;
     await openClawState.cleanup();
@@ -1727,7 +1735,11 @@ test("sessions.create preserves a committed worktree when initial-turn setup fai
     worktreeId = owned?.id;
   } finally {
     if (worktreeId) {
-      await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
+      await managedWorktrees.remove({
+        id: worktreeId,
+        reason: "test-cleanup",
+        allowSnapshotLoss: true,
+      });
     }
     closeOpenClawStateDatabaseForTest();
     testState.agentConfig = undefined;
@@ -1903,7 +1915,11 @@ test.each([
       expect(dashboardTitleGenerationMocks.generate).toHaveBeenCalledOnce();
     } finally {
       if (worktreeId) {
-        await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
+        await managedWorktrees.remove({
+          id: worktreeId,
+          reason: "test-cleanup",
+          allowSnapshotLoss: true,
+        });
       }
       setActivePluginRegistry(createEmptyPluginRegistry());
       closeOpenClawStateDatabaseForTest();
@@ -2001,7 +2017,11 @@ test.each([
       expect(dashboardTitleGenerationMocks.generate).toHaveBeenCalledOnce();
     } finally {
       if (worktreeId) {
-        await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
+        await managedWorktrees.remove({
+          id: worktreeId,
+          reason: "test-cleanup",
+          allowSnapshotLoss: true,
+        });
       }
       closeOpenClawStateDatabaseForTest();
       testState.agentConfig = undefined;
@@ -2037,7 +2057,11 @@ test("sessions.create keeps the crustacean fallback when no title source exists"
     expect(dashboardTitleGenerationMocks.generate).not.toHaveBeenCalled();
   } finally {
     if (worktreeId) {
-      await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
+      await managedWorktrees.remove({
+        id: worktreeId,
+        reason: "test-cleanup",
+        allowSnapshotLoss: true,
+      });
     }
     closeOpenClawStateDatabaseForTest();
     testState.agentConfig = undefined;
@@ -2555,7 +2579,11 @@ test("sessions.create skips the worktree setup script for non-admin callers", as
     await expect(fs.stat(path.join(worktree, "setup-marker.txt"))).rejects.toThrow();
   } finally {
     if (worktreeId) {
-      await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
+      await managedWorktrees.remove({
+        id: worktreeId,
+        reason: "test-cleanup",
+        allowSnapshotLoss: true,
+      });
     }
     closeOpenClawStateDatabaseForTest();
     testState.agentConfig = undefined;
@@ -2699,7 +2727,11 @@ test("sessions.create reset-in-place detaches the prior worktree permission boun
     releaseWorktreeRemoval();
     restoreRemoveIfLossless();
     if (worktreeId && getRegistryWorktree(process.env, worktreeId)?.removedAt === undefined) {
-      await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
+      await managedWorktrees.remove({
+        id: worktreeId,
+        reason: "test-cleanup",
+        allowSnapshotLoss: true,
+      });
     }
     closeOpenClawStateDatabaseForTest();
     testState.agentConfig = undefined;

src/gateway/server.sessions.delete-worktree-lifecycle.test.ts

@@ -127,7 +127,11 @@ test("sessions.delete snapshots and removes session worktrees", async () => {
       dirtyWorktreeId &&
       getRegistryWorktree(process.env, dirtyWorktreeId)?.removedAt === undefined
     ) {
-      await managedWorktrees.remove({ id: dirtyWorktreeId, reason: "test-cleanup", force: true });
+      await managedWorktrees.remove({
+        id: dirtyWorktreeId,
+        reason: "test-cleanup",
+        allowSnapshotLoss: true,
+      });
     }
     closeOpenClawStateDatabaseForTest();
     testState.agentConfig = undefined;
@@ -257,7 +261,7 @@ test("sessions.delete keeps same-key successor worktree creation behind exact cl
       await managedWorktrees.remove({
         id: successorWorktreeId,
         reason: "test-cleanup",
-        force: true,
+        allowSnapshotLoss: true,
       });
     }
     closeOpenClawStateDatabaseForTest();
@@ -316,7 +320,11 @@ test("sessions.delete reports the exact preserved worktree when cleanup fails",
   } finally {
     removeSpy.mockRestore();
     if (worktreeId && getRegistryWorktree(process.env, worktreeId)?.removedAt === undefined) {
-      await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
+      await managedWorktrees.remove({
+        id: worktreeId,
+        reason: "test-cleanup",
+        allowSnapshotLoss: true,
+      });
     }
     closeOpenClawStateDatabaseForTest();
     testState.agentConfig = undefined;
@@ -383,7 +391,7 @@ test("sessions.delete preserves an entry-bound worktree owned by another princip
       await managedWorktrees.remove({
         id: foreignWorktree.id,
         reason: "test-cleanup",
-        force: true,
+        allowSnapshotLoss: true,
       });
     }
     closeOpenClawStateDatabaseForTest();