From 8e2b74ae2ea7170830415ac46ed769ee5c9968ee Mon Sep 17 00:00:00 2001 From: Jiang Bohan Date: Wed, 15 Apr 2026 19:12:56 +0800 Subject: [PATCH] fix(daemon): normalize repo URL and clarify reposVersion intent - TrimSpace incoming repoURL in ensureRepoReady to prevent unnecessary server refreshes when CLI passes URLs with whitespace - Add comment on reposVersion field clarifying it is stored for future version-based skip optimization - Add concurrency safety comment on syncWorkspacesFromAPI skip logic - Add test for URL trimming fast-path behavior --- server/internal/daemon/daemon.go | 6 ++++-- server/internal/daemon/daemon_test.go | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/server/internal/daemon/daemon.go b/server/internal/daemon/daemon.go index 6c36f5d936..d0472299b9 100644 --- a/server/internal/daemon/daemon.go +++ b/server/internal/daemon/daemon.go @@ -28,7 +28,7 @@ var ErrRepoNotConfigured = errors.New("repo is not configured for this workspace type workspaceState struct { workspaceID string runtimeIDs []string - reposVersion string + reposVersion string // stored for future use: skip refresh when version unchanged allowedRepoURLs map[string]struct{} lastRepoSyncErr string repoRefreshMu sync.Mutex @@ -318,6 +318,8 @@ func (d *Daemon) ensureRepoReady(ctx context.Context, workspaceID, repoURL strin return fmt.Errorf("repo cache not initialized") } + repoURL = strings.TrimSpace(repoURL) + d.mu.Lock() ws, ok := d.workspaces[workspaceID] d.mu.Unlock() @@ -406,7 +408,7 @@ func (d *Daemon) syncWorkspacesFromAPI(ctx context.Context) error { var registered int for id, name := range apiIDs { if currentIDs[id] { - continue + continue // important: never replace existing workspaceState; ensureRepoReady holds ws.repoRefreshMu from the original pointer } resp, err := d.registerRuntimesForWorkspace(ctx, id) if err != nil { diff --git a/server/internal/daemon/daemon_test.go b/server/internal/daemon/daemon_test.go index 014f18285c..25883566fb 100644 --- a/server/internal/daemon/daemon_test.go +++ b/server/internal/daemon/daemon_test.go @@ -342,6 +342,29 @@ func TestEnsureRepoReadyFastPathDoesNotRefresh(t *testing.T) { } } +func TestEnsureRepoReadyTrimsURL(t *testing.T) { + t.Parallel() + + sourceRepo := createDaemonTestRepo(t) + var refreshCalls atomic.Int32 + d := newRepoReadyTestDaemon(t, func(w http.ResponseWriter, r *http.Request) { + refreshCalls.Add(1) + http.Error(w, "unexpected refresh", http.StatusInternalServerError) + }) + if err := d.repoCache.Sync("ws-1", []repocache.RepoInfo{{URL: sourceRepo}}); err != nil { + t.Fatalf("seed repo cache: %v", err) + } + d.workspaces["ws-1"] = newWorkspaceState("ws-1", nil, "v1", []RepoData{{URL: sourceRepo}}) + + // URL with trailing whitespace should still hit the fast path. + if err := d.ensureRepoReady(context.Background(), "ws-1", " "+sourceRepo+" "); err != nil { + t.Fatalf("ensureRepoReady with padded URL: %v", err) + } + if got := refreshCalls.Load(); got != 0 { + t.Fatalf("expected no refresh calls for trimmed URL, got %d", got) + } +} + func TestEnsureRepoReadyRefreshesOnMiss(t *testing.T) { t.Parallel()