Merge branch 'main' into add-repo-collaborators

Author: JoannaaKL

pkg/github/context_tools_test.go

@@ -192,10 +192,7 @@ func Test_GetMe_IFC_InsidersMode(t *testing.T) {
 		require.NoError(t, err)
 
 		assert.Equal(t, "trusted", ifcMap["integrity"])
-		confList, ok := ifcMap["confidentiality"].([]any)
-		require.True(t, ok, "confidentiality should be a list")
-		require.Len(t, confList, 1)
-		assert.Equal(t, "public", confList[0])
+		assert.Equal(t, "public", ifcMap["confidentiality"])
 	})
 }
 

pkg/github/helper_test.go

@@ -31,7 +31,6 @@ const (
 	GetReposByOwnerByRepo                = "GET /repos/{owner}/{repo}"
 	GetReposBranchesByOwnerByRepo        = "GET /repos/{owner}/{repo}/branches"
 	GetReposTagsByOwnerByRepo            = "GET /repos/{owner}/{repo}/tags"
-	GetReposCollaboratorsByOwnerByRepo   = "GET /repos/{owner}/{repo}/collaborators"
 	GetReposCommitsByOwnerByRepo         = "GET /repos/{owner}/{repo}/commits"
 	GetReposCommitsByOwnerByRepoByRef    = "GET /repos/{owner}/{repo}/commits/{ref}"
 	GetReposContentsByOwnerByRepoByPath  = "GET /repos/{owner}/{repo}/contents/{path}"

pkg/github/issues.go

@@ -298,11 +298,7 @@ Options are:
 
 			// attachIFC adds the IFC label to a successful tool result when
 			// InsidersMode is enabled. If the visibility lookup fails the
-			// label is omitted rather than misclassifying the result. If
-			// only the collaborators lookup fails for a private repo we
-			// fall back to the owner so the reader set is never empty. The
-			// label matches list_issues semantics: per-repo visibility,
-			// integrity always untrusted.
+			// label is omitted rather than misclassifying the result.
 			attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
 				if r == nil || r.IsError || !deps.GetFlags(ctx).InsidersMode {
 					return r
@@ -311,19 +307,10 @@ Options are:
 				if err != nil {
 					return r
 				}
-				var readers []string
-				if isPrivate {
-					if collaborators, err := FetchRepoCollaborators(ctx, client, owner, repo); err == nil {
-						readers = collaborators
-					}
-					if len(readers) == 0 {
-						readers = []string{owner}
-					}
-				}
 				if r.Meta == nil {
 					r.Meta = mcp.Meta{}
 				}
-				r.Meta["ifc"] = ifc.LabelListIssues(isPrivate, readers)
+				r.Meta["ifc"] = ifc.LabelListIssues(isPrivate)
 				return r
 			}
 
@@ -1034,36 +1021,18 @@ func searchIssuesIFCPostProcess(deps ToolDependencies) searchPostProcessFn {
 
 		uniqueRepos := uniqueSearchIssuesRepos(result)
 		visibilities := make([]bool, 0, len(uniqueRepos))
-		readerSets := make([][]string, 0, len(uniqueRepos))
 		for _, r := range uniqueRepos {
 			isPrivate, err := FetchRepoIsPrivate(ctx, client, r.owner, r.repo)
 			if err != nil {
 				return
 			}
 			visibilities = append(visibilities, isPrivate)
-			if !isPrivate {
-				readerSets = append(readerSets, nil)
-				continue
-			}
-			collaborators, err := FetchRepoCollaborators(ctx, client, r.owner, r.repo)
-			if err != nil {
-				return
-			}
-			// Preserve an empty collaborator set as-is. Substituting the
-			// owner here would corrupt the cross-repo intersection (the
-			// owner could appear in another repo's collaborator list and
-			// widen the joined reader set incorrectly).
-			readerSets = append(readerSets, collaborators)
 		}
 
-		label, ok := ifc.LabelSearchIssues(visibilities, readerSets)
-		if !ok {
-			return
-		}
 		if callResult.Meta == nil {
 			callResult.Meta = mcp.Meta{}
 		}
-		callResult.Meta["ifc"] = label
+		callResult.Meta["ifc"] = ifc.LabelSearchIssues(visibilities)
 	}
 }
 
@@ -1728,22 +1697,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
 				if result.Meta == nil {
 					result.Meta = mcp.Meta{}
 				}
-				var readers []string
-				if isPrivate {
-					restClient, err := deps.GetClient(ctx)
-					if err == nil {
-						if collaborators, err := FetchRepoCollaborators(ctx, restClient, owner, repo); err == nil {
-							readers = collaborators
-						}
-					}
-					// Fall back to the repository owner so the reader set is
-					// never empty for a private repository even if the
-					// collaborators lookup fails.
-					if len(readers) == 0 {
-						readers = []string{owner}
-					}
-				}
-				result.Meta["ifc"] = ifc.LabelListIssues(isPrivate, readers)
+				result.Meta["ifc"] = ifc.LabelListIssues(isPrivate)
 			}
 			return result, nil, nil
 		})

pkg/github/issues_test.go

@@ -297,10 +297,6 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
 		handlers := map[string]http.HandlerFunc{
 			GetReposIssuesByOwnerByRepoByIssueNumber:         mockResponse(t, http.StatusOK, mockIssue),
 			GetReposIssuesCommentsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockComments),
-			GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
-				{Login: github.Ptr("octocat")},
-				{Login: github.Ptr("alice")},
-			}),
 		}
 		if repoStatus != 0 && repoStatus != http.StatusOK {
 			handlers[GetReposByOwnerByRepo] = mockResponse(t, repoStatus, "boom")
@@ -356,7 +352,7 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
 		require.NotNil(t, result.Meta)
 		ifcMap := unmarshalIFC(t, result.Meta["ifc"])
 		assert.Equal(t, "untrusted", ifcMap["integrity"])
-		assert.Equal(t, []any{"public"}, ifcMap["confidentiality"])
+		assert.Equal(t, "public", ifcMap["confidentiality"])
 	})
 
 	t.Run("insiders mode enabled on private repo with get_comments emits private untrusted", func(t *testing.T) {
@@ -374,7 +370,7 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
 		require.NotNil(t, result.Meta)
 		ifcMap := unmarshalIFC(t, result.Meta["ifc"])
 		assert.Equal(t, "untrusted", ifcMap["integrity"])
-		assert.Equal(t, []any{"octocat", "alice"}, ifcMap["confidentiality"])
+		assert.Equal(t, "private", ifcMap["confidentiality"])
 	})
 
 	t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) {
@@ -829,23 +825,17 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
 	}
 
 	type repoFixture struct {
-		owner               string
-		repo                string
-		isPrivate           bool
-		collaborators       []string
-		repoStatus          int
-		collaboratorsStatus int
+		owner      string
+		repo       string
+		isPrivate  bool
+		repoStatus int
 	}
 
 	repoHandlers := func(repos []repoFixture) map[string]http.HandlerFunc {
 		repoByPath := map[string]repoFixture{}
 		for _, r := range repos {
 			repoByPath["/repos/"+r.owner+"/"+r.repo] = r
 		}
-		collaboratorsByPath := map[string]repoFixture{}
-		for _, r := range repos {
-			collaboratorsByPath["/repos/"+r.owner+"/"+r.repo+"/collaborators"] = r
-		}
 		return map[string]http.HandlerFunc{
 			GetReposByOwnerByRepo: func(w http.ResponseWriter, req *http.Request) {
 				r, ok := repoByPath[req.URL.Path]
@@ -864,25 +854,6 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
 				w.WriteHeader(http.StatusOK)
 				_, _ = w.Write(body)
 			},
-			GetReposCollaboratorsByOwnerByRepo: func(w http.ResponseWriter, req *http.Request) {
-				r, ok := collaboratorsByPath[req.URL.Path]
-				if !ok {
-					w.WriteHeader(http.StatusOK)
-					_, _ = w.Write([]byte("[]"))
-					return
-				}
-				if r.collaboratorsStatus != 0 && r.collaboratorsStatus != http.StatusOK {
-					w.WriteHeader(r.collaboratorsStatus)
-					return
-				}
-				users := make([]*github.User, len(r.collaborators))
-				for i, login := range r.collaborators {
-					users[i] = &github.User{Login: github.Ptr(login)}
-				}
-				body, _ := json.Marshal(users)
-				w.WriteHeader(http.StatusOK)
-				_, _ = w.Write(body)
-			},
 		}
 	}
 
@@ -909,7 +880,7 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
 		assert.Nil(t, result.Meta)
 	})
 
-	t.Run("insiders mode enabled with single public repo emits public untrusted", func(t *testing.T) {
+	t.Run("insiders mode all public emits public untrusted", func(t *testing.T) {
 		searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "public-repo", 1)}}
 		deps := BaseDeps{
 			Client: github.NewClient(makeMockClient(searchResult, []repoFixture{{owner: "octocat", repo: "public-repo"}})),
@@ -925,17 +896,17 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
 		require.NotNil(t, result.Meta)
 		ifcMap := unmarshalIFC(t, result.Meta["ifc"])
 		assert.Equal(t, "untrusted", ifcMap["integrity"])
-		assert.Equal(t, []any{"public"}, ifcMap["confidentiality"])
+		assert.Equal(t, "public", ifcMap["confidentiality"])
 	})
 
-	t.Run("insiders mode mixed public and private keeps the private readers", func(t *testing.T) {
+	t.Run("insiders mode mixed public and private emits private untrusted", func(t *testing.T) {
 		searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{
 			makeIssue("octocat", "private-repo", 1),
 			makeIssue("octocat", "public-repo", 2),
 		}}
 		deps := BaseDeps{
 			Client: github.NewClient(makeMockClient(searchResult, []repoFixture{
-				{owner: "octocat", repo: "private-repo", isPrivate: true, collaborators: []string{"alice"}},
+				{owner: "octocat", repo: "private-repo", isPrivate: true},
 				{owner: "octocat", repo: "public-repo"},
 			})),
 			Flags: FeatureFlags{InsidersMode: true},
@@ -950,32 +921,7 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
 		require.NotNil(t, result.Meta)
 		ifcMap := unmarshalIFC(t, result.Meta["ifc"])
 		assert.Equal(t, "untrusted", ifcMap["integrity"])
-		assert.Equal(t, []any{"alice"}, ifcMap["confidentiality"])
-	})
-
-	t.Run("insiders mode two private repos intersect collaborators", func(t *testing.T) {
-		searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{
-			makeIssue("octocat", "repo-a", 1),
-			makeIssue("octocat", "repo-b", 2),
-		}}
-		deps := BaseDeps{
-			Client: github.NewClient(makeMockClient(searchResult, []repoFixture{
-				{owner: "octocat", repo: "repo-a", isPrivate: true, collaborators: []string{"alice", "bob", "carol"}},
-				{owner: "octocat", repo: "repo-b", isPrivate: true, collaborators: []string{"bob", "carol", "dan"}},
-			})),
-			Flags: FeatureFlags{InsidersMode: true},
-		}
-		handler := serverTool.Handler(deps)
-
-		request := createMCPRequest(reqParams)
-		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
-		require.NoError(t, err)
-		require.False(t, result.IsError)
-
-		require.NotNil(t, result.Meta)
-		ifcMap := unmarshalIFC(t, result.Meta["ifc"])
-		assert.Equal(t, "untrusted", ifcMap["integrity"])
-		assert.Equal(t, []any{"bob", "carol"}, ifcMap["confidentiality"])
+		assert.Equal(t, "private", ifcMap["confidentiality"])
 	})
 
 	t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) {
@@ -999,27 +945,6 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
 		}
 	})
 
-	t.Run("insiders mode skips ifc label when collaborators lookup fails", func(t *testing.T) {
-		searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "private-repo", 1)}}
-		deps := BaseDeps{
-			Client: github.NewClient(makeMockClient(searchResult, []repoFixture{
-				{owner: "octocat", repo: "private-repo", isPrivate: true, collaboratorsStatus: http.StatusInternalServerError},
-			})),
-			Flags: FeatureFlags{InsidersMode: true},
-		}
-		handler := serverTool.Handler(deps)
-
-		request := createMCPRequest(reqParams)
-		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
-		require.NoError(t, err)
-		require.False(t, result.IsError, "tool call should still succeed when collaborators lookup fails")
-
-		if result.Meta != nil {
-			_, hasIFC := result.Meta["ifc"]
-			assert.False(t, hasIFC, "ifc label should be omitted when collaborators lookup fails")
-		}
-	})
-
 	t.Run("insiders mode empty results emits public untrusted", func(t *testing.T) {
 		searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{}}
 		deps := BaseDeps{
@@ -1036,7 +961,7 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
 		require.NotNil(t, result.Meta)
 		ifcMap := unmarshalIFC(t, result.Meta["ifc"])
 		assert.Equal(t, "untrusted", ifcMap["integrity"])
-		assert.Equal(t, []any{"public"}, ifcMap["confidentiality"])
+		assert.Equal(t, "public", ifcMap["confidentiality"])
 	})
 }
 
@@ -1804,24 +1729,13 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
 		require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
 
 		assert.Equal(t, "untrusted", ifcMap["integrity"])
-		confList, ok := ifcMap["confidentiality"].([]any)
-		require.True(t, ok, "confidentiality should be a list")
-		require.Len(t, confList, 1)
-		assert.Equal(t, "public", confList[0])
+		assert.Equal(t, "public", ifcMap["confidentiality"])
 	})
 
-	t.Run("insiders mode enabled on private repo emits private untrusted label with collaborators", func(t *testing.T) {
+	t.Run("insiders mode enabled on private repo emits private untrusted label", func(t *testing.T) {
 		matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
 		gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
-		restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
-			GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
-				{Login: github.Ptr("octocat")},
-				{Login: github.Ptr("alice")},
-				{Login: github.Ptr("bob")},
-			}),
-		}))
 		deps := BaseDeps{
-			Client:    restClient,
 			GQLClient: gqlClient,
 			Flags:     FeatureFlags{InsidersMode: true},
 		}
@@ -1842,36 +1756,7 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
 		require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
 
 		assert.Equal(t, "untrusted", ifcMap["integrity"])
-		confList, ok := ifcMap["confidentiality"].([]any)
-		require.True(t, ok, "confidentiality should be a list")
-		assert.Equal(t, []any{"octocat", "alice", "bob"}, confList)
-	})
-
-	t.Run("insiders mode enabled on private repo falls back to owner when collaborators lookup fails", func(t *testing.T) {
-		matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
-		gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
-		restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
-			GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusInternalServerError, "boom"),
-		}))
-		deps := BaseDeps{
-			Client:    restClient,
-			GQLClient: gqlClient,
-			Flags:     FeatureFlags{InsidersMode: true},
-		}
-		handler := serverTool.Handler(deps)
-
-		request := createMCPRequest(reqParams)
-		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
-		require.NoError(t, err)
-		require.False(t, result.IsError)
-
-		require.NotNil(t, result.Meta)
-		ifcJSON, err := json.Marshal(result.Meta["ifc"])
-		require.NoError(t, err)
-		var ifcMap map[string]any
-		require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
-
-		assert.Equal(t, []any{"octocat"}, ifcMap["confidentiality"])
+		assert.Equal(t, "private", ifcMap["confidentiality"])
 	})
 }