search_issues: address Copilot review findings

- LabelSearchIssues now returns (SecurityLabel, bool); the bool is
  false when len(repoVisibilities) != len(readerSets), so callers can
  omit the label rather than emit one computed from inconsistent
  inputs.
- searchIssuesIFCPostProcess no longer substitutes [owner] when the
  collaborators API returns an empty list. The substitution was
  inconsistent with the cross-repo intersection semantics: the owner
  could appear in another matched private repo's collaborator list and
  thereby widen the joined reader set incorrectly. Empty collaborator
  sets are now passed through unchanged.
- Add a subtest exercising the collaborators-failure branch (500 on
  /repos/{owner}/{repo}/collaborators), asserting the tool still
  succeeds and result.Meta["ifc"] is absent.
- Extend the LabelSearchIssues table tests with the slice-length
  mismatch case.

Addresses the three Copilot findings on #2456.

Author: gokhanarkan

pkg/github/issues.go

@@ -1018,16 +1018,21 @@ func searchIssuesIFCPostProcess(deps ToolDependencies) searchPostProcessFn {
 			if err != nil {
 				return
 			}
-			if len(collaborators) == 0 {
-				collaborators = []string{r.owner}
-			}
+			// 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"] = ifc.LabelSearchIssues(visibilities, readerSets)
+		callResult.Meta["ifc"] = label
 	}
 }
 

pkg/github/issues_test.go

@@ -708,11 +708,12 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
 	}
 
 	type repoFixture struct {
-		owner         string
-		repo          string
-		isPrivate     bool
-		collaborators []string
-		repoStatus    int
+		owner               string
+		repo                string
+		isPrivate           bool
+		collaborators       []string
+		repoStatus          int
+		collaboratorsStatus int
 	}
 
 	repoHandlers := func(repos []repoFixture) map[string]http.HandlerFunc {
@@ -749,6 +750,10 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
 					_, _ = 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)}
@@ -873,6 +878,27 @@ 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{

pkg/ifc/ifc.go

@@ -104,17 +104,22 @@ func LabelGetFileContents(isPrivate bool, readers []string) SecurityLabel {
 //
 // repoVisibilities[i] reports whether the i-th matched repository is private;
 // readerSets[i] is that repository's reader set (only consulted for private
-// repos). The two slices must have the same length.
-func LabelSearchIssues(repoVisibilities []bool, readerSets [][]string) SecurityLabel {
+// repos). The two slices must have the same length; the second return value
+// is false when they do not, in which case the caller should omit the label
+// rather than emit one computed from inconsistent inputs.
+func LabelSearchIssues(repoVisibilities []bool, readerSets [][]string) (SecurityLabel, bool) {
+	if len(repoVisibilities) != len(readerSets) {
+		return SecurityLabel{}, false
+	}
 	if len(repoVisibilities) == 0 {
-		return PublicUntrusted()
+		return PublicUntrusted(), true
 	}
 	for _, isPrivate := range repoVisibilities {
 		if !isPrivate {
-			return PublicUntrusted()
+			return PublicUntrusted(), true
 		}
 	}
-	return PrivateUntrusted(intersectReaders(readerSets))
+	return PrivateUntrusted(intersectReaders(readerSets)), true
 }
 
 // intersectReaders returns the readers present in every set, preserving the

pkg/ifc/ifc_test.go

@@ -13,55 +13,72 @@ func TestLabelSearchIssues(t *testing.T) {
 		name             string
 		visibilities     []bool
 		readers          [][]string
+		wantOK           bool
 		wantIntegrity    Integrity
 		wantConfidential []Confidentiality
 	}{
 		{
 			name:             "empty result is treated as public",
+			wantOK:           true,
 			wantIntegrity:    IntegrityUntrusted,
 			wantConfidential: []Confidentiality{ConfidentialityPublic},
 		},
 		{
 			name:             "single public repo",
 			visibilities:     []bool{false},
 			readers:          [][]string{nil},
+			wantOK:           true,
 			wantIntegrity:    IntegrityUntrusted,
 			wantConfidential: []Confidentiality{ConfidentialityPublic},
 		},
 		{
 			name:             "mixed public and private collapses to public",
 			visibilities:     []bool{true, false},
 			readers:          [][]string{{"alice"}, nil},
+			wantOK:           true,
 			wantIntegrity:    IntegrityUntrusted,
 			wantConfidential: []Confidentiality{ConfidentialityPublic},
 		},
 		{
 			name:             "two private repos with intersecting collaborators",
 			visibilities:     []bool{true, true},
 			readers:          [][]string{{"alice", "bob", "carol"}, {"bob", "carol", "dan"}},
+			wantOK:           true,
 			wantIntegrity:    IntegrityUntrusted,
 			wantConfidential: []Confidentiality{"bob", "carol"},
 		},
 		{
 			name:             "private repos with no overlap yield empty reader set",
 			visibilities:     []bool{true, true},
 			readers:          [][]string{{"alice"}, {"bob"}},
+			wantOK:           true,
 			wantIntegrity:    IntegrityUntrusted,
 			wantConfidential: []Confidentiality{},
 		},
 		{
 			name:             "intersection preserves first-set order and dedupes",
 			visibilities:     []bool{true, true, true},
 			readers:          [][]string{{"alice", "bob", "alice"}, {"bob", "alice"}, {"alice", "bob"}},
+			wantOK:           true,
 			wantIntegrity:    IntegrityUntrusted,
 			wantConfidential: []Confidentiality{"alice", "bob"},
 		},
+		{
+			name:         "mismatched slice lengths return ok=false",
+			visibilities: []bool{true, true},
+			readers:      [][]string{{"alice"}},
+			wantOK:       false,
+		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-			label := LabelSearchIssues(tc.visibilities, tc.readers)
+			label, ok := LabelSearchIssues(tc.visibilities, tc.readers)
+			assert.Equal(t, tc.wantOK, ok)
+			if !tc.wantOK {
+				return
+			}
 			assert.Equal(t, tc.wantIntegrity, label.Integrity)
 			if len(tc.wantConfidential) == 0 {
 				assert.Empty(t, label.Confidentiality)