fix(config): align session maintenance validation (#108046)

src/commands/doctor/shared/legacy-config-migrations.runtime.session.test.ts

@@ -1,316 +1,74 @@
-// Session migration tests cover doctor legacy config migration for zero-duration
-// pruneAfter and resetArchiveRetention values.
 import { describe, expect, it } from "vitest";
-import { LEGACY_CONFIG_MIGRATIONS_RUNTIME_SESSION } from "./legacy-config-migrations.runtime.session.js";
+import { findLegacyConfigIssues } from "../../../config/legacy.js";
+import { applyLegacyDoctorMigrations } from "./legacy-config-compat.js";
 
-function getMigration() {
-  const m = LEGACY_CONFIG_MIGRATIONS_RUNTIME_SESSION.find(
-    (candidate) => candidate.id === "session.maintenance.resetArchiveRetention-zero",
-  );
-  if (!m) {
-    throw new Error("migration not found");
-  }
-  return m;
+const ZERO_DURATIONS = [0, "0", "0ms", "0s", "0m", "0h", "0d", "0.0h", "0h0m"];
+
+type MaintenanceKey = "pruneAfter" | "resetArchiveRetention";
+
+function configWith(key: MaintenanceKey, value: unknown) {
+  return { session: { maintenance: { [key]: value } } };
 }
 
-function getRule(id: string) {
-  const rules = getMigration().legacyRules;
-  if (!rules) {
-    throw new Error("legacyRules missing");
-  }
-  // Two rules share the same path; distinguish by matching the message prefix.
-  const found = rules.find((candidate) => candidate.message.includes(id));
-  if (!found) {
-    throw new Error(`rule not found: ${id}`);
-  }
-  return found;
-}
+describe.each([
+  { key: "pruneAfter" as const, outcome: "30d" },
+  { key: "resetArchiveRetention" as const, outcome: "keep-by-default" },
+])("session.maintenance.$key zero-duration migration", ({ key, outcome }) => {
+  it.each(ZERO_DURATIONS)("detects and removes %s", (value) => {
+    const raw = configWith(key, value);
+    const issues = findLegacyConfigIssues(raw);
 
-const pruneAfterRule = getRule("pruneAfter");
-const resetArchiveRetentionRule = getRule("resetArchiveRetention");
-const migration = getMigration();
+    expect(issues.some((issue) => issue.message.includes(key))).toBe(true);
 
-describe("session.maintenance zero-duration migration", () => {
-  // ── pruneAfter rule match ──────────────────────────────────
-
-  describe("pruneAfter rule", () => {
-    it("detects literal zero strings", () => {
-      expect(pruneAfterRule.match?.({ pruneAfter: "0h" }, {} as Record<string, unknown>)).toBe(
-        true,
-      );
-      expect(pruneAfterRule.match?.({ pruneAfter: "0d" }, {} as Record<string, unknown>)).toBe(
-        true,
-      );
-      expect(pruneAfterRule.match?.({ pruneAfter: "0ms" }, {} as Record<string, unknown>)).toBe(
-        true,
-      );
-    });
-
-    it("detects bare-number zero", () => {
-      expect(pruneAfterRule.match?.({ pruneAfter: "0" }, {} as Record<string, unknown>)).toBe(true);
-    });
-
-    it("detects zero in other units", () => {
-      expect(pruneAfterRule.match?.({ pruneAfter: "0s" }, {} as Record<string, unknown>)).toBe(
-        true,
-      );
-      expect(pruneAfterRule.match?.({ pruneAfter: "0m" }, {} as Record<string, unknown>)).toBe(
-        true,
-      );
-    });
-
-    it("detects decimal and composite zero", () => {
-      expect(pruneAfterRule.match?.({ pruneAfter: "0.0h" }, {} as Record<string, unknown>)).toBe(
-        true,
-      );
-      expect(pruneAfterRule.match?.({ pruneAfter: "0h0m" }, {} as Record<string, unknown>)).toBe(
-        true,
-      );
-    });
-
-    it("detects numeric zero", () => {
-      expect(pruneAfterRule.match?.({ pruneAfter: 0 }, {} as Record<string, unknown>)).toBe(true);
-    });
-
-    it("does not match positive durations", () => {
-      expect(pruneAfterRule.match?.({ pruneAfter: "30d" }, {} as Record<string, unknown>)).toBe(
-        false,
-      );
-      expect(pruneAfterRule.match?.({ pruneAfter: "24h" }, {} as Record<string, unknown>)).toBe(
-        false,
-      );
-      expect(pruneAfterRule.match?.({ pruneAfter: 30 }, {} as Record<string, unknown>)).toBe(false);
-    });
-
-    it("does not match missing or unparseable", () => {
-      expect(pruneAfterRule.match?.({}, {} as Record<string, unknown>)).toBe(false);
-      expect(pruneAfterRule.match?.({ pruneAfter: "abc" }, {} as Record<string, unknown>)).toBe(
-        false,
-      );
-    });
-
-    it("detects zero pruneAfter even with positive resetArchiveRetention", () => {
-      expect(
-        pruneAfterRule.match?.(
-          { resetArchiveRetention: "30d", pruneAfter: "0h" },
-          {} as Record<string, unknown>,
-        ),
-      ).toBe(true);
-    });
-
-    it("message mentions pruneAfter and 30d default", () => {
-      expect(pruneAfterRule.message).toContain("pruneAfter");
-      expect(pruneAfterRule.message).toContain("30d");
-    });
+    const result = applyLegacyDoctorMigrations(raw);
+    expect(result.next?.session).toEqual({ maintenance: {} });
+    expect(result.changes).toHaveLength(1);
+    expect(result.changes[0]).toContain(key);
+    expect(result.changes[0]).toContain(outcome);
+    expect(applyLegacyDoctorMigrations(result.next)).toEqual({ next: null, changes: [] });
   });
 
-  // ── resetArchiveRetention rule match ───────────────────────
-
-  describe("resetArchiveRetention rule", () => {
-    it("detects literal zero strings", () => {
-      expect(
-        resetArchiveRetentionRule.match?.(
-          { resetArchiveRetention: "0h" },
-          {} as Record<string, unknown>,
-        ),
-      ).toBe(true);
-      expect(
-        resetArchiveRetentionRule.match?.(
-          { resetArchiveRetention: "0d" },
-          {} as Record<string, unknown>,
-        ),
-      ).toBe(true);
-    });
-
-    it("detects bare-number zero", () => {
-      expect(
-        resetArchiveRetentionRule.match?.(
-          { resetArchiveRetention: "0" },
-          {} as Record<string, unknown>,
-        ),
-      ).toBe(true);
-    });
-
-    it("detects numeric zero", () => {
-      expect(
-        resetArchiveRetentionRule.match?.(
-          { resetArchiveRetention: 0 },
-          {} as Record<string, unknown>,
-        ),
-      ).toBe(true);
-    });
-
-    it("does not match positive or false", () => {
-      expect(
-        resetArchiveRetentionRule.match?.(
-          { resetArchiveRetention: "30d" },
-          {} as Record<string, unknown>,
-        ),
-      ).toBe(false);
-      expect(
-        resetArchiveRetentionRule.match?.(
-          { resetArchiveRetention: false },
-          {} as Record<string, unknown>,
-        ),
-      ).toBe(false);
-    });
-
-    it("does not match missing or unparseable", () => {
-      expect(resetArchiveRetentionRule.match?.({}, {} as Record<string, unknown>)).toBe(false);
-      expect(
-        resetArchiveRetentionRule.match?.(
-          { resetArchiveRetention: "abc" },
-          {} as Record<string, unknown>,
-        ),
-      ).toBe(false);
-    });
-
-    it("message mentions resetArchiveRetention and the keep default", () => {
-      expect(resetArchiveRetentionRule.message).toContain("resetArchiveRetention");
-      expect(resetArchiveRetentionRule.message).toContain("keep-by-default");
-    });
-
-    it("does not match when only pruneAfter is present", () => {
-      expect(
-        resetArchiveRetentionRule.match?.({ pruneAfter: "0h" }, {} as Record<string, unknown>),
-      ).toBe(false);
-    });
+  it.each(["500ms", "24h", "30d", 30])("preserves positive duration %s", (value) => {
+    const raw = configWith(key, value);
+    expect(findLegacyConfigIssues(raw).some((issue) => issue.message.includes(key))).toBe(false);
+    expect(applyLegacyDoctorMigrations(raw)).toEqual({ next: null, changes: [] });
   });
 
-  // ── Migration apply (doctor --fix) ─────────────────────────
-
-  describe("apply", () => {
-    it("removes zero-duration resetArchiveRetention and reports the keep default", () => {
-      for (const val of ["0h", "0d", "0ms", "0", "0s", "0m", "0.0h", "0h0m"]) {
-        const changes: string[] = [];
-        const raw = { session: { maintenance: { resetArchiveRetention: val } } };
-        migration.apply(raw, changes);
-
-        expect(raw.session?.maintenance).not.toHaveProperty("resetArchiveRetention");
-        expect(changes).toHaveLength(1);
-        expect(changes[0]).toContain("resetArchiveRetention");
-        expect(changes[0]).toContain(val);
-        expect(changes[0]).toContain("keep-by-default");
-      }
-    });
-
-    it("removes zero-duration pruneAfter and reports change", () => {
-      for (const val of ["0h", "0d", "0ms", "0", "0s", "0m"]) {
-        const changes: string[] = [];
-        const raw = { session: { maintenance: { pruneAfter: val } } };
-        migration.apply(raw, changes);
-
-        expect(raw.session?.maintenance).not.toHaveProperty("pruneAfter");
-        expect(changes).toHaveLength(1);
-        expect(changes[0]).toContain("pruneAfter");
-        expect(changes[0]).toContain(val);
-        expect(changes[0]).toContain("30d");
-      }
-    });
-
-    it("removes numeric zero values for both fields", () => {
-      // pruneAfter: 30d
-      {
-        const changes: string[] = [];
-        const raw = { session: { maintenance: { pruneAfter: 0 } } };
-        migration.apply(raw, changes);
-        expect(changes[0]).toContain("30d");
-      }
-      // resetArchiveRetention: keep-by-default
-      {
-        const changes: string[] = [];
-        const raw = { session: { maintenance: { resetArchiveRetention: 0 } } };
-        migration.apply(raw, changes);
-        expect(raw.session?.maintenance).not.toHaveProperty("resetArchiveRetention");
-        expect(changes[0]).toContain("keep-by-default");
-      }
-    });
-
-    it("preserves positive durations", () => {
-      for (const val of ["30d", "7d", "500ms"]) {
-        const changes: string[] = [];
-        const raw = { session: { maintenance: { resetArchiveRetention: val } } };
-        migration.apply(raw, changes);
-        expect((raw.session as Record<string, unknown>)?.maintenance).toEqual({
-          resetArchiveRetention: val,
-        });
-        expect(changes).toHaveLength(0);
-      }
-    });
-
-    it("preserves false (documented disable)", () => {
-      const changes: string[] = [];
-      const raw = { session: { maintenance: { resetArchiveRetention: false } } };
-      migration.apply(raw, changes);
-      expect((raw.session as Record<string, unknown>)?.maintenance).toEqual({
-        resetArchiveRetention: false,
-      });
-      expect(changes).toHaveLength(0);
-    });
-
-    it("removes only zero pruneAfter when resetArchiveRetention is positive", () => {
-      const changes: string[] = [];
-      const raw = {
-        session: { maintenance: { resetArchiveRetention: "30d", pruneAfter: "0h" } },
-      };
-      migration.apply(raw, changes);
-
-      expect(raw.session?.maintenance).not.toHaveProperty("pruneAfter");
-      expect(raw.session?.maintenance).toHaveProperty("resetArchiveRetention", "30d");
-      expect(changes).toHaveLength(1);
-      expect(changes[0]).toContain("pruneAfter");
-      expect(changes[0]).toContain("30d");
-    });
-
-    it("removes both fields when both are zero", () => {
-      const changes: string[] = [];
-      const raw = {
-        session: { maintenance: { resetArchiveRetention: "0h", pruneAfter: 0 } },
-      };
-      migration.apply(raw, changes);
-
-      expect(raw.session?.maintenance).not.toHaveProperty("resetArchiveRetention");
-      expect(raw.session?.maintenance).not.toHaveProperty("pruneAfter");
-      expect(changes).toHaveLength(2);
-      expect(changes[0]).toContain("resetArchiveRetention");
-      expect(changes[0]).toContain("keep-by-default");
-      expect(changes[1]).toContain("pruneAfter");
-      expect(changes[1]).toContain("30d");
-    });
-
-    it("handles empty maintenance section", () => {
-      const changes: string[] = [];
-      const raw = { session: { maintenance: {} } };
-      migration.apply(raw, changes);
-      expect((raw.session as Record<string, unknown>)?.maintenance).toEqual({});
-      expect(changes).toHaveLength(0);
-    });
-
-    it("does nothing when session/maintenance is not an object", () => {
-      for (const raw of [
-        { session: "not-an-object" },
-        { session: { maintenance: "not-an-object" } },
-      ]) {
-        const changes: string[] = [];
-        migration.apply(raw, changes);
-        expect(changes).toHaveLength(0);
-      }
-    });
-
-    // ── Rule re-applies after migration ──────────────────────
-
-    it("pruneAfter rule no longer matches after apply removed it", () => {
-      const raw = { session: { maintenance: { pruneAfter: "0h" } } };
-      expect(pruneAfterRule.match?.(raw.session.maintenance, raw)).toBe(true);
-      migration.apply(raw, []);
-      expect(pruneAfterRule.match?.(raw.session?.maintenance, raw)).toBe(false);
-    });
-
-    it("resetArchiveRetention rule no longer matches after apply removed it", () => {
-      const raw = { session: { maintenance: { resetArchiveRetention: "0h" } } };
-      expect(resetArchiveRetentionRule.match?.(raw.session.maintenance, raw)).toBe(true);
-      migration.apply(raw, []);
-      expect(resetArchiveRetentionRule.match?.(raw.session?.maintenance, raw)).toBe(false);
-    });
+  it("leaves invalid values for schema validation", () => {
+    const raw = configWith(key, "invalid");
+    expect(findLegacyConfigIssues(raw).some((issue) => issue.message.includes(key))).toBe(false);
+    expect(applyLegacyDoctorMigrations(raw)).toEqual({ next: null, changes: [] });
+  });
+});
+
+describe("session maintenance zero-duration migration interactions", () => {
+  it("preserves the documented reset archive disable value", () => {
+    const raw = configWith("resetArchiveRetention", false);
+    expect(findLegacyConfigIssues(raw)).toEqual([]);
+    expect(applyLegacyDoctorMigrations(raw)).toEqual({ next: null, changes: [] });
+  });
+
+  it("removes both zero durations in one pass", () => {
+    const raw = {
+      session: { maintenance: { pruneAfter: 0, resetArchiveRetention: "0h" } },
+    };
+    const result = applyLegacyDoctorMigrations(raw);
+
+    expect(result.next?.session).toEqual({ maintenance: {} });
+    expect(result.changes).toHaveLength(2);
+    expect(result.changes.join("\n")).toContain("30d");
+    expect(result.changes.join("\n")).toContain("keep-by-default");
+  });
+
+  it("removes only the zero field", () => {
+    const raw = {
+      session: { maintenance: { pruneAfter: "0h", resetArchiveRetention: "30d" } },
+    };
+    const result = applyLegacyDoctorMigrations(raw);
+
+    expect(result.next?.session).toEqual({
+      maintenance: { resetArchiveRetention: "30d" },
+    });
+    expect(result.changes).toHaveLength(1);
   });
 });

src/commands/doctor/shared/legacy-config-migrations.runtime.session.ts

@@ -34,20 +34,12 @@ function isZeroDuration(val: unknown): boolean {
   }
 }
 
-function isZeroDurationPruneAfter(raw: unknown): boolean {
+function hasZeroDuration(raw: unknown, key: "pruneAfter" | "resetArchiveRetention"): boolean {
   const maintenance = getRecord(raw);
-  if (!maintenance || !Object.hasOwn(maintenance, "pruneAfter")) {
+  if (!maintenance || !Object.hasOwn(maintenance, key)) {
     return false;
   }
-  return isZeroDuration(maintenance.pruneAfter);
-}
-
-function isZeroDurationResetArchiveRetention(raw: unknown): boolean {
-  const maintenance = getRecord(raw);
-  if (!maintenance || !Object.hasOwn(maintenance, "resetArchiveRetention")) {
-    return false;
-  }
-  return isZeroDuration(maintenance.resetArchiveRetention);
+  return isZeroDuration(maintenance[key]);
 }
 
 const LEGACY_SESSION_MAINTENANCE_ROTATE_BYTES_RULE: LegacyConfigRule = {
@@ -68,14 +60,14 @@ const SESSION_MAINTENANCE_PRUNE_AFTER_ZERO_RULE: LegacyConfigRule = {
   path: ["session", "maintenance"],
   message:
     'session.maintenance.pruneAfter is a zero duration — this causes immediate deletion of eligible stale/non-preserved session entries. Run "openclaw doctor --fix" to remove it so the documented 30d default applies.',
-  match: isZeroDurationPruneAfter,
+  match: (raw) => hasZeroDuration(raw, "pruneAfter"),
 };
 
 const SESSION_MAINTENANCE_RESET_ARCHIVE_RETENTION_ZERO_RULE: LegacyConfigRule = {
   path: ["session", "maintenance"],
   message:
     'session.maintenance.resetArchiveRetention is a zero duration — this causes immediate deletion of all reset transcript archives. Run "openclaw doctor --fix" to remove it so the keep-by-default archive retention applies.',
-  match: isZeroDurationResetArchiveRetention,
+  match: (raw) => hasZeroDuration(raw, "resetArchiveRetention"),
 };
 
 /** Legacy config migration specs for session runtime config compatibility. */
@@ -107,7 +99,7 @@ export const LEGACY_CONFIG_MIGRATIONS_RUNTIME_SESSION: LegacyConfigMigrationSpec
     },
   }),
   defineLegacyConfigMigration({
-    id: "session.maintenance.resetArchiveRetention-zero",
+    id: "session.maintenance.zero-duration-retention",
     describe: "Remove zero-duration session maintenance values so documented defaults apply",
     legacyRules: [
       SESSION_MAINTENANCE_PRUNE_AFTER_ZERO_RULE,

src/config/types.base.ts

@@ -257,7 +257,7 @@ export type SessionMaintenanceMode = "enforce" | "warn";
 
 /** Session-store cleanup policy for transcript count, age, archives, and disk budget. */
 export type SessionMaintenanceConfig = {
-  /** Whether to enforce maintenance or warn only. Default: "warn". */
+  /** Whether to enforce maintenance or warn only. Default: "enforce". */
   mode?: SessionMaintenanceMode;
   /** Remove session entries older than this duration (e.g. "30d", "12h"). Default: "30d". */
   pruneAfter?: string | number;

src/config/zod-schema.session-maintenance-extensions.test.ts

@@ -60,6 +60,15 @@ describe("SessionSchema maintenance extensions", () => {
     expect(result.success).toBe(true);
   });
 
+  it("accepts disabling the session disk budget", () => {
+    const result = SessionSchema.safeParse({
+      maintenance: {
+        maxDiskBytes: false,
+      },
+    });
+    expect(result.success).toBe(true);
+  });
+
   it("rejects invalid maintenance extension values", () => {
     expect(() =>
       SessionSchema.parse({

src/config/zod-schema.session.ts

@@ -24,6 +24,25 @@ const SessionResetConfigSchema = z
   })
   .strict();
 
+const PositiveDurationSchema = z.union([z.string(), z.number()]).superRefine((value, ctx) => {
+  try {
+    const ms = parseDurationMs(normalizeStringifiedOptionalString(value) ?? "", {
+      defaultUnit: "d",
+    });
+    if (ms <= 0) {
+      ctx.addIssue({
+        code: z.ZodIssueCode.custom,
+        message: "duration must be positive (use ms, s, m, h, d), e.g. 30d",
+      });
+    }
+  } catch {
+    ctx.addIssue({
+      code: z.ZodIssueCode.custom,
+      message: "invalid duration (use ms, s, m, h, d)",
+    });
+  }
+});
+
 export const SessionSendPolicySchema = createAllowDenyChannelRulesSchema();
 
 export const SessionSchema = z
@@ -84,58 +103,17 @@ export const SessionSchema = z
     maintenance: z
       .object({
         mode: z.enum(["enforce", "warn"]).optional(),
-        pruneAfter: z.union([z.string(), z.number()]).optional(),
+        pruneAfter: PositiveDurationSchema.optional(),
         /** @deprecated Use pruneAfter instead. */
         pruneDays: z.number().int().positive().optional(),
         maxEntries: z.number().int().positive().optional(),
-        resetArchiveRetention: z.union([z.string(), z.number(), z.literal(false)]).optional(),
-        maxDiskBytes: z.union([z.string(), z.number()]).optional(),
+        resetArchiveRetention: z.union([PositiveDurationSchema, z.literal(false)]).optional(),
+        maxDiskBytes: z.union([z.string(), z.number(), z.literal(false)]).optional(),
         highWaterBytes: z.union([z.string(), z.number()]).optional(),
       })
       .strict()
       .superRefine((val, ctx) => {
-        if (val.pruneAfter !== undefined) {
-          try {
-            const ms = parseDurationMs(normalizeStringifiedOptionalString(val.pruneAfter) ?? "", {
-              defaultUnit: "d",
-            });
-            if (ms <= 0) {
-              ctx.addIssue({
-                code: z.ZodIssueCode.custom,
-                path: ["pruneAfter"],
-                message: "duration must be positive (use ms, s, m, h, d), e.g. 30d",
-              });
-            }
-          } catch {
-            ctx.addIssue({
-              code: z.ZodIssueCode.custom,
-              path: ["pruneAfter"],
-              message: "invalid duration (use ms, s, m, h, d)",
-            });
-          }
-        }
-        if (val.resetArchiveRetention !== undefined && val.resetArchiveRetention !== false) {
-          try {
-            const ms = parseDurationMs(
-              normalizeStringifiedOptionalString(val.resetArchiveRetention) ?? "",
-              { defaultUnit: "d" },
-            );
-            if (ms <= 0) {
-              ctx.addIssue({
-                code: z.ZodIssueCode.custom,
-                path: ["resetArchiveRetention"],
-                message: "duration must be positive (use ms, s, m, h, d), e.g. 30d",
-              });
-            }
-          } catch {
-            ctx.addIssue({
-              code: z.ZodIssueCode.custom,
-              path: ["resetArchiveRetention"],
-              message: "invalid duration (use ms, s, m, h, d)",
-            });
-          }
-        }
-        if (val.maxDiskBytes !== undefined) {
+        if (val.maxDiskBytes !== undefined && val.maxDiskBytes !== false) {
           try {
             parseByteSize(normalizeStringifiedOptionalString(val.maxDiskBytes) ?? "", {
               defaultUnit: "b",