From 9c0fc30927bea3f6b2a049e15145e99fc0862758 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Tue, 20 Oct 2026 20:32:14 +0800 Subject: [PATCH] fix(config-backup): the retention fallback follows the folder, a clock-ahead snapshot cannot make every pass write a copy --- docs/api-reference.md | 2 +- docs/wiki/Config-Backups.md | 12 ++++--- src/config-backup.ts | 41 +++++++++++++----------- src/web/public/index.html | 4 +-- src/web/public/settings-ui.js | 10 ++++-- test/config-backup.test.ts | 34 +++++++++++++++++++- test/routes/config-backup-routes.test.ts | 33 +++++++++++++++++++ 7 files changed, 108 insertions(+), 28 deletions(-) diff --git a/docs/api-reference.md b/docs/api-reference.md index 45553419..e5e27151 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1158,7 +1158,7 @@ Timestamped snapshots of the files that hold what a user has customized (`src/co Tracked files (a fixed list, relative to the data dir): `settings.json`, `clis.json`, `custom-model-hosts.json`, `webhook.json`, `intents.json`, `linked-cases.json`, `remote-hosts.json`, `remote-cases.json`, `docker-hosts.json`, `docker-cases.json`, `webviews.json`, `push-keys.json`, `users.json`, `.env`. Session state (`state.json`, which also holds cron jobs and respawn presets), logs, the hook secret and caches are not. -Settings (synced, in `settings.json`): `configBackupEnabled` (absent = on), `configBackupDir` (absolute path, `~` expanded; empty = `/backups/config`), `configBackupKeepCount` (2–500, default 20), `configBackupKeepDays` (0–3650, default 30; `0` = no age limit). Retention only ever deletes snapshot folders Codeman made there, and never the newest one. Each snapshot is a `0700` folder named `cfg-YYYYMMDD-HHMMSS` (server local time; `-2`, `-3`… on a same-second collision) holding the tracked files under their own names plus a `manifest.json` (`id`, `createdAt`, `reason` of `startup`/`auto`/`manual`/`pre-restore`, `appVersion`, an `origin` id, and a SHA-256 and size per file), all `0600`. Only folders with that name and a valid manifest are listed, restored or pruned. Full guide: `docs/wiki/Config-Backups.md`. +Settings (synced, in `settings.json`): `configBackupEnabled` (absent = on), `configBackupDir` (absolute path, `~` expanded; empty = `/backups/config`), `configBackupKeepCount` (2–500, default 20), `configBackupKeepDays` (0–3650, default 30; `0` = no age limit). Retention only ever deletes snapshot folders Codeman made there, and never the newest one. Each snapshot is a `0700` folder named `cfg-YYYYMMDD-HHMMSS` (server local time; `-2`, `-3`… on a same-second collision) holding the tracked files under their own names plus a `manifest.json` (`id`, `createdAt`, `reason` of `startup`/`auto`/`manual`/`pre-restore`, `appVersion`, the change-check `hash`, an `origin` id, the `keepCount` and `keepDays` in force, and a SHA-256 and size per file), all `0600`. A snapshot is never dated earlier than the instance's newest one, so a clock that read ahead once does not make every later pass write a copy. All four routes answer `503 OPERATION_FAILED` when a custom folder does not respond, and diff and restore answer `409` for a symlinked or group/world-accessible backup as well as a checksum mismatch. Only folders with that name and a valid manifest are listed, restored or pruned. Full guide: `docs/wiki/Config-Backups.md`. | Method | Path | Body | Notes | | ------ | ------------------------------------ | ---- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | diff --git a/docs/wiki/Config-Backups.md b/docs/wiki/Config-Backups.md index f3dd9962..62736e73 100644 --- a/docs/wiki/Config-Backups.md +++ b/docs/wiki/Config-Backups.md @@ -61,7 +61,8 @@ Each backup is one folder named for the moment it was made, in the server's loca ``` ~/.codeman/backups/config/ cfg-20261011-120305/ - manifest.json id, createdAt (epoch ms), reason, appVersion, SHA-256 + size per file + manifest.json id, createdAt (epoch ms), reason, appVersion, hash, origin, + keepCount and keepDays in force, SHA-256 + size per file settings.json the original file names, unchanged clis.json custom-model-hosts.json @@ -73,7 +74,7 @@ Each backup is one folder named for the moment it was made, in the server's loca | Part | Meaning | | ---- | ------- | | `cfg-YYYYMMDD-HHMMSS` | Date and time (server local time) the backup was made. A collision in the same second gets `-2`, `-3`, … | -| `manifest.json` | The record of the backup: `reason`, `appVersion`, and a SHA-256 and size for every file. | +| `manifest.json` | The record of the backup: `reason`, `appVersion`, the change-check `hash`, the `origin` id, the retention in force (`keepCount`, `keepDays`), and a SHA-256 and size for every file. | | `reason` | `startup`, `auto` (a change was detected), `manual` (Back up now) or `pre-restore`. It is in the manifest and the Settings list, not in the folder name. | | `.tmp-…` | A backup being written. It is renamed into place when complete and is never listed. | @@ -193,9 +194,12 @@ query and fragment. network share copies those credentials there, with whatever permissions that location has. Choose a private location, or turn the feature off if you cannot. - **Retention survives a bad `settings.json`.** If it does not parse, a pass still takes its - snapshot but deletes nothing. If the four backup settings are missing (a reset file), the pass + snapshot but deletes nothing. If a retention setting is missing (a reset file), the pass prunes with the retention the newest backup that recorded one holds, not the 20 copies and 30 - days defaults, and Settings shows those values until you save. While `settings.json` does not + days defaults, and Settings shows those values. Saving sends a retention value only if it was + saved before or you edited it there, so saving other settings (or re-entering a custom folder + after a reset) does not write the displayed 20 and 30 over it; the first pass in the re-entered + folder finds the retention its own backups recorded. While `settings.json` does not parse, a pass reads it as empty, so its snapshot goes to the default folder even if a custom folder is set or backups are switched off; nothing is widened (same modes, inside the data directory). diff --git a/src/config-backup.ts b/src/config-backup.ts index c641af37..34b4ec86 100644 --- a/src/config-backup.ts +++ b/src/config-backup.ts @@ -208,13 +208,6 @@ export function readConfigBackupOrigin(dataDir: string): string | null { } } -/** True when any of the four `configBackup*` keys is present (the Settings form always sends all four). */ -export function hasConfigBackupSettings(raw: Record): boolean { - return ['configBackupEnabled', 'configBackupDir', 'configBackupKeepCount', 'configBackupKeepDays'].some( - (k) => k in raw - ); -} - const sha256 = (data: Buffer | string): string => createHash('sha256').update(data).digest('hex'); /** @@ -352,8 +345,12 @@ export function createConfigBackup(opts: CreateConfigBackupOptions): ConfigBacku if (latest && latest.hash === hash) return { status: 'unchanged', id: latest.id }; } + // Never dated earlier than this instance's newest snapshot. One written while the clock read ahead + // (a dual-boot machine, a VM resumed with a stale clock) would otherwise stay newest after the + // correction, differ from every later pass, and have each one write a copy until real time caught up. + const createdAt = existing[0] ? Math.max(now, existing[0].createdAt + 1) : now; mkdirSync(opts.backupDir, { recursive: true, mode: 0o700 }); - const base = `cfg-${stamp(now)}`; + const base = `cfg-${stamp(createdAt)}`; let id = base; for (let n = 2; existsSync(join(opts.backupDir, id)); n++) id = `${base}-${n}`; @@ -363,7 +360,7 @@ export function createConfigBackup(opts: CreateConfigBackupOptions): ConfigBacku try { const manifest: ConfigBackupManifest = { id, - createdAt: now, + createdAt, reason: opts.reason, appVersion: opts.appVersion ?? '', hash, @@ -415,9 +412,11 @@ export function latestBackupRetention(dir: string, origin: string): { keepCount: } /** - * The settings in effect when `raw` (the parsed settings.json) has none of the four `configBackup*` - * keys, as after a reset: the retention the newest backup recorded instead of the defaults. One - * place for the timer pass and for what the GET route reports, so they cannot disagree. + * The settings in effect, with each retention key that `raw` (the parsed settings.json) lacks, as + * after a reset, taken from what the newest backup recorded in `dir` instead of the defaults. It + * looks at the retention keys only (not the folder), so re-entering a custom folder after a reset + * finds that folder's own manifests. One place for the timer pass and for what the GET route + * reports, so they cannot disagree. */ export function withRecordedRetention( settings: ConfigBackupSettings, @@ -425,9 +424,16 @@ export function withRecordedRetention( dir: string, origin: string | null ): ConfigBackupSettings { - if (hasConfigBackupSettings(raw) || !origin) return settings; + const lacksCount = !('configBackupKeepCount' in raw); + const lacksDays = !('configBackupKeepDays' in raw); + if ((!lacksCount && !lacksDays) || !origin) return settings; const kept = latestBackupRetention(dir, origin); - return kept ? { ...settings, ...kept } : settings; + if (!kept) return settings; + return { + ...settings, + ...(lacksCount && { keepCount: kept.keepCount }), + ...(lacksDays && { keepDays: kept.keepDays }), + }; } export interface PruneOptions { @@ -523,11 +529,10 @@ function keepCurrentBackupSettings(snapshot: Buffer, livePath: string): Buffer { const live = JSON.parse(readFileSync(livePath, 'utf8')) as Record; if (!restored || typeof restored !== 'object' || !live || typeof live !== 'object') return snapshot; let changed = false; - // A reset live file has none of the four keys. Taking "absent" for the retention too would bring - // the defaults back (20 copies, 30 days) on the next pass, so the backup's own retention stays. - const liveHasSettings = hasConfigBackupSettings(live); + // A reset live file lacks the retention keys. Taking "absent" for them would bring the defaults + // back (20 copies, 30 days) on the next pass, so the backup's own retention stays. for (const key of BACKUP_SETTING_KEYS) { - if (!liveHasSettings && (key === 'configBackupKeepCount' || key === 'configBackupKeepDays')) continue; + if (!(key in live) && (key === 'configBackupKeepCount' || key === 'configBackupKeepDays')) continue; if (JSON.stringify(restored[key]) === JSON.stringify(live[key])) continue; changed = true; if (key in live) restored[key] = live[key]; diff --git a/src/web/public/index.html b/src/web/public/index.html index 55150e12..d2e9b155 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -3264,14 +3264,14 @@ Keep this many backups 2 to 500 (with 1, every change would delete the state before it). The newest backup is always kept. - +
Delete backups older than (days) A backup is removed once a newer one has existed this many days. 0 keeps backups until the count above pushes them out.
- +
diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 03d3e239..99662c23 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -2887,8 +2887,14 @@ Object.assign(CodemanApp.prototype, { mcpSyncEnabled: document.getElementById('appSettingsMcpSync').checked, configBackupEnabled: document.getElementById('appSettingsConfigBackup').checked, configBackupDir: document.getElementById('appSettingsConfigBackupDir').value.trim(), - configBackupKeepCount: Math.min(500, Math.max(2, parseInt(document.getElementById('appSettingsConfigBackupKeepCount').value, 10) || 20)), - configBackupKeepDays: Math.min(3650, Math.max(0, Number.isNaN(parseInt(document.getElementById('appSettingsConfigBackupKeepDays').value, 10)) ? 30 : parseInt(document.getElementById('appSettingsConfigBackupKeepDays').value, 10))), + // Sent only once saved before or edited here: a value merely displayed from the server (after a reset) + // would otherwise be written back and replace the retention the backups recorded. + ...(_prev.configBackupKeepCount !== undefined || document.getElementById('appSettingsConfigBackupKeepCount').dataset.touched + ? { configBackupKeepCount: Math.min(500, Math.max(2, parseInt(document.getElementById('appSettingsConfigBackupKeepCount').value, 10) || 20)) } + : {}), + ...(_prev.configBackupKeepDays !== undefined || document.getElementById('appSettingsConfigBackupKeepDays').dataset.touched + ? { configBackupKeepDays: Math.min(3650, Math.max(0, Number.isNaN(parseInt(document.getElementById('appSettingsConfigBackupKeepDays').value, 10)) ? 30 : parseInt(document.getElementById('appSettingsConfigBackupKeepDays').value, 10))) } + : {}), readMyMindEnabled: document.getElementById('appSettingsReadMyMind').checked, ultracodeFloatingWindows: document.getElementById('appSettingsUltracodeFloatingWindows').checked, showMultiMonitorButton: document.getElementById('appSettingsShowMultiMonitorButton').checked, diff --git a/test/config-backup.test.ts b/test/config-backup.test.ts index a045939e..2cbaa58e 100644 --- a/test/config-backup.test.ts +++ b/test/config-backup.test.ts @@ -465,6 +465,32 @@ describe('diffConfigBackups', () => { }); }); +describe('createConfigBackup: a snapshot dated ahead of the clock', () => { + it('is not written again on every pass once the clock is corrected', () => { + put('settings.json', '{"v":0}'); + createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 }); + put('settings.json', '{"v":1}'); + const ahead = createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 + 2 * 3_600_000 }); + expect(ahead.status).toBe('created'); + put('settings.json', '{"v":2}'); // the clock is back at T0 + 1 minute + const corrected = createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 + 60_000 }); + expect(corrected.status).toBe('created'); + const [newest, second] = listConfigBackups(backups); + expect(newest.id).toBe((corrected as { id: string }).id); // dated no earlier than the one before it + expect(newest.createdAt).toBeGreaterThan(second.createdAt); + for (let i = 0; i < 24; i++) { + const r = createConfigBackup({ + dataDir: data, + backupDir: backups, + reason: 'auto', + now: T0 + 120_000 + i * 300_000, + }); + expect(r.status, `pass ${i}`).toBe('unchanged'); + } + expect(listConfigBackups(backups)).toHaveLength(3); + }); +}); + describe('instances sharing one backup folder', () => { it('keeps to its own snapshots: no churn, no prune, no restore of the other', () => { const other = join(root, 'other-data'); @@ -608,6 +634,12 @@ describe('restore trusts only what this module wrote', () => { put('settings.json', JSON.stringify({ displayName: 'new', configBackupDir: '/new', configBackupKeepDays: 7 })); restoreConfigBackup({ dataDir: data, backupDir: backups, id }); const after = JSON.parse(readFileSync(join(data, 'settings.json'), 'utf8')); - expect(after).toEqual({ displayName: 'old', configBackupDir: '/new', configBackupKeepDays: 7 }); + // the folder and days follow the live file; the live file has no keep count (reset), so the backup's 5 stays + expect(after).toEqual({ + displayName: 'old', + configBackupDir: '/new', + configBackupKeepCount: 5, + configBackupKeepDays: 7, + }); }); }); diff --git a/test/routes/config-backup-routes.test.ts b/test/routes/config-backup-routes.test.ts index a87d71be..40dce415 100644 --- a/test/routes/config-backup-routes.test.ts +++ b/test/routes/config-backup-routes.test.ts @@ -250,6 +250,39 @@ describe('retention survives an unreadable or reset settings.json', () => { } ); + it("re-entering a custom folder after a reset finds that folder's recorded retention", async () => { + const folder = join(custom, 'kept'); + settings({ configBackupDir: folder, configBackupKeepCount: 100, configBackupKeepDays: 0 }); + for (let i = 0; i < 30; i++) { + createConfigBackup({ + dataDir: getDataDir(), + backupDir: folder, + reason: 'auto', + force: true, + now: BASE + i * DAY, + keepCount: 100, + keepDays: 0, + }); + } + writeFileSync(SETTINGS_PATH, '{}'); // the reset drops the folder too + await runConfigBackup('auto'); // goes to the default folder, which holds none of those manifests + // the user enters the folder again and saves; the form no longer sends retention it only displayed + writeFileSync(SETTINGS_PATH, JSON.stringify({ configBackupDir: folder, configBackupEnabled: true })); + const { app } = await createRouteTestHarness(registerConfigBackupRoutes); + const reported = (await app.inject({ method: 'GET', url: '/api/config-backups' })).json().data.settings; + expect(reported).toMatchObject({ keepCount: 100, keepDays: 0 }); + const out = await runConfigBackup('auto'); + expect(out?.removed).toEqual([]); + expect(listConfigBackups(folder).length).toBeGreaterThanOrEqual(30); + }); + + it('only the retention key the file lacks falls back to the recorded one', async () => { + seed(); + writeFileSync(SETTINGS_PATH, JSON.stringify({ configBackupKeepDays: 3650 })); + await runConfigBackup('auto'); + expect(listConfigBackups(dir())).toHaveLength(31); // keepCount 100 from the manifests, not the default 20 + }); + it('a settings.json that sets the retention still wins', async () => { seed(); writeFileSync(SETTINGS_PATH, JSON.stringify({ configBackupKeepCount: 5, configBackupKeepDays: 0 }));