Skip to content

Commit 84ddade

Browse files
big-guyclaude
andauthored
Recover from a corrupt config.json instead of silently resetting (#151)
## Problem A malformed or unreadable `config.json` was swallowed by a bare `catch` in `loadConfig`, the app booted on defaults, and the first save **overwrote the bad file** — destroying the user's only copy with no backup. Writes were also non-atomic, so a crash mid-write could corrupt the file in the first place. ## What this does On a corrupt load: - **Atomic writes** (temp + rename) so a partial write can't corrupt the file. - The unreadable file is **quarantined** to `config.corrupt-<ts>.json`. - Further config writes are **suspended** so the broken original survives for hand-editing. - A new `configHealth` slice carries the error to the renderer, which shows **`InvalidConfigModal`** — an in-app Monaco JSON editor preloaded with the raw bytes: - **Save & Retry** (⌘S) writes + validates and re-applies if it parses, otherwise shows the error inline so the user keeps editing. - **Reset to defaults** starts fresh (the quarantine backup is kept). - The modal **preempts onboarding** — a corrupt load lands on empty defaults, which would otherwise trigger first-run setup underneath it. Re-applying a fixed config is environment-aware: - **Production** relaunches for a clean reboot off the file. - Under **`electron-vite dev`**, relaunch blanks the screen (it tears down the Vite dev server), so dev re-seeds the store + FSMs in place (`reapplyConfigFromDisk` + `Store.replaceState`) and reloads the window. A **missing** file is still a clean first-run default load. **Headless** boots have no UI, so they quarantine + reset immediately. ## Testing - `npm run typecheck`, `npx electron-vite build`, and `npx vitest run` all pass. - New `persistence-recovery.test.ts` covers quarantine, suspended writes, atomic writes, validate, and reset (using a temp userData dir). - Manually verified in dev: corrupt config → modal with editor → fix JSON → Save & Retry → window reloads into a working session; the bad file is preserved while suspended. ## Not covered (follow-up) Valid-JSON-but-wrong-shape configs (e.g. `repoRoots` not an array, malformed `panes`) parse fine so they don't hit this recovery path; they can still degrade boot. That's a separate per-field validation/boot-resilience pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1353b00 commit 84ddade

14 files changed

Lines changed: 526 additions & 15 deletions

src/main/build-initial-state.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { initialUpdater } from '../shared/state/updater'
1919
import { initialRepoConfigs } from '../shared/state/repo-configs'
2020
import { initialSettings } from '../shared/state/settings'
2121
import { initialScratchpad } from '../shared/state/scratchpad'
22+
import { initialConfigHealth } from '../shared/state/config-health'
2223

2324
// The bug we're guarding against: a slice's `initial<Slice>` constant gains
2425
// a new field but the main-process Store seed in index.ts forgets to include
@@ -47,7 +48,8 @@ describe('buildInitialAppState', () => {
4748
updater: initialUpdater,
4849
repoConfigs: initialRepoConfigs,
4950
settings: initialSettings,
50-
scratchpad: initialScratchpad
51+
scratchpad: initialScratchpad,
52+
configHealth: initialConfigHealth
5153
} as const
5254

5355
for (const [name, initial] of Object.entries(slices)) {

src/main/build-initial-state.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { initialSnooze } from '../shared/state/snooze'
1313
import { initialAnnouncements } from '../shared/state/announcements'
1414
import { initialScratchpad } from '../shared/state/scratchpad'
1515
import { initialSshBootstrap } from '../shared/state/ssh-bootstrap'
16+
import { initialConfigHealth, type ConfigLoadError } from '../shared/state/config-health'
1617
import {
1718
initialSettings,
1819
DEFAULT_LIGHT_THEME,
@@ -52,10 +53,11 @@ function flattenScratchpadNotes(
5253

5354
export function buildInitialAppState(
5455
config: Config,
55-
opts: { hasGithubToken: boolean }
56+
opts: { hasGithubToken: boolean; configLoadError?: ConfigLoadError | null }
5657
): AppState {
5758
return {
5859
prs: initialPRs,
60+
configHealth: { ...initialConfigHealth, loadError: opts.configLoadError ?? null },
5961
onboarding: {
6062
...initialOnboarding,
6163
quest: config.onboarding?.quest ?? 'hidden'

src/main/desktop-shell.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,14 @@ import type { CompoundServerTransport } from './transport-compound'
3838
import type { PtyManager } from './pty-manager'
3939
import type { WorktreesFSM } from './worktrees-fsm'
4040
import type { Config } from './persistence'
41-
import { saveConfig, saveConfigSync, THEME_APP_BG } from './persistence'
41+
import {
42+
saveConfig,
43+
saveConfigSync,
44+
THEME_APP_BG,
45+
readRawConfigText,
46+
saveRawConfigText,
47+
discardCorruptConfigAndReset
48+
} from './persistence'
4249
import { DEFAULT_LIGHT_THEME, DEFAULT_DARK_THEME } from '../shared/state/settings'
4350
import { registerWindowControlHandlers } from './window-controls'
4451
import { sealAllActive } from './activity'
@@ -121,6 +128,11 @@ export interface DesktopShellStartDeps {
121128
* starting pollers, etc. Owned by index.ts because most of it is
122129
* mode-agnostic. */
123130
runBoot: () => Promise<void> | void
131+
/** Dev-only: re-load config.json and re-seed the store + FSMs in place,
132+
* used by the corrupt-config handlers when not packaged (relaunch blanks
133+
* the screen under the dev server). Owned by index.ts (touches the store,
134+
* FSMs, and shared config object). */
135+
reapplyConfigFromDisk: () => Promise<void>
124136
/** Reference held by index.ts so its handlers can stop the watcher
125137
* during quitAndInstall. */
126138
getStopWatchingStatus: () => (() => void) | null
@@ -155,6 +167,7 @@ export function startDesktopShell(deps: DesktopShellStartDeps): DesktopShellStar
155167
worktreesFSM,
156168
config,
157169
runBoot,
170+
reapplyConfigFromDisk,
158171
getStopWatchingStatus,
159172
setStopWatchingStatus,
160173
onRepoAdded,
@@ -637,6 +650,38 @@ export function startDesktopShell(deps: DesktopShellStartDeps): DesktopShellStar
637650
shell.openExternal(url)
638651
})
639652

653+
// --- Corrupt-config recovery (see persistence.ts) ---
654+
// These fire while the renderer is showing InvalidConfigModal. Production
655+
// relaunches for a clean reboot off the fixed file; dev can't (relaunch
656+
// tears down the Vite dev server → blank screen) so it re-seeds in place
657+
// and reloads the renderer.
658+
const applyRecoveredConfig = async (): Promise<void> => {
659+
if (app.isPackaged) {
660+
app.relaunch()
661+
app.exit(0)
662+
return
663+
}
664+
await reapplyConfigFromDisk()
665+
for (const win of BrowserWindow.getAllWindows()) win.reload()
666+
}
667+
668+
transport.onRequest('config:readRawConfig', (_ctx) => {
669+
return { text: readRawConfigText() }
670+
})
671+
672+
transport.onRequest('config:saveRawConfigAndRetry', async (_ctx, text: string) => {
673+
const result = saveRawConfigText(text)
674+
if (!result.ok) return { ok: false as const, error: result.error }
675+
await applyRecoveredConfig()
676+
return { ok: true as const }
677+
})
678+
679+
transport.onRequest('config:resetConfigToDefaults', async (_ctx) => {
680+
discardCorruptConfigAndReset()
681+
await applyRecoveredConfig()
682+
return { ok: true as const }
683+
})
684+
640685
transport.onRequest('config:openThemesFolder', async (_ctx) => {
641686
const { themesDir } = await import('./themes-loader')
642687
const dir = themesDir()

src/main/index.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ import {
4747
loadConfig,
4848
saveConfig,
4949
saveConfigSync,
50+
getConfigLoadError,
51+
discardCorruptConfigAndReset,
5052
DEFAULT_CLAUDE_COMMAND,
5153
AVAILABLE_THEMES,
5254
THEME_APP_BG,
@@ -314,7 +316,19 @@ const ptyManager = new PtyManager()
314316
let config = loadConfig()
315317
let stopWatchingStatus: (() => void) | null = null
316318

317-
const store = new Store(buildInitialAppState(config, { hasGithubToken: hasSecret('githubToken') }))
319+
// Desktop surfaces a corrupt-config load via the recovery modal (configHealth
320+
// slice → InvalidConfigModal). Headless has no UI to drive the fix, so it
321+
// abandons the bad file and boots on defaults (the quarantine copy is kept).
322+
let configLoadError = getConfigLoadError()
323+
if (configLoadError && runtime !== 'electron') {
324+
log('config', `corrupt config on headless boot, resetting: ${configLoadError.message}`)
325+
config = discardCorruptConfigAndReset()
326+
configLoadError = null
327+
}
328+
329+
const store = new Store(
330+
buildInitialAppState(config, { hasGithubToken: hasSecret('githubToken'), configLoadError })
331+
)
318332

319333
// Scan for user-authored themes once the store exists. Done as a
320334
// dispatch (rather than seeding into buildInitialAppState) so the
@@ -3584,6 +3598,37 @@ const desktopHooks = {
35843598
stopAutoUpdateChecks: (): void => {}
35853599
}
35863600

3601+
// Dev-only re-apply of a freshly-fixed config without relaunching (relaunch
3602+
// blanks the screen under `electron-vite dev` — see desktop-shell). `config`
3603+
// is mutated in place because desktop-shell and many IPC handlers hold it by
3604+
// reference; reassigning would strand them on the stale object. The caller
3605+
// reloads the window afterward so the renderer re-fetches the new snapshot.
3606+
async function reapplyConfigFromDisk(): Promise<void> {
3607+
const fresh = loadConfig()
3608+
const mutable = config as unknown as Record<string, unknown>
3609+
for (const key of Object.keys(mutable)) {
3610+
delete mutable[key]
3611+
}
3612+
Object.assign(config, fresh)
3613+
3614+
store.replaceState(
3615+
buildInitialAppState(config, {
3616+
hasGithubToken: hasSecret('githubToken'),
3617+
configLoadError: getConfigLoadError()
3618+
})
3619+
)
3620+
3621+
const repoConfigsMap: Record<string, RepoConfig> = {}
3622+
for (const root of config.repoRoots || []) {
3623+
repoConfigsMap[root] = loadRepoConfig(root)
3624+
}
3625+
store.dispatch({ type: 'repoConfigs/loaded', payload: repoConfigsMap })
3626+
3627+
await panesFSM.restoreFromConfig(config.panes)
3628+
await worktreesFSM.refreshList()
3629+
void prPoller.refreshAll()
3630+
}
3631+
35873632
async function runBoot(): Promise<void> {
35883633
log('app', `started, log file: ${getLogFilePath()}`)
35893634

@@ -3957,6 +4002,7 @@ if (desktopShellMod && desktopEarly) {
39574002
worktreesFSM,
39584003
config,
39594004
runBoot,
4005+
reapplyConfigFromDisk,
39604006
getStopWatchingStatus: () => stopWatchingStatus,
39614007
setStopWatchingStatus: (next) => {
39624008
stopWatchingStatus = next
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
2+
import { mkdtempSync, rmSync, writeFileSync, readFileSync, existsSync, readdirSync } from 'fs'
3+
import { join } from 'path'
4+
import { tmpdir } from 'os'
5+
6+
// Point persistence at a throwaway userData dir per test.
7+
let DIR = ''
8+
vi.mock('./paths', () => ({
9+
userDataDir: () => DIR
10+
}))
11+
12+
import {
13+
loadConfig,
14+
saveConfigSync,
15+
getConfigLoadError,
16+
validateConfigFile,
17+
discardCorruptConfigAndReset
18+
} from './persistence'
19+
20+
const configPath = (): string => join(DIR, 'config.json')
21+
22+
beforeEach(() => {
23+
DIR = mkdtempSync(join(tmpdir(), 'harness-cfg-'))
24+
})
25+
afterEach(() => {
26+
rmSync(DIR, { recursive: true, force: true })
27+
})
28+
29+
describe('loadConfig corrupt-file handling', () => {
30+
it('a missing file is a clean default load (no error, writes enabled)', () => {
31+
const cfg = loadConfig()
32+
expect(getConfigLoadError()).toBeNull()
33+
expect(cfg.repoRoots).toEqual([])
34+
// Writes work after a clean load.
35+
saveConfigSync({ ...cfg, repoRoots: ['/x'] })
36+
expect(JSON.parse(readFileSync(configPath(), 'utf-8')).repoRoots).toEqual(['/x'])
37+
})
38+
39+
it('malformed JSON: returns defaults, records error, quarantines, suspends writes', () => {
40+
writeFileSync(configPath(), '{ this is not json')
41+
const cfg = loadConfig()
42+
43+
// Falls back to defaults rather than throwing.
44+
expect(cfg.repoRoots).toEqual([])
45+
46+
// Error captured.
47+
const err = getConfigLoadError()
48+
expect(err).not.toBeNull()
49+
expect(err?.configPath).toBe(configPath())
50+
expect(err?.backupPath).toBeTruthy()
51+
52+
// Quarantine copy holds the original bad content.
53+
expect(readFileSync(err!.backupPath!, 'utf-8')).toBe('{ this is not json')
54+
55+
// Writes are suspended — the bad original survives for hand-editing.
56+
saveConfigSync({ ...cfg, repoRoots: ['/should-not-persist'] })
57+
expect(readFileSync(configPath(), 'utf-8')).toBe('{ this is not json')
58+
})
59+
60+
it('validateConfigFile reflects the on-disk state without applying it', () => {
61+
writeFileSync(configPath(), 'garbage')
62+
loadConfig()
63+
expect(validateConfigFile().ok).toBe(false)
64+
65+
// Simulate the user fixing the file by hand.
66+
writeFileSync(configPath(), JSON.stringify({ repoRoots: ['/fixed'] }))
67+
expect(validateConfigFile()).toEqual({ ok: true })
68+
})
69+
70+
it('discardCorruptConfigAndReset re-enables writes and persists defaults', () => {
71+
writeFileSync(configPath(), 'nope')
72+
loadConfig()
73+
expect(getConfigLoadError()).not.toBeNull()
74+
75+
const fresh = discardCorruptConfigAndReset()
76+
expect(getConfigLoadError()).toBeNull()
77+
78+
// Default config written over the bad file, and it parses.
79+
const onDisk = JSON.parse(readFileSync(configPath(), 'utf-8'))
80+
expect(onDisk.repoRoots).toEqual([])
81+
expect(fresh.connections?.[0].kind).toBe('local')
82+
83+
// Writes resumed.
84+
saveConfigSync({ ...fresh, repoRoots: ['/y'] })
85+
expect(JSON.parse(readFileSync(configPath(), 'utf-8')).repoRoots).toEqual(['/y'])
86+
})
87+
88+
it('atomic write leaves no stray .tmp file behind', () => {
89+
const cfg = loadConfig()
90+
saveConfigSync({ ...cfg, repoRoots: ['/z'] })
91+
const leftovers = readdirSync(DIR).filter((f) => f.endsWith('.tmp'))
92+
expect(leftovers).toEqual([])
93+
expect(existsSync(configPath())).toBe(true)
94+
})
95+
})

0 commit comments

Comments
 (0)