Repository navigation
refactor(sidecars): move boot and the watchers out of index.ts - #2215
Conversation
bootSidecars() moves to sidecars/boot.ts and takes its paths, the packaged flag and the secret as a BootContext, instead of reading app, process.resourcesPath and safeStorage. The three watchers get their own files and tests. index.ts builds the context from Electron and passes the crash notice to the window in as the crash handler. Nothing under sidecars/ imports electron, so the container can run the same code. Fixes MODSetter#2103
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to This refactor moves sidecar startup and watcher code into separate modules without intended behavior changes. No concrete merge-blocking risk was found in the supplied context. Security Architecture Review
Pre-merge checks |
|
|
@llwand1 is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Evidence for the linked checks (follow-up commit 1. Docstring coverage — added JSDoc to the functions the diff touches ( 2. 3. Tests — the watcher suites added by this step: And 4. No Electron coupling — nothing under (The word "Electron" appears only inside comments.) Ready for review — happy to address any feedback. |
|
Merged into
Small, for when you're there: no test checks the stop function each watcher returns (a |
What
bootSidecars()moves fromindex.tstosidecars/boot.ts. It takes its paths, the packaged flag and the secret as aBootContext, instead of readingapp,process.resourcesPathandsafeStorage.watch-generation-preset.ts,watch-audio-models.ts,watch-image-model.ts, with tests of their own. The opencode watcher stays inboot.ts, since only boot owns that lifecycle.index.tsbuilds the context from Electron and passes its crash notice to the window in as the crash handler.SidecarControl(the running children and how to change them), so a test hands it fakes and spawns nothing.bootSidecars()now returns before waiting on/health: the caller holds the sidecars and the watchers' stop before anything can fail, so a failed startup still reaps them.Why
Step 3 of 14 of #2100, so the container can start the same sidecars without Electron. No file under
sidecars/importselectronnow.Fixes #2103
How to test
cd surfsense_local/electron && pnpm typecheckcd surfsense_local/electron && pnpm testpnpm devboots as beforeHigh-level PR Summary
This PR refactors the sidecar boot process by moving
bootSidecars()fromindex.tsinto a newsidecars/boot.tsfile and extracting each model watcher (generation preset, audio models, image models) into separate testable modules. The refactoring introduces aBootContextinterface that decouples the boot logic from Electron-specific APIs, allowing the same code to run in a containerized environment. Each watcher now receives aSidecarControlinterface for managing child processes, enabling unit tests with fake implementations that don't spawn actual processes. The boot function now returns immediately with handles to the sidecars and watchers, deferring health checks to the caller to ensure proper cleanup even if startup fails.⏱️ Estimated Review Time: 30-90 minutes
💡 Review Order Suggestion
surfsense_local/electron/src/main/sidecars/file-stamp.tssurfsense_local/electron/src/main/sidecars/supervisor.tssurfsense_local/electron/src/main/sidecars/watch-image-model.tssurfsense_local/electron/src/main/sidecars/watch-generation-preset.tssurfsense_local/electron/src/main/sidecars/watch-audio-models.tssurfsense_local/electron/src/main/sidecars/watch-image-model.test.tssurfsense_local/electron/src/main/sidecars/watch-generation-preset.test.tssurfsense_local/electron/src/main/sidecars/watch-audio-models.test.tssurfsense_local/electron/src/main/sidecars/boot.tssurfsense_local/electron/src/main/index.tsSummary by CodeRabbit