fix: guard hotkey close(nil) panic, non-blocking tray sends, D-Bus leak cleanup

Author: bosiakov

infra/autostart/autostart_linux.go

@@ -35,7 +35,7 @@ func Enable() error {
 	content := "[Desktop Entry]\n" +
 		"Type=Application\n" +
 		"Name=Debrief\n" +
-		"Exec=" + exePath + "\n" +
+		"Exec=\"" + exePath + "\"\n" +
 		"X-GNOME-Autostart-enabled=true\n" +
 		"StartupNotify=false\n" +
 		"Terminal=false\n"

infra/config/config.go

@@ -52,11 +52,21 @@ func LoadConfig(path string) (*Config, error) {
 		return DefaultConfig(), nil
 	}
 
+	cfg.clamp()
+
 	log.Printf("Loaded config from %s", path)
 
 	return &cfg, nil
 }
 
+// clamp ensures all fields are within valid ranges.
+func (c *Config) clamp() {
+	if c.HotkeyPreset < 0 || c.HotkeyPreset > MaxHotkeyPreset {
+		log.Printf("Config: HotkeyPreset %d out of range [0, %d], resetting to 0", c.HotkeyPreset, MaxHotkeyPreset)
+		c.HotkeyPreset = 0
+	}
+}
+
 // SaveConfig writes configuration to disk, creating the directory if needed.
 func (c *Config) SaveConfig(path string) error {
 	dir := filepath.Dir(path)

infra/config/constants.go

@@ -24,6 +24,10 @@ const ConfigFileName = "config.json"
 // LogFileName is the name of the application log file.
 const LogFileName = "debrief.log"
 
+// MaxHotkeyPreset is the maximum valid hotkey preset index.
+// Must match hotkey.PresetCount - 1.
+const MaxHotkeyPreset = 2
+
 // PollingInterval is the fallback interval for polling history file changes
 // when filesystem notifications (fsnotify) are unavailable.
 const PollingInterval = 5 * time.Second

infra/hotkey/gnome_linux.go

@@ -84,6 +84,18 @@ func (g *gnomeBackend) Register() error {
 
 	g.conn = conn
 
+	// closeOnError releases D-Bus resources if registration fails partway.
+	cleanup := true
+	defer func() {
+		if cleanup {
+			if closeErr := conn.Close(); closeErr != nil {
+				log.Printf("Hotkey GNOME: error closing D-Bus connection: %v", closeErr)
+			}
+
+			g.conn = nil
+		}
+	}()
+
 	reply, err := conn.RequestName(gnomeDBusName, dbus.NameFlagDoNotQueue|dbus.NameFlagReplaceExisting)
 	if err != nil {
 		return fmt.Errorf("failed to request D-Bus name %s: %w", gnomeDBusName, err)
@@ -150,6 +162,8 @@ func (g *gnomeBackend) Register() error {
 
 	log.Printf("Hotkey GNOME: Registered shortcut with binding %s", trigger)
 
+	cleanup = false // Success — don't close the connection
+
 	return nil
 }
 

infra/hotkey/hotkey.go

@@ -166,15 +166,24 @@ func (m *Manager) UpdateHotkey(mods []hk.Modifier, key hk.Key, modStrs []string,
 		m.registered = false
 	}
 
-	m.b = newBackend(mods, key, modStrs, keyStr)
+	b := newBackend(mods, key, modStrs, keyStr)
 
 	log.Printf("Registering hotkey: %v + %s", modStrs, keyStr)
 
-	if err := m.b.Register(); err != nil {
+	if err := b.Register(); err != nil {
+		// Clean up any resources the backend acquired during construction
+		// (e.g., portal backend opens a D-Bus connection in newPortalBackend).
+		if unregErr := b.Unregister(); unregErr != nil {
+			log.Printf("Failed to clean up failed hotkey backend: %v", unregErr)
+		}
+
 		log.Printf("Failed to register hotkey: %v", err)
+
 		return fmt.Errorf("failed to register hotkey (%v + %s): %w", modStrs, keyStr, err)
 	}
 
+	m.b = b
+
 	m.registered = true
 	m.done = make(chan struct{})
 

infra/hotkey/portal_linux.go

@@ -34,6 +34,7 @@ type portalBackend struct {
 	sessionPath      dbus.ObjectPath
 	preferredTrigger string
 	keyChan          chan struct{}
+	done             chan struct{}
 	mu               sync.Mutex
 	closed           bool
 }
@@ -81,6 +82,7 @@ func newPortalBackend(modStrs []string, keyStr string) (*portalBackend, error) {
 		conn:             conn,
 		preferredTrigger: buildTriggerString(modStrs, keyStr),
 		keyChan:          make(chan struct{}, 1),
+		done:             make(chan struct{}),
 	}, nil
 }
 
@@ -169,6 +171,7 @@ func (p *portalBackend) Register() error {
 	call = obj.Call(portalShortcutIf+".BindShortcuts", 0,
 		p.sessionPath, shortcuts, "", bindOpts)
 	if call.Err != nil {
+		p.closeSession()
 		return fmt.Errorf("BindShortcuts call failed: %w", call.Err)
 	}
 
@@ -178,10 +181,12 @@ func (p *portalBackend) Register() error {
 
 	responseCode, _, err = waitResponse(sigChan, expectedBindPath)
 	if err != nil {
+		p.closeSession()
 		return fmt.Errorf("BindShortcuts response failed: %w", err)
 	}
 
 	if responseCode != 0 {
+		p.closeSession()
 		return fmt.Errorf("BindShortcuts denied (response code: %d)", responseCode)
 	}
 
@@ -218,31 +223,58 @@ func (p *portalBackend) listenActivated() {
 
 	log.Println("Hotkey portal: Listening for Activated signals")
 
-	for sig := range sigChan {
-		if sig.Name != portalShortcutIf+".Activated" {
-			continue
-		}
+	for {
+		select {
+		case <-p.done:
+			log.Println("Hotkey portal: Listener stopped")
+			return
+		case sig, ok := <-sigChan:
+			if !ok {
+				log.Println("Hotkey portal: Signal channel closed")
+				return
+			}
 
-		// Activated signal body: (session_handle, shortcut_id, timestamp, options)
-		if len(sig.Body) < minResponseBodyLen {
-			continue
-		}
+			if sig.Name != portalShortcutIf+".Activated" {
+				continue
+			}
 
-		id, ok := sig.Body[1].(string)
-		if !ok || id != shortcutID {
-			continue
-		}
+			// Activated signal body: (session_handle, shortcut_id, timestamp, options)
+			if len(sig.Body) < minResponseBodyLen {
+				continue
+			}
+
+			id, ok := sig.Body[1].(string)
+			if !ok || id != shortcutID {
+				continue
+			}
 
-		log.Println("Hotkey portal: Shortcut activated")
+			log.Println("Hotkey portal: Shortcut activated")
 
-		select {
-		case p.keyChan <- struct{}{}:
-		default:
-			// Channel full, skip duplicate
+			select {
+			case p.keyChan <- struct{}{}:
+			default:
+				// Channel full, skip duplicate
+			}
 		}
 	}
 }
 
+// closeSession closes the portal session via D-Bus.
+// Safe to call even if no session was created.
+func (p *portalBackend) closeSession() {
+	if p.sessionPath == "" {
+		return
+	}
+
+	sessionObj := p.conn.Object(portalDest, p.sessionPath)
+
+	if err := sessionObj.Call(portalSessionIf+".Close", 0).Err; err != nil {
+		log.Printf("Hotkey portal: warning: failed to close session: %v", err)
+	}
+
+	p.sessionPath = ""
+}
+
 // Unregister closes the portal session and D-Bus connection.
 func (p *portalBackend) Unregister() error {
 	p.mu.Lock()
@@ -253,15 +285,9 @@ func (p *portalBackend) Unregister() error {
 	}
 
 	p.closed = true
+	close(p.done)
 
-	// Close the portal session
-	if p.sessionPath != "" {
-		sessionObj := p.conn.Object(portalDest, p.sessionPath)
-
-		if err := sessionObj.Call(portalSessionIf+".Close", 0).Err; err != nil {
-			log.Printf("Hotkey portal: warning: failed to close session: %v", err)
-		}
-	}
+	p.closeSession()
 
 	if err := p.conn.Close(); err != nil {
 		return fmt.Errorf("failed to close D-Bus connection: %w", err)

infra/hotkey/x11.go

@@ -58,7 +58,9 @@ func (b *nativeBackend) Register() error {
 
 func (b *nativeBackend) Unregister() error {
 	b.once.Do(func() {
-		close(b.done)
+		if b.done != nil {
+			close(b.done)
+		}
 	})
 
 	if err := b.hk.Unregister(); err != nil {

infra/platform/platform.go

@@ -1,6 +1,7 @@
 package platform
 
 import (
+	"log"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -29,6 +30,8 @@ func ExpandPath(path string) string {
 			if path[1] == '/' || path[1] == '\\' {
 				return filepath.Join(home, path[2:])
 			}
+		} else {
+			log.Printf("Warning: failed to expand ~ in path %q: %v", path, err)
 		}
 	}
 

infra/tray/dispatch_darwin.go

@@ -21,7 +21,12 @@ static void dispatchTrayStart(void) {
 */
 import "C"
 
-var startFunc func()
+import "sync"
+
+var (
+	startFunc     func()
+	startFuncOnce sync.Once
+)
 
 //export goTrayStartCallback
 func goTrayStartCallback() {
@@ -34,6 +39,8 @@ func goTrayStartCallback() {
 // thread via dispatch_async(dispatch_get_main_queue(), ...).
 // This is required because NSStatusItem/NSMenu must be created on the main thread.
 func dispatchStartOnMainThread(f func()) {
-	startFunc = f
-	C.dispatchTrayStart()
+	startFuncOnce.Do(func() {
+		startFunc = f
+		C.dispatchTrayStart()
+	})
 }

infra/tray/menu.go

@@ -51,10 +51,16 @@ func SetupMenu(windowSignalChan chan<- string, shouldQuit chan<- bool) {
 			for {
 				select {
 				case <-mShow.ClickedCh:
-					handlers.WindowSignal <- "show"
+					select {
+					case handlers.WindowSignal <- "show":
+					default:
+					}
 
 				case <-mHide.ClickedCh:
-					handlers.WindowSignal <- "hide"
+					select {
+					case handlers.WindowSignal <- "hide":
+					default:
+					}
 
 				case <-mQuit.ClickedCh:
 					handlers.ShouldQuit <- true