From 9a5c14c58b6ed1a5fba38c4712c14cd354e45d18 Mon Sep 17 00:00:00 2001 From: Eva Ho Date: Fri, 19 Dec 2025 15:27:15 -0500 Subject: [PATCH] address comments --- app/cmd/app/app.go | 19 ++++--- app/cmd/app/app_darwin.go | 8 --- app/cmd/app/app_darwin.h | 1 - app/cmd/app/app_darwin.m | 12 ---- app/ui/app/src/api.ts | 14 ----- app/ui/app/src/components/Settings.tsx | 4 +- app/ui/ui.go | 47 +--------------- app/updater/updater.go | 76 ++++++++++---------------- app/wintray/menus.go | 28 ---------- app/wintray/tray.go | 1 - 10 files changed, 45 insertions(+), 165 deletions(-) diff --git a/app/cmd/app/app.go b/app/cmd/app/app.go index 0b1880234..7bc73cd75 100644 --- a/app/cmd/app/app.go +++ b/app/cmd/app/app.go @@ -267,15 +267,12 @@ func main() { }, Store: st, ToolRegistry: toolRegistry, - Dev: devMode, - Logger: slog.Default(), - Updater: upd, + Dev: devMode, + Logger: slog.Default(), + Updater: upd, UpdateAvailableFunc: func() { UpdateAvailable("") }, - ClearUpdateAvailableFunc: func() { - ClearUpdateAvailable() - }, } srv := &http.Server{ @@ -295,6 +292,12 @@ func main() { upd.StartBackgroundUpdaterChecker(ctx, UpdateAvailable) + // Check for pending updates on startup (show tray notification if update is ready) + if updater.IsUpdatePending() { + slog.Debug("update pending on startup, showing tray notification") + UpdateAvailable("") + } + hasCompletedFirstRun, err := st.HasCompletedFirstRun() if err != nil { slog.Error("failed to load has completed first run", "error", err) @@ -356,13 +359,15 @@ func startHiddenTasks() { // CLI triggered app startup use-case slog.Info("deferring pending update for fast startup") } else { - // Check if auto-update is enabled before upgrading + // Check if auto-update is enabled before automatically upgrading st := &store.Store{} settings, err := st.Settings() if err != nil { slog.Warn("failed to load settings for upgrade check", "error", err) } else if !settings.AutoUpdateEnabled { slog.Info("auto-update disabled, skipping automatic upgrade at startup") + // Still show tray notification so user knows update is ready + UpdateAvailable("") return } diff --git a/app/cmd/app/app_darwin.go b/app/cmd/app/app_darwin.go index 5d5138304..8e886a124 100644 --- a/app/cmd/app/app_darwin.go +++ b/app/cmd/app/app_darwin.go @@ -172,14 +172,6 @@ func UpdateAvailable(ver string) error { return nil } -func ClearUpdateAvailable() error { - slog.Debug("clearing update notification") - if updater.BundlePath != "" { - C.clearUpdateAvailable() - } - return nil -} - func osRun(_ func(), hasCompletedFirstRun, startHidden bool) { registerLaunchAgent(hasCompletedFirstRun) diff --git a/app/cmd/app/app_darwin.h b/app/cmd/app/app_darwin.h index c9c510872..4a5ba055f 100644 --- a/app/cmd/app/app_darwin.h +++ b/app/cmd/app/app_darwin.h @@ -30,7 +30,6 @@ void StartUpdate(); void darwinStartHiddenTasks(); void launchApp(const char *appPath); void updateAvailable(); -void clearUpdateAvailable(); void quit(); void uiRequest(char *path); void registerSelfAsLoginItem(bool firstTimeRun); diff --git a/app/cmd/app/app_darwin.m b/app/cmd/app/app_darwin.m index dd817ce18..d5095ab25 100644 --- a/app/cmd/app/app_darwin.m +++ b/app/cmd/app/app_darwin.m @@ -241,12 +241,6 @@ bool firstTimeRun,startHidden; // Set in run before initialization [self showIcon]; } -- (void)clearUpdateAvailable { - self.updateAvailable = NO; - [self.statusItem.menu.itemArray[3] setHidden:YES]; - [self.statusItem.menu.itemArray[4] setHidden:YES]; -} - - (void)aboutOllama { [[NSApplication sharedApplication] orderFrontStandardAboutPanel:nil]; [NSApp activateIgnoringOtherApps:YES]; @@ -979,12 +973,6 @@ void updateAvailable() { }); } -void clearUpdateAvailable() { - dispatch_async(dispatch_get_main_queue(), ^{ - [appDelegate clearUpdateAvailable]; - }); -} - void quit() { dispatch_async(dispatch_get_main_queue(), ^{ [appDelegate quit]; diff --git a/app/ui/app/src/api.ts b/app/ui/app/src/api.ts index 80debe508..689051f9b 100644 --- a/app/ui/app/src/api.ts +++ b/app/ui/app/src/api.ts @@ -453,20 +453,6 @@ export async function checkForUpdate(): Promise<{ return data.updateInfo; } -export async function downloadUpdate(version: string): Promise { - const response = await fetch(`${API_BASE}/api/v1/update/download`, { - method: "POST", - headers: { - "Content-Type": "application/json", - }, - body: JSON.stringify({ version }), - }); - if (!response.ok) { - const error = await response.text(); - throw new Error(error || "Failed to download update"); - } -} - export async function installUpdate(): Promise { const response = await fetch(`${API_BASE}/api/v1/update/install`, { method: "POST", diff --git a/app/ui/app/src/components/Settings.tsx b/app/ui/app/src/components/Settings.tsx index e628f25a7..c3ae9b82d 100644 --- a/app/ui/app/src/components/Settings.tsx +++ b/app/ui/app/src/components/Settings.tsx @@ -375,14 +375,14 @@ export default function Settings() { {settings.AutoUpdateEnabled ? ( <> - Automatically downloads updates and restarts the app. + Automatically downloads updates when available.
Current version: {updateInfo?.currentVersion || "Loading..."}
) : ( <> - You must manually check for updates. + Manually download updates.
diff --git a/app/ui/ui.go b/app/ui/ui.go index d55f848b2..0b8beb599 100644 --- a/app/ui/ui.go +++ b/app/ui/ui.go @@ -109,15 +109,13 @@ type Server struct { Dev bool // Updater for checking and downloading updates - Updater UpdaterInterface - UpdateAvailableFunc func() - ClearUpdateAvailableFunc func() + Updater UpdaterInterface + UpdateAvailableFunc func() } // UpdaterInterface defines the methods we need from the updater type UpdaterInterface interface { CheckForUpdate(ctx context.Context) (bool, string, error) - DownloadUpdate(ctx context.Context, updateVersion string) error InstallAndRestart() error CancelOngoingDownload() TriggerImmediateCheck() @@ -300,7 +298,6 @@ func (s *Server) Handler() http.Handler { mux.Handle("GET /api/v1/settings", handle(s.getSettings)) mux.Handle("POST /api/v1/settings", handle(s.settings)) mux.Handle("GET /api/v1/update/check", handle(s.checkForUpdate)) - mux.Handle("POST /api/v1/update/download", handle(s.downloadUpdate)) mux.Handle("POST /api/v1/update/install", handle(s.installUpdate)) // Ollama proxy endpoints @@ -1469,13 +1466,10 @@ func (s *Server) settings(w http.ResponseWriter, r *http.Request) error { // Handle auto-update toggle changes if old.AutoUpdateEnabled != settings.AutoUpdateEnabled { if !settings.AutoUpdateEnabled { - // Auto-update disabled: cancel any ongoing download and clear tray notification + // Auto-update disabled: cancel any ongoing download if s.Updater != nil { s.Updater.CancelOngoingDownload() } - if s.ClearUpdateAvailableFunc != nil { - s.ClearUpdateAvailableFunc() - } } else { // Auto-update re-enabled: show notification if update is already staged, or trigger immediate check if (updater.IsUpdatePending() || updater.UpdateDownloaded) && s.UpdateAvailableFunc != nil { @@ -1589,41 +1583,6 @@ func (s *Server) checkForUpdate(w http.ResponseWriter, r *http.Request) error { return json.NewEncoder(w).Encode(response) } -func (s *Server) downloadUpdate(w http.ResponseWriter, r *http.Request) error { - if r.Method != "POST" { - return fmt.Errorf("method not allowed") - } - - if s.Updater == nil { - return fmt.Errorf("updater not available") - } - - var req struct { - Version string `json:"version"` - } - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - return fmt.Errorf("invalid request body: %w", err) - } - - if req.Version == "" { - return fmt.Errorf("version is required") - } - - err := s.Updater.DownloadUpdate(r.Context(), req.Version) - if err != nil { - s.log().Error("failed to download update", "error", err, "version", req.Version) - return fmt.Errorf("failed to download update: %w", err) - } - - response := map[string]any{ - "success": true, - "message": "Update downloaded successfully", - } - - w.Header().Set("Content-Type", "application/json") - return json.NewEncoder(w).Encode(response) -} - func (s *Server) installUpdate(w http.ResponseWriter, r *http.Request) error { if r.Method != "POST" { return fmt.Errorf("method not allowed") diff --git a/app/updater/updater.go b/app/updater/updater.go index d05a375d8..b29a4f83a 100644 --- a/app/updater/updater.go +++ b/app/updater/updater.go @@ -303,35 +303,37 @@ func (u *Updater) StartBackgroundUpdaterChecker(ctx context.Context, cb func(str // Regular interval check } - // Always check for updates - available, resp := u.checkForUpdate(ctx) - if !available { - continue - } + // Always check for updates + available, resp := u.checkForUpdate(ctx) + if !available { + continue + } - // Update is available - check if auto-update is enabled - settings, err := u.Store.Settings() - if err != nil { - slog.Error("failed to load settings", "error", err) - continue - } + // Update is available - check if auto-update is enabled for downloading + settings, err := u.Store.Settings() + if err != nil { + slog.Error("failed to load settings", "error", err) + continue + } - if !settings.AutoUpdateEnabled { - // Auto-update disabled - don't download, just log - slog.Debug("update available but auto-update disabled", "version", resp.UpdateVersion) - continue - } + if !settings.AutoUpdateEnabled { + // Auto-update disabled - don't download, just log + slog.Debug("update available but auto-update disabled", "version", resp.UpdateVersion) + continue + } - // Auto-update is enabled - download and notify - err = u.DownloadNewRelease(ctx, resp) - if err != nil { - slog.Error("failed to download new release", "error", err) - } else { - err = cb(resp.UpdateVersion) - if err != nil { - slog.Warn("failed to register update available with tray", "error", err) - } - } + // Auto-update is enabled - download + err = u.DownloadNewRelease(ctx, resp) + if err != nil { + slog.Error("failed to download new release", "error", err) + continue + } + + // Download successful - show tray notification (regardless of toggle state) + err = cb(resp.UpdateVersion) + if err != nil { + slog.Warn("failed to register update available with tray", "error", err) + } } }() } @@ -341,28 +343,6 @@ func (u *Updater) CheckForUpdate(ctx context.Context) (bool, string, error) { return available, resp.UpdateVersion, nil } -func (u *Updater) DownloadUpdate(ctx context.Context, updateVersion string) error { - // First check for update to get the actual download URL - available, updateResp := u.checkForUpdate(ctx) - - if !available { - return fmt.Errorf("no update available") - } - if updateResp.UpdateVersion != updateVersion { - slog.Error("version mismatch", "requested", updateVersion, "available", updateResp.UpdateVersion) - return fmt.Errorf("version mismatch: requested %s, available %s", updateVersion, updateResp.UpdateVersion) - } - - slog.Info("downloading update", "version", updateVersion) - err := u.DownloadNewRelease(ctx, updateResp) - if err != nil { - return err - } - - slog.Info("update downloaded successfully", "version", updateVersion) - return nil -} - func (u *Updater) InstallAndRestart() error { if !UpdateDownloaded { return fmt.Errorf("no update downloaded") diff --git a/app/wintray/menus.go b/app/wintray/menus.go index 6e752ef57..106953b7a 100644 --- a/app/wintray/menus.go +++ b/app/wintray/menus.go @@ -47,34 +47,6 @@ func (t *winTray) initMenus() error { return nil } -func (t *winTray) ClearUpdateAvailable() error { - if t.updateNotified { - slog.Debug("clearing update notification and menu items") - if err := t.removeMenuItem(updateSeparatorMenuID, 0); err != nil { - return fmt.Errorf("unable to remove menu entries %w", err) - } - if err := t.removeMenuItem(updateAvailableMenuID, 0); err != nil { - return fmt.Errorf("unable to remove menu entries %w", err) - } - if err := t.removeMenuItem(updateMenuID, 0); err != nil { - return fmt.Errorf("unable to remove menu entries %w", err) - } - if err := t.removeMenuItem(separatorMenuID, 0); err != nil { - return fmt.Errorf("unable to remove menu entries %w", err) - } - iconFilePath, err := iconBytesToFilePath(wt.normalIcon) - if err != nil { - return fmt.Errorf("unable to write icon data to temp file: %w", err) - } - if err := t.setIcon(iconFilePath); err != nil { - return fmt.Errorf("unable to set icon: %w", err) - } - t.updateNotified = false - t.pendingUpdate = false - } - return nil -} - func (t *winTray) UpdateAvailable(ver string) error { if !t.updateNotified { slog.Debug("updating menu and sending notification for new update") diff --git a/app/wintray/tray.go b/app/wintray/tray.go index 0271f47d4..0c1a2ed0f 100644 --- a/app/wintray/tray.go +++ b/app/wintray/tray.go @@ -41,7 +41,6 @@ type TrayCallbacks interface { Quit() TrayRun() UpdateAvailable(ver string) error - ClearUpdateAvailable() error GetIconHandle() windows.Handle }