From ffdd0641c80f73c3868f730280674d3ab7aadcd6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 22:05:16 -0700 Subject: [PATCH] fix(workboard): retry managed worktree cleanup after hook failures (#126162) * fix(workboard): retry managed worktree cleanup * fix(workboard): keep workspace mutation type local --- extensions/workboard/index.ts | 18 +- .../workboard/src/dispatcher-workspace.ts | 71 +++- .../workboard/src/dispatcher.races.test.ts | 50 +++ extensions/workboard/src/dispatcher.test.ts | 10 +- extensions/workboard/src/dispatcher.ts | 31 +- .../lifecycle-sync.cleanup-recovery.test.ts | 308 ++++++++++++++++++ extensions/workboard/src/lifecycle-sync.ts | 127 ++++++-- extensions/workboard/src/store-normalizers.ts | 12 +- 8 files changed, 544 insertions(+), 83 deletions(-) create mode 100644 extensions/workboard/src/lifecycle-sync.cleanup-recovery.test.ts diff --git a/extensions/workboard/index.ts b/extensions/workboard/index.ts index f6de46750309..a0b8c436c26f 100644 --- a/extensions/workboard/index.ts +++ b/extensions/workboard/index.ts @@ -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({ diff --git a/extensions/workboard/src/dispatcher-workspace.ts b/extensions/workboard/src/dispatcher-workspace.ts index b30b24f52cb5..ecfbe291184d 100644 --- a/extensions/workboard/src/dispatcher-workspace.ts +++ b/extensions/workboard/src/dispatcher-workspace.ts @@ -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; + +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; - runId: string; + worktrees: WorkboardWorktreeCleanupRuntime; + card: WorkboardCard; + workspaceMutation?: WorkboardWorkspaceMutation; }): Promise { - 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: { diff --git a/extensions/workboard/src/dispatcher.races.test.ts b/extensions/workboard/src/dispatcher.races.test.ts index 9c6a8ce8050e..b2d0d32f66a1 100644 --- a/extensions/workboard/src/dispatcher.races.test.ts +++ b/extensions/workboard/src/dispatcher.races.test.ts @@ -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 }); + } + }); }); diff --git a/extensions/workboard/src/dispatcher.test.ts b/extensions/workboard/src/dispatcher.test.ts index 8dfcc07d89fe..f670bb7615f5 100644 --- a/extensions/workboard/src/dispatcher.test.ts +++ b/extensions/workboard/src/dispatcher.test.ts @@ -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 () => { diff --git a/extensions/workboard/src/dispatcher.ts b/extensions/workboard/src/dispatcher.ts index 0bf348586ad6..b3716038897f 100644 --- a/extensions/workboard/src/dispatcher.ts +++ b/extensions/workboard/src/dispatcher.ts @@ -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); + } + } } } diff --git a/extensions/workboard/src/lifecycle-sync.cleanup-recovery.test.ts b/extensions/workboard/src/lifecycle-sync.cleanup-recovery.test.ts new file mode 100644 index 000000000000..894ee389423d --- /dev/null +++ b/extensions/workboard/src/lifecycle-sync.cleanup-recovery.test.ts @@ -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 }); + } + }); +}); diff --git a/extensions/workboard/src/lifecycle-sync.ts b/extensions/workboard/src/lifecycle-sync.ts index 5fdd6e9864d6..50b2b96f0f50 100644 --- a/extensions/workboard/src/lifecycle-sync.ts +++ b/extensions/workboard/src/lifecycle-sync.ts @@ -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 { +}): 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 { 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 { 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; @@ -418,6 +442,31 @@ export function createWorkboardLifecycleService(params: { let generation = 0; let timer: ReturnType | 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); diff --git a/extensions/workboard/src/store-normalizers.ts b/extensions/workboard/src/store-normalizers.ts index 7ca153bde0e0..bcb54c04f01f 100644 --- a/extensions/workboard/src/store-normalizers.ts +++ b/extensions/workboard/src/store-normalizers.ts @@ -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", );