fix(sessions): warn when reset retains worktree (#126771)

This commit is contained in:
Peter Steinberger
2026-08-20 11:04:24 -07:00
committed by GitHub
parent a2051c9bb2
commit b439c2f588
2 changed files with 124 additions and 1 deletions
+111
View File
@@ -46,6 +46,7 @@ import {
attachGatewayLocalUserIngress,
prepareGatewayLocalUserIngress,
} from "./local-user-ingress.js";
import { sessionLog } from "./server-methods/sessions-shared.js";
import { listSessionGroups } from "./session-groups.js";
import {
resolveSessionMutationAuthorization,
@@ -2595,6 +2596,116 @@ test("sessions.create skips the worktree setup script for non-admin callers", as
}
});
test.each([
{ name: "a dirty checkout", outcome: "dirty" },
{ name: "a concurrently finalized checkout", outcome: "finalized" },
{ name: "successful cleanup", outcome: "removed" },
{ name: "a cleanup exception", outcome: "failed" },
] as const)(
"sessions.create reset-in-place reports cleanup truth for $name",
async ({ outcome }) => {
const openClawState = await createOpenClawTestState({
layout: "state-only",
prefix: "openclaw-reset-retained-worktree-",
});
const root = openClawState.root;
const workspace = await initializeGitWorkspace(root);
const origin = path.join(root, "origin.git");
await execFileAsync("git", ["init", "--bare", origin]);
await execFileAsync("git", ["-C", workspace, "remote", "add", "origin", origin]);
await execFileAsync("git", ["-C", workspace, "push", "-u", "origin", "main"]);
closeOpenClawStateDatabaseForTest();
testState.agentConfig = { workspace };
testState.sessionConfig = { dmScope: "main" };
const { storePath } = await createSessionStoreDir();
await writeSessionStore({ entries: { main: sessionStoreEntry("sess-retained-parent") } });
const warnSpy = vi.spyOn(sessionLog, "warn").mockImplementation(() => {});
const originalRemoveIfLossless = managedWorktrees.removeIfLossless.bind(managedWorktrees);
let restoreRemoveIfLossless = () => {};
let worktreeId: string | undefined;
try {
const created = await directSessionReq<{
worktree: { id: string; path: string; branch: string };
}>(
"sessions.create",
{ agentId: "main", parentSessionKey: "main", emitCommandHooks: true, worktree: true },
{ client: { connect: { scopes: ["operator.admin"] } } as never },
);
expect(created.ok).toBe(true);
const worktree = created.payload!.worktree;
worktreeId = worktree.id;
const dirtyFile = path.join(worktree.path, "retained-work.txt");
if (outcome === "dirty") {
await fs.writeFile(dirtyFile, "preserve my work\n");
} else if (outcome === "finalized" || outcome === "failed") {
const removeSpy = vi
.spyOn(managedWorktrees, "removeIfLossless")
.mockImplementation(async (id) => {
if (outcome === "failed") {
throw new Error("simulated cleanup failure");
}
await originalRemoveIfLossless(id);
return false;
});
restoreRemoveIfLossless = () => removeSpy.mockRestore();
}
const reset = await directSessionReq<{
entry: { spawnedCwd?: string; sessionRoot?: string; worktree?: unknown };
}>(
"sessions.create",
{ agentId: "main", parentSessionKey: "main", emitCommandHooks: true },
{ client: { connect: { scopes: ["operator.write"] } } as never },
);
expect(reset.ok).toBe(true);
expect(reset.payload).not.toHaveProperty("worktreePreserved");
expect(reset.payload?.entry.spawnedCwd).toBeUndefined();
expect(reset.payload?.entry.sessionRoot).toBeUndefined();
expect(reset.payload?.entry.worktree).toBeUndefined();
expect(
loadSessionEntry({ sessionKey: "agent:main:main", storePath })?.worktree,
).toBeUndefined();
if (outcome === "dirty") {
expect(getRegistryWorktree(process.env, worktree.id)).toMatchObject({
runEndCleanup: { outcome: "retained-dirty" },
});
await expect(fs.readFile(dirtyFile, "utf8")).resolves.toBe("preserve my work\n");
expect(warnSpy).toHaveBeenCalledOnce();
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining(worktree.branch));
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining(worktree.path));
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("retained-dirty"));
} else if (outcome === "failed") {
expect(getRegistryWorktree(process.env, worktree.id)?.removedAt).toBeUndefined();
await expect(fs.access(worktree.path)).resolves.toBeUndefined();
expect(warnSpy).toHaveBeenCalledExactlyOnceWith(
"failed to finalize session worktree lifecycle: simulated cleanup failure",
);
} else {
expect(getRegistryWorktree(process.env, worktree.id)?.removedAt).toEqual(
expect.any(Number),
);
await expect(fs.access(worktree.path)).rejects.toThrow();
expect(warnSpy).not.toHaveBeenCalled();
}
} finally {
restoreRemoveIfLossless();
warnSpy.mockRestore();
if (worktreeId && getRegistryWorktree(process.env, worktreeId)?.removedAt === undefined) {
await managedWorktrees.remove({
id: worktreeId,
reason: "test-cleanup",
allowSnapshotLoss: true,
});
}
closeOpenClawStateDatabaseForTest();
testState.agentConfig = undefined;
testState.sessionConfig = undefined;
await openClawState.cleanup();
}
},
);
test("sessions.create reset-in-place detaches the prior worktree permission boundary", async () => {
const openClawState = await createOpenClawTestState({
layout: "state-only",
+13 -1
View File
@@ -3,7 +3,9 @@
import { randomUUID } from "node:crypto";
import { cleanupSessionResources } from "@openclaw/ai/internal/runtime";
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice";
import { ErrorCodes, errorShape } from "../../packages/gateway-protocol/src/index.js";
import { sanitizeForLog } from "../../packages/terminal-core/src/ansi.js";
import { getAcpSessionManager } from "../acp/control-plane/manager.js";
import { tryPrepareFreshManagerRuntimeSession } from "../acp/control-plane/manager.runtime-resume-state.js";
import { getAcpRuntimeBackend } from "../acp/runtime/registry.js";
@@ -1742,7 +1744,17 @@ export async function performGatewaySessionReset(params: {
// Preserve reset notifications and unbinding order, but finalize the exact
// old checkout before the fence opens to same-key successors.
try {
await managedWorktrees.removeIfLossless(detachedWorktreeId);
if (!(await managedWorktrees.removeIfLossless(detachedWorktreeId))) {
const retained = managedWorktrees.findLiveById(detachedWorktreeId);
if (retained) {
const safePath = truncateUtf16Safe(sanitizeForLog(retained.path), 256);
reportLifecycleCleanupError(
new Error(
`worktree retained: branch=${retained.branch} path=${safePath} outcome=${retained.runEndCleanup?.outcome}`,
),
);
}
}
} catch (error) {
reportLifecycleCleanupError(error);
}