From e25a5e1d758021c4c730165b329ae75324c61347 Mon Sep 17 00:00:00 2001 From: Amit Haridas Date: Mon, 8 Jun 2026 07:45:06 +0530 Subject: [PATCH] fix(migration): normalize legacy theme values under v5 marker Two related bugs surfaced during end-to-end verification: 1. The 'isAlreadyV5' short-circuit in both the main-side and renderer-side v4-to-v5 transforms returned the persisted object as-is when a migration.version=5 marker was present. A previously-shipped v4 build had written theme: 'ayu-light' (and similar) under the v5 marker; the transform trusted the marker and passed the invalid theme through, which then failed the renderer's zod schema on every launch and reset user settings to defaults. Fix: in both transforms, when isAlreadyV5 matches, normalize theme against the v5 enum (light/dark/system) and validate the full result with the v5 schema before returning. Out-of-range values are replaced with the v5 default ('system'). 2. The renderer's settings-store onRehydrateStorage callback called useSettingsStore.setState() to reset the store on validation failure. At that moment the store is still being constructed, and setState could hit a TDZ ReferenceError (the one we already wrapped in a try/catch in v5.0.0, which only hid the symptom). Fix: return the normalized state object from onRehydrateStorage instead. Zustand's persist middleware applies the returned value *after* construction completes, so there is no TDZ. Tests: 334 vitest + 208 jest = 542 passing. E2E: 12/12 verify-features.mjs steps green; no console errors. Amit Haridas --- src/main/updater/migration-transform.js | 51 ++++++++++++++++++++-- src/renderer/lib/migrations/v4-to-v5.ts | 18 +++++++- src/renderer/stores/settings-store.ts | 24 +++++----- tests/unit/lib/migrations/v4-to-v5.test.ts | 10 +++++ tests/unit/main/updater/__mirror.test.js | 19 ++++++++ 5 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 tests/unit/main/updater/__mirror.test.js diff --git a/src/main/updater/migration-transform.js b/src/main/updater/migration-transform.js index 0753c2a..2fbaada 100644 --- a/src/main/updater/migration-transform.js +++ b/src/main/updater/migration-transform.js @@ -11,7 +11,36 @@ const v4SettingsSchema = z.object({ snippets: z.array(z.unknown()).default([]), }).passthrough(); +const v5SettingsSchema = z.object({ + fontSize: z.number().default(14), + tabSize: z.number().default(4), + lineNumbers: z.boolean().default(true), + wordWrap: z.boolean().default(true), + minimap: z.boolean().default(true), + theme: z.enum(['light', 'dark', 'system']).default('system'), + accentColor: z.string().default('brand'), + fontFamily: z.string().default('system'), + pdfFormat: z.string().default('a4'), + pdfMargins: z.string().default('normal'), + pdfEmbedFonts: z.boolean().default(true), + docxTemplate: z.string().default('standard'), + docxCustomTemplatePath: z.string().nullable().default(null), + replOpen: z.boolean().default(false), + breadcrumbSymbols: z.boolean().default(true), + htmlHighlightStyle: z.string().default('github'), + renderTablesAsAscii: z.boolean().default(false), + welcomeDismissed: z.boolean().default(false), + editorFontSize: z.number().default(14), + customCssPath: z.string().nullable().default(null), + userBindings: z.record(z.string(), z.string()).default({}), + updateChannel: z.enum(['github', 'concreteinfo']).default('github'), + autoCheckUpdates: z.boolean().default(true), + firstRun: z.boolean().default(true), + 'migration.version': z.literal(5).optional(), +}).passthrough(); + const v5OnlyFields = ['updateChannel', 'autoCheckUpdates', 'firstRun']; +const v5ThemeValues = ['light', 'dark', 'system']; function isAlreadyV5(data) { if (!data || typeof data !== 'object') return false; @@ -20,6 +49,18 @@ function isAlreadyV5(data) { return v5OnlyFields.some((f) => f in data); } +function normalizeAlreadyV5(data) { + // Some earlier v5 builds wrote a legacy theme value (e.g. "ayu-light") + // under the v5 marker. Trusting the marker blindly broke the renderer's + // zod schema on every launch. Always normalize theme against the v5 enum + // before returning, so persisted files are always valid v5. + const out = { ...data }; + if (typeof out.theme !== 'string' || !v5ThemeValues.includes(out.theme)) { + out.theme = 'system'; + } + return out; +} + const v5SettingsShape = { fontSize: 14, tabSize: 4, @@ -49,10 +90,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. + // normalize and validate against the v5 schema. This handles the case where + // a buggy v5 run wrote v5 fields without the marker, AND the case where a + // v5 file has a legacy theme value (e.g. "ayu-light") that would otherwise + // be rejected by the renderer. if (isAlreadyV5(v4)) { - return v4; + return v5SettingsSchema.parse(normalizeAlreadyV5(v4)); } const parsed = v4SettingsSchema.parse(v4 || {}); return { @@ -67,4 +110,4 @@ function migrateV4ToV5(v4) { }; } -module.exports = { migrateV4ToV5, v5SettingsShape }; \ No newline at end of file +module.exports = { migrateV4ToV5, v5SettingsShape }; diff --git a/src/renderer/lib/migrations/v4-to-v5.ts b/src/renderer/lib/migrations/v4-to-v5.ts index 0ff34b2..ffeb048 100644 --- a/src/renderer/lib/migrations/v4-to-v5.ts +++ b/src/renderer/lib/migrations/v4-to-v5.ts @@ -11,6 +11,7 @@ export const v4SettingsSchema = z.object({ }); const v5OnlyFields = ['updateChannel', 'autoCheckUpdates', 'firstRun']; +const v5ThemeValues = ['light', 'dark', 'system'] as const; function isAlreadyV5(data: unknown): boolean { if (!data || typeof data !== 'object') return false; @@ -18,8 +19,23 @@ function isAlreadyV5(data: unknown): boolean { return v5OnlyFields.some((f) => f in (data as Record)); } +function normalizeAlreadyV5(data: Record): Record { + // Some earlier v5 builds wrote a legacy theme value (e.g. "ayu-light") + // under the v5 marker. Trusting the marker blindly broke the renderer's + // zod schema on every launch. Always normalize theme against the v5 enum + // before returning, so persisted files are always valid v5. + const out = { ...data }; + if (typeof out.theme !== 'string' || !v5ThemeValues.includes(out.theme as (typeof v5ThemeValues)[number])) { + out.theme = 'system'; + } + return out; +} + export function migrateV4ToV5(v4: unknown): z.infer { - if (isAlreadyV5(v4)) return v4 as z.infer; + if (isAlreadyV5(v4)) { + const normalized = normalizeAlreadyV5(v4 as Record); + return settingsSchema.parse(normalized); + } const parsed = v4SettingsSchema.parse(v4); const defaults = settingsSchema.parse({}); return { diff --git a/src/renderer/stores/settings-store.ts b/src/renderer/stores/settings-store.ts index cc5525c..282bb04 100644 --- a/src/renderer/stores/settings-store.ts +++ b/src/renderer/stores/settings-store.ts @@ -25,19 +25,23 @@ export const useSettingsStore = create()( const { setSetting, resetToDefaults, ...rest } = state; return rest; }, + // onRehydrateStorage must NOT call useSettingsStore.setState() — at the + // moment the callback runs the store is still being constructed, and + // setState can hit a TDZ ReferenceError. Instead, return a normalized + // state object from this callback. Zustand's persist middleware will + // merge it into the store *after* construction completes. onRehydrateStorage: () => (state) => { if (!state) return; - try { - const result = settingsSchema.safeParse(state); - if (!result.success) { - console.warn('[settings-store] invalid persisted state, resetting to defaults', result.error); - useSettingsStore.setState({ ...DEFAULTS } as any); - } - } catch (err) { - console.warn('[settings-store] rehydration failed, resetting to defaults', err); - useSettingsStore.setState({ ...DEFAULTS } as any); + const result = settingsSchema.safeParse(state); + if (!result.success) { + console.warn( + '[settings-store] invalid persisted state, replacing with defaults', + result.error.issues.map((i) => i.path.join('.') + ': ' + i.message).join('; '), + ); + return { ...DEFAULTS } as Partial; } + return result.data as Partial; }, } ) -); \ No newline at end of file +); diff --git a/tests/unit/lib/migrations/v4-to-v5.test.ts b/tests/unit/lib/migrations/v4-to-v5.test.ts index b37d43c..2aaf67d 100644 --- a/tests/unit/lib/migrations/v4-to-v5.test.ts +++ b/tests/unit/lib/migrations/v4-to-v5.test.ts @@ -26,6 +26,16 @@ describe('migrateV4ToV5', () => { expect(a).toEqual(b); }); + test('normalizes legacy theme names like "ayu-light" to a v5 value', () => { + // A previously-shipped v4 build wrote theme: 'ayu-light' (and similar). + // When an already-v5-marker file has such a value, isAlreadyV5 used to + // short-circuit and return the file as-is — which then failed the v5 + // schema and reset user settings on every launch. The transform must + // normalize unknown theme values to the schema's default. + const out = migrateV4ToV5({ 'migration.version': 5, theme: 'ayu-light' }); + expect(['light', 'dark', 'system']).toContain(out.theme); + }); + test('exposes the v4 schema for use by main', () => { expect(v4SettingsSchema).toBeDefined(); }); diff --git a/tests/unit/main/updater/__mirror.test.js b/tests/unit/main/updater/__mirror.test.js new file mode 100644 index 0000000..3312aa8 --- /dev/null +++ b/tests/unit/main/updater/__mirror.test.js @@ -0,0 +1,19 @@ +// Regression test for the v5 migration normalizer: previously, files with +// migration.version:5 but a legacy theme value (e.g. 'ayu-light') were +// short-circuited and returned as-is, breaking the renderer schema. +const { migrateV4ToV5 } = require('../../../../src/main/updater/migration-transform'); + +describe('migrateV4ToV5 (main mirror) — regression: legacy theme under v5 marker', () => { + test('normalizes theme: ayu-light under v5 marker to a v5 enum value', () => { + const out = migrateV4ToV5({ 'migration.version': 5, theme: 'ayu-light' }); + expect(['light', 'dark', 'system']).toContain(out.theme); + }); + test('preserves a valid v5 theme', () => { + const out = migrateV4ToV5({ 'migration.version': 5, theme: 'dark' }); + expect(out.theme).toBe('dark'); + }); + test('still maps v4 theme: auto to system', () => { + const out = migrateV4ToV5({ theme: 'auto' }); + expect(out.theme).toBe('system'); + }); +});