Refactor pkg mutex sites to use deferred unlocks consistently

pkg/cli/mcp_server_cache.go

@@ -37,24 +37,33 @@ func newMCPCacheStore() *mcpCacheStore {
 	}
 }
 
+func (c *mcpCacheStore) getPermissionEntry(cacheKey string) (*permissionEntry, bool) {
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+	entry, ok := c.permissions[cacheKey]
+	return entry, ok
+}
+
+func (c *mcpCacheStore) deletePermissionEntryIfUnchanged(cacheKey string, entry *permissionEntry) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if currentEntry, ok := c.permissions[cacheKey]; ok && currentEntry == entry {
+		delete(c.permissions, cacheKey)
+	}
+}
+
 // GetPermission returns the cached permission for the given actor and repo, or ("", false) on cache miss.
 func (c *mcpCacheStore) GetPermission(actor, repo string) (string, bool) {
 	cacheKey := actor + ":" + repo
-	c.mu.RLock()
-	entry, ok := c.permissions[cacheKey]
+	entry, ok := c.getPermissionEntry(cacheKey)
 	if ok && time.Since(entry.timestamp) < c.permissionTTL {
-		perm := entry.permission
-		c.mu.RUnlock()
-		mcpServerCacheLog.Printf("Permission cache hit: actor=%s, repo=%s, permission=%s", actor, repo, perm)
-		return perm, true
+		mcpServerCacheLog.Printf("Permission cache hit: actor=%s, repo=%s, permission=%s", actor, repo, entry.permission)
+		return entry.permission, true
 	}
-	c.mu.RUnlock()
 	if ok {
 		// Expired — remove it
 		mcpServerCacheLog.Printf("Permission cache entry expired for actor=%s, repo=%s", actor, repo)
-		c.mu.Lock()
-		delete(c.permissions, cacheKey)
-		c.mu.Unlock()
+		c.deletePermissionEntryIfUnchanged(cacheKey, entry)
 	}
 	mcpServerCacheLog.Printf("Permission cache miss: actor=%s, repo=%s", actor, repo)
 	return "", false
@@ -65,11 +74,11 @@ func (c *mcpCacheStore) SetPermission(actor, repo, permission string) {
 	cacheKey := actor + ":" + repo
 	mcpServerCacheLog.Printf("Caching permission: actor=%s, repo=%s, permission=%s", actor, repo, permission)
 	c.mu.Lock()
+	defer c.mu.Unlock()
 	c.permissions[cacheKey] = &permissionEntry{
 		permission: permission,
 		timestamp:  time.Now(),
 	}
-	c.mu.Unlock()
 }
 
 // GetRepo returns the cached repository name, or ("", false) on cache miss.
@@ -88,11 +97,11 @@ func (c *mcpCacheStore) GetRepo() (string, bool) {
 func (c *mcpCacheStore) SetRepo(repository string) {
 	mcpServerCacheLog.Printf("Caching repository: %s", repository)
 	c.mu.Lock()
+	defer c.mu.Unlock()
 	c.repo = &repoEntry{
 		repository: repository,
 		timestamp:  time.Now(),
 	}
-	c.mu.Unlock()
 }
 
 var mcpCache = newMCPCacheStore()

pkg/cli/mcp_server_cache_test.go

@@ -81,6 +81,27 @@ func TestMCPCacheStore_PermissionExpiry(t *testing.T) {
 	}
 }
 
+func TestMCPCacheStore_DeletePermissionEntryIfUnchanged_PreservesRefreshedEntry(t *testing.T) {
+	cache := newMCPCacheStore()
+	cacheKey := "actor:owner/repo"
+	expiredEntry := &permissionEntry{
+		permission: "read",
+		timestamp:  time.Now().Add(-2 * cache.permissionTTL),
+	}
+	cache.permissions[cacheKey] = expiredEntry
+
+	cache.SetPermission("actor", "owner/repo", "write")
+	cache.deletePermissionEntryIfUnchanged(cacheKey, expiredEntry)
+
+	entry, ok := cache.getPermissionEntry(cacheKey)
+	if !ok {
+		t.Fatal("expected refreshed permission entry to remain cached")
+	}
+	if entry.permission != "write" {
+		t.Fatalf("expected refreshed permission to be preserved, got %q", entry.permission)
+	}
+}
+
 func TestMCPCacheStore_RepoExpiry(t *testing.T) {
 	cache := newMCPCacheStore()
 	cache.repoTTL = 10 * time.Millisecond

pkg/cli/repo.go

@@ -91,14 +91,15 @@ func getCurrentRepoSlugUncached() (string, error) {
 // GetCurrentRepoSlug gets the current repository slug with caching.
 // This is the recommended function to use for repository access across the codebase.
 func GetCurrentRepoSlug() (string, error) {
-	currentRepoSlugCache.mu.Lock()
-	if !currentRepoSlugCache.done {
-		currentRepoSlugCache.result, currentRepoSlugCache.err = getCurrentRepoSlugUncached()
-		currentRepoSlugCache.done = true
-	}
-	result := currentRepoSlugCache.result
-	err := currentRepoSlugCache.err
-	currentRepoSlugCache.mu.Unlock()
+	result, err := func() (string, error) {
+		currentRepoSlugCache.mu.Lock()
+		defer currentRepoSlugCache.mu.Unlock()
+		if !currentRepoSlugCache.done {
+			currentRepoSlugCache.result, currentRepoSlugCache.err = getCurrentRepoSlugUncached()
+			currentRepoSlugCache.done = true
+		}
+		return currentRepoSlugCache.result, currentRepoSlugCache.err
+	}()
 
 	if err != nil {
 		return "", err

pkg/logger/logger.go

@@ -112,11 +112,7 @@ func (l *Logger) Printf(format string, args ...any) {
 	if !l.enabled {
 		return
 	}
-	l.mu.Lock()
-	now := time.Now()
-	diff := now.Sub(l.lastLog)
-	l.lastLog = now
-	l.mu.Unlock()
+	diff := l.tickTime()
 
 	message := fmt.Sprintf(format, args...)
 	lipgloss.Fprintf(os.Stderr, "%s %s +%s\n", l.label, message, timeutil.FormatDuration(diff))
@@ -129,14 +125,19 @@ func (l *Logger) Print(args ...any) {
 	if !l.enabled {
 		return
 	}
+	diff := l.tickTime()
+
+	message := fmt.Sprint(args...)
+	lipgloss.Fprintf(os.Stderr, "%s %s +%s\n", l.label, message, timeutil.FormatDuration(diff))
+}
+
+func (l *Logger) tickTime() time.Duration {
 	l.mu.Lock()
+	defer l.mu.Unlock()
 	now := time.Now()
 	diff := now.Sub(l.lastLog)
 	l.lastLog = now
-	l.mu.Unlock()
-
-	message := fmt.Sprint(args...)
-	lipgloss.Fprintf(os.Stderr, "%s %s +%s\n", l.label, message, timeutil.FormatDuration(diff))
+	return diff
 }
 
 // computeEnabled computes whether a namespace matches the DEBUG patterns

pkg/workflow/repository_features_validation.go

@@ -157,14 +157,15 @@ func (c *Compiler) validateRepositoryFeatures(workflowData *WorkflowData) error
 
 // getCurrentRepository gets the current repository from git context (with caching)
 func getCurrentRepository() (string, error) {
-	currentRepositoryCache.mu.Lock()
-	if !currentRepositoryCache.done {
-		currentRepositoryCache.result, currentRepositoryCache.err = getCurrentRepositoryUncached()
-		currentRepositoryCache.done = true
-	}
-	result := currentRepositoryCache.result
-	err := currentRepositoryCache.err
-	currentRepositoryCache.mu.Unlock()
+	result, err := func() (string, error) {
+		currentRepositoryCache.mu.Lock()
+		defer currentRepositoryCache.mu.Unlock()
+		if !currentRepositoryCache.done {
+			currentRepositoryCache.result, currentRepositoryCache.err = getCurrentRepositoryUncached()
+			currentRepositoryCache.done = true
+		}
+		return currentRepositoryCache.result, currentRepositoryCache.err
+	}()
 
 	if err != nil {
 		return "", err