mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 02:29:33 +01:00
fix(config-backup): a restore that fails partway names what it restored and what failed
Merge-time fix for #661. A restore that could not write one file (a full disk, a directory where the file should be) used to stop there and answer a bare 500, naming neither the files it had already put back nor the pre-restore copy, and the Settings form was not re-read, so the next Save wrote the old settings back over a restored settings.json. restoreConfigBackup now writes every verified file it can, then throws ConfigBackupPartialRestoreError with restored, failed (name and error), untouched and safetySnapshot. The route runs the same reload hooks and audit for the files written as a full restore does, and answers 500 with the error envelope plus those fields (and restartRequired) in data, as the tab-layout 409 carries its layout. Settings refills the form when settings.json is among the restored files, also on that path. Tests: a unit case where a later file is still restored, the clis.json-as-a-directory route case, and two vm cases for the Settings handler. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
8a4ff775fb
commit
0952a2ee47
8 files changed
+196
-33
No files matched your search
@@ -9,9 +9,14 @@ import vm from 'node:vm';
|
||||
|
||||
const SRC = readFileSync(new URL('../src/web/public/settings-ui.js', import.meta.url), 'utf-8');
|
||||
|
||||
function makeApp(restored: string[], confirmed = true) {
|
||||
function makeApp(
|
||||
restored: string[],
|
||||
confirmed = true,
|
||||
reply: { ok: boolean; body: unknown } = { ok: true, body: { success: true, data: { restored, restartRequired: [] } } }
|
||||
) {
|
||||
const calls: string[] = [];
|
||||
const confirms: string[] = [];
|
||||
const toasts: Array<[string, string]> = [];
|
||||
function CodemanApp(this: unknown) {}
|
||||
const ctx = vm.createContext({
|
||||
CodemanApp,
|
||||
@@ -29,16 +34,13 @@ function makeApp(restored: string[], confirmed = true) {
|
||||
files: [{ name: 'settings.json' }, { name: 'users.json' }, { name: '.env' }, { name: 'clis.json' }],
|
||||
},
|
||||
],
|
||||
_apiPost: async () => ({
|
||||
ok: true,
|
||||
json: async () => ({ success: true, data: { restored, restartRequired: [] } }),
|
||||
}),
|
||||
_apiPost: async () => ({ ok: reply.ok, json: async () => reply.body }),
|
||||
loadAppSettingsFromServer: async () => void calls.push('reload-settings'),
|
||||
openAppSettings: () => void calls.push('refill-form'),
|
||||
loadConfigBackups: () => void calls.push('reload-list'),
|
||||
showToast: () => {},
|
||||
showToast: (m: string, type: string) => void toasts.push([m, type]),
|
||||
});
|
||||
return { app, calls, confirms };
|
||||
return { app, calls, confirms, toasts };
|
||||
}
|
||||
|
||||
describe('restoreConfigBackup (Settings UI)', () => {
|
||||
@@ -54,6 +56,38 @@ describe('restoreConfigBackup (Settings UI)', () => {
|
||||
expect(calls).toEqual(['reload-list']);
|
||||
});
|
||||
|
||||
it('a restore that failed partway still refills the form when settings.json was among the files written', async () => {
|
||||
const { app, calls, toasts } = makeApp([], true, {
|
||||
ok: false,
|
||||
body: {
|
||||
success: false,
|
||||
errorCode: 'OPERATION_FAILED',
|
||||
error: 'Restore incomplete: could not write clis.json (EISDIR); settings.json restored',
|
||||
data: {
|
||||
restored: ['settings.json'],
|
||||
failed: [{ name: 'clis.json', error: 'EISDIR' }],
|
||||
safetySnapshot: 'cfg-2',
|
||||
restartRequired: ['settings.json'],
|
||||
},
|
||||
},
|
||||
});
|
||||
await (app.restoreConfigBackup as (id: string) => Promise<void>)('cfg-1');
|
||||
expect(calls).toEqual(['reload-settings', 'refill-form', 'reload-list']);
|
||||
expect(toasts).toHaveLength(1);
|
||||
expect(toasts[0][1]).toBe('error');
|
||||
expect(toasts[0][0]).toContain('clis.json');
|
||||
});
|
||||
|
||||
it('a refused restore (nothing written) only reports the error', async () => {
|
||||
const { app, calls, toasts } = makeApp([], true, {
|
||||
ok: false,
|
||||
body: { success: false, errorCode: 'CONFLICT', error: 'settings.json does not match its checksum' },
|
||||
});
|
||||
await (app.restoreConfigBackup as (id: string) => Promise<void>)('cfg-1');
|
||||
expect(calls).toEqual([]);
|
||||
expect(toasts).toEqual([['settings.json does not match its checksum', 'error']]);
|
||||
});
|
||||
|
||||
it('names the files being replaced and calls out the credential ones', async () => {
|
||||
const { app, confirms } = makeApp(['settings.json'], false);
|
||||
await (app.restoreConfigBackup as (id: string) => Promise<void>)('cfg-1');
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
CONFIG_BACKUP_FILES,
|
||||
ConfigBackupCorruptError,
|
||||
ConfigBackupNotFoundError,
|
||||
ConfigBackupPartialRestoreError,
|
||||
createConfigBackup,
|
||||
defaultConfigBackupDir,
|
||||
diffConfigBackups,
|
||||
@@ -345,6 +346,30 @@ describe('restoreConfigBackup', () => {
|
||||
}
|
||||
});
|
||||
|
||||
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":{}}');
|
||||
put('webhook.json', '{"enabled":false}');
|
||||
const made = createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 }) as { id: string };
|
||||
put('settings.json', '{"polluted":true}');
|
||||
put('webhook.json', '{"enabled":true}');
|
||||
rmSync(join(data, 'clis.json'));
|
||||
mkdirSync(join(data, 'clis.json')); // between the other two in the tracked list
|
||||
let caught: unknown;
|
||||
try {
|
||||
restoreConfigBackup({ dataDir: data, backupDir: backups, id: made.id, now: T0 + 1000 });
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(ConfigBackupPartialRestoreError);
|
||||
const err = caught as ConfigBackupPartialRestoreError;
|
||||
expect(err.restored).toEqual(['settings.json', 'webhook.json']);
|
||||
expect(err.failed.map((f) => f.name)).toEqual(['clis.json']);
|
||||
expect(err.safetySnapshot).toBeTruthy();
|
||||
expect(readFileSync(join(data, 'webhook.json'), 'utf8')).toBe('{"enabled":false}');
|
||||
expect(readdirSync(data).filter((n) => n.includes('.restore-'))).toEqual([]); // no temp left behind
|
||||
});
|
||||
|
||||
it('only ever writes allowlisted names, even if a manifest lists others', () => {
|
||||
put('settings.json', '{"a":1}');
|
||||
const made = createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 }) as { id: string };
|
||||
|
||||
@@ -90,6 +90,36 @@ describe('/api/config-backups', () => {
|
||||
expect(JSON.parse(readFileSync(SETTINGS_PATH, 'utf8')).displayName).toBe('before');
|
||||
});
|
||||
|
||||
it('a restore that fails partway names what it restored, what failed and the pre-restore copy', async () => {
|
||||
const { app } = await createRouteTestHarness(registerConfigBackupRoutes);
|
||||
const clis = join(dirname(SETTINGS_PATH), 'clis.json');
|
||||
writeFileSync(clis, '{"schemaVersion":1,"clis":{}}', { mode: 0o600 });
|
||||
const id = (await app.inject({ method: 'POST', url: '/api/config-backups' })).json().data.id;
|
||||
writeFileSync(SETTINGS_PATH, JSON.stringify({ displayName: 'polluted' }));
|
||||
// A file the server cannot replace: settings.json (first in the list) goes back, clis.json cannot.
|
||||
rmSync(clis);
|
||||
mkdirSync(clis);
|
||||
try {
|
||||
const res = await app.inject({ method: 'POST', url: `/api/config-backups/${id}/restore` });
|
||||
expect(res.statusCode).toBe(500);
|
||||
const body = res.json();
|
||||
expect(body).toMatchObject({ success: false, errorCode: 'OPERATION_FAILED' });
|
||||
expect(body.error).toContain('clis.json');
|
||||
expect(body.data.restored).toContain('settings.json');
|
||||
expect(body.data.failed.map((f: { name: string }) => f.name)).toEqual(['clis.json']);
|
||||
expect(body.data.restartRequired).toContain('settings.json');
|
||||
expect(JSON.parse(readFileSync(SETTINGS_PATH, 'utf8')).displayName).toBe('before');
|
||||
// The undo copy exists, holds the state the restore replaced, and is the one named.
|
||||
const safety = body.data.safetySnapshot;
|
||||
expect(safety).toBeTruthy();
|
||||
expect(body.error).toContain(safety);
|
||||
const listed = (await app.inject({ method: 'GET', url: '/api/config-backups' })).json().data.backups;
|
||||
expect(listed.find((b: { id: string }) => b.id === safety)?.reason).toBe('pre-restore');
|
||||
} finally {
|
||||
rmSync(clis, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('a restored intents.json is not reverted by the next captured prompt', async () => {
|
||||
const { app } = await createRouteTestHarness(registerConfigBackupRoutes);
|
||||
const dir = mkdtempSync(join(tmpdir(), 'cfg-backup-intent-'));
|
||||
|
||||
Reference in new issue
Block a user