mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-19 17:11:42 -06:00
fix(gateway): fence session worktree lifecycle (#121666)
Same-key replacement sessions no longer retain a managed worktree that predecessor deletion can remove.
This commit is contained in:
committed by
GitHub
parent
9935ca3b30
commit
59d545d81f
@@ -770,6 +770,11 @@ export class ManagedWorktreeService {
|
||||
return findLiveRegistryWorktreeByOwner(this.env, ownerKind, ownerId);
|
||||
}
|
||||
|
||||
findLiveById(id: string): ManagedWorktreeRecord | undefined {
|
||||
const record = getRegistryWorktree(this.env, id);
|
||||
return record?.removedAt === undefined ? record : undefined;
|
||||
}
|
||||
|
||||
/** Resolves the canonical registry root and the caller's own checkout root. */
|
||||
async resolveRepositoryPaths(repoRoot: string): Promise<{
|
||||
canonicalRoot: string;
|
||||
|
||||
@@ -3,6 +3,7 @@ import { randomUUID } from "node:crypto";
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { expectDefined } from "@openclaw/normalization-core";
|
||||
import { err, ok as resultOk } from "@openclaw/normalization-core/result";
|
||||
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
|
||||
import {
|
||||
ErrorCodes,
|
||||
@@ -29,6 +30,7 @@ import { resolveUserPath } from "../../utils.js";
|
||||
import { generateDashboardSessionTitle } from "../dashboard-session-title.js";
|
||||
import { ADMIN_SCOPE, authorizeOperatorScopesForRequiredScope } from "../method-scopes.js";
|
||||
import { buildDashboardSessionKey, createGatewaySession } from "../session-create-service.js";
|
||||
import type { PrepareGatewaySessionLifecycle } from "../session-lifecycle-preparation.js";
|
||||
import { resolveRequestedSessionAgentId as resolveRequestedGlobalAgentId } from "../session-request-agent.js";
|
||||
import { resolveSessionStoreAgentId } from "../session-store-key.js";
|
||||
import { readSessionMessageCountAsync } from "../session-transcript-readers.js";
|
||||
@@ -202,8 +204,7 @@ export const sessionCreateHandlers: GatewayRequestHandlers = {
|
||||
let sessionWorktree: Awaited<ReturnType<typeof managedWorktrees.create>> | undefined;
|
||||
const sessionExecCwd = requestedExecNode ? requestedCwd : undefined;
|
||||
let sessionCwd = requestedExecNode ? undefined : requestedCwd;
|
||||
let sessionSourceRoot: string | undefined;
|
||||
let provisionedSessionWorktree = false;
|
||||
let prepareLifecycle: PrepareGatewaySessionLifecycle | undefined;
|
||||
let generatedDisplayName: string | undefined;
|
||||
if (requestedCwd && !requestedExecNode && p.worktree !== true) {
|
||||
const targetAgentId = normalizeAgentId(
|
||||
@@ -292,107 +293,9 @@ export const sessionCreateHandlers: GatewayRequestHandlers = {
|
||||
);
|
||||
return;
|
||||
}
|
||||
let requestedRepository: Awaited<ReturnType<typeof managedWorktrees.resolveRepositoryPaths>>;
|
||||
try {
|
||||
const requestedRepository = await managedWorktrees.resolveRepositoryPaths(workspace);
|
||||
sessionSourceRoot = requestedRepository.sourceRoot;
|
||||
const existing = managedWorktrees.findLiveByOwner("session", target.canonicalKey);
|
||||
let existingDirectory = false;
|
||||
if (existing) {
|
||||
try {
|
||||
existingDirectory = fs.lstatSync(existing.path).isDirectory();
|
||||
} catch {
|
||||
// Missing registry targets are replaced; periodic GC retires their stale rows.
|
||||
}
|
||||
}
|
||||
if (existing && existingDirectory) {
|
||||
if (existing.repoRoot !== requestedRepository.canonicalRoot) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
"session worktree belongs to a different repository",
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
// Adopting an existing checkout cannot honor a different name or a
|
||||
// new base; fail loudly instead of silently ignoring the request.
|
||||
if (
|
||||
(requestedWorktreeName && existing.name !== requestedWorktreeName) ||
|
||||
requestedWorktreeBaseRef
|
||||
) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
`session is already bound to worktree ${existing.name} (${existing.branch})`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
sessionWorktree = existing;
|
||||
} else {
|
||||
const scopes = Array.isArray(client?.connect.scopes) ? client.connect.scopes : [];
|
||||
if (
|
||||
!requestedWorktreeName &&
|
||||
!normalizeOptionalString(p.label) &&
|
||||
(initialMessage || initialAttachments)
|
||||
) {
|
||||
try {
|
||||
const requestedTitleModel =
|
||||
catalogTarget?.target.model ?? normalizeOptionalString(p.model);
|
||||
let titleModelEntry:
|
||||
| Pick<SessionEntry, "authProfileOverride" | "modelOverride" | "providerOverride">
|
||||
| undefined;
|
||||
if (requestedTitleModel) {
|
||||
const defaultModel = resolveDefaultModelForAgent({
|
||||
cfg,
|
||||
agentId: target.agentId,
|
||||
});
|
||||
const selection = resolveSessionPatchModelSelection({
|
||||
cfg,
|
||||
catalog: await context.loadGatewayModelCatalog({ agentId: target.agentId }),
|
||||
raw: requestedTitleModel,
|
||||
defaultProvider: defaultModel.provider,
|
||||
defaultModel: defaultModel.model,
|
||||
});
|
||||
if (selection.ok) {
|
||||
titleModelEntry = {
|
||||
providerOverride: selection.provider,
|
||||
modelOverride: selection.model,
|
||||
...(selection.profile ? { authProfileOverride: selection.profile } : {}),
|
||||
};
|
||||
}
|
||||
}
|
||||
generatedDisplayName =
|
||||
(await generateDashboardSessionTitle({
|
||||
cfg,
|
||||
agentId: target.agentId,
|
||||
entry: titleModelEntry,
|
||||
userMessage: initialMessage ?? "",
|
||||
attachments: initialAttachments,
|
||||
})) ?? undefined;
|
||||
} catch (error) {
|
||||
sessionLog.warn(`worktree title generation failed: ${formatErrorMessage(error)}`);
|
||||
}
|
||||
}
|
||||
sessionWorktree = await managedWorktrees.create({
|
||||
repoRoot: workspace,
|
||||
ownerKind: "session",
|
||||
ownerId: target.canonicalKey,
|
||||
name: requestedWorktreeName,
|
||||
suggestedName: slugifyWorktreeTitle(
|
||||
normalizeOptionalString(p.label) ?? generatedDisplayName ?? "",
|
||||
),
|
||||
baseRef: requestedWorktreeBaseRef,
|
||||
// Checkout hooks and .openclaw/worktree-setup.sh run repo code; keep them
|
||||
// admin-only so this write-scoped path cannot execute gated repo scripts.
|
||||
runSetupScript: scopes.includes(ADMIN_SCOPE),
|
||||
});
|
||||
provisionedSessionWorktree = true;
|
||||
}
|
||||
requestedRepository = await managedWorktrees.resolveRepositoryPaths(workspace);
|
||||
} catch (error) {
|
||||
if (error instanceof WorktreeRepositoryError) {
|
||||
respond(
|
||||
@@ -405,22 +308,151 @@ export const sessionCreateHandlers: GatewayRequestHandlers = {
|
||||
respond(false, undefined, errorShape(ErrorCodes.UNAVAILABLE, formatErrorMessage(error)));
|
||||
return;
|
||||
}
|
||||
// Nested workspaces run from the matching subdirectory inside the worktree, mirroring
|
||||
// how the session would have run in the source checkout; the worktree root would
|
||||
// silently change tool/file scope for subdirectory-configured agents.
|
||||
sessionCwd = sessionWorktree.path;
|
||||
try {
|
||||
const relative = path.relative(
|
||||
sessionSourceRoot ?? fs.realpathSync(sessionWorktree.repoRoot),
|
||||
fs.realpathSync(workspace),
|
||||
);
|
||||
if (relative && !relative.startsWith("..") && !path.isAbsolute(relative)) {
|
||||
sessionCwd = path.join(sessionWorktree.path, relative);
|
||||
fs.mkdirSync(sessionCwd, { recursive: true });
|
||||
|
||||
if (
|
||||
!requestedWorktreeName &&
|
||||
!normalizeOptionalString(p.label) &&
|
||||
(initialMessage || initialAttachments)
|
||||
) {
|
||||
try {
|
||||
const requestedTitleModel =
|
||||
catalogTarget?.target.model ?? normalizeOptionalString(p.model);
|
||||
let titleModelEntry:
|
||||
| Pick<SessionEntry, "authProfileOverride" | "modelOverride" | "providerOverride">
|
||||
| undefined;
|
||||
if (requestedTitleModel) {
|
||||
const defaultModel = resolveDefaultModelForAgent({ cfg, agentId: target.agentId });
|
||||
const selection = resolveSessionPatchModelSelection({
|
||||
cfg,
|
||||
catalog: await context.loadGatewayModelCatalog({ agentId: target.agentId }),
|
||||
raw: requestedTitleModel,
|
||||
defaultProvider: defaultModel.provider,
|
||||
defaultModel: defaultModel.model,
|
||||
});
|
||||
if (selection.ok) {
|
||||
titleModelEntry = {
|
||||
providerOverride: selection.provider,
|
||||
modelOverride: selection.model,
|
||||
...(selection.profile ? { authProfileOverride: selection.profile } : {}),
|
||||
};
|
||||
}
|
||||
}
|
||||
generatedDisplayName =
|
||||
(await generateDashboardSessionTitle({
|
||||
cfg,
|
||||
agentId: target.agentId,
|
||||
entry: titleModelEntry,
|
||||
userMessage: initialMessage ?? "",
|
||||
attachments: initialAttachments,
|
||||
})) ?? undefined;
|
||||
} catch (error) {
|
||||
sessionLog.warn(`worktree title generation failed: ${formatErrorMessage(error)}`);
|
||||
}
|
||||
} catch {
|
||||
sessionCwd = sessionWorktree.path;
|
||||
}
|
||||
|
||||
const scopes = Array.isArray(client?.connect.scopes) ? client.connect.scopes : [];
|
||||
prepareLifecycle = async (lifecycleTarget) => {
|
||||
try {
|
||||
const boundId = normalizeOptionalString(lifecycleTarget.entry?.worktree?.id);
|
||||
let existing = boundId ? managedWorktrees.findLiveById(boundId) : undefined;
|
||||
if (
|
||||
existing &&
|
||||
(existing.ownerKind !== "session" || existing.ownerId !== lifecycleTarget.key)
|
||||
) {
|
||||
return err(
|
||||
errorShape(ErrorCodes.UNAVAILABLE, "session worktree binding has a different owner"),
|
||||
);
|
||||
}
|
||||
existing ??= managedWorktrees.findLiveByOwner("session", lifecycleTarget.key);
|
||||
let existingDirectory = false;
|
||||
if (existing) {
|
||||
try {
|
||||
existingDirectory = fs.lstatSync(existing.path).isDirectory();
|
||||
} catch {
|
||||
// Missing registry targets are replaced by create() under its owner lease.
|
||||
}
|
||||
}
|
||||
let provisioned = false;
|
||||
if (existing && existingDirectory) {
|
||||
if (existing.repoRoot !== requestedRepository.canonicalRoot) {
|
||||
return err(
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
"session worktree belongs to a different repository",
|
||||
),
|
||||
);
|
||||
}
|
||||
if (
|
||||
(requestedWorktreeName && existing.name !== requestedWorktreeName) ||
|
||||
requestedWorktreeBaseRef
|
||||
) {
|
||||
return err(
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
`session is already bound to worktree ${existing.name} (${existing.branch})`,
|
||||
),
|
||||
);
|
||||
}
|
||||
sessionWorktree = existing;
|
||||
} else {
|
||||
sessionWorktree = await managedWorktrees.create({
|
||||
repoRoot: workspace,
|
||||
ownerKind: "session",
|
||||
ownerId: lifecycleTarget.key,
|
||||
name: requestedWorktreeName,
|
||||
suggestedName: slugifyWorktreeTitle(
|
||||
normalizeOptionalString(p.label) ?? generatedDisplayName ?? "",
|
||||
),
|
||||
baseRef: requestedWorktreeBaseRef,
|
||||
// Checkout hooks and .openclaw/worktree-setup.sh run repo code; keep them
|
||||
// admin-only so this write-scoped path cannot execute gated repo scripts.
|
||||
runSetupScript: scopes.includes(ADMIN_SCOPE),
|
||||
});
|
||||
provisioned = true;
|
||||
}
|
||||
// Nested workspaces run from the matching subdirectory inside the worktree.
|
||||
sessionCwd = sessionWorktree.path;
|
||||
try {
|
||||
const relative = path.relative(
|
||||
requestedRepository.sourceRoot,
|
||||
fs.realpathSync(workspace),
|
||||
);
|
||||
if (relative && !relative.startsWith("..") && !path.isAbsolute(relative)) {
|
||||
sessionCwd = path.join(sessionWorktree.path, relative);
|
||||
fs.mkdirSync(sessionCwd, { recursive: true });
|
||||
}
|
||||
} catch {
|
||||
sessionCwd = sessionWorktree.path;
|
||||
}
|
||||
const preparedWorktree = sessionWorktree;
|
||||
return resultOk({
|
||||
spawnedCwd: sessionCwd,
|
||||
worktree: {
|
||||
id: preparedWorktree.id,
|
||||
branch: preparedWorktree.branch,
|
||||
repoRoot: preparedWorktree.repoRoot,
|
||||
},
|
||||
...(provisioned
|
||||
? {
|
||||
rollback: async () => {
|
||||
await managedWorktrees.remove({
|
||||
id: preparedWorktree.id,
|
||||
reason: "session-create-failed",
|
||||
force: true,
|
||||
});
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
} catch (error) {
|
||||
if (error instanceof WorktreeRepositoryError) {
|
||||
return err(
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "agent workspace is not a git checkout"),
|
||||
);
|
||||
}
|
||||
return err(errorShape(ErrorCodes.UNAVAILABLE, formatErrorMessage(error)));
|
||||
}
|
||||
};
|
||||
}
|
||||
let runPayload: Record<string, unknown> | undefined;
|
||||
let runError: unknown;
|
||||
@@ -498,19 +530,18 @@ export const sessionCreateHandlers: GatewayRequestHandlers = {
|
||||
: {}),
|
||||
}
|
||||
: undefined,
|
||||
spawnedCwd: sessionCwd,
|
||||
worktree: sessionWorktree
|
||||
? {
|
||||
id: sessionWorktree.id,
|
||||
branch: sessionWorktree.branch,
|
||||
repoRoot: sessionWorktree.repoRoot,
|
||||
}
|
||||
: undefined,
|
||||
spawnedCwd: p.worktree === true ? undefined : sessionCwd,
|
||||
prepareLifecycle,
|
||||
onLifecycleCleanupError: (error) => {
|
||||
sessionLog.warn(
|
||||
`failed to finalize session worktree lifecycle: ${formatErrorMessage(error)}`,
|
||||
);
|
||||
},
|
||||
execNode: requestedExecNode,
|
||||
execCwd: sessionExecCwd,
|
||||
clearExecBinding: !requestedExecNode,
|
||||
// A plain New Chat with no cwd must not inherit the prior session cwd.
|
||||
clearSpawnedCwd: !sessionCwd,
|
||||
clearSpawnedCwd: p.worktree !== true && !sessionCwd,
|
||||
fork: p.fork,
|
||||
succeedsParent: p.succeedsParent,
|
||||
emitCommandHooks: p.emitCommandHooks,
|
||||
@@ -559,36 +590,9 @@ export const sessionCreateHandlers: GatewayRequestHandlers = {
|
||||
},
|
||||
});
|
||||
if (!created.ok) {
|
||||
if (sessionWorktree && provisionedSessionWorktree) {
|
||||
try {
|
||||
await managedWorktrees.remove({
|
||||
id: sessionWorktree.id,
|
||||
reason: "session-create-failed",
|
||||
force: true,
|
||||
});
|
||||
} catch (error) {
|
||||
sessionLog.warn(
|
||||
`failed to clean up worktree after session creation failed: ${formatErrorMessage(error)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
respond(false, undefined, created.error);
|
||||
return;
|
||||
}
|
||||
// Leaving an isolated checkout via a plain New Chat detaches the session from its
|
||||
// worktree; remove it when lossless so the reset does not orphan a protected worktree.
|
||||
if (p.worktree !== true) {
|
||||
try {
|
||||
const owned = managedWorktrees.findLiveByOwner("session", created.key);
|
||||
if (owned) {
|
||||
await managedWorktrees.removeIfLossless(owned.id);
|
||||
}
|
||||
} catch (error) {
|
||||
sessionLog.warn(
|
||||
`failed to release worktree for reset session ${created.key}: ${formatErrorMessage(error)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
if (created.resetExisting) {
|
||||
await captureCreatedSessionBaseline({
|
||||
key: created.key,
|
||||
|
||||
@@ -209,6 +209,8 @@ export const sessionDeleteHandlers: GatewayRequestHandlers = {
|
||||
let deleteBlockedByWorkerPlacement = false;
|
||||
let deleteBlockedByArchiveOrOwnership = false;
|
||||
let preparedDeleteSessionId: string | undefined;
|
||||
let deletedWorktreeId: string | undefined;
|
||||
let worktreePreserved: { id: string; branch: string; path: string } | undefined;
|
||||
const deletion = await runExclusiveSessionLifecycleMutation({
|
||||
scope: storePath,
|
||||
identities: deleteLifecycleIdentities,
|
||||
@@ -381,6 +383,7 @@ export const sessionDeleteHandlers: GatewayRequestHandlers = {
|
||||
...(requestedAgentId ? { agentId: requestedAgentId } : {}),
|
||||
});
|
||||
const postCleanupEntry = postCleanupTarget.entry;
|
||||
deletedWorktreeId = normalizeOptionalString(postCleanupEntry?.worktree?.id);
|
||||
sessionMutationAuthorization?.assertCurrent();
|
||||
if (
|
||||
!expectedLifecycleRevisionMatches(postCleanupEntry) ||
|
||||
@@ -440,6 +443,44 @@ export const sessionDeleteHandlers: GatewayRequestHandlers = {
|
||||
reason: "session-delete",
|
||||
emitHooks: p.emitLifecycleHooks !== false,
|
||||
});
|
||||
// Hooks and unbinding retain their historical post-delete order. The
|
||||
// generation-scoped purge and checkout cleanup still finish before
|
||||
// this fence opens, so a same-key successor cannot be mistaken for it.
|
||||
const deletedSessionKey = target.canonicalKey ?? key;
|
||||
handleSessionStateSessionDeleted(
|
||||
deletedSessionKey,
|
||||
requestedAgentId ?? resolveSessionStoreAgentId(cfg, deletedSessionKey),
|
||||
);
|
||||
const deletedWorktree = deletedWorktreeId
|
||||
? managedWorktrees.findLiveById(deletedWorktreeId)
|
||||
: undefined;
|
||||
if (deletedWorktree) {
|
||||
worktreePreserved = {
|
||||
id: deletedWorktree.id,
|
||||
branch: deletedWorktree.branch,
|
||||
path: deletedWorktree.path,
|
||||
};
|
||||
if (
|
||||
deletedWorktree.ownerKind !== "session" ||
|
||||
deletedWorktree.ownerId !== deletedSessionKey
|
||||
) {
|
||||
sessionLog.warn(
|
||||
`refusing to clean up worktree ${deletedWorktree.id} for deleted session ${deletedSessionKey}: registry owner is ${deletedWorktree.ownerKind}${deletedWorktree.ownerId ? ` ${deletedWorktree.ownerId}` : ""}`,
|
||||
);
|
||||
} else {
|
||||
try {
|
||||
await managedWorktrees.remove({
|
||||
id: deletedWorktree.id,
|
||||
reason: "session-delete",
|
||||
});
|
||||
worktreePreserved = undefined;
|
||||
} catch (error) {
|
||||
sessionLog.warn(
|
||||
`failed to clean up worktree for deleted session ${deletedSessionKey}: ${formatErrorMessage(error)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
},
|
||||
@@ -451,32 +492,6 @@ export const sessionDeleteHandlers: GatewayRequestHandlers = {
|
||||
const archivedTranscripts = deletion.archivedTranscripts;
|
||||
const archived = archivedTranscripts.map((entryLocal) => entryLocal.archivedPath);
|
||||
|
||||
// Session deletion ends worktree ownership. Snapshot before removal so
|
||||
// inherited unpushed history or local edits do not leave an ownerless checkout.
|
||||
let worktreePreserved: { id: string; branch: string; path: string } | undefined;
|
||||
if (deleted) {
|
||||
// requestedAgentId wins: "global" canonical keys resolve to the default store
|
||||
// agent, which would purge the wrong agent's rows for explicit-agent deletes.
|
||||
handleSessionStateSessionDeleted(
|
||||
target.canonicalKey ?? key,
|
||||
requestedAgentId ?? resolveSessionStoreAgentId(cfg, target.canonicalKey ?? key),
|
||||
);
|
||||
let worktree: ReturnType<typeof managedWorktrees.findLiveByOwner> = undefined;
|
||||
try {
|
||||
worktree = managedWorktrees.findLiveByOwner("session", target.canonicalKey);
|
||||
if (worktree) {
|
||||
await managedWorktrees.remove({ id: worktree.id, reason: "session-delete" });
|
||||
}
|
||||
} catch (error) {
|
||||
if (worktree) {
|
||||
worktreePreserved = { id: worktree.id, branch: worktree.branch, path: worktree.path };
|
||||
}
|
||||
sessionLog.warn(
|
||||
`failed to clean up worktree for deleted session ${target.canonicalKey}: ${formatErrorMessage(error)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
respond(
|
||||
true,
|
||||
{
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { GATEWAY_CLIENT_CAPS } from "../../../packages/gateway-protocol/src/client-info.js";
|
||||
import { managedWorktrees } from "../../agents/worktrees/service.js";
|
||||
import { upsertSessionEntry } from "../../config/sessions/session-accessor.js";
|
||||
import { clearAgentRunContext, registerAgentRunContext } from "../../infra/agent-run-registry.js";
|
||||
import { closeOpenClawAgentDatabasesForTest } from "../../state/openclaw-agent-db.js";
|
||||
@@ -17,8 +16,24 @@ import { taskSuggestionsHandlers } from "./task-suggestions.js";
|
||||
import type { GatewayClient, GatewayRequestContext, RespondFn } from "./types.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({ handleChatSend: vi.fn() }));
|
||||
const sessionReadState = vi.hoisted(() => ({ mode: "normal" as "normal" | "present" | "throw" }));
|
||||
|
||||
vi.mock("./chat-send-handler.js", () => ({ handleChatSend: mocks.handleChatSend }));
|
||||
vi.mock("../session-utils.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../session-utils.js")>();
|
||||
return {
|
||||
...actual,
|
||||
loadSessionEntryReadOnly: (...args: Parameters<typeof actual.loadSessionEntryReadOnly>) => {
|
||||
if (sessionReadState.mode === "throw") {
|
||||
throw new Error("session inspection unavailable");
|
||||
}
|
||||
const loaded = actual.loadSessionEntryReadOnly(...args);
|
||||
return sessionReadState.mode === "present"
|
||||
? { ...loaded, entry: { sessionId: "surviving-session", updatedAt: 1 } }
|
||||
: loaded;
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
type Method =
|
||||
| "taskSuggestions.list"
|
||||
@@ -70,6 +85,7 @@ async function dismissPendingTaskSuggestions(): Promise<void> {
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
sessionReadState.mode = "normal";
|
||||
await dismissPendingTaskSuggestions();
|
||||
mocks.handleChatSend.mockReset();
|
||||
mocks.handleChatSend.mockImplementation(async ({ respond }: { respond: RespondFn }) => {
|
||||
@@ -122,6 +138,18 @@ async function createSourceSuggestion() {
|
||||
return (requirePayload(created) as { taskId: string }).taskId;
|
||||
}
|
||||
|
||||
async function createLocalTaskSuggestion() {
|
||||
const created = await call("taskSuggestions.create", {
|
||||
title: "Add coverage",
|
||||
prompt: "Add the missing regression test.",
|
||||
tldr: "The edge case is untested.",
|
||||
cwd: GIT_CWD,
|
||||
sessionKey: "agent:main:main",
|
||||
agentId: "main",
|
||||
});
|
||||
return (requirePayload(created) as { taskId: string }).taskId;
|
||||
}
|
||||
|
||||
describe("task suggestion gateway methods", () => {
|
||||
it("creates, lists, and resolves an ephemeral suggestion", async () => {
|
||||
const created = await call("taskSuggestions.create", {
|
||||
@@ -688,15 +716,7 @@ describe("task suggestion gateway methods", () => {
|
||||
});
|
||||
|
||||
it("rolls back an empty session and keeps a failed seed suggestion pending", async () => {
|
||||
const created = await call("taskSuggestions.create", {
|
||||
title: "Add coverage",
|
||||
prompt: "Add the missing regression test.",
|
||||
tldr: "The edge case is untested.",
|
||||
cwd: GIT_CWD,
|
||||
sessionKey: "agent:main:main",
|
||||
agentId: "main",
|
||||
});
|
||||
const taskId = (requirePayload(created) as { taskId: string }).taskId;
|
||||
const taskId = await createLocalTaskSuggestion();
|
||||
let sessionKey = "";
|
||||
vi.spyOn(sessionCreateHandlers, "sessions.create").mockImplementation(
|
||||
async ({ params, respond }) => {
|
||||
@@ -741,55 +761,71 @@ describe("task suggestion gateway methods", () => {
|
||||
expect(listed.response?.[1]).toMatchObject({ suggestions: [{ id: taskId }] });
|
||||
});
|
||||
|
||||
it("rolls back a preallocated session when creation throws after persistence", async () => {
|
||||
const created = await call("taskSuggestions.create", {
|
||||
title: "Add coverage",
|
||||
prompt: "Add the missing regression test.",
|
||||
tldr: "The edge case is untested.",
|
||||
cwd: GIT_CWD,
|
||||
sessionKey: "agent:main:main",
|
||||
agentId: "main",
|
||||
});
|
||||
const taskId = (requirePayload(created) as { taskId: string }).taskId;
|
||||
let sessionKey = "";
|
||||
vi.spyOn(sessionCreateHandlers, "sessions.create").mockImplementation(async ({ params }) => {
|
||||
sessionKey = (params as { key: string }).key;
|
||||
throw new Error("initial dispatch failed");
|
||||
});
|
||||
const deleteSession = vi
|
||||
.spyOn(sessionDeleteHandlers, "sessions.delete")
|
||||
.mockImplementation(async ({ params, respond }) => {
|
||||
expect(params).toMatchObject({ key: sessionKey, agentId: "main" });
|
||||
respond(true, { ok: true, deleted: true }, undefined);
|
||||
it.each([true, false])(
|
||||
"rolls back a preallocated session when delete reports deleted=$deleted",
|
||||
async (deleted) => {
|
||||
const taskId = await createLocalTaskSuggestion();
|
||||
let sessionKey = "";
|
||||
vi.spyOn(sessionCreateHandlers, "sessions.create").mockImplementation(async ({ params }) => {
|
||||
sessionKey = (params as { key: string }).key;
|
||||
throw new Error("initial dispatch failed");
|
||||
});
|
||||
const deleteSession = vi
|
||||
.spyOn(sessionDeleteHandlers, "sessions.delete")
|
||||
.mockImplementation(async ({ params, respond }) => {
|
||||
expect(params).toMatchObject({ key: sessionKey, agentId: "main" });
|
||||
respond(true, { ok: true, deleted }, undefined);
|
||||
});
|
||||
|
||||
const accepted = await call("taskSuggestions.accept", { taskId });
|
||||
const listed = await call("taskSuggestions.list", {});
|
||||
const accepted = await call("taskSuggestions.accept", { taskId });
|
||||
const listed = await call("taskSuggestions.list", {});
|
||||
|
||||
expect(accepted.response?.[0]).toBe(false);
|
||||
expect(accepted.response?.[2]).toMatchObject({ message: "initial dispatch failed" });
|
||||
expect(deleteSession).toHaveBeenCalledTimes(1);
|
||||
expect(listed.response?.[1]).toMatchObject({ suggestions: [{ id: taskId }] });
|
||||
});
|
||||
expect(accepted.response?.[0]).toBe(false);
|
||||
expect(accepted.response?.[2]).toMatchObject({ message: "initial dispatch failed" });
|
||||
expect(deleteSession).toHaveBeenCalledTimes(1);
|
||||
expect(listed.response?.[1]).toMatchObject({ suggestions: [{ id: taskId }] });
|
||||
},
|
||||
);
|
||||
|
||||
it("expires a suggestion when partial session rollback cannot finish", async () => {
|
||||
const created = await call("taskSuggestions.create", {
|
||||
title: "Add coverage",
|
||||
prompt: "Add the missing regression test.",
|
||||
tldr: "The edge case is untested.",
|
||||
cwd: GIT_CWD,
|
||||
sessionKey: "agent:main:main",
|
||||
agentId: "main",
|
||||
});
|
||||
const taskId = (requirePayload(created) as { taskId: string }).taskId;
|
||||
it.each([
|
||||
["delete rejects", "reject"],
|
||||
["delete throws", "throw"],
|
||||
["the session row survives", "survives"],
|
||||
["delete preserves the worktree", "preserved"],
|
||||
["session inspection throws", "inspect-throws"],
|
||||
] as const)("expires a suggestion when rollback is incomplete: %s", async (_name, failure) => {
|
||||
const taskId = await createLocalTaskSuggestion();
|
||||
vi.spyOn(sessionCreateHandlers, "sessions.create").mockRejectedValue(
|
||||
new Error("initial dispatch failed"),
|
||||
);
|
||||
sessionReadState.mode =
|
||||
failure === "survives" ? "present" : failure === "inspect-throws" ? "throw" : "normal";
|
||||
vi.spyOn(sessionDeleteHandlers, "sessions.delete").mockImplementation(async ({ respond }) => {
|
||||
respond(false, undefined, { code: "UNAVAILABLE", message: "still active" });
|
||||
if (failure === "throw") {
|
||||
throw new Error("delete handler failed");
|
||||
}
|
||||
if (failure === "reject") {
|
||||
respond(false, undefined, { code: "UNAVAILABLE", message: "still active" });
|
||||
return;
|
||||
}
|
||||
respond(
|
||||
true,
|
||||
{
|
||||
ok: true,
|
||||
deleted: true,
|
||||
...(failure === "preserved"
|
||||
? {
|
||||
worktreePreserved: {
|
||||
id: "preserved-worktree",
|
||||
path: "/preserved-worktree",
|
||||
branch: "openclaw/preserved-worktree",
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
vi.spyOn(managedWorktrees, "findLiveByOwner").mockReturnValue({ id: "wt_partial" } as never);
|
||||
vi.spyOn(managedWorktrees, "remove").mockRejectedValue(new Error("still active"));
|
||||
|
||||
const accepted = await call("taskSuggestions.accept", { taskId });
|
||||
const listed = await call("taskSuggestions.list", {});
|
||||
@@ -804,47 +840,6 @@ describe("task suggestion gateway methods", () => {
|
||||
expect(listed.response?.[1]).toEqual({ suggestions: [] });
|
||||
});
|
||||
|
||||
it("abandons an acceptance when rollback inspection throws", async () => {
|
||||
const created = await call("taskSuggestions.create", {
|
||||
title: "Add coverage",
|
||||
prompt: "Add the missing regression test.",
|
||||
tldr: "The edge case is untested.",
|
||||
cwd: GIT_CWD,
|
||||
sessionKey: "agent:main:main",
|
||||
agentId: "main",
|
||||
});
|
||||
const taskId = (requirePayload(created) as { taskId: string }).taskId;
|
||||
vi.spyOn(sessionCreateHandlers, "sessions.create").mockRejectedValue(
|
||||
new Error("initial dispatch failed"),
|
||||
);
|
||||
vi.spyOn(sessionDeleteHandlers, "sessions.delete").mockImplementation(async ({ respond }) => {
|
||||
respond(true, { ok: true, deleted: true }, undefined);
|
||||
});
|
||||
vi.spyOn(managedWorktrees, "findLiveByOwner").mockImplementation(() => {
|
||||
throw new Error("worktree registry unavailable");
|
||||
});
|
||||
const broadcast = vi.fn();
|
||||
|
||||
await expect(call("taskSuggestions.accept", { taskId }, broadcast)).rejects.toThrow(
|
||||
"worktree registry unavailable",
|
||||
);
|
||||
|
||||
expect(broadcast).toHaveBeenCalledTimes(1);
|
||||
expect(broadcast).toHaveBeenCalledWith(
|
||||
"task.suggestion",
|
||||
{ action: "resolved", taskId, resolution: "expired" },
|
||||
{ dropIfSlow: true },
|
||||
);
|
||||
const listed = await call("taskSuggestions.list", {});
|
||||
expect(listed.response?.[1]).toEqual({ suggestions: [] });
|
||||
const retry = await call("taskSuggestions.accept", { taskId });
|
||||
expect(retry.response?.[0]).toBe(false);
|
||||
expect(retry.response?.[2]).toMatchObject({
|
||||
code: "INVALID_REQUEST",
|
||||
message: "task suggestion cannot be accepted: dismissed",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects a relative cwd before recording or broadcasting", async () => {
|
||||
const result = await call("taskSuggestions.create", {
|
||||
title: "Add coverage",
|
||||
|
||||
@@ -14,7 +14,6 @@ import {
|
||||
} from "../../../packages/gateway-protocol/src/index.js";
|
||||
import { resolveDefaultAgentId } from "../../agents/agent-scope.js";
|
||||
import { insideGitCheckout } from "../../agents/worktrees/git.js";
|
||||
import { managedWorktrees } from "../../agents/worktrees/service.js";
|
||||
import { resolveSessionWorkStartError } from "../../config/sessions.js";
|
||||
import { formatErrorMessage } from "../../infra/errors.js";
|
||||
import { normalizeAgentId, parseAgentSessionKey } from "../../routing/session-key.js";
|
||||
@@ -70,9 +69,13 @@ async function rollbackSuggestedTaskSession(params: {
|
||||
agentId?: string;
|
||||
options: GatewayRequestHandlerOptions;
|
||||
}): Promise<boolean> {
|
||||
let deletionConfirmed = false;
|
||||
let deletionResponse: { ok: true; worktreePreserved: boolean } | { ok: false } | undefined;
|
||||
try {
|
||||
await sessionDeleteHandlers["sessions.delete"]?.({
|
||||
const deleteSession = sessionDeleteHandlers["sessions.delete"];
|
||||
if (!deleteSession) {
|
||||
return false;
|
||||
}
|
||||
await deleteSession({
|
||||
...params.options,
|
||||
params: {
|
||||
key: params.key,
|
||||
@@ -81,41 +84,33 @@ async function rollbackSuggestedTaskSession(params: {
|
||||
emitLifecycleHooks: false,
|
||||
},
|
||||
respond: (ok, payload) => {
|
||||
deletionConfirmed = Boolean(
|
||||
ok &&
|
||||
payload &&
|
||||
typeof payload === "object" &&
|
||||
typeof (payload as { deleted?: unknown }).deleted === "boolean",
|
||||
);
|
||||
if (
|
||||
!ok ||
|
||||
!payload ||
|
||||
typeof payload !== "object" ||
|
||||
typeof (payload as { deleted?: unknown }).deleted !== "boolean"
|
||||
) {
|
||||
deletionResponse = { ok: false };
|
||||
return;
|
||||
}
|
||||
deletionResponse = {
|
||||
ok: true,
|
||||
worktreePreserved:
|
||||
(payload as { worktreePreserved?: unknown }).worktreePreserved !== undefined,
|
||||
};
|
||||
},
|
||||
});
|
||||
} catch {
|
||||
// The state probes below determine whether the preallocated session key
|
||||
// and its worktree were fully removed despite a handler-level failure.
|
||||
return false;
|
||||
}
|
||||
if (!deletionResponse?.ok || deletionResponse.worktreePreserved) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
if (
|
||||
!deletionConfirmed &&
|
||||
loadSessionEntryReadOnly(params.key, { agentId: params.agentId }).entry
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
return !loadSessionEntryReadOnly(params.key, { agentId: params.agentId }).entry;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
const worktree = managedWorktrees.findLiveByOwner("session", params.key);
|
||||
if (worktree) {
|
||||
try {
|
||||
await managedWorktrees.remove({
|
||||
id: worktree.id,
|
||||
reason: "suggested-task-seed-failed",
|
||||
force: true,
|
||||
});
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return managedWorktrees.findLiveByOwner("session", params.key) === undefined;
|
||||
}
|
||||
|
||||
async function failSuggestedTaskSession(params: {
|
||||
|
||||
@@ -10,6 +10,7 @@ import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
|
||||
import { findGitCheckoutRoot } from "../agents/worktrees/git.js";
|
||||
import {
|
||||
findLiveRegistryWorktreeByOwner,
|
||||
getRegistryWorktree,
|
||||
listRegistryWorktrees,
|
||||
} from "../agents/worktrees/registry.js";
|
||||
import { managedWorktrees } from "../agents/worktrees/service.js";
|
||||
@@ -26,6 +27,7 @@ import { resolveSqliteTargetFromSessionStorePath } from "../config/sessions/sess
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { createEmptyPluginRegistry } from "../plugins/registry-empty.js";
|
||||
import { setActivePluginRegistry } from "../plugins/runtime.js";
|
||||
import { isSessionLifecycleMutationActive } from "../sessions/session-lifecycle-admission.js";
|
||||
import { listSessionStateEventsSince } from "../sessions/session-state-events.js";
|
||||
import {
|
||||
closeOpenClawAgentDatabasesForTest,
|
||||
@@ -55,6 +57,7 @@ import {
|
||||
sessionHookMocks,
|
||||
sessionLifecycleHookMocks,
|
||||
seedSessionTranscript,
|
||||
threadBindingMocks,
|
||||
} from "./test/server-sessions.test-helpers.js";
|
||||
|
||||
type EnsureSessionDiffBaseline =
|
||||
@@ -893,6 +896,162 @@ test("sessions.create rejects draft visibility when policy disables drafts", asy
|
||||
});
|
||||
});
|
||||
|
||||
test("sessions.create provisions its worktree inside the target lifecycle fence", async () => {
|
||||
const openClawState = await createOpenClawTestState({
|
||||
layout: "state-only",
|
||||
prefix: "openclaw-session-worktree-fence-",
|
||||
});
|
||||
const workspace = await initializeGitWorkspace(openClawState.root);
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = { workspace };
|
||||
const { storePath } = await createSessionStoreDir();
|
||||
const key = "agent:main:dashboard:worktree-fence";
|
||||
const originalCreate = managedWorktrees.create.bind(managedWorktrees);
|
||||
const createSpy = vi.spyOn(managedWorktrees, "create").mockImplementation(async (params) => {
|
||||
expect(isSessionLifecycleMutationActive(storePath, [key])).toBe(true);
|
||||
return await originalCreate(params);
|
||||
});
|
||||
let worktreeId: string | undefined;
|
||||
try {
|
||||
const created = await directSessionReq<{
|
||||
worktree: { id: string; path: string; branch: string };
|
||||
}>(
|
||||
"sessions.create",
|
||||
{ key, agentId: "main", worktree: true },
|
||||
{ client: { connect: { scopes: ["operator.admin"] } } as never },
|
||||
);
|
||||
expect(created.ok).toBe(true);
|
||||
worktreeId = created.payload?.worktree.id;
|
||||
expect(createSpy).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
createSpy.mockRestore();
|
||||
if (worktreeId) {
|
||||
await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
|
||||
}
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = undefined;
|
||||
await openClawState.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test("sessions.create rolls back failed provisioning before a same-key creator proceeds", async () => {
|
||||
const openClawState = await createOpenClawTestState({
|
||||
layout: "state-only",
|
||||
prefix: "openclaw-session-worktree-rollback-",
|
||||
});
|
||||
const workspace = await initializeGitWorkspace(openClawState.root);
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = { workspace };
|
||||
testState.sessionConfig = { sharing: { drafts: false } };
|
||||
const { storePath } = await createSessionStoreDir();
|
||||
const key = "agent:main:dashboard:worktree-rollback";
|
||||
const adminClient = { connect: { scopes: ["operator.admin"] } } as never;
|
||||
const originalRemove = managedWorktrees.remove.bind(managedWorktrees);
|
||||
let failedWorktreeId: string | undefined;
|
||||
let successorWorktreeId: string | undefined;
|
||||
let releaseRollback = () => {};
|
||||
const rollbackGate = new Promise<void>((resolve) => {
|
||||
releaseRollback = resolve;
|
||||
});
|
||||
let markRollbackStarted = () => {};
|
||||
const rollbackStarted = new Promise<void>((resolve) => {
|
||||
markRollbackStarted = resolve;
|
||||
});
|
||||
const removeSpy = vi.spyOn(managedWorktrees, "remove").mockImplementation(async (params) => {
|
||||
if (params.reason === "session-create-failed") {
|
||||
failedWorktreeId = params.id;
|
||||
markRollbackStarted();
|
||||
expect(isSessionLifecycleMutationActive(storePath, [key])).toBe(true);
|
||||
await rollbackGate;
|
||||
}
|
||||
return await originalRemove(params);
|
||||
});
|
||||
try {
|
||||
const failedPromise = directSessionReq(
|
||||
"sessions.create",
|
||||
{
|
||||
key,
|
||||
agentId: "main",
|
||||
visibility: "draft",
|
||||
worktree: true,
|
||||
},
|
||||
{ client: adminClient },
|
||||
);
|
||||
await rollbackStarted;
|
||||
let successorSettled = false;
|
||||
const successorPromise = directSessionReq<{
|
||||
entry: { worktree?: { id: string; branch: string; repoRoot: string } };
|
||||
worktree: { id: string; path: string; branch: string };
|
||||
}>("sessions.create", { key, agentId: "main", worktree: true }, { client: adminClient }).then(
|
||||
(result) => {
|
||||
successorSettled = true;
|
||||
return result;
|
||||
},
|
||||
);
|
||||
await Promise.resolve();
|
||||
expect(successorSettled).toBe(false);
|
||||
|
||||
releaseRollback();
|
||||
const [failed, successor] = await Promise.all([failedPromise, successorPromise]);
|
||||
expect(failed).toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
code: "INVALID_REQUEST",
|
||||
message: "session visibility is disabled: draft",
|
||||
details: { code: "SESSION_VISIBILITY_DISABLED", visibility: "draft" },
|
||||
},
|
||||
});
|
||||
expect(failedWorktreeId).toBeTruthy();
|
||||
expect(getRegistryWorktree(process.env, failedWorktreeId!)).toMatchObject({
|
||||
removedAt: expect.any(Number),
|
||||
});
|
||||
expect(successor.ok).toBe(true);
|
||||
const successorWorktree = successor.payload!.worktree;
|
||||
successorWorktreeId = successorWorktree.id;
|
||||
expect(successorWorktree.id).not.toBe(failedWorktreeId);
|
||||
await expect(fs.access(successorWorktree.path)).resolves.toBeUndefined();
|
||||
expect(loadSessionEntry({ sessionKey: key, storePath })?.worktree).toEqual({
|
||||
id: successorWorktree.id,
|
||||
branch: successorWorktree.branch,
|
||||
repoRoot: workspace,
|
||||
});
|
||||
|
||||
const adoptedFailure = await directSessionReq(
|
||||
"sessions.create",
|
||||
{ key, agentId: "main", visibility: "draft", worktree: true },
|
||||
{ client: adminClient },
|
||||
);
|
||||
expect(adoptedFailure).toMatchObject({
|
||||
ok: false,
|
||||
error: { message: "sessions.create visibility requires a new session" },
|
||||
});
|
||||
expect(
|
||||
removeSpy.mock.calls.some(
|
||||
([params]) =>
|
||||
params.reason === "session-create-failed" && params.id === successorWorktree.id,
|
||||
),
|
||||
).toBe(false);
|
||||
expect(getRegistryWorktree(process.env, successorWorktree.id)?.removedAt).toBeUndefined();
|
||||
} finally {
|
||||
releaseRollback();
|
||||
removeSpy.mockRestore();
|
||||
if (
|
||||
successorWorktreeId &&
|
||||
getRegistryWorktree(process.env, successorWorktreeId)?.removedAt === undefined
|
||||
) {
|
||||
await managedWorktrees.remove({
|
||||
id: successorWorktreeId,
|
||||
reason: "test-cleanup",
|
||||
force: true,
|
||||
});
|
||||
}
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = undefined;
|
||||
testState.sessionConfig = undefined;
|
||||
await openClawState.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test("sessions.create provisions and reuses a session worktree for later runs", async () => {
|
||||
const openClawState = await createOpenClawTestState({
|
||||
layout: "state-only",
|
||||
@@ -1512,6 +1671,8 @@ test("sessions.create reset-in-place persists the returned worktree cwd", async
|
||||
const { storePath } = await createSessionStoreDir();
|
||||
await writeSessionStore({ entries: { main: sessionStoreEntry("sess-reset-parent") } });
|
||||
let worktreeId: string | undefined;
|
||||
let releaseWorktreeRemoval = () => {};
|
||||
let restoreRemoveIfLossless = () => {};
|
||||
try {
|
||||
const created = await directSessionReq<{
|
||||
key: string;
|
||||
@@ -1542,9 +1703,32 @@ test("sessions.create reset-in-place persists the returned worktree cwd", async
|
||||
worktree?.path,
|
||||
);
|
||||
|
||||
// A later plain New Chat on the same main session must leave the worktree: cwd clears
|
||||
// and the (clean) session worktree is lossless-removed rather than left orphaned.
|
||||
const reset = await directSessionReq<{
|
||||
// Pause the exact old-binding removal before destructive work. A same-key
|
||||
// worktree reset must remain fenced until that prior generation is gone.
|
||||
const originalRemoveIfLossless = managedWorktrees.removeIfLossless.bind(managedWorktrees);
|
||||
const removalGate = new Promise<void>((resolve) => {
|
||||
releaseWorktreeRemoval = resolve;
|
||||
});
|
||||
let markRemovalStarted = () => {};
|
||||
const removalStarted = new Promise<void>((resolve) => {
|
||||
markRemovalStarted = resolve;
|
||||
});
|
||||
const removeIfLosslessSpy = vi
|
||||
.spyOn(managedWorktrees, "removeIfLossless")
|
||||
.mockImplementation(async (id) => {
|
||||
if (id === worktree?.id) {
|
||||
expect(threadBindingMocks.unbindThreadBindingsBySessionKey).toHaveBeenCalledWith({
|
||||
targetSessionKey: "agent:main:main",
|
||||
reason: "session-reset",
|
||||
});
|
||||
markRemovalStarted();
|
||||
expect(isSessionLifecycleMutationActive(storePath, ["agent:main:main"])).toBe(true);
|
||||
await removalGate;
|
||||
}
|
||||
return await originalRemoveIfLossless(id);
|
||||
});
|
||||
restoreRemoveIfLossless = () => removeIfLosslessSpy.mockRestore();
|
||||
const resetPromise = directSessionReq<{
|
||||
key: string;
|
||||
entry: { spawnedCwd?: string };
|
||||
resolved: { modelProvider?: string; model?: string };
|
||||
@@ -1553,23 +1737,53 @@ test("sessions.create reset-in-place persists the returned worktree cwd", async
|
||||
{ agentId: "main", parentSessionKey: "main", emitCommandHooks: true },
|
||||
{ client: { connect: { scopes: ["operator.write"] } } as never },
|
||||
);
|
||||
await removalStarted;
|
||||
let successorSettled = false;
|
||||
const successorPromise = directSessionReq<{
|
||||
entry: { spawnedCwd?: string; worktree?: { id: string; branch: string; repoRoot: string } };
|
||||
worktree: { id: string; path: string; branch: string };
|
||||
}>(
|
||||
"sessions.create",
|
||||
{
|
||||
key: "agent:main:main",
|
||||
agentId: "main",
|
||||
worktree: true,
|
||||
},
|
||||
{ client: { connect: { scopes: ["operator.admin"] } } as never },
|
||||
).then((result) => {
|
||||
successorSettled = true;
|
||||
return result;
|
||||
});
|
||||
await Promise.resolve();
|
||||
expect(successorSettled).toBe(false);
|
||||
releaseWorktreeRemoval();
|
||||
const [reset, successor] = await Promise.all([resetPromise, successorPromise]);
|
||||
restoreRemoveIfLossless();
|
||||
expect(reset.ok).toBe(true);
|
||||
expect(reset.payload?.entry.spawnedCwd).toBeUndefined();
|
||||
expect(reset.payload?.resolved).toEqual({
|
||||
modelProvider: "openai",
|
||||
model: "current-model",
|
||||
});
|
||||
expect(
|
||||
listRegistryWorktrees(process.env).filter(
|
||||
(record) =>
|
||||
record.ownerKind === "session" &&
|
||||
record.ownerId === "agent:main:main" &&
|
||||
record.removedAt === undefined,
|
||||
),
|
||||
).toHaveLength(0);
|
||||
worktreeId = undefined;
|
||||
expect(getRegistryWorktree(process.env, worktree!.id)?.removedAt).toEqual(expect.any(Number));
|
||||
expect(successor.ok).toBe(true);
|
||||
const successorWorktree = successor.payload!.worktree;
|
||||
expect(successorWorktree.id).not.toBe(worktree?.id);
|
||||
worktreeId = successorWorktree.id;
|
||||
await expect(fs.access(successorWorktree.path)).resolves.toBeUndefined();
|
||||
expect(loadSessionEntry({ sessionKey: "agent:main:main", storePath })).toMatchObject({
|
||||
spawnedCwd: successorWorktree.path,
|
||||
worktree: {
|
||||
id: successorWorktree.id,
|
||||
branch: successorWorktree.branch,
|
||||
repoRoot: workspace,
|
||||
},
|
||||
});
|
||||
expect(getRegistryWorktree(process.env, successorWorktree.id)?.removedAt).toBeUndefined();
|
||||
} finally {
|
||||
if (worktreeId) {
|
||||
releaseWorktreeRemoval();
|
||||
restoreRemoveIfLossless();
|
||||
if (worktreeId && getRegistryWorktree(process.env, worktreeId)?.removedAt === undefined) {
|
||||
await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
|
||||
}
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
|
||||
@@ -0,0 +1,307 @@
|
||||
// Session delete worktree lifecycle tests protect exact-generation cleanup and
|
||||
// same-key successor admission.
|
||||
import { execFile } from "node:child_process";
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
import { expect, test, vi } from "vitest";
|
||||
import { getRegistryWorktree } from "../agents/worktrees/registry.js";
|
||||
import {
|
||||
acquireWorktreeRunLease,
|
||||
resolveWorktreeIdForPath,
|
||||
} from "../agents/worktrees/run-lease.js";
|
||||
import { managedWorktrees } from "../agents/worktrees/service.js";
|
||||
import { loadSessionEntry } from "../config/sessions/session-accessor.js";
|
||||
import { isSessionLifecycleMutationActive } from "../sessions/session-lifecycle-admission.js";
|
||||
import { listSessionStateEventsSince } from "../sessions/session-state-events.js";
|
||||
import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js";
|
||||
import { createOpenClawTestState } from "../test-utils/openclaw-test-state.js";
|
||||
import { testState, writeSessionStore } from "./test-helpers.js";
|
||||
import {
|
||||
directSessionReq,
|
||||
sessionStoreEntry,
|
||||
setupGatewaySessionsTestHarness,
|
||||
threadBindingMocks,
|
||||
} from "./test/server-sessions.test-helpers.js";
|
||||
|
||||
const { createSessionStoreDir } = setupGatewaySessionsTestHarness();
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
||||
async function initializeRemoteBackedGitWorkspace(root: string): Promise<string> {
|
||||
const workspace = path.join(root, "workspace");
|
||||
const remote = path.join(root, "remote.git");
|
||||
await fs.mkdir(workspace, { recursive: true });
|
||||
await execFileAsync("git", ["-C", workspace, "init", "-b", "main"]);
|
||||
await execFileAsync("git", ["-C", workspace, "config", "user.name", "OpenClaw Test"]);
|
||||
await execFileAsync("git", [
|
||||
"-C",
|
||||
workspace,
|
||||
"config",
|
||||
"user.email",
|
||||
"openclaw-test@example.invalid",
|
||||
]);
|
||||
await fs.writeFile(path.join(workspace, "README.md"), "base\n");
|
||||
await execFileAsync("git", ["-C", workspace, "add", "README.md"]);
|
||||
await execFileAsync("git", ["-C", workspace, "commit", "-m", "initial"]);
|
||||
await execFileAsync("git", ["clone", "--bare", workspace, remote]);
|
||||
await execFileAsync("git", ["-C", workspace, "remote", "add", "origin", remote]);
|
||||
await execFileAsync("git", ["-C", workspace, "push", "-u", "origin", "main"]);
|
||||
return await fs.realpath(workspace);
|
||||
}
|
||||
|
||||
test("sessions.delete keeps same-key successor worktree creation behind exact cleanup", async () => {
|
||||
const openClawState = await createOpenClawTestState({
|
||||
layout: "state-only",
|
||||
prefix: "openclaw-delete-worktree-successor-",
|
||||
});
|
||||
const workspace = await initializeRemoteBackedGitWorkspace(openClawState.root);
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = { workspace };
|
||||
const { storePath } = await createSessionStoreDir();
|
||||
const key = "agent:main:dashboard:delete-worktree-successor";
|
||||
const creatorProfileId = "delete-worktree-successor-creator";
|
||||
const adminClient = {
|
||||
connect: { scopes: ["operator.admin"] },
|
||||
authenticatedUserProfile: {
|
||||
profileId: creatorProfileId,
|
||||
displayName: "Delete Worktree Test",
|
||||
hasAvatar: false,
|
||||
updatedAt: 1,
|
||||
},
|
||||
} as never;
|
||||
let successorWorktreeId: string | undefined;
|
||||
let releaseRemoval = () => {};
|
||||
const removalGate = new Promise<void>((resolve) => {
|
||||
releaseRemoval = resolve;
|
||||
});
|
||||
const originalRemove = managedWorktrees.remove.bind(managedWorktrees);
|
||||
let markRemovalStarted = () => {};
|
||||
const removalStarted = new Promise<void>((resolve) => {
|
||||
markRemovalStarted = resolve;
|
||||
});
|
||||
const removeSpy = vi.spyOn(managedWorktrees, "remove");
|
||||
try {
|
||||
const predecessor = await directSessionReq<{
|
||||
sessionId: string;
|
||||
worktree: { id: string; path: string; branch: string };
|
||||
}>("sessions.create", { key, agentId: "main", worktree: true }, { client: adminClient });
|
||||
expect(predecessor.ok).toBe(true);
|
||||
const predecessorSessionId = predecessor.payload!.sessionId;
|
||||
const predecessorWorktree = predecessor.payload!.worktree;
|
||||
|
||||
removeSpy.mockImplementation(async (params) => {
|
||||
if (params.id === predecessorWorktree.id && params.reason === "session-delete") {
|
||||
expect(isSessionLifecycleMutationActive(storePath, [key, predecessorSessionId])).toBe(true);
|
||||
expect(threadBindingMocks.unbindThreadBindingsBySessionKey).toHaveBeenCalledWith({
|
||||
targetSessionKey: key,
|
||||
reason: "session-delete",
|
||||
});
|
||||
markRemovalStarted();
|
||||
await removalGate;
|
||||
}
|
||||
return await originalRemove(params);
|
||||
});
|
||||
|
||||
const deletion = directSessionReq<{ deleted: boolean }>("sessions.delete", {
|
||||
key,
|
||||
expectedSessionId: predecessorSessionId,
|
||||
});
|
||||
await removalStarted;
|
||||
let successorSettled = false;
|
||||
const successorPromise = directSessionReq<{
|
||||
sessionId: string;
|
||||
entry: { spawnedCwd?: string; worktree?: { id: string; branch: string; repoRoot: string } };
|
||||
worktree: { id: string; path: string; branch: string };
|
||||
}>("sessions.create", { key, agentId: "main", worktree: true }, { client: adminClient }).then(
|
||||
(result) => {
|
||||
successorSettled = true;
|
||||
return result;
|
||||
},
|
||||
);
|
||||
await Promise.resolve();
|
||||
expect(successorSettled).toBe(false);
|
||||
|
||||
releaseRemoval();
|
||||
const [deleted, successor] = await Promise.all([deletion, successorPromise]);
|
||||
expect(deleted).toMatchObject({ ok: true, payload: { deleted: true } });
|
||||
expect(successor.ok).toBe(true);
|
||||
const successorSessionId = successor.payload!.sessionId;
|
||||
const successorWorktree = successor.payload!.worktree;
|
||||
successorWorktreeId = successorWorktree.id;
|
||||
expect(successorSessionId).not.toBe(predecessorSessionId);
|
||||
expect(successorWorktree.id).not.toBe(predecessorWorktree.id);
|
||||
await expect(fs.access(successorWorktree.path)).resolves.toBeUndefined();
|
||||
expect(getRegistryWorktree(process.env, successorWorktree.id)?.id).toBe(successorWorktree.id);
|
||||
expect(getRegistryWorktree(process.env, successorWorktree.id)?.removedAt).toBeUndefined();
|
||||
const persisted = loadSessionEntry({ sessionKey: key, storePath });
|
||||
expect(persisted).toMatchObject({
|
||||
sessionId: successorSessionId,
|
||||
spawnedCwd: successorWorktree.path,
|
||||
worktree: {
|
||||
id: successorWorktree.id,
|
||||
branch: successorWorktree.branch,
|
||||
repoRoot: workspace,
|
||||
},
|
||||
});
|
||||
expect(
|
||||
listSessionStateEventsSince(key, "main", 0, 20).events.filter(
|
||||
(event) => event.kind === "created",
|
||||
),
|
||||
).toEqual([
|
||||
expect.objectContaining({
|
||||
sessionId: successorSessionId,
|
||||
actorType: "human",
|
||||
actorId: creatorProfileId,
|
||||
}),
|
||||
]);
|
||||
const admittedWorktreeId = await resolveWorktreeIdForPath({
|
||||
sessionEntry: persisted,
|
||||
candidatePaths: [persisted?.spawnedCwd],
|
||||
});
|
||||
expect(admittedWorktreeId).toBe(successorWorktree.id);
|
||||
const runLease = await acquireWorktreeRunLease(admittedWorktreeId!);
|
||||
await runLease.release();
|
||||
} finally {
|
||||
releaseRemoval();
|
||||
removeSpy.mockRestore();
|
||||
if (
|
||||
successorWorktreeId &&
|
||||
getRegistryWorktree(process.env, successorWorktreeId)?.removedAt === undefined
|
||||
) {
|
||||
await managedWorktrees.remove({
|
||||
id: successorWorktreeId,
|
||||
reason: "test-cleanup",
|
||||
force: true,
|
||||
});
|
||||
}
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = undefined;
|
||||
await openClawState.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test("sessions.delete reports the exact preserved worktree when cleanup fails", async () => {
|
||||
const openClawState = await createOpenClawTestState({
|
||||
layout: "state-only",
|
||||
prefix: "openclaw-delete-worktree-preserved-",
|
||||
});
|
||||
const workspace = await initializeRemoteBackedGitWorkspace(openClawState.root);
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = { workspace };
|
||||
const { storePath } = await createSessionStoreDir();
|
||||
const key = "agent:main:dashboard:delete-worktree-preserved";
|
||||
const adminClient = { connect: { scopes: ["operator.admin"] } } as never;
|
||||
const originalRemove = managedWorktrees.remove.bind(managedWorktrees);
|
||||
let worktreeId: string | undefined;
|
||||
const removeSpy = vi.spyOn(managedWorktrees, "remove");
|
||||
try {
|
||||
const created = await directSessionReq<{
|
||||
worktree: { id: string; path: string; branch: string };
|
||||
}>("sessions.create", { key, agentId: "main", worktree: true }, { client: adminClient });
|
||||
expect(created.ok).toBe(true);
|
||||
const worktree = created.payload!.worktree;
|
||||
worktreeId = worktree.id;
|
||||
removeSpy.mockImplementation(async (params) => {
|
||||
if (params.id === worktree.id && params.reason === "session-delete") {
|
||||
throw new Error("simulated cleanup failure");
|
||||
}
|
||||
return await originalRemove(params);
|
||||
});
|
||||
|
||||
const deleted = await directSessionReq<{
|
||||
deleted: boolean;
|
||||
worktreePreserved?: { id: string; path: string; branch: string };
|
||||
}>("sessions.delete", { key });
|
||||
|
||||
expect(deleted).toMatchObject({
|
||||
ok: true,
|
||||
payload: {
|
||||
deleted: true,
|
||||
worktreePreserved: {
|
||||
id: worktree.id,
|
||||
path: worktree.path,
|
||||
branch: worktree.branch,
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(loadSessionEntry({ sessionKey: key, storePath })).toBeUndefined();
|
||||
expect(getRegistryWorktree(process.env, worktree.id)?.removedAt).toBeUndefined();
|
||||
await expect(fs.access(worktree.path)).resolves.toBeUndefined();
|
||||
} finally {
|
||||
removeSpy.mockRestore();
|
||||
if (worktreeId && getRegistryWorktree(process.env, worktreeId)?.removedAt === undefined) {
|
||||
await managedWorktrees.remove({ id: worktreeId, reason: "test-cleanup", force: true });
|
||||
}
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = undefined;
|
||||
await openClawState.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test("sessions.delete preserves an entry-bound worktree owned by another principal", async () => {
|
||||
const openClawState = await createOpenClawTestState({
|
||||
layout: "state-only",
|
||||
prefix: "openclaw-delete-worktree-owner-mismatch-",
|
||||
});
|
||||
const workspace = await initializeRemoteBackedGitWorkspace(openClawState.root);
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = { workspace };
|
||||
await createSessionStoreDir();
|
||||
const key = "agent:main:dashboard:delete-worktree-owner-mismatch";
|
||||
const foreignWorktree = await managedWorktrees.create({
|
||||
repoRoot: workspace,
|
||||
ownerKind: "manual",
|
||||
ownerId: "foreign-owner",
|
||||
name: "foreign-owner",
|
||||
});
|
||||
await writeSessionStore({
|
||||
entries: {
|
||||
[key]: sessionStoreEntry("session-owner-mismatch", {
|
||||
spawnedCwd: foreignWorktree.path,
|
||||
worktree: {
|
||||
id: foreignWorktree.id,
|
||||
branch: foreignWorktree.branch,
|
||||
repoRoot: foreignWorktree.repoRoot,
|
||||
},
|
||||
}),
|
||||
},
|
||||
});
|
||||
const removeSpy = vi.spyOn(managedWorktrees, "remove");
|
||||
try {
|
||||
const deleted = await directSessionReq<{
|
||||
deleted: boolean;
|
||||
worktreePreserved?: { id: string; path: string; branch: string };
|
||||
}>("sessions.delete", { key, deleteTranscript: false });
|
||||
|
||||
expect(deleted).toMatchObject({
|
||||
ok: true,
|
||||
payload: {
|
||||
deleted: true,
|
||||
worktreePreserved: {
|
||||
id: foreignWorktree.id,
|
||||
path: foreignWorktree.path,
|
||||
branch: foreignWorktree.branch,
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(removeSpy).not.toHaveBeenCalled();
|
||||
expect(getRegistryWorktree(process.env, foreignWorktree.id)).toMatchObject({
|
||||
ownerKind: "manual",
|
||||
ownerId: "foreign-owner",
|
||||
});
|
||||
expect(getRegistryWorktree(process.env, foreignWorktree.id)?.removedAt).toBeUndefined();
|
||||
await expect(fs.access(foreignWorktree.path)).resolves.toBeUndefined();
|
||||
} finally {
|
||||
removeSpy.mockRestore();
|
||||
if (getRegistryWorktree(process.env, foreignWorktree.id)?.removedAt === undefined) {
|
||||
await managedWorktrees.remove({
|
||||
id: foreignWorktree.id,
|
||||
reason: "test-cleanup",
|
||||
force: true,
|
||||
});
|
||||
}
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
testState.agentConfig = undefined;
|
||||
await openClawState.cleanup();
|
||||
}
|
||||
});
|
||||
@@ -76,6 +76,11 @@ import { createLazyRuntimeModule } from "../shared/lazy-runtime.js";
|
||||
import { normalizeSessionDeliveryState } from "../utils/delivery-context.shared.js";
|
||||
import { ADMIN_SCOPE } from "./operator-scopes.js";
|
||||
import { buildForkedGatewaySessionEntry } from "./session-create-fork-entry.js";
|
||||
import {
|
||||
type PreparedGatewaySessionLifecycle,
|
||||
type PrepareGatewaySessionLifecycle,
|
||||
rollbackGatewaySessionPreparation,
|
||||
} from "./session-lifecycle-preparation.js";
|
||||
import { resolvePluginSessionOwnershipError } from "./session-plugin-ownership.js";
|
||||
import { resolveRequestedSessionAgentId } from "./session-request-agent.js";
|
||||
import { isSessionVisibilityAllowed, resolveSessionVisibility } from "./session-sharing.js";
|
||||
@@ -249,8 +254,9 @@ export async function createGatewaySession(params: {
|
||||
deny: string[];
|
||||
};
|
||||
spawnedCwd?: string;
|
||||
/** Managed worktree bound to the new session; persisted alongside spawnedCwd. */
|
||||
worktree?: { id: string; branch: string; repoRoot: string };
|
||||
/** Prepares session-owned resources while the target lifecycle fence is held. */
|
||||
prepareLifecycle?: PrepareGatewaySessionLifecycle;
|
||||
onLifecycleCleanupError?: (error: unknown) => void;
|
||||
/** Bind session exec to host=node with this node id; caller scope-checks. */
|
||||
execNode?: string;
|
||||
/** Working directory interpreted only by execNode. */
|
||||
@@ -642,7 +648,10 @@ export async function createGatewaySession(params: {
|
||||
commandSource: params.commandSource,
|
||||
...(params.creation ? { creation: params.creation } : {}),
|
||||
...(spawnedCwd ? { spawnedCwd } : {}),
|
||||
...(params.worktree ? { worktree: params.worktree } : {}),
|
||||
...(params.prepareLifecycle ? { prepareLifecycle: params.prepareLifecycle } : {}),
|
||||
...(params.onLifecycleCleanupError
|
||||
? { onLifecycleCleanupError: params.onLifecycleCleanupError }
|
||||
: {}),
|
||||
...(params.execNode ? { execNode: params.execNode } : {}),
|
||||
...(execCwd ? { execCwd } : {}),
|
||||
...(params.clearExecBinding ? { clearExecBinding: true } : {}),
|
||||
@@ -670,6 +679,8 @@ export async function createGatewaySession(params: {
|
||||
|
||||
let createdContext: CreatedGatewaySession | undefined;
|
||||
let createdNewEntry = false;
|
||||
let preparedLifecycle: PreparedGatewaySessionLifecycle | undefined;
|
||||
let lifecyclePreparationCommitted = false;
|
||||
const spawnToolPolicy =
|
||||
params.spawnToolPolicy && canonicalParentSessionKey
|
||||
? {
|
||||
@@ -775,6 +786,22 @@ export async function createGatewaySession(params: {
|
||||
}
|
||||
|
||||
const target = creationTarget;
|
||||
const currentTargetEntry = loadSessionEntryReadOnly(target.canonicalKey, {
|
||||
agentId: target.agentId,
|
||||
}).entry;
|
||||
const preparationResult = params.prepareLifecycle
|
||||
? await params.prepareLifecycle({
|
||||
agentId: target.agentId,
|
||||
entry: currentTargetEntry,
|
||||
key: target.canonicalKey,
|
||||
storePath: target.storePath,
|
||||
})
|
||||
: undefined;
|
||||
if (preparationResult && !preparationResult.ok) {
|
||||
return { ok: false, error: preparationResult.error };
|
||||
}
|
||||
preparedLifecycle = preparationResult?.value;
|
||||
|
||||
const created = await createSessionEntryWithTranscript<ErrorShape>(
|
||||
{
|
||||
agentId: target.agentId,
|
||||
@@ -928,7 +955,9 @@ export async function createGatewaySession(params: {
|
||||
return patched;
|
||||
}
|
||||
sessionEntries[target.canonicalKey] = patched.entry;
|
||||
const spawnedCwd = normalizeOptionalString(params.spawnedCwd);
|
||||
const spawnedCwd = normalizeOptionalString(
|
||||
preparedLifecycle?.spawnedCwd ?? params.spawnedCwd,
|
||||
);
|
||||
const execNode = normalizeOptionalString(params.execNode);
|
||||
const execCwd = normalizeOptionalString(params.execCwd);
|
||||
const initialAgentHarnessId = params.initialEntry
|
||||
@@ -987,7 +1016,7 @@ export async function createGatewaySession(params: {
|
||||
// Session worktrees adopt cwd only during admin-gated creation; public patching stays
|
||||
// restricted to spawned subagent and ACP lineage.
|
||||
...(spawnedCwd ? { spawnedCwd } : {}),
|
||||
...(params.worktree ? { worktree: params.worktree } : {}),
|
||||
...(preparedLifecycle?.worktree ? { worktree: preparedLifecycle.worktree } : {}),
|
||||
...(execNode ? { execHost: "node", execNode, ...(execCwd ? { execCwd } : {}) } : {}),
|
||||
...(initialAgentHarnessId ? { agentHarnessId: initialAgentHarnessId } : {}),
|
||||
...(createdNewEntry && params.authorizedPluginId && !params.catalogTarget
|
||||
@@ -1135,6 +1164,16 @@ export async function createGatewaySession(params: {
|
||||
entry: created.entry,
|
||||
storePath: target.storePath,
|
||||
};
|
||||
lifecyclePreparationCommitted = true;
|
||||
if (createdNewEntry) {
|
||||
// The created fact belongs to this row generation; record it before a
|
||||
// same-key delete can acquire the lifecycle fence and purge that state.
|
||||
recordSessionCreated({
|
||||
sessionKey: createdContext.key,
|
||||
agentId: createdContext.agentId,
|
||||
entry: createdContext.entry,
|
||||
});
|
||||
}
|
||||
|
||||
if (canonicalParentSessionKey && parentSessionTarget && params.emitCommandHooks === true) {
|
||||
const parentEntry = currentParentSessionEntry;
|
||||
@@ -1205,15 +1244,16 @@ export async function createGatewaySession(params: {
|
||||
const result = await runExclusiveSessionLifecycleMutation({
|
||||
targets: lifecycleTargets,
|
||||
run: createChildSession,
|
||||
finalize: async () => {
|
||||
if (!lifecyclePreparationCommitted) {
|
||||
await rollbackGatewaySessionPreparation({
|
||||
prepared: preparedLifecycle,
|
||||
onError: params.onLifecycleCleanupError,
|
||||
});
|
||||
}
|
||||
},
|
||||
});
|
||||
if (result.ok && !result.resetExisting && createdContext) {
|
||||
if (createdNewEntry) {
|
||||
recordSessionCreated({
|
||||
sessionKey: createdContext.key,
|
||||
agentId: createdContext.agentId,
|
||||
entry: createdContext.entry,
|
||||
});
|
||||
}
|
||||
await params.afterCreate?.(createdContext);
|
||||
}
|
||||
return result;
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import type { Result } from "@openclaw/normalization-core/result";
|
||||
import type { ErrorShape } from "../../packages/gateway-protocol/src/index.js";
|
||||
import type { SessionEntry } from "../config/sessions/types.js";
|
||||
|
||||
export type PreparedGatewaySessionLifecycle = {
|
||||
spawnedCwd?: string;
|
||||
worktree?: NonNullable<SessionEntry["worktree"]>;
|
||||
rollback?: () => Promise<void>;
|
||||
};
|
||||
|
||||
export type PrepareGatewaySessionLifecycle = (target: {
|
||||
agentId: string;
|
||||
entry?: SessionEntry;
|
||||
key: string;
|
||||
storePath: string;
|
||||
}) => Promise<Result<PreparedGatewaySessionLifecycle, ErrorShape>>;
|
||||
|
||||
export async function rollbackGatewaySessionPreparation(params: {
|
||||
onError?: (error: unknown) => void;
|
||||
prepared?: PreparedGatewaySessionLifecycle;
|
||||
}): Promise<void> {
|
||||
try {
|
||||
await params.prepared?.rollback?.();
|
||||
} catch (error) {
|
||||
params.onError?.(error);
|
||||
}
|
||||
}
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
import { clearAllCliSessions } from "../agents/cli-session.js";
|
||||
import { resetRegisteredAgentHarnessSessions } from "../agents/harness/registry.js";
|
||||
import { resolveSessionModelRef } from "../agents/session-model-ref.js";
|
||||
import { managedWorktrees } from "../agents/worktrees/service.js";
|
||||
import { stopSubagentsForRequester } from "../auto-reply/reply/abort.js";
|
||||
import {
|
||||
buildSessionEndHookPayload,
|
||||
@@ -91,6 +92,11 @@ import {
|
||||
noteActiveSessionForShutdown,
|
||||
} from "./active-sessions-shutdown-tracker.js";
|
||||
import { findDirectChildSessionsForParent } from "./session-child-sessions.js";
|
||||
import {
|
||||
type PreparedGatewaySessionLifecycle,
|
||||
type PrepareGatewaySessionLifecycle,
|
||||
rollbackGatewaySessionPreparation,
|
||||
} from "./session-lifecycle-preparation.js";
|
||||
import { resolvePluginSessionOwnershipError } from "./session-plugin-ownership.js";
|
||||
import { notifyGatewaySessionReset } from "./session-reset-notifications.js";
|
||||
import {
|
||||
@@ -1001,8 +1007,9 @@ export async function performGatewaySessionReset(params: {
|
||||
key: string;
|
||||
agentId?: string;
|
||||
spawnedCwd?: string;
|
||||
/** Managed worktree adopted by this reset; cleared together with spawnedCwd. */
|
||||
worktree?: { id: string; branch: string; repoRoot: string };
|
||||
/** Prepares session-owned resources while the target lifecycle fence is held. */
|
||||
prepareLifecycle?: PrepareGatewaySessionLifecycle;
|
||||
onLifecycleCleanupError?: (error: unknown) => void;
|
||||
/** Bind session exec to host=node with this node id; caller scope-checks. */
|
||||
execNode?: string;
|
||||
/** Working directory interpreted only by execNode. */
|
||||
@@ -1077,6 +1084,13 @@ export async function performGatewaySessionReset(params: {
|
||||
if (!resetTarget.ok) {
|
||||
return resetTarget;
|
||||
}
|
||||
const reportLifecycleCleanupError = (error: unknown) => {
|
||||
if (params.onLifecycleCleanupError) {
|
||||
params.onLifecycleCleanupError(error);
|
||||
return;
|
||||
}
|
||||
logVerbose(`session lifecycle resource cleanup failed: ${String(error)}`);
|
||||
};
|
||||
const initialResetEntry = loadSessionEntry(
|
||||
params.key,
|
||||
resetTarget.requestedAgentId ? { agentId: resetTarget.requestedAgentId } : undefined,
|
||||
@@ -1149,6 +1163,8 @@ export async function performGatewaySessionReset(params: {
|
||||
let admittedWorkReleased = true;
|
||||
let resetPreparationError: ReturnType<typeof errorShape> | undefined;
|
||||
let preparedResetSessionId: string | undefined;
|
||||
let preparedLifecycle: PreparedGatewaySessionLifecycle | undefined;
|
||||
let lifecyclePreparationCommitted = false;
|
||||
return await runExclusiveSessionLifecycleMutation({
|
||||
scope: resetTarget.storePath,
|
||||
identities: resetLifecycleIdentities,
|
||||
@@ -1220,6 +1236,19 @@ export async function performGatewaySessionReset(params: {
|
||||
identities: resetLifecycleIdentities,
|
||||
timeoutMs: SESSION_WORK_ADMISSION_DRAIN_TIMEOUT_MS,
|
||||
});
|
||||
if (admittedWorkReleased && params.prepareLifecycle) {
|
||||
const prepared = await params.prepareLifecycle({
|
||||
agentId: resetTarget.target.agentId,
|
||||
entry: currentEntry,
|
||||
key: resetTarget.target.canonicalKey,
|
||||
storePath: resetTarget.storePath,
|
||||
});
|
||||
if (!prepared.ok) {
|
||||
resetPreparationError = prepared.error;
|
||||
return;
|
||||
}
|
||||
preparedLifecycle = prepared.value;
|
||||
}
|
||||
},
|
||||
run: async () => {
|
||||
const { cfg, target, storePath, requestedAgentId } = resetTarget;
|
||||
@@ -1307,6 +1336,9 @@ export async function performGatewaySessionReset(params: {
|
||||
};
|
||||
}
|
||||
const hadExistingEntry = Boolean(entry);
|
||||
const detachedWorktreeId = params.clearSpawnedCwd
|
||||
? normalizeOptionalString(entry?.worktree?.id)
|
||||
: undefined;
|
||||
const resetLifecycleRevision = entry?.lifecycleRevision;
|
||||
const agentId = normalizeAgentId(target.agentId ?? resolveDefaultAgentId(cfg));
|
||||
const workspaceDir = resolveAgentWorkspaceDir(cfg, agentId);
|
||||
@@ -1579,10 +1611,10 @@ export async function performGatewaySessionReset(params: {
|
||||
spawnedWorkspaceDir: currentEntry?.spawnedWorkspaceDir,
|
||||
spawnedCwd: params.clearSpawnedCwd
|
||||
? undefined
|
||||
: (params.spawnedCwd ?? currentEntry?.spawnedCwd),
|
||||
: (preparedLifecycle?.spawnedCwd ?? params.spawnedCwd ?? currentEntry?.spawnedCwd),
|
||||
worktree: params.clearSpawnedCwd
|
||||
? undefined
|
||||
: (params.worktree ?? currentEntry?.worktree),
|
||||
: (preparedLifecycle?.worktree ?? currentEntry?.worktree),
|
||||
parentSessionKey: currentEntry?.parentSessionKey,
|
||||
...creationStamp,
|
||||
forkSource: currentEntry?.forkSource,
|
||||
@@ -1696,6 +1728,7 @@ export async function performGatewaySessionReset(params: {
|
||||
});
|
||||
const lifecycle: Awaited<ReturnType<typeof resetSessionEntryLifecycle>> =
|
||||
await lifecyclePromise;
|
||||
lifecyclePreparationCommitted = !resetSkipped;
|
||||
if (!resetSkipped) {
|
||||
const resetSessionKey = target.canonicalKey ?? params.key;
|
||||
handleSessionStateSessionReset(resetSessionKey);
|
||||
@@ -1746,6 +1779,15 @@ export async function performGatewaySessionReset(params: {
|
||||
reason: "session-reset",
|
||||
});
|
||||
}
|
||||
if (!resetSkipped && detachedWorktreeId) {
|
||||
// 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);
|
||||
} catch (error) {
|
||||
reportLifecycleCleanupError(error);
|
||||
}
|
||||
}
|
||||
return {
|
||||
ok: true,
|
||||
key: target.canonicalKey,
|
||||
@@ -1755,6 +1797,14 @@ export async function performGatewaySessionReset(params: {
|
||||
storePath,
|
||||
};
|
||||
},
|
||||
finalize: async () => {
|
||||
if (!lifecyclePreparationCommitted) {
|
||||
await rollbackGatewaySessionPreparation({
|
||||
prepared: preparedLifecycle,
|
||||
onError: reportLifecycleCleanupError,
|
||||
});
|
||||
}
|
||||
},
|
||||
});
|
||||
}
|
||||
/* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// Tests lifecycle/work admission ordering across canonical keys and backing ids.
|
||||
import { setImmediate as waitForImmediate } from "node:timers/promises";
|
||||
import { importFreshModule } from "openclaw/plugin-sdk/test-fixtures";
|
||||
import { expect, it } from "vitest";
|
||||
import { expect, it, vi } from "vitest";
|
||||
import { createDeferred } from "../../test/helpers/promise.js";
|
||||
import { runExclusiveSessionStoreWrite } from "../config/sessions/store-writer.js";
|
||||
import {
|
||||
@@ -257,6 +258,70 @@ it("counts one multi-identity lifecycle mutation once across module instances",
|
||||
expect(second.getActiveSessionLifecycleMutationCount()).toBe(0);
|
||||
});
|
||||
|
||||
it("keeps a same-identity mutation queued until finalization completes", async () => {
|
||||
const target = { scope: "store-finalize-order", identities: ["session-finalize-order"] };
|
||||
const finalizeStarted = createDeferred();
|
||||
const releaseFinalize = createDeferred();
|
||||
let secondRan = false;
|
||||
const first = runExclusiveSessionLifecycleMutation({
|
||||
...target,
|
||||
run: async () => {},
|
||||
finalize: async () => {
|
||||
finalizeStarted.resolve();
|
||||
await releaseFinalize.promise;
|
||||
},
|
||||
});
|
||||
await finalizeStarted.promise;
|
||||
|
||||
const second = runExclusiveSessionLifecycleMutation({
|
||||
...target,
|
||||
run: async () => {
|
||||
secondRan = true;
|
||||
},
|
||||
});
|
||||
await waitForImmediate();
|
||||
expect(secondRan).toBe(false);
|
||||
|
||||
releaseFinalize.resolve();
|
||||
await Promise.all([first, second]);
|
||||
});
|
||||
|
||||
it("finalizes a lifecycle mutation when its run throws", async () => {
|
||||
const runError = new Error("lifecycle run failed");
|
||||
const finalize = vi.fn(async () => {});
|
||||
|
||||
await expect(
|
||||
runExclusiveSessionLifecycleMutation({
|
||||
scope: "store-finalize-run-error",
|
||||
identities: ["session-finalize-run-error"],
|
||||
run: async () => {
|
||||
throw runError;
|
||||
},
|
||||
finalize,
|
||||
}),
|
||||
).rejects.toBe(runError);
|
||||
expect(finalize).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("releases lifecycle state when finalization throws", async () => {
|
||||
const target = { scope: "store-finalize-error", identities: ["session-finalize-error"] };
|
||||
const finalizeError = new Error("lifecycle finalizer failed");
|
||||
|
||||
await expect(
|
||||
runExclusiveSessionLifecycleMutation({
|
||||
...target,
|
||||
run: async () => {},
|
||||
finalize: async () => {
|
||||
throw finalizeError;
|
||||
},
|
||||
}),
|
||||
).rejects.toBe(finalizeError);
|
||||
expect(isSessionLifecycleMutationActive(target.scope, target.identities)).toBe(false);
|
||||
await expect(
|
||||
runExclusiveSessionLifecycleMutation({ ...target, run: async () => "next" }),
|
||||
).resolves.toBe("next");
|
||||
});
|
||||
|
||||
it("counts a cross-store lifecycle mutation once and fences every target", async () => {
|
||||
const mutationStarted = createDeferred();
|
||||
const releaseMutation = createDeferred();
|
||||
|
||||
@@ -48,6 +48,7 @@ type SessionLifecycleMutationTarget = {
|
||||
type SessionLifecycleMutationParams<T> = {
|
||||
kind?: SessionLifecycleMutationKind;
|
||||
prepare?: () => Promise<void>;
|
||||
finalize?: () => Promise<void>;
|
||||
run: () => Promise<T>;
|
||||
signal?: AbortSignal;
|
||||
} & (SessionLifecycleMutationTarget | { targets: Iterable<SessionLifecycleMutationTarget> });
|
||||
@@ -241,34 +242,40 @@ export async function runExclusiveSessionLifecycleMutation<T>(
|
||||
await params.prepare?.();
|
||||
return await runWithSessionIdentityLocks(identities, 0, params.run);
|
||||
} finally {
|
||||
await runWithSessionIdentityLocks(identities, 0, async () => {
|
||||
for (const identity of identities) {
|
||||
if (params.kind) {
|
||||
const kinds = ACTIVE_SESSION_LIFECYCLE_MUTATION_KINDS.get(identity);
|
||||
const remainingKindCount = (kinds?.get(params.kind) ?? 1) - 1;
|
||||
if (remainingKindCount > 0) {
|
||||
kinds?.set(params.kind, remainingKindCount);
|
||||
} else {
|
||||
kinds?.delete(params.kind);
|
||||
if (kinds?.size === 0) {
|
||||
ACTIVE_SESSION_LIFECYCLE_MUTATION_KINDS.delete(identity);
|
||||
// Resource finalization is part of the mutation: successors remain
|
||||
// fenced until rollback or exact-generation cleanup has completed.
|
||||
try {
|
||||
await params.finalize?.();
|
||||
} finally {
|
||||
await runWithSessionIdentityLocks(identities, 0, async () => {
|
||||
for (const identity of identities) {
|
||||
if (params.kind) {
|
||||
const kinds = ACTIVE_SESSION_LIFECYCLE_MUTATION_KINDS.get(identity);
|
||||
const remainingKindCount = (kinds?.get(params.kind) ?? 1) - 1;
|
||||
if (remainingKindCount > 0) {
|
||||
kinds?.set(params.kind, remainingKindCount);
|
||||
} else {
|
||||
kinds?.delete(params.kind);
|
||||
if (kinds?.size === 0) {
|
||||
ACTIVE_SESSION_LIFECYCLE_MUTATION_KINDS.delete(identity);
|
||||
}
|
||||
}
|
||||
}
|
||||
const remaining = (ACTIVE_SESSION_LIFECYCLE_MUTATIONS.get(identity) ?? 1) - 1;
|
||||
if (remaining > 0) {
|
||||
ACTIVE_SESSION_LIFECYCLE_MUTATIONS.set(identity, remaining);
|
||||
continue;
|
||||
}
|
||||
ACTIVE_SESSION_LIFECYCLE_MUTATIONS.delete(identity);
|
||||
const waiters = SESSION_LIFECYCLE_IDLE_WAITERS.get(identity);
|
||||
SESSION_LIFECYCLE_IDLE_WAITERS.delete(identity);
|
||||
for (const resolve of waiters ?? []) {
|
||||
resolve();
|
||||
}
|
||||
}
|
||||
const remaining = (ACTIVE_SESSION_LIFECYCLE_MUTATIONS.get(identity) ?? 1) - 1;
|
||||
if (remaining > 0) {
|
||||
ACTIVE_SESSION_LIFECYCLE_MUTATIONS.set(identity, remaining);
|
||||
continue;
|
||||
}
|
||||
ACTIVE_SESSION_LIFECYCLE_MUTATIONS.delete(identity);
|
||||
const waiters = SESSION_LIFECYCLE_IDLE_WAITERS.get(identity);
|
||||
SESSION_LIFECYCLE_IDLE_WAITERS.delete(identity);
|
||||
for (const resolve of waiters ?? []) {
|
||||
resolve();
|
||||
}
|
||||
}
|
||||
ACTIVE_SESSION_LIFECYCLE_MUTATION_RUNS.delete(mutationRun);
|
||||
});
|
||||
ACTIVE_SESSION_LIFECYCLE_MUTATION_RUNS.delete(mutationRun);
|
||||
});
|
||||
}
|
||||
}
|
||||
}),
|
||||
"mutation",
|
||||
|
||||
Reference in New Issue
Block a user