From 7a99547b22d60582414a6b5d0ffb108bfff4df44 Mon Sep 17 00:00:00 2001
From: Alan Wizemann <alan@wizemann.com>
Date: Thu, 23 Apr 2026 00:59:58 +0200
Subject: [PATCH] fix: address code-review findings from Apr 22 commits
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three follow-ups from reviewing 1989fee (sidebar-width persist) and
4163595 (default server on launch):

- `SplitViewAutosaveFinder` hardcoded `"ScarfMainSidebar"` for every
  window. Since Scarf's `WindowGroup` spawns one window per `ServerID`,
  all windows shared the same `NSSplitView.autosaveName` — AppKit
  documents that name as required-unique, and in practice per-window
  widths collapsed onto a single UserDefaults key. Thread the window's
  `ServerContext` in through `@Environment(\.serverContext)` (already
  wired at `WindowGroup` construction) and suffix the name with the
  server UUID.
- `setDefaultServer` fired `onEntriesChanged`, whose sole consumer is
  `ServerLiveStatusRegistry.rebuild()` for menu-bar fanout. Flipping a
  default flag doesn't change the set of servers; the callback was
  semantic noise. Drop the call — SwiftUI views still redraw on the
  flag flip via `@Observable`'s tracking of `entries`.
- The filled-yellow star in `ManageServersView` had a no-op action
  inside `if !isDefault { ... }` but still animated its pressed state
  on click. Replace the conditional with `.disabled(isDefault)` so the
  row is visually inert when it already is the default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 scarf/scarf/Core/Persistence/ServerRegistry.swift        | 6 +++++-
 .../scarf/Features/Servers/Views/ManageServersView.swift | 9 +++++----
 scarf/scarf/Navigation/SidebarView.swift                 | 3 ++-
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/scarf/scarf/Core/Persistence/ServerRegistry.swift b/scarf/scarf/Core/Persistence/ServerRegistry.swift
index 666f2da..246a242 100644
--- a/scarf/scarf/Core/Persistence/ServerRegistry.swift
+++ b/scarf/scarf/Core/Persistence/ServerRegistry.swift
@@ -82,6 +82,11 @@ final class ServerRegistry {
     /// Flip the default server to `id`. Passing `ServerContext.local.id`
     /// clears the flag on every remote entry, making Local the implicit
     /// default. Passing an unknown ID is a no-op. Persisted on return.
+    ///
+    /// Intentionally doesn't fire `onEntriesChanged` — that hook means "the
+    /// set of servers changed" and drives the menu-bar fanout rebuild. A
+    /// default-flag flip doesn't change the set; SwiftUI views reading
+    /// `defaultServerID` redraw via `@Observable`'s tracking of `entries`.
     func setDefaultServer(_ id: ServerID) {
         var changed = false
         for idx in entries.indices {
@@ -93,7 +98,6 @@ final class ServerRegistry {
         }
         if changed {
             save()
-            onEntriesChanged?()
         }
     }
 
diff --git a/scarf/scarf/Features/Servers/Views/ManageServersView.swift b/scarf/scarf/Features/Servers/Views/ManageServersView.swift
index 504c842..f702b48 100644
--- a/scarf/scarf/Features/Servers/Views/ManageServersView.swift
+++ b/scarf/scarf/Features/Servers/Views/ManageServersView.swift
@@ -143,19 +143,20 @@ struct ManageServersView: View {
     }
 
     /// A star button that marks the open-on-launch default. Filled + yellow
-    /// on the current default row (and non-interactive — clicking it is a
-    /// no-op since the flag is already set); outline + secondary elsewhere,
-    /// clicking promotes that row to default.
+    /// on the current default row (disabled, since clicking would be a
+    /// no-op); outline + secondary elsewhere, clicking promotes that row
+    /// to default.
     @ViewBuilder
     private func defaultStar(for id: ServerID, currentDefault: ServerID) -> some View {
         let isDefault = id == currentDefault
         Button {
-            if !isDefault { registry.setDefaultServer(id) }
+            registry.setDefaultServer(id)
         } label: {
             Image(systemName: isDefault ? "star.fill" : "star")
                 .foregroundStyle(isDefault ? .yellow : .secondary)
         }
         .buttonStyle(.borderless)
+        .disabled(isDefault)
         .help(isDefault ? "Opens on launch" : "Set as default — open this server when Scarf launches.")
     }
 
diff --git a/scarf/scarf/Navigation/SidebarView.swift b/scarf/scarf/Navigation/SidebarView.swift
index ee91083..d8cbc5f 100644
--- a/scarf/scarf/Navigation/SidebarView.swift
+++ b/scarf/scarf/Navigation/SidebarView.swift
@@ -2,6 +2,7 @@ import SwiftUI
 
 struct SidebarView: View {
     @Environment(AppCoordinator.self) private var coordinator
+    @Environment(\.serverContext) private var serverContext
 
     var body: some View {
         @Bindable var coordinator = coordinator
@@ -59,6 +60,6 @@ struct SidebarView: View {
         }
         .listStyle(.sidebar)
         .navigationTitle("Scarf")
-        .splitViewAutosaveName("ScarfMainSidebar")
+        .splitViewAutosaveName("ScarfMainSidebar.\(serverContext.id)")
     }
 }