Hide lockdown logic behind shouldFilter function

Author: JoannaaKL

pkg/github/issues.go

@@ -331,11 +331,11 @@ func GetIssue(ctx context.Context, client *github.Client, cache *lockdown.RepoAc
 		}
 		login := issue.GetUser().GetLogin()
 		if login != "" {
-			info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
+			shouldFilter, err := cache.ShouldFilterContent(ctx, login, owner, repo)
 			if err != nil {
 				return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
 			}
-			if info.ViewerLogin != login && !info.IsPrivate && !info.HasPushAccess {
+			if shouldFilter {
 				return mcp.NewToolResultError("access to issue details is restricted by lockdown mode"), nil
 			}
 		}
@@ -394,16 +394,11 @@ func GetIssueComments(ctx context.Context, client *github.Client, cache *lockdow
 			if login == "" {
 				continue
 			}
-			info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
+			shouldFilter, err := cache.ShouldFilterContent(ctx, login, owner, repo)
 			if err != nil {
 				return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
 			}
-			// Do not filter content for private repositories or if the comment author is the viewer
-			if info.IsPrivate || info.ViewerLogin == login {
-				filteredComments = comments
-				break
-			}
-			if info.HasPushAccess {
+			if !shouldFilter {
 				filteredComments = append(filteredComments, comment)
 			}
 		}
@@ -459,16 +454,11 @@ func GetSubIssues(ctx context.Context, client *github.Client, cache *lockdown.Re
 			if login == "" {
 				continue
 			}
-			info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
+			shouldFilter, err := cache.ShouldFilterContent(ctx, login, owner, repo)
 			if err != nil {
 				return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
 			}
-			// Repo is private or the comment author is the viewer, do not filter content
-			if info.IsPrivate || info.ViewerLogin == login {
-				filteredSubIssues = subIssues
-				break
-			}
-			if info.HasPushAccess {
+			if !shouldFilter {
 				filteredSubIssues = append(filteredSubIssues, subIssue)
 			}
 		}

pkg/github/pullrequests.go

@@ -141,12 +141,12 @@ func GetPullRequest(ctx context.Context, client *github.Client, cache *lockdown.
 		}
 		login := pr.GetUser().GetLogin()
 		if login != "" {
-			info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
+			shouldFilter, err := cache.ShouldFilterContent(ctx, login, owner, repo)
 			if err != nil {
 				return nil, fmt.Errorf("failed to check content removal: %w", err)
 			}
 
-			if info.ViewerLogin != login && !info.IsPrivate && !info.HasPushAccess {
+			if shouldFilter {
 				return mcp.NewToolResultError("access to pull request is restricted by lockdown mode"), nil
 			}
 		}
@@ -303,16 +303,11 @@ func GetPullRequestReviewComments(ctx context.Context, client *github.Client, ca
 			if user == nil {
 				continue
 			}
-			info, err := cache.GetRepoAccessInfo(ctx, user.GetLogin(), owner, repo)
+			shouldFilter, err := cache.ShouldFilterContent(ctx, user.GetLogin(), owner, repo)
 			if err != nil {
 				return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
 			}
-			// Do not filter content for private repositories or if the comment author is the viewer
-			if info.IsPrivate || info.ViewerLogin == user.GetLogin() {
-				filteredComments = comments
-				break
-			}
-			if info.HasPushAccess {
+			if !shouldFilter {
 				filteredComments = append(filteredComments, comment)
 			}
 		}
@@ -354,14 +349,11 @@ func GetPullRequestReviews(ctx context.Context, client *github.Client, cache *lo
 		for _, review := range reviews {
 			login := review.GetUser().GetLogin()
 			if login != "" {
-				info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
+				shouldFilter, err := cache.ShouldFilterContent(ctx, login, owner, repo)
 				if err != nil {
 					return nil, fmt.Errorf("failed to check lockdown mode: %w", err)
 				}
-				if info.IsPrivate || info.ViewerLogin == login {
-					filteredReviews = reviews
-				}
-				if info.HasPushAccess {
+				if !shouldFilter {
 					filteredReviews = append(filteredReviews, review)
 				}
 				reviews = filteredReviews

pkg/lockdown/lockdown.go

@@ -109,9 +109,19 @@ type CacheStats struct {
 	Evictions int64
 }
 
-// GetRepoAccessInfo returns repository access metadata for the provided user.
-// Results are cached per repository to avoid repeated GraphQL round-trips.
-func (c *RepoAccessCache) GetRepoAccessInfo(ctx context.Context, username, owner, repo string) (RepoAccessInfo, error) {
+func (c *RepoAccessCache) ShouldFilterContent(ctx context.Context, username, owner, repo string) (bool, error) {
+	repoInfo, err := c.getRepoAccessInfo(ctx, username, owner, repo)
+	if err != nil {
+		c.logDebug("error checking repo access info for content filtering", "owner", owner, "repo", repo, "user", username, "error", err)
+		return false, err
+	}
+	if repoInfo.IsPrivate || repoInfo.ViewerLogin == username {
+		return false, nil
+	}
+	return !repoInfo.HasPushAccess, nil
+}
+
+func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner, repo string) (RepoAccessInfo, error) {
 	if c == nil {
 		return RepoAccessInfo{}, fmt.Errorf("nil repo access cache")
 	}

pkg/lockdown/lockdown_test.go

@@ -96,15 +96,15 @@ func TestRepoAccessCacheEvictsAfterTTL(t *testing.T) {
 	ctx := t.Context()
 
 	cache, transport := newMockRepoAccessCache(t, 5*time.Millisecond)
-	info, err := cache.GetRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	info, err := cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
 	require.NoError(t, err)
 	require.Equal(t, testUser, info.ViewerLogin)
 	require.True(t, info.HasPushAccess)
 	require.EqualValues(t, 1, transport.CallCount())
 
 	time.Sleep(20 * time.Millisecond)
 
-	info, err = cache.GetRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	info, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
 	require.NoError(t, err)
 	require.Equal(t, testUser, info.ViewerLogin)
 	require.True(t, info.HasPushAccess)