Skip to content

Commit dbe6308

Browse files
authored
fix: replace intrusive update modal with toast and fix double-restart bug (#841)
1 parent 653587a commit dbe6308

4 files changed

Lines changed: 281 additions & 125 deletions

File tree

apps/twig/src/main/services/app-lifecycle/service.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,51 @@ export class AppLifecycleService {
3434
process.exit(1);
3535
}
3636

37+
async cleanupForUpdate(): Promise<void> {
38+
log.info("Cleanup for update started");
39+
40+
// Shut down watchers
41+
log.info("Shutting down native watchers");
42+
try {
43+
const watcherRegistry = container.get<WatcherRegistryService>(
44+
MAIN_TOKENS.WatcherRegistryService,
45+
);
46+
await watcherRegistry.shutdownAll();
47+
} catch (error) {
48+
log.warn("Failed to shutdown watcher registry", error);
49+
}
50+
51+
// Kill all tracked processes
52+
try {
53+
const processTracking = container.get<ProcessTrackingService>(
54+
MAIN_TOKENS.ProcessTrackingService,
55+
);
56+
const snapshot = await processTracking.getSnapshot(true);
57+
log.info("Process snapshot before update", {
58+
tracked: {
59+
shell: snapshot.tracked.shell.length,
60+
agent: snapshot.tracked.agent.length,
61+
child: snapshot.tracked.child.length,
62+
},
63+
});
64+
65+
if (
66+
snapshot.tracked.shell.length +
67+
snapshot.tracked.agent.length +
68+
snapshot.tracked.child.length >
69+
0
70+
) {
71+
log.info("Killing all tracked processes before update");
72+
processTracking.killAll();
73+
}
74+
} catch (error) {
75+
log.warn("Failed to kill processes before update", error);
76+
}
77+
78+
// Skip container unbind, PostHog shutdown - app is restarting anyway
79+
log.info("Cleanup for update complete");
80+
}
81+
3782
async shutdown(): Promise<void> {
3883
if (this._isShuttingDown) {
3984
log.warn("Shutdown already in progress, forcing exit");

apps/twig/src/main/services/updates/service.test.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const { mockApp, mockAutoUpdater, mockLifecycleService } = vi.hoisted(() => ({
1717
},
1818
mockLifecycleService: {
1919
shutdown: vi.fn(() => Promise.resolve()),
20+
cleanupForUpdate: vi.fn(() => Promise.resolve()),
2021
setQuittingForUpdate: vi.fn(),
2122
},
2223
}));
@@ -341,9 +342,27 @@ describe("UpdatesService", () => {
341342

342343
const result = await service.installUpdate();
343344
expect(result).toEqual({ installed: true });
344-
expect(mockLifecycleService.shutdown).toHaveBeenCalled();
345+
346+
// Verify setQuittingForUpdate is called first
345347
expect(mockLifecycleService.setQuittingForUpdate).toHaveBeenCalled();
348+
349+
// Verify cleanupForUpdate is called (not full shutdown)
350+
expect(mockLifecycleService.cleanupForUpdate).toHaveBeenCalled();
351+
expect(mockLifecycleService.shutdown).not.toHaveBeenCalled();
352+
353+
// Verify quitAndInstall is called after cleanup
346354
expect(mockAutoUpdater.quitAndInstall).toHaveBeenCalled();
355+
356+
// Verify order: setQuittingForUpdate -> cleanupForUpdate -> quitAndInstall
357+
const setQuittingOrder =
358+
mockLifecycleService.setQuittingForUpdate.mock.invocationCallOrder[0];
359+
const cleanupOrder =
360+
mockLifecycleService.cleanupForUpdate.mock.invocationCallOrder[0];
361+
const quitAndInstallOrder =
362+
mockAutoUpdater.quitAndInstall.mock.invocationCallOrder[0];
363+
364+
expect(setQuittingOrder).toBeLessThan(cleanupOrder);
365+
expect(cleanupOrder).toBeLessThan(quitAndInstallOrder);
347366
});
348367

349368
it("returns false if quitAndInstall throws", async () => {

apps/twig/src/main/services/updates/service.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,13 @@ export class UpdatesService extends TypedEventEmitter<UpdatesEvents> {
115115
});
116116

117117
try {
118-
await this.lifecycleService.shutdown();
118+
// Set the flag FIRST so before-quit handler won't prevent quit
119119
this.lifecycleService.setQuittingForUpdate();
120120

121+
// Do lightweight cleanup: kill processes, shut down watchers
122+
// Skip container teardown so before-quit handler can still access services
123+
await this.lifecycleService.cleanupForUpdate();
124+
121125
autoUpdater.quitAndInstall();
122126
return { installed: true };
123127
} catch (error) {

0 commit comments

Comments
 (0)