fix: prevent excessive disk usage from repo backups (#235)

* fix: prevent excessive disk usage from repo backups (#234) Legacy configs with backupBeforeSync: true but no explicit backupStrategy silently resolved to "always", creating full git bundles on every sync cycle. This caused repo-backups to grow to 17GB+ for users with many repositories. Changes: - Fix resolveBackupStrategy to map backupBeforeSync: true → "on-force-push" instead of "always", so legacy configs only backup when force-push is detected - Fix config mapper to always set backupStrategy explicitly ("on-force-push") preventing the backward-compat fallback from triggering - Lower default backupRetentionCount from 20 to 5 bundles per repo - Add time-based retention (backupRetentionDays, default 30 days) alongside count-based retention, with safety net to always keep at least 1 bundle - Add "high disk usage" warning on "Always Backup" UI option - Update docs and tests to reflect new defaults and behavior * fix: preserve legacy backupBeforeSync:false on UI round-trip and expose retention days P1: mapDbToUiConfig now checks backupBeforeSync === false before defaulting backupStrategy, preventing legacy "disabled" configs from silently becoming "on-force-push" after any auto-save round-trip. P3: Added "Snapshot retention days" input field to the backup settings UI, matching the documented setting in FORCE_PUSH_PROTECTION.md.
2026-03-24 22:58:03 +03:00 · 2026-03-18 15:05:00 +05:30
parent 4629ab4335
commit ddd071f7e5
10 changed files with 94 additions and 23 deletions
--- a/docs/FORCE_PUSH_PROTECTION.md
+++ b/docs/FORCE_PUSH_PROTECTION.md
@@ -78,7 +78,11 @@ These appear when any non-disabled strategy is selected:

 ### Snapshot Retention Count

-How many backup snapshots to keep per repository. Oldest snapshots are deleted when this limit is exceeded. Default: **20**.
+How many backup snapshots to keep per repository. Oldest snapshots are deleted when this limit is exceeded. Default: **5**.
+
+### Snapshot Retention Days
+
+Maximum age (in days) for backup snapshots. Bundles older than this are deleted during retention enforcement, though at least one bundle is always kept. Set to `0` to disable time-based retention. Default: **30**.

 ### Snapshot Directory

@@ -96,7 +100,7 @@ The old `backupBeforeSync` boolean is still recognized:

 | Old Setting | New Equivalent |
 |---|---|
-| `backupBeforeSync: true` | `backupStrategy: "always"` |
+| `backupBeforeSync: true` | `backupStrategy: "on-force-push"` |
 | `backupBeforeSync: false` | `backupStrategy: "disabled"` |
 | Neither set | `backupStrategy: "on-force-push"` (new default) |

--- a/src/components/config/ConfigTabs.tsx
+++ b/src/components/config/ConfigTabs.tsx
@@ -51,7 +51,8 @@ export function ConfigTabs() {
      starredReposMode: 'dedicated-org',
      preserveOrgStructure: false,
      backupStrategy: "on-force-push",
-      backupRetentionCount: 20,
+      backupRetentionCount: 5,
+      backupRetentionDays: 30,
      backupDirectory: 'data/repo-backups',
      blockSyncOnBackupFailure: true,
    },
--- a/src/components/config/GitHubConfigForm.tsx
+++ b/src/components/config/GitHubConfigForm.tsx
@@ -234,7 +234,7 @@ export function GitHubConfigForm({
                  {
                    value: "always",
                    label: "Always Backup",
-                    desc: "Snapshot before every sync",
+                    desc: "Snapshot before every sync (high disk usage)",
                  },
                  {
                    value: "on-force-push",
@@ -272,7 +272,7 @@ export function GitHubConfigForm({

              {(giteaConfig.backupStrategy ?? "on-force-push") !== "disabled" && (
                <>
-                  <div className="grid grid-cols-1 md:grid-cols-2 gap-4">
+                  <div className="grid grid-cols-1 md:grid-cols-3 gap-4">
                    <div>
                      <label htmlFor="backup-retention" className="block text-sm font-medium mb-1.5">
                        Snapshot retention count
@@ -282,11 +282,11 @@ export function GitHubConfigForm({
                        name="backupRetentionCount"
                        type="number"
                        min={1}
-                        value={giteaConfig.backupRetentionCount ?? 20}
+                        value={giteaConfig.backupRetentionCount ?? 5}
                        onChange={(e) => {
                          const newConfig = {
                            ...giteaConfig,
-                            backupRetentionCount: Math.max(1, Number.parseInt(e.target.value, 10) || 20),
+                            backupRetentionCount: Math.max(1, Number.parseInt(e.target.value, 10) || 5),
                          };
                          setGiteaConfig(newConfig);
                          if (onGiteaAutoSave) onGiteaAutoSave(newConfig);
@@ -294,6 +294,28 @@ export function GitHubConfigForm({
                        className="w-full rounded-md border border-input bg-background px-3 py-2 text-sm shadow-sm transition-colors placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring"
                      />
                    </div>
+                    <div>
+                      <label htmlFor="backup-retention-days" className="block text-sm font-medium mb-1.5">
+                        Snapshot retention days
+                      </label>
+                      <input
+                        id="backup-retention-days"
+                        name="backupRetentionDays"
+                        type="number"
+                        min={0}
+                        value={giteaConfig.backupRetentionDays ?? 30}
+                        onChange={(e) => {
+                          const newConfig = {
+                            ...giteaConfig,
+                            backupRetentionDays: Math.max(0, Number.parseInt(e.target.value, 10) || 0),
+                          };
+                          setGiteaConfig(newConfig);
+                          if (onGiteaAutoSave) onGiteaAutoSave(newConfig);
+                        }}
+                        className="w-full rounded-md border border-input bg-background px-3 py-2 text-sm shadow-sm transition-colors placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring"
+                      />
+                      <p className="text-xs text-muted-foreground mt-1">0 = no time-based limit</p>
+                    </div>
                    <div>
                      <label htmlFor="backup-directory" className="block text-sm font-medium mb-1.5">
                        Snapshot directory
--- a/src/lib/db/schema.ts
+++ b/src/lib/db/schema.ts
@@ -75,7 +75,8 @@ export const giteaConfigSchema = z.object({
  mirrorMilestones: z.boolean().default(false),
  backupStrategy: backupStrategyEnum.default("on-force-push"),
  backupBeforeSync: z.boolean().default(true), // Deprecated: kept for backward compat, use backupStrategy
-  backupRetentionCount: z.number().int().min(1).default(20),
+  backupRetentionCount: z.number().int().min(1).default(5),
+  backupRetentionDays: z.number().int().min(0).default(30),
  backupDirectory: z.string().optional(),
  blockSyncOnBackupFailure: z.boolean().default(true),
 });
--- a/src/lib/gitea-enhanced.test.ts
+++ b/src/lib/gitea-enhanced.test.ts
@@ -575,7 +575,7 @@ describe("Enhanced Gitea Operations", () => {
          token: "encrypted-token",
          defaultOwner: "testuser",
          mirrorReleases: false,
-          backupBeforeSync: true,
+          backupStrategy: "always",
          blockSyncOnBackupFailure: true,
        },
      };
--- a/src/lib/repo-backup.test.ts
+++ b/src/lib/repo-backup.test.ts
@@ -162,8 +162,8 @@ describe("resolveBackupStrategy", () => {
    expect(resolveBackupStrategy(makeConfig({ backupStrategy: "block-on-force-push" }))).toBe("block-on-force-push");
  });

-  test("maps backupBeforeSync: true → 'always' (backward compat)", () => {
-    expect(resolveBackupStrategy(makeConfig({ backupBeforeSync: true }))).toBe("always");
+  test("maps backupBeforeSync: true → 'on-force-push' (backward compat, prevents silent always-backup)", () => {
+    expect(resolveBackupStrategy(makeConfig({ backupBeforeSync: true }))).toBe("on-force-push");
  });

  test("maps backupBeforeSync: false → 'disabled' (backward compat)", () => {
--- a/src/lib/repo-backup.ts
+++ b/src/lib/repo-backup.ts
@@ -65,13 +65,17 @@ async function runGit(args: string[], tokenToMask: string): Promise<void> {
  }
 }

-async function enforceRetention(repoBackupDir: string, keepCount: number): Promise<void> {
+async function enforceRetention(
+  repoBackupDir: string,
+  keepCount: number,
+  retentionDays: number = 0,
+): Promise<void> {
  const entries = await readdir(repoBackupDir);
  const bundleFiles = entries
    .filter((name) => name.endsWith(".bundle"))
    .map((name) => path.join(repoBackupDir, name));

-  if (bundleFiles.length <= keepCount) return;
+  if (bundleFiles.length === 0) return;

  const filesWithMtime = await Promise.all(
    bundleFiles.map(async (filePath) => ({
@@ -81,9 +85,33 @@ async function enforceRetention(repoBackupDir: string, keepCount: number): Promi
  );

  filesWithMtime.sort((a, b) => b.mtimeMs - a.mtimeMs);
-  const toDelete = filesWithMtime.slice(keepCount);

-  await Promise.all(toDelete.map((entry) => rm(entry.filePath, { force: true })));
+  const toDelete = new Set<string>();
+
+  // Count-based retention: keep only the N most recent
+  if (filesWithMtime.length > keepCount) {
+    for (const entry of filesWithMtime.slice(keepCount)) {
+      toDelete.add(entry.filePath);
+    }
+  }
+
+  // Time-based retention: delete bundles older than retentionDays
+  if (retentionDays > 0) {
+    const cutoffMs = Date.now() - retentionDays * 86_400_000;
+    for (const entry of filesWithMtime) {
+      if (entry.mtimeMs < cutoffMs) {
+        toDelete.add(entry.filePath);
+      }
+    }
+    // Always keep at least 1 bundle even if it's old
+    if (toDelete.size === filesWithMtime.length && filesWithMtime.length > 0) {
+      toDelete.delete(filesWithMtime[0].filePath);
+    }
+  }
+
+  if (toDelete.size > 0) {
+    await Promise.all([...toDelete].map((fp) => rm(fp, { force: true })));
+  }
 }

 export function isPreSyncBackupEnabled(): boolean {
@@ -126,9 +154,12 @@ export function resolveBackupStrategy(config: Partial<Config>): BackupStrategy {
  }

  // 2. Legacy backupBeforeSync boolean → map to strategy
+  //    Note: backupBeforeSync: true now maps to "on-force-push" (not "always")
+  //    because mappers default backupBeforeSync to true, causing every legacy config
+  //    to silently resolve to "always" and create full git bundles on every sync.
  const legacy = config.giteaConfig?.backupBeforeSync;
  if (legacy !== undefined) {
-    return legacy ? "always" : "disabled";
+    return legacy ? "on-force-push" : "disabled";
  }

  // 3. Env var (new)
@@ -251,7 +282,13 @@ export async function createPreSyncBundleBackup({
    1,
    Number.isFinite(config.giteaConfig?.backupRetentionCount)
      ? Number(config.giteaConfig?.backupRetentionCount)
-      : parsePositiveInt(process.env.PRE_SYNC_BACKUP_KEEP_COUNT, 20)
+      : parsePositiveInt(process.env.PRE_SYNC_BACKUP_KEEP_COUNT, 5)
+  );
+  const retentionDays = Math.max(
+    0,
+    Number.isFinite(config.giteaConfig?.backupRetentionDays)
+      ? Number(config.giteaConfig?.backupRetentionDays)
+      : parsePositiveInt(process.env.PRE_SYNC_BACKUP_RETENTION_DAYS, 30)
  );

  await mkdir(repoBackupDir, { recursive: true });
@@ -268,7 +305,7 @@ export async function createPreSyncBundleBackup({
    await runGit(["clone", "--mirror", authCloneUrl, mirrorClonePath], giteaToken);
    await runGit(["-C", mirrorClonePath, "bundle", "create", bundlePath, "--all"], giteaToken);

-    await enforceRetention(repoBackupDir, retention);
+    await enforceRetention(repoBackupDir, retention, retentionDays);
    return { bundlePath };
  } finally {
    await rm(tmpDir, { recursive: true, force: true });
--- a/src/lib/utils/config-defaults.ts
+++ b/src/lib/utils/config-defaults.ts
@@ -95,7 +95,8 @@ export async function createDefaultConfig({ userId, envOverrides = {} }: Default
      pullRequestConcurrency: 5,
      backupStrategy: "on-force-push",
      backupBeforeSync: true, // Deprecated: kept for backward compat
-      backupRetentionCount: 20,
+      backupRetentionCount: 5,
+      backupRetentionDays: 30,
      backupDirectory: "data/repo-backups",
      blockSyncOnBackupFailure: true,
    },
--- a/src/lib/utils/config-mapper.ts
+++ b/src/lib/utils/config-mapper.ts
@@ -101,9 +101,10 @@ export function mapUiToDbConfig(
    mirrorPullRequests: mirrorOptions.mirrorMetadata && mirrorOptions.metadataComponents.pullRequests,
    mirrorLabels: mirrorOptions.mirrorMetadata && mirrorOptions.metadataComponents.labels,
    mirrorMilestones: mirrorOptions.mirrorMetadata && mirrorOptions.metadataComponents.milestones,
-    backupStrategy: giteaConfig.backupStrategy,
+    backupStrategy: giteaConfig.backupStrategy || "on-force-push",
    backupBeforeSync: giteaConfig.backupBeforeSync ?? true,
-    backupRetentionCount: giteaConfig.backupRetentionCount ?? 20,
+    backupRetentionCount: giteaConfig.backupRetentionCount ?? 5,
+    backupRetentionDays: giteaConfig.backupRetentionDays ?? 30,
    backupDirectory: giteaConfig.backupDirectory?.trim() || undefined,
    blockSyncOnBackupFailure: giteaConfig.blockSyncOnBackupFailure ?? true,
  };
@@ -146,9 +147,12 @@ export function mapDbToUiConfig(dbConfig: any): {
    personalReposOrg: undefined, // Not stored in current schema
    issueConcurrency: dbConfig.giteaConfig?.issueConcurrency ?? 3,
    pullRequestConcurrency: dbConfig.giteaConfig?.pullRequestConcurrency ?? 5,
-    backupStrategy: dbConfig.giteaConfig?.backupStrategy || undefined,
+    backupStrategy: dbConfig.giteaConfig?.backupStrategy ||
+      // Respect legacy backupBeforeSync: false → "disabled" mapping on round-trip
+      (dbConfig.giteaConfig?.backupBeforeSync === false ? "disabled" : "on-force-push"),
    backupBeforeSync: dbConfig.giteaConfig?.backupBeforeSync ?? true,
-    backupRetentionCount: dbConfig.giteaConfig?.backupRetentionCount ?? 20,
+    backupRetentionCount: dbConfig.giteaConfig?.backupRetentionCount ?? 5,
+    backupRetentionDays: dbConfig.giteaConfig?.backupRetentionDays ?? 30,
    backupDirectory: dbConfig.giteaConfig?.backupDirectory || "data/repo-backups",
    blockSyncOnBackupFailure: dbConfig.giteaConfig?.blockSyncOnBackupFailure ?? true,
  };
--- a/src/types/config.ts
+++ b/src/types/config.ts
@@ -22,6 +22,7 @@ export interface GiteaConfig {
  backupStrategy?: BackupStrategy;
  backupBeforeSync?: boolean; // Deprecated: kept for backward compat, use backupStrategy
  backupRetentionCount?: number;
+  backupRetentionDays?: number;
  backupDirectory?: string;
  blockSyncOnBackupFailure?: boolean;
 }