From 57cedf56efbb5e32b98cb29f39e898f2339cbcb9 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 27 Aug 2026 10:39:10 +0800 Subject: [PATCH] fix(tasks): skip completed deleted-agent stores (#130554) --- ...tasks-session-registry-maintenance.test.ts | 203 ++++++++++++++---- .../tasks-session-registry-maintenance.ts | 30 ++- src/commands/tasks.ts | 2 +- 3 files changed, 192 insertions(+), 43 deletions(-) diff --git a/src/commands/tasks-session-registry-maintenance.test.ts b/src/commands/tasks-session-registry-maintenance.test.ts index 0ea8c8393925..0e36f9722029 100644 --- a/src/commands/tasks-session-registry-maintenance.test.ts +++ b/src/commands/tasks-session-registry-maintenance.test.ts @@ -1,13 +1,21 @@ -// Covers the session-registry sweep's cron-store failure behavior: an -// unreadable cron store must skip the sweep, not prune running transcripts. +// Covers session-registry sweep isolation: unreadable cron facts fail the whole +// sweep closed, while completed agent deletions are reported as per-store skips. +import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { beginAgentDeletion } from "../agents/agent-lifecycle-registry.js"; import { resetConfigRuntimeState } from "../config/config.js"; import { loadSessionEntry, replaceSessionEntry } from "../config/sessions/session-accessor.js"; +import { resolveSqliteTargetFromSessionStorePath } from "../config/sessions/session-sqlite-target.js"; +import type { RuntimeEnv } from "../runtime.js"; import { closeOpenClawAgentDatabasesForTest } from "../state/openclaw-agent-db.js"; +import * as taskRegistryMaintenance from "../tasks/task-registry.maintenance.js"; import { withOpenClawTestState } from "../test-utils/openclaw-test-state.js"; +import type { OpenClawTestState } from "../test-utils/openclaw-test-state.js"; import { runSessionRegistryMaintenance } from "./tasks-session-registry-maintenance.js"; +import { tasksMaintenanceCommand } from "./tasks.js"; +const DAY_MS = 24 * 60 * 60_000; const mocks = vi.hoisted(() => ({ cronStoreLoadError: undefined as Error | undefined, })); @@ -25,54 +33,177 @@ vi.mock("../cron/store.js", async (importOriginal) => { }; }); +function writeAgentDeletion( + state: OpenClawTestState, + agentId: string, + cleanupCompleted: boolean, +): void { + const deletion = beginAgentDeletion({ + agentId, + agentDir: state.agentDir(agentId), + workspaceDir: state.path(`workspace-${agentId}`), + sessionsDir: state.sessionsDir(agentId), + deleteFiles: false, + }); + if (cleanupCompleted) { + deletion.finish(); + } +} + +async function writeStaleCronSession(storePath: string, agentId: string): Promise { + const sessionKey = `agent:${agentId}:cron:done-job:run:old-run`; + await replaceSessionEntry( + { sessionKey, storePath }, + { sessionId: `${agentId}-old-run`, updatedAt: Date.now() - 8 * DAY_MS }, + ); + return sessionKey; +} + +async function withMaintenanceState(run: (state: OpenClawTestState) => Promise) { + await withOpenClawTestState( + { layout: "state-only", prefix: "openclaw-session-registry-maintenance-" }, + async (state) => { + resetConfigRuntimeState(); + await run(state); + }, + ); +} + +function createRuntime(): RuntimeEnv { + return { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn(), + } as unknown as RuntimeEnv; +} + describe("runSessionRegistryMaintenance", () => { afterEach(() => { mocks.cronStoreLoadError = undefined; + taskRegistryMaintenance.stopTaskRegistryMaintenance(); + taskRegistryMaintenance.resetTaskRegistryMaintenanceRuntimeForTests(); resetConfigRuntimeState(); closeOpenClawAgentDatabasesForTest(); }); it("skips the sweep instead of pruning when the cron store is unreadable", async () => { - await withOpenClawTestState( - { layout: "state-only", prefix: "openclaw-session-registry-maintenance-" }, - async (state) => { - resetConfigRuntimeState(); - const storePath = path.join(state.sessionsDir("main"), "sessions.json"); - const staleCronKey = "agent:main:cron:maybe-running:run:old-run"; - await replaceSessionEntry( - { sessionKey: staleCronKey, storePath }, - { sessionId: "maybe-running", updatedAt: Date.now() - 8 * 24 * 60 * 60_000 }, - ); - mocks.cronStoreLoadError = new Error("SQLITE_CORRUPT: database disk image is malformed"); + await withMaintenanceState(async (state) => { + const storePath = path.join(state.sessionsDir("main"), "sessions.json"); + const staleCronKey = "agent:main:cron:maybe-running:run:old-run"; + await replaceSessionEntry( + { sessionKey: staleCronKey, storePath }, + { sessionId: "maybe-running", updatedAt: Date.now() - 8 * DAY_MS }, + ); + mocks.cronStoreLoadError = new Error("SQLITE_CORRUPT: database disk image is malformed"); - const summary = await runSessionRegistryMaintenance({ apply: true }); + const summary = await runSessionRegistryMaintenance({ apply: true }); - expect(summary.skippedReason).toContain("cron store unreadable"); - expect(summary.pruned).toBe(0); - // The possibly-running cron transcript survives until cron facts are readable. - expect(loadSessionEntry({ sessionKey: staleCronKey, storePath })).toBeDefined(); - }, - ); + expect(summary.skippedReason).toContain("cron store unreadable"); + expect(summary.pruned).toBe(0); + // The possibly-running cron transcript survives until cron facts are readable. + expect(loadSessionEntry({ sessionKey: staleCronKey, storePath })).toBeDefined(); + }); }); it("prunes stale rows when the cron store is readable", async () => { - await withOpenClawTestState( - { layout: "state-only", prefix: "openclaw-session-registry-maintenance-" }, - async (state) => { - resetConfigRuntimeState(); - const storePath = path.join(state.sessionsDir("main"), "sessions.json"); - const staleKey = "agent:main:cron:done-job:run:old-run"; - await replaceSessionEntry( - { sessionKey: staleKey, storePath }, - { sessionId: "old-run", updatedAt: Date.now() - 8 * 24 * 60 * 60_000 }, - ); + await withMaintenanceState(async (state) => { + const storePath = path.join(state.sessionsDir("main"), "sessions.json"); + const staleKey = "agent:main:cron:done-job:run:old-run"; + await replaceSessionEntry( + { sessionKey: staleKey, storePath }, + { sessionId: "old-run", updatedAt: Date.now() - 8 * DAY_MS }, + ); - const summary = await runSessionRegistryMaintenance({ apply: true }); + const summary = await runSessionRegistryMaintenance({ apply: true }); - expect(summary.skippedReason).toBeUndefined(); - expect(summary.pruned).toBe(1); - expect(loadSessionEntry({ sessionKey: staleKey, storePath })).toBeUndefined(); - }, - ); + expect(summary.skippedReason).toBeUndefined(); + expect(summary.pruned).toBe(1); + expect(loadSessionEntry({ sessionKey: staleKey, storePath })).toBeUndefined(); + }); + }); + + it.each([ + { apply: false, mainEntrySurvives: true }, + { apply: true, mainEntrySurvives: false }, + ])( + "reports completed agent deletions while maintaining healthy stores (apply=$apply)", + async ({ apply, mainEntrySurvives }) => { + await withMaintenanceState(async (state) => { + const mainStorePath = path.join(state.sessionsDir("main"), "sessions.json"); + const retiredStorePath = path.join(state.sessionsDir("retired"), "sessions.json"); + const mainKey = await writeStaleCronSession(mainStorePath, "main"); + await writeStaleCronSession(retiredStorePath, "retired"); + writeAgentDeletion(state, "retired", true); + closeOpenClawAgentDatabasesForTest(); + + const summary = await runSessionRegistryMaintenance({ apply }); + + expect(summary).toMatchObject({ + pruned: 1, + skippedStores: 1, + stores: expect.arrayContaining([ + expect.objectContaining({ agentId: "main", pruned: 1 }), + { + agentId: "retired", + storePath: retiredStorePath, + skippedReason: "agent-deletion-complete", + }, + ]), + }); + expect( + loadSessionEntry({ sessionKey: mainKey, storePath: mainStorePath }) !== undefined, + ).toBe(mainEntrySurvives); + if (!apply) { + const jsonRuntime = createRuntime(); + await tasksMaintenanceCommand({ json: true }, jsonRuntime); + expect(JSON.parse(String(vi.mocked(jsonRuntime.log).mock.calls[0]?.[0]))).toMatchObject({ + maintenance: { + sessions: { + skippedStores: 1, + stores: expect.arrayContaining([ + { + agentId: "retired", + storePath: retiredStorePath, + skippedReason: "agent-deletion-complete", + }, + ]), + }, + }, + }); + const textRuntime = createRuntime(); + await tasksMaintenanceCommand({}, textRuntime); + expect(vi.mocked(textRuntime.log).mock.calls.flat().join("\n")).toContain( + "1 skipped store", + ); + } + }); + }, + ); + + it("keeps incomplete agent deletions terminal", async () => { + await withMaintenanceState(async (state) => { + const retiredStorePath = path.join(state.sessionsDir("retired"), "sessions.json"); + await writeStaleCronSession(retiredStorePath, "retired"); + writeAgentDeletion(state, "retired", false); + closeOpenClawAgentDatabasesForTest(); + + await expect(runSessionRegistryMaintenance({ apply: false })).rejects.toThrow( + "OpenClaw agent database is unavailable while agent retired is deleted.", + ); + }); + }); + + it("keeps corrupt discovered stores terminal", async () => { + await withMaintenanceState(async (state) => { + const retiredStorePath = path.join(state.sessionsDir("retired"), "sessions.json"); + const sqlitePath = resolveSqliteTargetFromSessionStorePath(retiredStorePath).path; + if (!sqlitePath) { + throw new Error("expected retired store to resolve to SQLite"); + } + await fs.mkdir(path.dirname(sqlitePath), { recursive: true }); + await fs.writeFile(sqlitePath, "not a sqlite database"); + + await expect(runSessionRegistryMaintenance({ apply: false })).rejects.toThrow(); + }); }); }); diff --git a/src/commands/tasks-session-registry-maintenance.ts b/src/commands/tasks-session-registry-maintenance.ts index d0d16c9d3974..e63f97eb3c34 100644 --- a/src/commands/tasks-session-registry-maintenance.ts +++ b/src/commands/tasks-session-registry-maintenance.ts @@ -7,22 +7,31 @@ import { } from "../config/sessions.js"; import { loadCronJobsStoreSync, resolveCronJobsStorePath } from "../cron/store.js"; import { formatErrorMessage } from "../infra/errors.js"; +import { readAgentDeletionJournal } from "../state/agent-deletion-journal.js"; const SESSION_REGISTRY_RETENTION_MS = 7 * 24 * 60 * 60_000; -type SessionRegistryMaintenanceStoreSummary = { +type SessionRegistryMaintenanceStoreIdentity = { agentId: string; storePath: string; - beforeCount: number; - afterCount: number; - pruned: number; - preservedRunning: number; }; +type SessionRegistryMaintenanceStoreSummary = + | (SessionRegistryMaintenanceStoreIdentity & { + beforeCount: number; + afterCount: number; + pruned: number; + preservedRunning: number; + }) + | (SessionRegistryMaintenanceStoreIdentity & { + skippedReason: "agent-deletion-complete"; + }); + type SessionRegistryMaintenanceSummary = { retentionMs: number; runningCronJobs: number; pruned: number; + skippedStores: number; stores: SessionRegistryMaintenanceStoreSummary[]; /** Set when the sweep did not run; pruning without cron facts would archive live transcripts. */ skippedReason?: string; @@ -78,12 +87,20 @@ export async function runSessionRegistryMaintenance(params: { retentionMs: SESSION_REGISTRY_RETENTION_MS, runningCronJobs: 0, pruned: 0, + skippedStores: 0, stores: [], skippedReason: `cron store unreadable: ${runningCronJobs.reason}`, }; } const stores: SessionRegistryMaintenanceStoreSummary[] = []; for (const target of resolveAllAgentSessionStoreTargetsSync(cfg)) { + const deletion = readAgentDeletionJournal(target.agentId); + if (deletion?.cleanupCompleted) { + // Completed tombstones intentionally keep retired stores unavailable. + // Record that lifecycle outcome instead of reopening the fenced database. + stores.push({ ...target, skippedReason: "agent-deletion-complete" }); + continue; + } const result = await runSessionRegistryMaintenanceForStore({ apply: params.apply, retentionMs: SESSION_REGISTRY_RETENTION_MS, @@ -102,7 +119,8 @@ export async function runSessionRegistryMaintenance(params: { return { retentionMs: SESSION_REGISTRY_RETENTION_MS, runningCronJobs: runningCronJobs.count, - pruned: stores.reduce((total, store) => total + store.pruned, 0), + pruned: stores.reduce((total, store) => total + ("pruned" in store ? store.pruned : 0), 0), + skippedStores: stores.filter((store) => "skippedReason" in store).length, stores, }; } diff --git a/src/commands/tasks.ts b/src/commands/tasks.ts index 52f2ad39dc6a..136e836d3fd1 100644 --- a/src/commands/tasks.ts +++ b/src/commands/tasks.ts @@ -651,7 +651,7 @@ export async function tasksMaintenanceCommand( info( sessionMaintenance.skippedReason ? `Session registry: sweep skipped (${sessionMaintenance.skippedReason})` - : `Session registry: ${sessionMaintenance.pruned} prune · ${sessionMaintenance.runningCronJobs} running automations`, + : `Session registry: ${sessionMaintenance.pruned} prune · ${sessionMaintenance.runningCronJobs} running automations · ${sessionMaintenance.skippedStores} skipped ${sessionMaintenance.skippedStores === 1 ? "store" : "stores"}`, ), ); runtime.log(