feat: Add settings persistence for Electron app

[mattgodbolt]

Summary

• Persist user settings (model, coProcessor, display mode, etc.) and media paths (discs, tapes) across Electron sessions using electron-store
• Uses observer pattern: Config emits "settings-changed" events, main.js emits "media-changed" events, Electron subscribes and saves
• Settings restored as query params on app startup, seamlessly integrating with existing URL-based configuration

Test plan

• Open Electron app, change model in Configuration modal, close and reopen app - model should persist
• Load a disc from "Browse Example Discs", close and reopen app - disc should auto-load
• Load a disc from STH archive, close and reopen app - disc should auto-load
• Load a local disc via File menu, close and reopen app - disc should auto-load
• Same tests for tape images
• Verify command-line args still override saved settings

🤖 Generated with Claude Code

[mattgodbolt]

@claude What do you think?

[claude]

Claude finished @mattgodbolt's task —— View job

---

Code Review for Settings Persistence Feature

Todo List

• Read repository guidelines (CLAUDE.md)
• Analyze all changed files
• Review the observer pattern implementation
• Check for security and best practices
• Provide comprehensive feedback

Overall Assessment

This is a well-implemented feature that adds settings persistence to the Electron app using a clean observer pattern. The implementation follows the repository's code style guidelines and integrates seamlessly with the existing URL-based configuration system.

✅ Strengths

1. Clean Observer Pattern: The implementation uses EventEmitter properly - Config emits settings-changed when configuration modal closes, and main.js emits media-changed when media URLs change.

2. Non-invasive Integration: Settings are restored as query parameters on startup, which integrates perfectly with the existing URL-based configuration system without requiring architectural changes.

3. Secure Implementation: Uses electron-store (a well-established library) and follows Electron security best practices with proper contextBridge usage in preload.js.

4. Appropriate Dependency: Adding electron-store as an optionalDependency is correct since it's only needed for Electron builds.

5. Good Separation of Concerns: Each file has clear responsibilities - main.js handles storage, electron.js handles IPC, config.js handles UI events.

📋 Code Quality Details

src/config.js:28-29

Good implementation of the observer pattern:

if (Object.keys(this.changed).length > 0) {
    this.emit("settings-changed", this.changed);
}

Only emits when there are actual changes, preventing unnecessary saves.

src/main.js:769,775

Clean media change notifications:

config.emit("media-changed", { disc1: name });
config.emit("media-changed", { tape: name });

Proper use of the observer pattern for media changes.

src/app/electron.js:51-57

Well-structured event listeners:

config.on("settings-changed", (settings) => {
    api.saveSettings(settings);
});
config.on("media-changed", (media) => {
    api.saveSettings(media);
});

src/app/app.js:79-85

Smart query parameter merging with command-line override:

const savedSettings = store.get("settings", {});
const query = { ...savedSettings };
if (args.disc1 && !args.noboot) query.autoboot = true;
if (args.disc1) query.disc1 = getFileParam(args.disc1);
if (args.disc2) query.disc2 = getFileParam(args.disc2);

Command-line args properly override saved settings.

🔍 Minor Considerations

1. Error Handling: The current implementation doesn't handle electron-store errors, but this is generally acceptable since electron-store handles most error cases internally.

2. Storage Scope: The implementation saves all changed settings in a single object. This could theoretically accumulate unused settings over time, but this is unlikely to be a practical issue.

3. Test Coverage: The test plan shows good manual testing coverage, though some items are still pending.

🎯 Architecture Assessment

The implementation demonstrates excellent architectural thinking:

• Backward Compatibility: Web version continues to work unchanged
• Progressive Enhancement: Electron features are added without affecting core functionality
• Loose Coupling: Settings persistence is decoupled from the core emulation logic
• Standard Patterns: Uses established patterns (observer, query parameters) rather than inventing new ones

💡 Technical Excellence

The code follows the repository's guidelines well:

• ✅ Uses ES6+ features appropriately
• ✅ Follows camelCase naming conventions
• ✅ Uses const/let instead of var
• ✅ Proper error handling with try/catch where needed
• ✅ No magic numbers or hardcoded values
• ✅ Clean separation between Electron and web code

🔒 Security Review

The implementation follows Electron security best practices:

• ✅ Uses contextBridge for secure IPC communication
• ✅ No direct node.js access in renderer process
• ✅ Proper use of preload script
• ✅ Settings are stored locally (no network transmission)

Final Recommendation

✅ Approve - This is a high-quality implementation that adds valuable functionality without compromising the existing codebase. The observer pattern is well-executed, the integration is seamless, and the code follows all repository standards.

The feature successfully addresses the need for settings persistence in Electron while maintaining the flexibility and URL-based architecture that makes jsbeeb work so well in browser environments.