diff --git a/docs/api-reference.md b/docs/api-reference.md index 82af1322..613f99a6 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1165,7 +1165,7 @@ Settings (synced, in `settings.json`): `configBackupEnabled` (absent = on), `con | `GET` | `/api/config-backups` | none | `settings` in effect, `dir`, `defaultDir`, `tracked[]` and `backups[]` (`id`, `createdAt`, `reason`, `appVersion`, `files[]`, `bytes`), newest first. | | `POST` | `/api/config-backups` | none | Snapshot now, even if nothing changed (`reason: manual`), then apply retention. `status` is `created` or `empty` (nothing tracked exists yet); `removed[]` lists the ids retention deleted. `503` when a custom folder does not answer. | | `GET` | `/api/config-backups/diff?from=&to=` | none | What differs between two states: `from`/`to` are a backup id or `current` (the running config; the default for `to`). `files[]` (`name`, `status` `added`/`removed`/`changed`/`unchanged`, `changes[]` of `path`, `kind`, `before`/`after`, `truncated`, `detail`) and `changed`. Credential values are never returned (`redacted: true`). `400` without `from`; `404` unknown id; `409` if a file fails its checksum. | -| `POST` | `/api/config-backups/:id/restore` | none | Put a snapshot's files back, atomically, after a `pre-restore` snapshot of the current ones. `404` unknown id; `409 CONFLICT` if a file fails its checksum (nothing changed). Returns `restored[]`, `untouched[]` (tracked files present now that the backup did not hold; left as they are), `safetySnapshot` (the `pre-restore` id) and `restartRequired[]` (files the running server only reads at start: `.env`, `push-keys.json`, `settings.json`). A file that cannot be written (a full disk, a directory in its place) does not stop the others: the answer is then `500 OPERATION_FAILED` with the same fields in `data` plus `failed[]` (`name`, `error`); the files listed in `restored[]` are the backup's and are reloaded like a full restore. | +| `POST` | `/api/config-backups/:id/restore` | none | Put a snapshot's files back, atomically, after a `pre-restore` snapshot of the current ones (none when nothing changed since the newest backup). `404` unknown id; `409 CONFLICT` if a file fails its checksum (nothing changed). Returns `restored[]`, `untouched[]` (tracked files present now that the backup did not hold; left as they are), `safetySnapshot` (the `pre-restore` id, or the newest backup's when nothing had changed since it) and `restartRequired[]` (files the running server only reads at start: `.env`, `push-keys.json`, `settings.json`). A file that cannot be written (a full disk, a directory in its place) does not stop the others: the answer is then `500 OPERATION_FAILED` with the same fields in `data` plus `failed[]` (`name`, `error`); the files listed in `restored[]` are the backup's and are reloaded like a full restore. | ## Webhook notifications diff --git a/docs/wiki/Config-Backups.md b/docs/wiki/Config-Backups.md index 3918af94..8ca07216 100644 --- a/docs/wiki/Config-Backups.md +++ b/docs/wiki/Config-Backups.md @@ -48,7 +48,8 @@ cron jobs. `intents.json` as unchanged when only prompts differ, yet a restore drops the prompts captured since. - **On demand**, with **Back up now** (always writes one, even if nothing changed). -- **Before a restore**, as a `pre-restore` safety copy of the files about to be replaced. +- **Before a restore**, as a `pre-restore` safety copy of the files about to be replaced (unless + nothing changed since the newest backup). ## Where they go and what they are called @@ -132,16 +133,17 @@ From the list, press **Restore** on a backup and confirm. Codeman then: mode for every file, so it would refuse every backup there). The symlink test is not. Compare applies the same checks to the backups it reads. 2. **Takes a `pre-restore` backup** of the files about to be replaced, so a restore can itself - be undone. + be undone. When nothing changed since the newest backup (the same comparison an automatic + backup makes), no copy is made and the response names that backup instead (`safetySnapshot`). 3. **Replaces each file atomically** (written beside it, then renamed) with mode `0600`. Only the tracked names are ever written, whatever a manifest lists. If a file cannot be written (a full disk, a folder where the file should be), the restore still writes the others and then reports an error that names the files it restored, the ones it could -not write, and the `pre-restore` backup that holds the files as they were before. The restored -files are in effect as after a full restore (Settings re-reads `settings.json` into the form), so -fix the cause and restore again, or restore the `pre-restore` backup to go back. +not write, and the backup that holds the files as they were before. The restored files are in +effect as after a full restore (Settings re-reads `settings.json` into the form), so fix the cause +and restore again, or restore that backup to go back. Restoring `settings.json` keeps the four backup settings (on or off, folder, how many, how many days) at their current values, so a restore from before a folder change does not point the list diff --git a/src/config-backup.ts b/src/config-backup.ts index 563f1c45..f9e315aa 100644 --- a/src/config-backup.ts +++ b/src/config-backup.ts @@ -654,15 +654,19 @@ export interface RestoreResult { restored: string[]; /** Tracked files present now that the snapshot did not hold; a restore leaves them as they are. */ untouched: string[]; - /** The snapshot taken of the live files before they were overwritten. */ + /** + * The snapshot holding the live files as they were before the restore: the `pre-restore` copy, or the + * newest backup when it already held exactly that state. Null only when no tracked file existed. + */ safetySnapshot: string | null; } /** * Put a snapshot's files back into the data dir. Every file is checked against the manifest's digest - * BEFORE anything is written, a `pre-restore` snapshot of the live files is taken first, and each - * file is replaced atomically (temp + rename) with mode 0600. A file that cannot be written does not - * stop the others; it throws {@link ConfigBackupPartialRestoreError} at the end, naming both lists. + * BEFORE anything is written, a `pre-restore` snapshot of the live files is taken first (unless the + * newest backup already holds that state, which is then reported instead), and each file is replaced + * atomically (temp + rename) with mode 0600. A file that cannot be written does not stop the others; + * it throws {@link ConfigBackupPartialRestoreError} at the end, naming both lists. */ export function restoreConfigBackup(opts: { dataDir: string; @@ -704,7 +708,8 @@ export function restoreConfigBackup(opts: { backupDir: opts.backupDir, reason: 'pre-restore', appVersion: opts.appVersion, - force: true, + // Not forced: when the live files equal the newest backup, that backup already is the undo copy, + // and a forced duplicate would spend a retention slot on nothing. now: opts.now, // The retention in force, so this newest snapshot does not hide it from the next pass. ...(latestBackupRetention(opts.backupDir, origin, existing) ?? {}), @@ -742,7 +747,7 @@ export function restoreConfigBackup(opts: { !failed.some((x) => x.name === n) && lstatSync(join(opts.dataDir, n), { throwIfNoEntry: false })?.isFile() ); - const safetySnapshot = safety.status === 'created' ? safety.id : null; + const safetySnapshot = safety.status === 'empty' ? null : safety.id; if (failed.length) throw new ConfigBackupPartialRestoreError(restored, failed, untouched, safetySnapshot); return { restored, untouched, safetySnapshot }; } diff --git a/test/config-backup-origin.test.ts b/test/config-backup-origin.test.ts index 4db940e5..bf6f26c4 100644 --- a/test/config-backup-origin.test.ts +++ b/test/config-backup-origin.test.ts @@ -51,6 +51,7 @@ describe('configBackupOrigin', () => { host.name = 'container-b'; // the container was recreated const origin = configBackupOrigin(data); expect(listConfigBackups(backups, origin)).toHaveLength(2); + writeFileSync(join(data, 'settings.json'), '{"a":2}'); // a change, so the restore takes a pre-restore copy expect(() => restoreConfigBackup({ dataDir: data, backupDir: backups, id: made.id })).not.toThrow(); const removed = pruneConfigBackups(backups, { keepCount: 1, keepDays: 0, origin }); expect(removed).toHaveLength(2); // 3 now: the restore took a pre-restore copy diff --git a/test/config-backup.test.ts b/test/config-backup.test.ts index fca4243f..2a7e1231 100644 --- a/test/config-backup.test.ts +++ b/test/config-backup.test.ts @@ -346,6 +346,19 @@ describe('restoreConfigBackup', () => { } }); + it('reuses the newest backup as the undo copy when the live files already equal it', () => { + put('settings.json', '{"a":1}'); + const first = createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 }) as { id: string }; + put('settings.json', '{"a":2}'); + const newest = createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 + DAY }) as { + id: string; + }; + const r = restoreConfigBackup({ dataDir: data, backupDir: backups, id: first.id, now: T0 + 2 * DAY }); + expect(r.safetySnapshot).toBe(newest.id); + expect(listConfigBackups(backups).map((b) => b.id)).toEqual([newest.id, first.id]); // no duplicate + expect(readFileSync(join(data, 'settings.json'), 'utf8')).toBe('{"a":1}'); + }); + it('a file it cannot write does not stop the others, and the error names both lists', () => { put('settings.json', '{"good":true}'); put('clis.json', '{"clis":{}}'); @@ -644,9 +657,12 @@ describe('restore trusts only what this module wrote', () => { const origin = configBackupOrigin(data); expect(latestBackupRetention(backups, origin)).toEqual({ keepCount: 100, keepDays: 0 }); const id = listConfigBackups(backups, origin).at(-1)!.id; + // A change since the newest backup, so the restore makes a pre-restore copy rather than reusing it. + put('settings.json', '{"a":3}'); const r = restoreConfigBackup({ dataDir: data, backupDir: backups, id, now: T0 + 2000 }); expect(r.safetySnapshot).toBeTruthy(); const pre = listConfigBackups(backups, origin).find((b) => b.id === r.safetySnapshot)!; + expect(pre.reason).toBe('pre-restore'); expect(JSON.parse(readFileSync(join(backups, pre.id, 'manifest.json'), 'utf8'))).toMatchObject({ keepCount: 100, keepDays: 0,