From aeadde5f4046e06dbde0adb3156f4391450ebb5a Mon Sep 17 00:00:00 2001
From: Amit Haridas <amit.wh@gmail.com>
Date: Mon, 8 Jun 2026 07:02:45 +0530
Subject: [PATCH] fix(migration): handle already-v5 settings and ensure
 'failed' branch writes v5 marker

---
 src/main/updater/migration-runner.js             |  4 ++++
 src/main/updater/migration-transform.js          | 15 +++++++++++++++
 src/renderer/lib/migrations/v4-to-v5.ts          |  9 +++++++++
 tests/unit/main/updater/migration-runner.test.js | 10 +++++++++-
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/src/main/updater/migration-runner.js b/src/main/updater/migration-runner.js
index 9f082a3..c1c8075 100644
--- a/src/main/updater/migration-runner.js
+++ b/src/main/updater/migration-runner.js
@@ -25,6 +25,10 @@ class MigrationRunner {
       return 'migrated';
     } catch (err) {
       console.error('[migration-runner] transform failed:', err.message);
+      // Back up the original and write v5 marker so future launches skip migration.
+      // Without this, every launch would fail again and the user stays on defaults.
+      fs.copyFileSync(this.file, this.backup);
+      fs.writeFileSync(this.file, JSON.stringify({ ...raw, 'migration.version': 5 }, null, 2));
       return 'failed';
     }
   }
diff --git a/src/main/updater/migration-transform.js b/src/main/updater/migration-transform.js
index 74a2774..0753c2a 100644
--- a/src/main/updater/migration-transform.js
+++ b/src/main/updater/migration-transform.js
@@ -11,6 +11,15 @@ const v4SettingsSchema = z.object({
   snippets: z.array(z.unknown()).default([]),
 }).passthrough();
 
+const v5OnlyFields = ['updateChannel', 'autoCheckUpdates', 'firstRun'];
+
+function isAlreadyV5(data) {
+  if (!data || typeof data !== 'object') return false;
+  if (data['migration.version'] === 5) return true;
+  // Check for v5-only fields — a v4 file would never have these
+  return v5OnlyFields.some((f) => f in data);
+}
+
 const v5SettingsShape = {
   fontSize: 14,
   tabSize: 4,
@@ -39,6 +48,12 @@ const v5SettingsShape = {
 };
 
 function migrateV4ToV5(v4) {
+  // If data already looks like v5 (has migration.version=5 or v5-only fields),
+  // return it as-is so the runner can mark it done without re-migrating.
+  // This handles the case where a buggy v5 run wrote v5 fields without the marker.
+  if (isAlreadyV5(v4)) {
+    return v4;
+  }
   const parsed = v4SettingsSchema.parse(v4 || {});
   return {
     ...v5SettingsShape,
diff --git a/src/renderer/lib/migrations/v4-to-v5.ts b/src/renderer/lib/migrations/v4-to-v5.ts
index 0a41d66..0ff34b2 100644
--- a/src/renderer/lib/migrations/v4-to-v5.ts
+++ b/src/renderer/lib/migrations/v4-to-v5.ts
@@ -10,7 +10,16 @@ export const v4SettingsSchema = z.object({
   snippets: z.array(z.unknown()).default([]),
 });
 
+const v5OnlyFields = ['updateChannel', 'autoCheckUpdates', 'firstRun'];
+
+function isAlreadyV5(data: unknown): boolean {
+  if (!data || typeof data !== 'object') return false;
+  if ((data as Record<string, unknown>)['migration.version'] === 5) return true;
+  return v5OnlyFields.some((f) => f in (data as Record<string, unknown>));
+}
+
 export function migrateV4ToV5(v4: unknown): z.infer<typeof settingsSchema> {
+  if (isAlreadyV5(v4)) return v4 as z.infer<typeof settingsSchema>;
   const parsed = v4SettingsSchema.parse(v4);
   const defaults = settingsSchema.parse({});
   return {
diff --git a/tests/unit/main/updater/migration-runner.test.js b/tests/unit/main/updater/migration-runner.test.js
index 7c3cc74..3238fbe 100644
--- a/tests/unit/main/updater/migration-runner.test.js
+++ b/tests/unit/main/updater/migration-runner.test.js
@@ -34,12 +34,20 @@ describe('MigrationRunner', () => {
     fs.rmSync(dir, { recursive: true });
   });
 
-  test('returns failed and preserves v4 on transform throw', () => {
+  test('returns failed and writes v5 marker so subsequent runs skip migration', () => {
     const dir = tmpDir();
     fs.writeFileSync(path.join(dir, 'settings.json'), JSON.stringify({ theme: 'light' }));
     const r = new MigrationRunner({ dir, transform: () => { throw new Error('boom'); } });
     expect(r.run()).toBe('failed');
+    // v5 marker is written so next launch skips migration
+    expect(JSON.parse(fs.readFileSync(path.join(dir, 'settings.json'), 'utf-8'))['migration.version']).toBe(5);
+    // Original data is preserved (theme: light is still there)
     expect(fs.readFileSync(path.join(dir, 'settings.json'), 'utf-8')).toContain('light');
+    // Backup exists
+    expect(fs.existsSync(path.join(dir, 'settings.v4.bak.json'))).toBe(true);
+    // Subsequent run skips
+    const r2 = new MigrationRunner({ dir, transform: () => { throw new Error('should not be called'); } });
+    expect(r2.run()).toBe('skipped');
     fs.rmSync(dir, { recursive: true });
   });
 });