mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
fix(workboard): retry managed worktree cleanup after hook failures (#126162)
* fix(workboard): retry managed worktree cleanup * fix(workboard): keep workspace mutation type local
This commit is contained in:
committed by
GitHub
parent
dcdfd737e5
commit
ffdd0641c8
@@ -4,7 +4,6 @@ import { registerWorkboardGatewayMethods } from "./runtime-api.js";
|
||||
import { createWorkboardAutomationNudgeService } from "./src/automation-nudge.js";
|
||||
import { createWorkboardChangeEventService } from "./src/change-events.js";
|
||||
import { registerWorkboardCommand } from "./src/command.js";
|
||||
import { cleanupWorkboardRunWorktree } from "./src/dispatcher-workspace.js";
|
||||
import {
|
||||
createWorkboardLifecycleService,
|
||||
readWorkboardLifecycleSessions,
|
||||
@@ -30,6 +29,7 @@ export default definePluginEntry({
|
||||
});
|
||||
const lifecycleSync = createWorkboardLifecycleService({
|
||||
store,
|
||||
worktrees: api.runtime.worktrees,
|
||||
readSessions: async (options) =>
|
||||
await readWorkboardLifecycleSessions(api.runtime.gateway, options),
|
||||
});
|
||||
@@ -68,16 +68,12 @@ export default definePluginEntry({
|
||||
api.on("gateway_start", () => lifecycleSync.onGatewayStart());
|
||||
api.on("gateway_stop", () => lifecycleSync.onGatewayStop());
|
||||
api.on("subagent_ended", async (event) => {
|
||||
await Promise.all([
|
||||
syncWorkboardSubagentEnded({ store, event, onMatched: automationNudge.nudge }),
|
||||
event.runId
|
||||
? cleanupWorkboardRunWorktree({
|
||||
store,
|
||||
worktrees: api.runtime.worktrees,
|
||||
runId: event.runId,
|
||||
})
|
||||
: undefined,
|
||||
]);
|
||||
await syncWorkboardSubagentEnded({
|
||||
store,
|
||||
worktrees: api.runtime.worktrees,
|
||||
event,
|
||||
onMatched: automationNudge.nudge,
|
||||
});
|
||||
});
|
||||
api.on("agent_end", async (event, context) => {
|
||||
await syncWorkboardAgentEnded({
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
import type { WorkboardCard } from "@openclaw/workboard-contract";
|
||||
// Workboard dispatch workspace helpers keep authority resolution outside the orchestration loop.
|
||||
import type { PluginRuntime } from "openclaw/plugin-sdk/plugin-runtime";
|
||||
import { canonicalPathFromExistingAncestor } from "openclaw/plugin-sdk/security-runtime";
|
||||
import {
|
||||
canonicalPathFromExistingAncestor,
|
||||
pathExists,
|
||||
} from "openclaw/plugin-sdk/security-runtime";
|
||||
import type { WorkboardStore } from "./store.js";
|
||||
import {
|
||||
assertCanonicalWorkboardRootAccess,
|
||||
@@ -27,21 +30,71 @@ export function managedWorktreeName(cardId: string): string {
|
||||
return `wb-${suffix}`.slice(0, 64).replace(/-$/, "");
|
||||
}
|
||||
|
||||
export async function cleanupWorkboardRunWorktree(params: {
|
||||
export type WorkboardWorktreeCleanupRuntime = Pick<PluginRuntime["worktrees"], "removeIfLossless">;
|
||||
|
||||
type WorkboardWorkspaceMutation = {
|
||||
before: WorkboardCard;
|
||||
after: WorkboardCard;
|
||||
};
|
||||
|
||||
function hasTerminalWorkboardExecution(card: WorkboardCard): boolean {
|
||||
const status = card.execution?.status ?? card.status;
|
||||
return status === "review" || status === "blocked" || status === "done";
|
||||
}
|
||||
|
||||
export function isWorkboardWorktreeCleanupCandidate(card: WorkboardCard): boolean {
|
||||
const workspace = card.metadata?.automation?.workspace;
|
||||
return Boolean(
|
||||
workspace?.kind === "worktree" &&
|
||||
workspace.path &&
|
||||
workspace.sourcePath &&
|
||||
hasTerminalWorkboardExecution(card),
|
||||
);
|
||||
}
|
||||
|
||||
export async function cleanupWorkboardCardWorktree(params: {
|
||||
store: WorkboardStore;
|
||||
worktrees: Pick<PluginRuntime["worktrees"], "removeIfLossless">;
|
||||
runId: string;
|
||||
worktrees: WorkboardWorktreeCleanupRuntime;
|
||||
card: WorkboardCard;
|
||||
workspaceMutation?: WorkboardWorkspaceMutation;
|
||||
}): Promise<void> {
|
||||
const card = (await params.store.list()).find((entry) => entry.runId === params.runId);
|
||||
const workspace = card?.metadata?.automation?.workspace;
|
||||
if (!card || workspace?.kind !== "worktree" || !workspace.path) {
|
||||
const current = await params.store.get(params.card.id);
|
||||
const workspace = (params.workspaceMutation?.after ?? current)?.metadata?.automation?.workspace;
|
||||
if (
|
||||
!current ||
|
||||
!hasTerminalWorkboardExecution(current) ||
|
||||
workspace?.kind !== "worktree" ||
|
||||
!workspace?.path ||
|
||||
!workspace.sourcePath
|
||||
) {
|
||||
return;
|
||||
}
|
||||
await params.worktrees.removeIfLossless({
|
||||
const removed = await params.worktrees.removeIfLossless({
|
||||
path: workspace.path,
|
||||
ownerKind: "workboard",
|
||||
ownerId: card.id,
|
||||
ownerId: params.card.id,
|
||||
});
|
||||
if (!removed && (await pathExists(workspace.path))) {
|
||||
return;
|
||||
}
|
||||
if (params.workspaceMutation) {
|
||||
await params.store.compensateWorkspaceMutation(
|
||||
params.workspaceMutation.before,
|
||||
params.workspaceMutation.after,
|
||||
);
|
||||
return;
|
||||
}
|
||||
await params.store.update(
|
||||
current.id,
|
||||
{
|
||||
workspace: {
|
||||
kind: "worktree",
|
||||
path: workspace.sourcePath,
|
||||
...(workspace.sourceBranch ? { branch: workspace.sourceBranch } : {}),
|
||||
},
|
||||
},
|
||||
{ expectedUpdatedAt: current.updatedAt },
|
||||
);
|
||||
}
|
||||
|
||||
export async function resolveDispatchWorkspaceAccess(params: {
|
||||
|
||||
@@ -1,3 +1,6 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { dispatchAndStartWorkboardCards } from "./dispatcher.js";
|
||||
import type { PersistedWorkboardCard, WorkboardKeyedStore } from "./persistence-types.js";
|
||||
@@ -125,4 +128,51 @@ describe("Workboard dispatcher lifecycle races", () => {
|
||||
expect(archived?.metadata?.claim).toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
it("retains a managed worktree when pre-start lossless cleanup declines", async () => {
|
||||
const managedPath = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-workboard-retained-"));
|
||||
const store = new WorkboardStore(createMemoryStore());
|
||||
const card = await store.create({
|
||||
title: "Retain failed worker checkout",
|
||||
status: "ready",
|
||||
workspace: { kind: "worktree", path: "/repo", branch: "main" },
|
||||
workspaceAccess: { unrestricted: true },
|
||||
});
|
||||
const removeIfLossless = vi.fn().mockResolvedValue(false);
|
||||
try {
|
||||
const result = await dispatchAndStartWorkboardCards({
|
||||
store,
|
||||
subagent: { run: vi.fn().mockRejectedValue(new Error("model unavailable")) },
|
||||
worktrees: {
|
||||
resolveCheckoutRoot: vi.fn().mockResolvedValue(undefined),
|
||||
create: vi.fn().mockResolvedValue({
|
||||
id: "managed-id",
|
||||
path: managedPath,
|
||||
branch: `openclaw/wb-${card.id}`,
|
||||
}),
|
||||
release: vi.fn(),
|
||||
removeIfLossless,
|
||||
},
|
||||
options: { now: 10, maxStarts: 1, materializeWorktree: true },
|
||||
});
|
||||
|
||||
expect(result.startFailures).toEqual([
|
||||
expect.objectContaining({ cardId: card.id, error: "model unavailable" }),
|
||||
]);
|
||||
expect(removeIfLossless).toHaveBeenCalledOnce();
|
||||
const blocked = await store.get(card.id);
|
||||
expect(blocked).toMatchObject({
|
||||
status: "blocked",
|
||||
metadata: {
|
||||
automation: {
|
||||
workspace: { kind: "worktree", path: managedPath, sourcePath: "/repo" },
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(blocked?.execution).toBeUndefined();
|
||||
expect(blocked?.runId).toBeUndefined();
|
||||
} finally {
|
||||
fs.rmSync(managedPath, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
// Workboard tests cover dispatcher plugin behavior.
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { cleanupWorkboardRunWorktree } from "./dispatcher-workspace.js";
|
||||
import { dispatchAndStartWorkboardCards } from "./dispatcher.js";
|
||||
import type { PersistedWorkboardCard, WorkboardKeyedStore } from "./persistence-types.js";
|
||||
import { WorkboardStore } from "./store.js";
|
||||
@@ -79,7 +78,7 @@ describe("dispatchAndStartWorkboardCards", () => {
|
||||
expect(execution).not.toHaveProperty("model");
|
||||
});
|
||||
|
||||
it("materializes managed worktrees, supplies cwd, persists them, and cleans up on run end", async () => {
|
||||
it("materializes managed worktrees, supplies cwd, and persists them", async () => {
|
||||
const store = new WorkboardStore(createMemoryStore());
|
||||
const card = await store.create({
|
||||
title: "Isolated worker",
|
||||
@@ -135,12 +134,7 @@ describe("dispatchAndStartWorkboardCards", () => {
|
||||
},
|
||||
});
|
||||
|
||||
await cleanupWorkboardRunWorktree({ store, worktrees, runId: "run-worktree" });
|
||||
expect(worktrees.removeIfLossless).toHaveBeenCalledWith({
|
||||
path: "/state/worktrees/fingerprint/wb-card",
|
||||
ownerKind: "workboard",
|
||||
ownerId: card.id,
|
||||
});
|
||||
expect(worktrees.removeIfLossless).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("requires explicit reauthorization for legacy cards under full-host dispatch", async () => {
|
||||
|
||||
@@ -15,6 +15,7 @@ import type { PluginRuntime } from "openclaw/plugin-sdk/plugin-runtime";
|
||||
import { canonicalPathFromExistingAncestor } from "openclaw/plugin-sdk/security-runtime";
|
||||
import {
|
||||
assertRestrictedWorkboardTarget,
|
||||
cleanupWorkboardCardWorktree,
|
||||
managedWorktreeName,
|
||||
resolveDispatchWorkspaceAccess,
|
||||
type ResolveAgentWorkspaceRuntime,
|
||||
@@ -581,25 +582,6 @@ async function runWorkboardDispatch(
|
||||
)
|
||||
.catch(() => undefined);
|
||||
} catch (error) {
|
||||
if (
|
||||
!runStarted &&
|
||||
materializedWorkspace?.kind === "worktree" &&
|
||||
materializedWorkspace.path &&
|
||||
params.worktrees
|
||||
) {
|
||||
await params.worktrees
|
||||
.removeIfLossless({
|
||||
path: materializedWorkspace.path,
|
||||
ownerKind: "workboard",
|
||||
ownerId: card.id,
|
||||
})
|
||||
.catch(() => undefined);
|
||||
if (workspaceMutation) {
|
||||
await params.store
|
||||
.compensateWorkspaceMutation(workspaceMutation.before, workspaceMutation.after)
|
||||
.catch(() => undefined);
|
||||
}
|
||||
}
|
||||
const message = formatErrorMessage(error);
|
||||
startFailures.push({ cardId: card.id, title: card.title, error: message });
|
||||
if (!claimValue || runStarted) {
|
||||
@@ -623,6 +605,17 @@ async function runWorkboardDispatch(
|
||||
} catch {
|
||||
// Leave the original start failure visible; dispatch will diagnose stale claims later.
|
||||
}
|
||||
if (params.worktrees) {
|
||||
const failedCard = await params.store.get(card.id).catch(() => undefined);
|
||||
if (failedCard) {
|
||||
await cleanupWorkboardCardWorktree({
|
||||
store: params.store,
|
||||
worktrees: params.worktrees,
|
||||
card: failedCard,
|
||||
...(workspaceMutation ? { workspaceMutation } : {}),
|
||||
}).catch(() => undefined);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,308 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import type { WorkboardExecution } from "@openclaw/workboard-contract";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { createWorkboardLifecycleService, syncWorkboardSubagentEnded } from "./lifecycle-sync.js";
|
||||
import { createWorkboardSqliteStores } from "./sqlite-store.js";
|
||||
import { WorkboardStore } from "./store.js";
|
||||
|
||||
const SESSION_KEY = "agent:main:subagent:workboard-cleanup-recovery";
|
||||
const RUN_ID = "run-cleanup-recovery";
|
||||
const MANAGED_PATH = "/state/worktrees/recovery/wb-card";
|
||||
const SOURCE_PATH = "/repo";
|
||||
|
||||
function openStore(dbPath: string) {
|
||||
const stores = createWorkboardSqliteStores({ dbPath });
|
||||
return { store: new WorkboardStore(stores.cards), stores };
|
||||
}
|
||||
|
||||
function execution(
|
||||
sessionKey: string,
|
||||
runId: string,
|
||||
status: WorkboardExecution["status"],
|
||||
): WorkboardExecution {
|
||||
return {
|
||||
id: `exec-${runId}`,
|
||||
kind: "agent-session",
|
||||
mode: "autonomous",
|
||||
status,
|
||||
sessionKey,
|
||||
runId,
|
||||
startedAt: 1000,
|
||||
updatedAt: 1000,
|
||||
};
|
||||
}
|
||||
|
||||
async function createManagedCard(
|
||||
store: WorkboardStore,
|
||||
options: {
|
||||
managedPath?: string;
|
||||
status?: "blocked" | "running" | "review";
|
||||
withExecutionAssociation?: boolean;
|
||||
} = {},
|
||||
) {
|
||||
const status = options.status ?? "running";
|
||||
const withExecutionAssociation = options.withExecutionAssociation !== false;
|
||||
return await store.create({
|
||||
title: "Recover managed worktree cleanup",
|
||||
status,
|
||||
...(withExecutionAssociation
|
||||
? {
|
||||
sessionKey: SESSION_KEY,
|
||||
runId: RUN_ID,
|
||||
execution: execution(SESSION_KEY, RUN_ID, status),
|
||||
}
|
||||
: {}),
|
||||
workspace: {
|
||||
kind: "worktree",
|
||||
path: options.managedPath ?? MANAGED_PATH,
|
||||
branch: "openclaw/wb-card",
|
||||
sourcePath: SOURCE_PATH,
|
||||
sourceBranch: "main",
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function doneSessionSnapshot(updatedAt: number) {
|
||||
return vi.fn().mockResolvedValue({
|
||||
sessions: [
|
||||
{
|
||||
key: SESSION_KEY,
|
||||
status: "done" as const,
|
||||
hasActiveRun: false,
|
||||
updatedAt,
|
||||
},
|
||||
],
|
||||
complete: true,
|
||||
});
|
||||
}
|
||||
|
||||
const context = { logger: { warn: vi.fn() } } as never;
|
||||
|
||||
describe("Workboard managed-worktree cleanup recovery", () => {
|
||||
it("retries cleanup after a hook failure and process restart", async () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-workboard-cleanup-recovery-"));
|
||||
const dbPath = path.join(dir, "workboard.sqlite");
|
||||
const initial = openStore(dbPath);
|
||||
const card = await createManagedCard(initial.store);
|
||||
const removeIfLossless = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error("worktree registry unavailable"))
|
||||
.mockResolvedValueOnce(true);
|
||||
const worktrees = { removeIfLossless };
|
||||
|
||||
await expect(
|
||||
syncWorkboardSubagentEnded({
|
||||
store: initial.store,
|
||||
worktrees,
|
||||
event: {
|
||||
targetSessionKey: SESSION_KEY,
|
||||
runId: RUN_ID,
|
||||
endedAt: card.updatedAt + 1,
|
||||
outcome: "ok",
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow("worktree registry unavailable");
|
||||
initial.stores.close();
|
||||
|
||||
const restarted = openStore(dbPath);
|
||||
const service = createWorkboardLifecycleService({
|
||||
store: restarted.store,
|
||||
readSessions: doneSessionSnapshot(card.updatedAt + 1),
|
||||
worktrees,
|
||||
});
|
||||
|
||||
try {
|
||||
await service.start(context);
|
||||
service.onGatewayStart();
|
||||
await vi.waitFor(() => expect(removeIfLossless).toHaveBeenCalledTimes(2));
|
||||
|
||||
expect(removeIfLossless).toHaveBeenLastCalledWith({
|
||||
path: MANAGED_PATH,
|
||||
ownerKind: "workboard",
|
||||
ownerId: card.id,
|
||||
});
|
||||
const recovered = await restarted.store.get(card.id);
|
||||
expect(recovered).toMatchObject({ status: "review", execution: { status: "review" } });
|
||||
expect(recovered?.metadata?.automation?.workspace).toEqual({
|
||||
kind: "worktree",
|
||||
path: SOURCE_PATH,
|
||||
branch: "main",
|
||||
});
|
||||
} finally {
|
||||
service.onGatewayStop();
|
||||
await service.stop?.(context);
|
||||
restarted.stores.close();
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("cleans a freshly reconciled terminal worktree in the initial restart sweep", async () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-workboard-cleanup-fresh-"));
|
||||
const dbPath = path.join(dir, "workboard.sqlite");
|
||||
const initial = openStore(dbPath);
|
||||
const card = await createManagedCard(initial.store);
|
||||
initial.stores.close();
|
||||
|
||||
const restarted = openStore(dbPath);
|
||||
const removeIfLossless = vi.fn().mockResolvedValue(true);
|
||||
const service = createWorkboardLifecycleService({
|
||||
store: restarted.store,
|
||||
readSessions: doneSessionSnapshot(card.updatedAt + 1),
|
||||
worktrees: { removeIfLossless },
|
||||
});
|
||||
|
||||
try {
|
||||
await service.start(context);
|
||||
service.onGatewayStart();
|
||||
await vi.waitFor(() => expect(removeIfLossless).toHaveBeenCalledOnce());
|
||||
|
||||
expect((await restarted.store.get(card.id))?.metadata?.automation?.workspace).toEqual({
|
||||
kind: "worktree",
|
||||
path: SOURCE_PATH,
|
||||
branch: "main",
|
||||
});
|
||||
} finally {
|
||||
service.onGatewayStop();
|
||||
await service.stop?.(context);
|
||||
restarted.stores.close();
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("persists a retained worktree obligation and retries it after restart", async () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-workboard-cleanup-retained-"));
|
||||
const dbPath = path.join(dir, "workboard.sqlite");
|
||||
const managedPath = path.join(dir, "managed-worktree");
|
||||
fs.mkdirSync(managedPath);
|
||||
const initial = openStore(dbPath);
|
||||
const card = await createManagedCard(initial.store, { managedPath, status: "review" });
|
||||
initial.stores.close();
|
||||
const removeIfLossless = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(false)
|
||||
.mockImplementationOnce(async () => {
|
||||
fs.rmSync(managedPath, { recursive: true });
|
||||
return true;
|
||||
});
|
||||
|
||||
const retained = openStore(dbPath);
|
||||
const firstService = createWorkboardLifecycleService({
|
||||
store: retained.store,
|
||||
readSessions: doneSessionSnapshot(card.updatedAt),
|
||||
worktrees: { removeIfLossless },
|
||||
});
|
||||
await firstService.start(context);
|
||||
firstService.onGatewayStart();
|
||||
await vi.waitFor(() => expect(removeIfLossless).toHaveBeenCalledOnce());
|
||||
firstService.onGatewayStop();
|
||||
await firstService.stop?.(context);
|
||||
expect((await retained.store.get(card.id))?.metadata?.automation?.workspace).toMatchObject({
|
||||
path: managedPath,
|
||||
sourcePath: SOURCE_PATH,
|
||||
});
|
||||
retained.stores.close();
|
||||
|
||||
const restarted = openStore(dbPath);
|
||||
const secondService = createWorkboardLifecycleService({
|
||||
store: restarted.store,
|
||||
readSessions: doneSessionSnapshot(card.updatedAt),
|
||||
worktrees: { removeIfLossless },
|
||||
});
|
||||
try {
|
||||
await secondService.start(context);
|
||||
secondService.onGatewayStart();
|
||||
await vi.waitFor(() => expect(removeIfLossless).toHaveBeenCalledTimes(2));
|
||||
expect((await restarted.store.get(card.id))?.metadata?.automation?.workspace).toEqual({
|
||||
kind: "worktree",
|
||||
path: SOURCE_PATH,
|
||||
branch: "main",
|
||||
});
|
||||
} finally {
|
||||
secondService.onGatewayStop();
|
||||
await secondService.stop?.(context);
|
||||
restarted.stores.close();
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("cleans a blocked pre-start worktree without an execution association", async () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-workboard-cleanup-blocked-"));
|
||||
const dbPath = path.join(dir, "workboard.sqlite");
|
||||
const initial = openStore(dbPath);
|
||||
const card = await createManagedCard(initial.store, {
|
||||
status: "blocked",
|
||||
withExecutionAssociation: false,
|
||||
});
|
||||
initial.stores.close();
|
||||
|
||||
const restarted = openStore(dbPath);
|
||||
const readSessions = vi.fn();
|
||||
const removeIfLossless = vi.fn().mockResolvedValue(true);
|
||||
const service = createWorkboardLifecycleService({
|
||||
store: restarted.store,
|
||||
readSessions,
|
||||
worktrees: { removeIfLossless },
|
||||
});
|
||||
try {
|
||||
await service.start(context);
|
||||
service.onGatewayStart();
|
||||
await vi.waitFor(() => expect(removeIfLossless).toHaveBeenCalledOnce());
|
||||
|
||||
expect(readSessions).not.toHaveBeenCalled();
|
||||
expect((await restarted.store.get(card.id))?.metadata?.automation?.workspace).toEqual({
|
||||
kind: "worktree",
|
||||
path: SOURCE_PATH,
|
||||
branch: "main",
|
||||
});
|
||||
} finally {
|
||||
service.onGatewayStop();
|
||||
await service.stop?.(context);
|
||||
restarted.stores.close();
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does not clean a matched card after a newer running attempt wins the race", async () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-workboard-cleanup-race-"));
|
||||
const dbPath = path.join(dir, "workboard.sqlite");
|
||||
const initial = openStore(dbPath);
|
||||
const card = await createManagedCard(initial.store);
|
||||
const newerSessionKey = "agent:newer:subagent:workboard-cleanup-recovery";
|
||||
const originalSync = initial.store.syncLifecycle.bind(initial.store);
|
||||
vi.spyOn(initial.store, "syncLifecycle").mockImplementationOnce(async (id, input) => {
|
||||
await initial.store.update(id, {
|
||||
sessionKey: newerSessionKey,
|
||||
runId: "newer-run",
|
||||
execution: execution(newerSessionKey, "newer-run", "running"),
|
||||
});
|
||||
return await originalSync(id, input);
|
||||
});
|
||||
const removeIfLossless = vi.fn().mockResolvedValue(true);
|
||||
|
||||
try {
|
||||
await expect(
|
||||
syncWorkboardSubagentEnded({
|
||||
store: initial.store,
|
||||
worktrees: { removeIfLossless },
|
||||
event: {
|
||||
targetSessionKey: SESSION_KEY,
|
||||
runId: RUN_ID,
|
||||
endedAt: card.updatedAt + 1,
|
||||
outcome: "ok",
|
||||
},
|
||||
}),
|
||||
).resolves.toBe(0);
|
||||
expect(removeIfLossless).not.toHaveBeenCalled();
|
||||
await expect(initial.store.get(card.id)).resolves.toMatchObject({
|
||||
status: "running",
|
||||
runId: "newer-run",
|
||||
execution: { status: "running", runId: "newer-run" },
|
||||
});
|
||||
} finally {
|
||||
initial.stores.close();
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -6,6 +6,11 @@ import type {
|
||||
import { resolveGlobalSingleton } from "openclaw/plugin-sdk/global-singleton";
|
||||
import { isRecord } from "openclaw/plugin-sdk/string-coerce-runtime";
|
||||
import type { OpenClawPluginApi, OpenClawPluginService } from "../api.js";
|
||||
import {
|
||||
cleanupWorkboardCardWorktree,
|
||||
isWorkboardWorktreeCleanupCandidate,
|
||||
type WorkboardWorktreeCleanupRuntime,
|
||||
} from "./dispatcher-workspace.js";
|
||||
import {
|
||||
workboardCardMatchesLifecycleLink,
|
||||
workboardCardSessionLookupKey,
|
||||
@@ -17,6 +22,7 @@ import type { WorkboardStore } from "./store.js";
|
||||
const WORKBOARD_LIFECYCLE_SWEEP_MS = 60_000;
|
||||
const WORKBOARD_STALE_SESSION_MS = 30 * 60 * 1000;
|
||||
const WORKBOARD_SESSION_SWEEP_LIMIT = 10_000;
|
||||
const WORKBOARD_WORKTREE_CLEANUP_SWEEP_LIMIT = 32;
|
||||
// Keep readiness across plugin-only reloads, while the singleton lifecycle
|
||||
// clears it before an in-process Gateway restart starts replacement services.
|
||||
const workboardLifecycleGatewayState = resolveGlobalSingleton(
|
||||
@@ -133,7 +139,7 @@ async function syncWorkboardLifecycleEvent(params: {
|
||||
observation: WorkboardLifecycleObservation;
|
||||
now: number;
|
||||
onMatched?: WorkboardLifecycleMatchHandler;
|
||||
}): Promise<number> {
|
||||
}): Promise<{ cards: readonly WorkboardCard[]; count: number }> {
|
||||
const cards = (await params.store.list()).filter(
|
||||
(card) => !card.metadata?.archivedAt && workboardCardMatchesLifecycleLink(card, params.source),
|
||||
);
|
||||
@@ -164,11 +170,12 @@ async function syncWorkboardLifecycleEvent(params: {
|
||||
...(params.source.sessionKey ? { sessionKey: params.source.sessionKey } : {}),
|
||||
}),
|
||||
]);
|
||||
return (await updates).filter(Boolean).length;
|
||||
return { cards, count: (await updates).filter(Boolean).length };
|
||||
}
|
||||
|
||||
export async function syncWorkboardSubagentEnded(params: {
|
||||
store: WorkboardStore;
|
||||
worktrees?: WorkboardWorktreeCleanupRuntime;
|
||||
event: {
|
||||
targetSessionKey: string;
|
||||
runId?: string;
|
||||
@@ -179,7 +186,7 @@ export async function syncWorkboardSubagentEnded(params: {
|
||||
onMatched?: WorkboardLifecycleMatchHandler;
|
||||
}): Promise<number> {
|
||||
const now = params.now ?? Date.now();
|
||||
return await syncWorkboardLifecycleEvent({
|
||||
const synced = await syncWorkboardLifecycleEvent({
|
||||
store: params.store,
|
||||
source: { sessionKey: params.event.targetSessionKey, runId: params.event.runId },
|
||||
observation: {
|
||||
@@ -189,6 +196,19 @@ export async function syncWorkboardSubagentEnded(params: {
|
||||
now,
|
||||
...(params.onMatched ? { onMatched: params.onMatched } : {}),
|
||||
});
|
||||
if (params.worktrees) {
|
||||
for (const matched of synced.cards) {
|
||||
const card = await params.store.get(matched.id);
|
||||
if (card) {
|
||||
await cleanupWorkboardCardWorktree({
|
||||
store: params.store,
|
||||
worktrees: params.worktrees,
|
||||
card,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
return synced.count;
|
||||
}
|
||||
|
||||
export async function syncWorkboardAgentEnded(params: {
|
||||
@@ -199,19 +219,21 @@ export async function syncWorkboardAgentEnded(params: {
|
||||
onMatched?: WorkboardLifecycleMatchHandler;
|
||||
}): Promise<number> {
|
||||
const now = params.now ?? Date.now();
|
||||
return await syncWorkboardLifecycleEvent({
|
||||
store: params.store,
|
||||
source: {
|
||||
sessionKey: params.context.sessionKey,
|
||||
runId: params.event.runId ?? params.context.runId,
|
||||
},
|
||||
observation: {
|
||||
state: params.event.success ? "succeeded" : "failed",
|
||||
sourceUpdatedAt: now,
|
||||
},
|
||||
now,
|
||||
...(params.onMatched ? { onMatched: params.onMatched } : {}),
|
||||
});
|
||||
return (
|
||||
await syncWorkboardLifecycleEvent({
|
||||
store: params.store,
|
||||
source: {
|
||||
sessionKey: params.context.sessionKey,
|
||||
runId: params.event.runId ?? params.context.runId,
|
||||
},
|
||||
observation: {
|
||||
state: params.event.success ? "succeeded" : "failed",
|
||||
sourceUpdatedAt: now,
|
||||
},
|
||||
now,
|
||||
...(params.onMatched ? { onMatched: params.onMatched } : {}),
|
||||
})
|
||||
).count;
|
||||
}
|
||||
|
||||
function lifecycleFromSession(
|
||||
@@ -254,6 +276,7 @@ function lifecycleFromSession(
|
||||
|
||||
async function syncWorkboardLifecycleSessions(params: {
|
||||
store: WorkboardStore;
|
||||
cards?: readonly WorkboardCard[];
|
||||
sessions: readonly WorkboardLifecycleSession[];
|
||||
complete?: boolean;
|
||||
now?: number;
|
||||
@@ -277,7 +300,7 @@ async function syncWorkboardLifecycleSessions(params: {
|
||||
}
|
||||
}
|
||||
let count = 0;
|
||||
for (const card of await params.store.list()) {
|
||||
for (const card of params.cards ?? (await params.store.list())) {
|
||||
if (card.metadata?.archivedAt) {
|
||||
continue;
|
||||
}
|
||||
@@ -410,6 +433,7 @@ export async function readWorkboardLifecycleSessions(
|
||||
|
||||
export function createWorkboardLifecycleService(params: {
|
||||
store: WorkboardStore;
|
||||
worktrees?: WorkboardWorktreeCleanupRuntime;
|
||||
readSessions: (
|
||||
options: WorkboardLifecycleSessionReadOptions,
|
||||
) => Promise<WorkboardLifecycleSessionSnapshot>;
|
||||
@@ -418,6 +442,31 @@ export function createWorkboardLifecycleService(params: {
|
||||
let generation = 0;
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
let begin: (() => void) | undefined;
|
||||
let cleanupCursor = 0;
|
||||
const cleanupWorktrees = async (
|
||||
cards: readonly WorkboardCard[],
|
||||
warn: (message: string) => void,
|
||||
) => {
|
||||
if (!params.worktrees) {
|
||||
return;
|
||||
}
|
||||
const candidates = cards.filter(isWorkboardWorktreeCleanupCandidate);
|
||||
const start = cleanupCursor % Math.max(candidates.length, 1);
|
||||
const rotated = [...candidates.slice(start), ...candidates.slice(0, start)];
|
||||
const batch = rotated.slice(0, WORKBOARD_WORKTREE_CLEANUP_SWEEP_LIMIT);
|
||||
cleanupCursor = candidates.length === 0 ? 0 : (start + batch.length) % candidates.length;
|
||||
for (const card of batch) {
|
||||
try {
|
||||
await cleanupWorkboardCardWorktree({
|
||||
store: params.store,
|
||||
worktrees: params.worktrees,
|
||||
card,
|
||||
});
|
||||
} catch (error) {
|
||||
warn(`workboard worktree cleanup failed for card ${card.id}: ${String(error)}`);
|
||||
}
|
||||
}
|
||||
};
|
||||
const stop = () => {
|
||||
generation += 1;
|
||||
begin = undefined;
|
||||
@@ -433,27 +482,39 @@ export function createWorkboardLifecycleService(params: {
|
||||
let begun = false;
|
||||
const reconcile = async () => {
|
||||
try {
|
||||
const cards = await params.store.list();
|
||||
if (
|
||||
generation !== owner ||
|
||||
!cards.some((card) => needsWorkboardLifecycleReconciliation(card))
|
||||
) {
|
||||
let cards = await params.store.list();
|
||||
if (generation !== owner) {
|
||||
return;
|
||||
}
|
||||
const snapshot = await params.readSessions({
|
||||
includeUnknown: cards.some(
|
||||
(card) => !card.metadata?.archivedAt && cardSessionKey(card) === "unknown",
|
||||
),
|
||||
});
|
||||
if (cards.some((card) => needsWorkboardLifecycleReconciliation(card))) {
|
||||
try {
|
||||
const snapshot = await params.readSessions({
|
||||
includeUnknown: cards.some(
|
||||
(card) => !card.metadata?.archivedAt && cardSessionKey(card) === "unknown",
|
||||
),
|
||||
});
|
||||
if (generation !== owner) {
|
||||
return;
|
||||
}
|
||||
await syncWorkboardLifecycleSessions({
|
||||
store: params.store,
|
||||
cards,
|
||||
...snapshot,
|
||||
now: params.now?.() ?? Date.now(),
|
||||
});
|
||||
if (generation !== owner) {
|
||||
return;
|
||||
}
|
||||
cards = await params.store.list();
|
||||
} catch (error) {
|
||||
ctx.logger.warn(`workboard lifecycle sync failed: ${String(error)}`);
|
||||
}
|
||||
}
|
||||
if (generation === owner) {
|
||||
await syncWorkboardLifecycleSessions({
|
||||
store: params.store,
|
||||
...snapshot,
|
||||
now: params.now?.() ?? Date.now(),
|
||||
});
|
||||
await cleanupWorktrees(cards, (message) => ctx.logger.warn(message));
|
||||
}
|
||||
} catch (error) {
|
||||
ctx.logger.warn(`workboard lifecycle sync failed: ${String(error)}`);
|
||||
ctx.logger.warn(`workboard lifecycle recovery failed: ${String(error)}`);
|
||||
} finally {
|
||||
if (generation === owner) {
|
||||
timer = setTimeout(() => void reconcile(), WORKBOARD_LIFECYCLE_SWEEP_MS);
|
||||
|
||||
@@ -387,10 +387,16 @@ function normalizeWorkspace(
|
||||
if (kind === "dir" && (!workspacePath || !isAbsoluteWorkspacePath(workspacePath))) {
|
||||
throw new Error("dir workspace path must be absolute.");
|
||||
}
|
||||
const branch = normalizeBoundedString(record.branch, fallback?.branch, 160, "workspace branch");
|
||||
const workspaceFallback = workspacePath === fallback?.path ? fallback : undefined;
|
||||
const branch = normalizeBoundedString(
|
||||
record.branch,
|
||||
workspaceFallback?.branch,
|
||||
160,
|
||||
"workspace branch",
|
||||
);
|
||||
const sourcePath = normalizeBoundedString(
|
||||
record.sourcePath,
|
||||
fallback?.sourcePath,
|
||||
workspaceFallback?.sourcePath,
|
||||
2000,
|
||||
"workspace source path",
|
||||
);
|
||||
@@ -399,7 +405,7 @@ function normalizeWorkspace(
|
||||
}
|
||||
const sourceBranch = normalizeBoundedString(
|
||||
record.sourceBranch,
|
||||
fallback?.sourceBranch,
|
||||
workspaceFallback?.sourceBranch,
|
||||
160,
|
||||
"workspace source branch",
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user