OPS-440: Fix N+1 GetTeamByID per team in List()

[arreyder]

Summary

Remove per-team GetTeamByID re-fetch in team List(). ListTeams already returns full *github.Team objects with all fields needed for resource creation.

Problem

After listing teams, each was individually re-fetched:

teams, resp, err := o.client.Teams.ListTeams(ctx, orgName, listOpts)
for _, team := range teams {
    fullTeam, resp, err := o.client.Teams.GetTeamByID(ctx, orgID, team.GetID())  // ← N+1

An org with 50 teams = 51 API calls instead of 1.

Validation

• ListTeams returns []*github.Team — same type as GetTeamByID
• GitHub's List Teams API includes members_count, repos_count, name, id, url, organization
• teamResource() uses GetMembersCount(), GetReposCount(), GetOrganization().GetID(), GetName(), GetID(), GetURL() — all populated by list response
• GetOrganization() returns zero-value if nil (safe with nil-safe getters)
• Parent resource ID constructed from orgID already in scope

Impact

Part of 7.9k requests/12h to api.github.com. This is one of several N+1 patterns (user GetByID, org membership are also tracked in OPS-440).

Test plan

• go build ./... passes
• Verify GitHub API request volume decreases

🤖 Generated with Claude Code

[linear]

OPS-440 baton-github: Fix N+1 queries in user/team/org listing + token refresh client churn (7.9k req/12h)

[github-actions]

Claude finished @arreyder's task in 56s —— View job

---

PR Review: OPS-440 — Fix N+1 GetTeamByID per team in List()

• Gather context and read changed files
• Analyze changes and post review

Review

Verdict: Approve ✅

This is a clean, correct optimization. The change removes an N+1 query pattern where each team returned by ListTeams was individually re-fetched via GetTeamByID.

Why this is safe:

• Both ListTeams and GetTeamByID return []*github.Team / *github.Team — the exact same type
• teamResource() uses GetMembersCount(), GetReposCount(), GetOrganization().GetID(), GetName(), GetID(), GetURL() — all standard fields included in the GitHub List Teams API response
• The go-github library's nil-safe getters (GetOrganization(), GetID(), etc.) handle any absent fields gracefully by returning zero values

Impact: For an org with N teams, this reduces API calls from N+1 to 1 per page (up to 100 teams/page with maxPageSize). Directly addresses the 7.9k requests/12h issue mentioned in OPS-440.

One minor note: The removed line had a //nolint:staticcheck comment about migrating from GetTeamByID to GetTeamBySlug. Since the call is now gone entirely, that TODO is resolved as a side effect.

No issues found. LGTM.

[github-actions]

Claude finished @arreyder's task in 1m 20s —— View job

---

PR Review: OPS-440 — Fix N+1 GetTeamByID per team in List()

• Gather context and read changed files
• Check existing comments for already-identified issues
• Review changes against criteria
• Post results

Blocking Issues: 0 | Suggestions: 0

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

---

Verdict: Approve ✅

Clean, correct optimization. Reviewed against all applicable criteria:

• R8 (Pagination): Pagination logic is unchanged and correct — nextPage from parseResp flows through bag.NextToken.
• Backward compatibility: teamResource signature change uses variadic orgID ...int64, so existing callers in org_role.go:241, repository.go:264, and team_test.go:31 continue to work without modification.
• orgID fallback is safe: When ListTeams doesn't populate the Organization field, GetOrganization().GetID() returns 0 (nil-safe getter), and the explicit orgID fallback kicks in. The Grants method reads this value from the team profile to call GetByID — so it's important this is correct, and it is.
• No breaking changes (B1-B8): Resource IDs, entitlement slugs, and parent hierarchy are all unchanged.
• Side benefit: Removes the //nolint:staticcheck TODO about migrating GetTeamByID → GetTeamBySlug, since the call is eliminated entirely.

[github-actions]

No blocking issues found.

[github-actions]

Claude finished @arreyder's task in 1m 10s —— View job

---

PR Review: OPS-440 — Fix N+1 GetTeamByID per team in List()

• Gather context and read changed files
• Analyze changes for correctness, security, and performance
• Post review

Verdict: Approve ✅

This is a clean, well-reasoned optimization that eliminates an N+1 query pattern in team listing.

---

Changes reviewed

pkg/connector/team.go (+10/-8):

1. N+1 removal (lines 111-114): The per-team GetTeamByID re-fetch inside the for loop is removed. ListTeams already returns []*github.Team — the same type — with all fields that teamResource() needs (GetMembersCount(), GetReposCount(), GetOrganization(), GetName(), GetID(), GetURL()). For an org with N teams, this reduces API calls from N+1 to 1 per page. Correct.

2. teamResource signature change (line 34): Added variadic orgID ...int64 parameter. This is backward-compatible — existing callers at org_role.go:241, repository.go:264, and team_test.go:31 pass only 2 args and continue to work unchanged.

3. orgID fallback logic (lines 35-38): When ListTeams doesn't populate the Organization field, GetOrganization().GetID() returns 0 via nil-safe getters, and the explicit orgID fallback kicks in. This is important because the Grants method at line 167 reads orgID from the team profile to call Organizations.GetByID. Without this fallback, orgID=0 would cause grant calculation to fail. Well handled.

test/mocks/github.go (+0/-1):

4. Removed //nolint:gosec directive (line 1): The directive was unused — no gosec findings in this file. Clean removal.

No issues found

• Pagination logic is untouched and correct — nextPage from parseResp flows through bag.NextToken
• Resource IDs, entitlement slugs, and parent hierarchy are unchanged (no breaking changes)
• The removed //nolint:staticcheck TODO about GetTeamByID → GetTeamBySlug is resolved as a side effect since the call is eliminated entirely

---