Add direct-collaborators-only config flag for repo grant sync perf

[btipling]

Summary

• Adds opt-in --direct-collaborators-only config flag (env: BATON_DIRECT_COLLABORATORS_ONLY)
• When enabled, switches ListCollaborators from Affiliation: "all" to "direct"
• Team-based repo access is discovered via existing ListTeams + GrantExpandable expansion path
• For large orgs (e.g. 5K+ repos, 6K+ users), this can reduce ListCollaborators pagination by ~97%
• Default off — no behavior change unless explicitly enabled

Expected data impact when enabled

• Users whose only repo access is via team membership are excluded from ListCollaborators. Their grants are produced by the SDK's grant expansion from team entitlements instead.
• Org base permission: if a member's only repo access comes from the org's default base permission (not direct assignment, not via any team), it is unclear whether GitHub's "direct" filter includes them. This needs validation before enabling in production.
• Grant source annotations differ: team-expanded grants carry source annotations showing inheritance from team membership, whereas "all" grants appear as direct.

Test plan

• Verify build and tests pass
• Test with --direct-collaborators-only=false (default) — behavior unchanged
• Test with --direct-collaborators-only=true on a small org — compare grant output
• Validate whether Affiliation: "direct" includes org base permission users
• Compare sync duration with flag on vs off for a large org

🤖 Generated with Claude Code

[github-actions]

Claude finished @btipling's task in 1m 34s —— View job

---

PR Review: Add direct-collaborators-only config flag

• Gather context and read changed files
• Analyze PR diff against origin/main
• Review code for correctness, security, and best practices
• Post review feedback

---

Overall Assessment

Clean, well-scoped change. The implementation is minimal, correctly threaded through all layers (config definition, generated struct, connector, builder, and tests), and defaults to off — no behavior change for existing users. The grant expansion path via GrantExpandable on team grants at repository.go:286 already ensures team-based access is captured, so the claim in the PR description holds.

No blocking issues found. A few observations and suggestions below.

Code Review

Correctness - Looks good

• The flag is properly wired: config.go -> conf.gen.go (regenerated) -> connector.go (both PAT and App paths) -> repositoryBuilder() -> Grants() method. No gaps in the plumbing.
• The affiliation toggle at repository.go:172-175 is straightforward and correct.
• Test updated at repository_test.go:28 to pass the new parameter. Test passes false which exercises the default (unchanged) behavior.

Config field groups - Looks good

• directCollaboratorsOnly is correctly added to both GithubPersonalAccessTokenGroup and GithubAppGroup field groups (config.go:100,107), so it's available regardless of auth method.

Generated code - Looks good

• conf.gen.go shows field reordering in the struct plus the new DirectCollaboratorsOnly field. This is consistent with the //go:generate go run ./gen directive — the reordering is just an artifact of the generator and doesn't affect behavior since mapstructure tags drive deserialization.

Suggestions (non-blocking)

1. Missing test for directCollaboratorsOnly=true: The existing test only passes false. Consider adding a test case that passes true and verifies that the ListCollaborators call uses Affiliation: "direct". This would exercise the new code path and catch regressions. The mock server in mocks.NewMockGitHub() could validate the query parameter.

2. repositoryBuilder parameter list is growing (repository.go:435): With 4 parameters now (client, orgCache, omitArchived, directCollaborators), consider whether an options struct pattern would be cleaner for future additions:

   type repositoryBuilderOpts struct {
       omitArchivedRepositories bool
       directCollaboratorsOnly  bool
   }

   Not necessary today, but worth noting if more boolean flags are expected.

3. PR description callout is important: The PR correctly flags that it's unclear whether Affiliation: "direct" includes users whose only access comes from the org's default base permission. This is a critical thing to validate before enabling in production. The GitHub API docs define "direct" as explicitly granted access, which would likely exclude base-permission-only users. This should be confirmed with testing.

Summary

The implementation is clean and correct. The flag is properly opt-in with safe defaults. The main risk isn't in the code but in the semantics of GitHub's "direct" affiliation filter — specifically whether org base permission users are included. That's correctly called out in the PR description as needing validation.

LGTM with the suggestion to add a directCollaboratorsOnly=true test case.

[github-actions]

Claude finished @btipling's task in 2m 8s —— View job

---

PR Review: Add direct-collaborators-only config flag

• Gather context and read changed files
• Analyze PR diff against origin/main
• Review code for correctness, security, and best practices
• Post review feedback

---

Overall Assessment

This PR contains two well-scoped changes across 2 commits. Both are clean, correctly wired through all layers, and default to safe behavior. No blocking issues found.

---

Commit 1: f913fd8 — Direct collaborators config flag

Correctness — Looks good

The flag is properly plumbed end-to-end:

• config.go:62-69 — field definition with clear description
• config.go:100,107 — added to both PAT and App auth groups
• conf.gen.go:16 — generated struct field
• connector.go:106,312,393 — stored on the GitHub struct, set in both newWithGithubPAT and newWithGithubApp
• repository.go:64,172-175 — the actual toggle logic

The affiliation switch at repository.go:172-175 is straightforward:

affiliation := "all"
if o.directCollaboratorsOnly {
    affiliation = "direct"
}

Team-based repo access is correctly covered by the GrantExpandable annotations at repository.go:290-296, which expand team repo grants into individual user grants via the SDK's grant expansion. So when directCollaboratorsOnly=true excludes team-access users from ListCollaborators, those grants are still produced through the expansion path.

Suggestion (non-blocking): The test at repository_test.go:28 only exercises directCollaboratorsOnly=false. A test with true that verifies the ListCollaborators call uses Affiliation: "direct" (perhaps by checking the mock server received the expected query parameter) would give confidence the new code path works correctly.

---

Commit 2: a0096a0 — Fix N+1 API calls for teams

Correctness — Looks good

This eliminates per-team GetTeamByID calls during team List(). Previously:

// Old: N+1 pattern
for _, team := range teams {
    fullTeam, resp, err := o.client.Teams.GetTeamByID(ctx, orgID, team.GetID())
    tr, err := teamResource(fullTeam, ...)
}

Now:

// New: direct use
for _, team := range teams {
    tr, err := teamResource(team, orgID, ...)
}

The teamResource() signature change (team.go:34) adds an explicit orgID int64 parameter because ListTeams() doesn't populate team.GetOrganization(). All 4 call sites are correctly updated:

Call site	orgID source	Correct?
team.go:113	parseResourceToGitHub(parentID)	Yes — parentID is the org
repository.go:282	parseResourceToGitHub(resource.ParentResourceId)	Yes — repo's parent is org
org_role.go:264	parseResourceToGitHub(resource.ParentResourceId)	Yes — role's parent is org
team_test.go:31	githubOrganization.GetID()	Yes — direct from test fixture

Data tradeoff: members_count and repos_count will now be 0 for teams synced via ListTeams(). I verified these fields are only stored in the team profile (team.go:38-39) and are not consumed by any other connector code — they're purely informational for the ConductorOne UI. The comments at team.go:36-37 and team.go:110-112 properly document this tradeoff. For orgs with many teams, eliminating the N+1 calls is a significant improvement.

---

Key risk to validate before enabling

As the PR description correctly flags: it is unclear whether GitHub's Affiliation: "direct" filter includes users whose only repo access comes from the org's default base permission. The GitHub API docs define "direct" as users added explicitly — which likely excludes base-permission-only users. This should be confirmed with testing before enabling in production, as those users' grants would not be produced through the team expansion path either.

---

Summary

Both changes are clean and correctly implemented. LGTM with the caveat that the org base permission behavior should be validated before enabling the flag in production, and a directCollaboratorsOnly=true test case would strengthen the coverage.

[github-actions]

Claude finished @btipling's task in 2m 25s —— View job

---

PR Review: Add direct-collaborators-only config flag for repo grant sync perf

• Gather context and read changed files
• Analyze PR diff against origin/main
• Review code for correctness, security, and best practices
• Post review feedback

---

Overall Assessment

Three well-structured commits: (1) the config flag + affiliation toggle, (2) N+1 fix for teams, (3) org→repo grant expansion to cover base permission users. All three are clean, correctly plumbed, and default to safe behavior. No blocking issues.

---

Commit 1: f913fd8 — Direct collaborators config flag

Correctness — Looks good

The flag is correctly wired end-to-end: config.go → conf.gen.go → connector.go (both PAT and App paths) → repositoryBuilder() → Grants(). The affiliation toggle at repository.go:209-212 is straightforward.

---

Commit 2: a0096a0 — Fix N+1 API calls for teams

Correctness — Looks good

Eliminates per-team GetTeamByID calls by using ListTeams() data directly. The orgID parameter is now passed explicitly to teamResource() since ListTeams() doesn't populate the organization object. All 4 call sites are updated correctly with the right orgID source. The members_count/repos_count tradeoff is properly documented in comments.

---

Commit 3: 055c205 — Org→repo grant expansion for base permission

Correctness — Looks good, with one observation

This is the most substantial change. When directCollaboratorsOnly=true, users whose only repo access comes from the org's default base permission won't appear in ListCollaborators(Affiliation: "direct"). This commit closes that gap by creating expandable grants that map org membership to repo access.

The logic:

• Org admins → all repo access levels (pull through admin) via expandable grants referencing the org:<id>:admin entitlement
• Org members → repo access levels based on default_repository_permission via expandable grants referencing the org:<id>:member entitlement

The entitlement ID format (<type>:<id>:<slug>) matches what org.go:163-178 produces, so the SDK's grant expansion will correctly resolve these. Session caching via getOrgBasePermission avoids redundant Organizations.Get calls across repos in the same org.

The orgBasePermissionToRepoPermissions mapping is correct per GitHub's permission model:

• "admin" → all 5 levels ✓
• "write" → pull, triage, push ✓ (maintain is not included because GitHub's "write" base permission doesn't grant it)
• "read" → pull ✓
• "none" / default → nil ✓

Suggestion (non-blocking): repository.go:179 — The error from getOrgBasePermission is logged at Debug level, but a failure here means users whose only repo access is via org base permission will be silently dropped from this sync. Consider Warn instead of Debug since this directly affects data completeness:

l.Warn("failed to fetch org base permission, skipping org expansion", zap.Error(err))

Fix this →

---

Tests

The existing test at repository_test.go:28 and team_test.go:28 are updated to pass the new parameters, exercising the default (false) behavior. As previous reviews noted, a test case with directCollaboratorsOnly=true that verifies the ListCollaborators call uses Affiliation: "direct" and that org expansion grants are created would strengthen coverage.

---

Error prefix inconsistency (minor nit)

The new code in getOrgBasePermission uses "baton-github:" as an error prefix (repository.go:487,504), while the existing code in this file uses "github-connector:" / "github-connectorv2:". Either convention is fine, but consistency within the file would be slightly cleaner.

---

Summary

Clean, well-scoped implementation across all three commits. The flag is properly opt-in with safe defaults. The N+1 fix is a clear win. The org→repo grant expansion correctly compensates for the gap introduced by Affiliation: "direct". The only actionable suggestion is upgrading the log level from Debug to Warn when getOrgBasePermission fails, since it impacts data completeness.

LGTM.

[github-actions]

Claude finished @btipling's task in 2m 34s —— View job

---

PR Review: Add direct-collaborators-only config flag for repo grant sync perf

• Gather context and read changed files
• Analyze PR diff against origin/main
• Review code for correctness, security, and best practices
• Post review feedback

---

Overall Assessment

Four well-structured commits across a clean, well-scoped change. The flag is properly opt-in with safe defaults, the N+1 fix is a clear win, and the org→repo grant expansion correctly compensates for users that Affiliation: "direct" would exclude. No blocking issues. Two actionable suggestions and a few observations below.

---

Commit-by-Commit Review

Commit 1: f913fd8 — Direct collaborators config flag

Correctness — Looks good

The flag is wired end-to-end with no gaps:

• config.go:62-69 → field definition with clear description
• config.go:100,107 → added to both PAT and App auth groups
• conf.gen.go:16 → generated struct field (DirectCollaboratorsOnly)
• connector.go:106,312,393 → stored on GitHub struct, set in both auth paths
• repository.go:66,209-212 → the actual affiliation toggle

The affiliation switch at repository.go:209-212 is straightforward and correct.

---

Commit 2: a0096a0 — Fix N+1 API calls for teams

Correctness — Looks good

Eliminates per-team GetTeamByID calls by using ListTeams() data directly. The orgID parameter is now passed explicitly to teamResource() since ListTeams() doesn't populate team.GetOrganization(). All 4 call sites are correctly updated:

Call site	orgID source	Correct?
team.go:113	parseResourceToGitHub(parentID)	Yes — parentID is the org
repository.go:319	parseResourceToGitHub(resource.ParentResourceId)	Yes — repo's parent is org
org_role.go:264	parseResourceToGitHub(resource.ParentResourceId)	Yes — role's parent is org
team_test.go:31	githubOrganization.GetID()	Yes — direct from test fixture

The members_count/repos_count tradeoff is properly documented in comments at team.go:36-37. I verified these fields are only stored in the team profile and not consumed by other connector logic — they're purely informational.

---

Commit 3: 055c205 — Org→repo grant expansion for base permission

Correctness — Looks good, with suggestions

This closes the gap where users whose only repo access comes from the org's default base permission won't appear in ListCollaborators(Affiliation: "direct").

Entitlement ID format verified: The manually constructed IDs at repository.go:183,195:

fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleAdmin)   // → "org:<id>:admin"
fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleMember)  // → "org:<id>:member"

match the SDK entitlement format used in org.go:163-178 (NewAssignmentEntitlement(resource, orgRoleMember) / NewPermissionEntitlement(resource, orgRoleAdmin)). The expansion will correctly resolve.

Session caching via getOrgBasePermission (repository.go:484-508) correctly avoids redundant Organizations.Get calls across repos in the same org.

Permission mapping at orgBasePermissionToRepoPermissions (repository.go:512-523) is correct per GitHub's permission model:

• "admin" → all 5 levels ✓
• "write" → pull, triage, push ✓ (maintain is correctly excluded — GitHub's "write" base permission doesn't grant it)
• "read" → pull ✓
• "none" / default → nil ✓

---

Commit 4: 6c99675 — CI fix: remove stale nolint directives, regenerate metadata

The //nolint:staticcheck comments were removed from GetTeamByID calls at repository.go:398,456. These calls are still present in the Grant/Revoke paths (where you need to look up the team by ID to get the slug), so the removal is just cleaning up stale directives — the calls themselves are still necessary.

Note on baton_capabilities.json: The api-key resource type was removed during regeneration. This type is conditionally registered (only when syncSecrets=true, see connector.go:124-126), so the generator likely only captures unconditionally registered types. Worth confirming this is expected behavior — if the capabilities file is consumed by ConductorOne to understand what the connector can do, the absence of api-key could affect secret sync workflows.

---

Suggestions

1. (Actionable) Upgrade log level for org base permission failure — repository.go:179

The current l.Debug(...) means a failure to fetch the org's base permission will silently drop grants for users whose only repo access comes from the org default permission. Since this directly impacts data completeness, Warn is more appropriate:

l.Warn("failed to fetch org base permission, skipping org expansion", zap.Error(err))

Fix this →

2. (Actionable) Missing test for directCollaboratorsOnly=true

The existing test at repository_test.go:28 only exercises directCollaboratorsOnly=false. A test case with true that verifies the ListCollaborators call uses Affiliation: "direct" and that org expansion grants are produced would strengthen coverage for the new code path. The mock server in mocks.NewMockGitHub() could validate the query parameter.

---

Key Risk (reiterated from PR description)

The PR correctly flags that it's unclear whether GitHub's Affiliation: "direct" includes users whose only access is via the org's base permission. The GitHub API docs define "direct" as users added explicitly, which would likely exclude base-permission-only users. Commit 3 addresses this gap via the org→repo expansion, but this should still be validated with real GitHub data before enabling in production — particularly to confirm the expansion produces equivalent grants to what Affiliation: "all" would have returned.

---

Summary

Clean implementation across all four commits. The flag is properly opt-in with safe defaults. The N+1 fix is a clear performance win. The org→repo grant expansion correctly compensates for the semantic gap introduced by Affiliation: "direct". The entitlement ID formats match. The two actionable suggestions (log level upgrade, additional test) are non-blocking.

LGTM.

[github-actions]

Claude finished @btipling's task in 2m 7s —— View job

---

PR Review: Add direct-collaborators-only config flag for repo grant sync perf

• Gather context and read changed files
• Analyze PR diff against origin/main
• Review code for correctness, security, and best practices
• Post review feedback

---

Overall Assessment

Five well-structured commits that add an opt-in performance flag, fix N+1 API calls for teams, add org→repo grant expansion, remove stale nolint directives, and restrict expansion to user resource types. The implementation is clean, correctly wired through all layers, and defaults to safe behavior (off). No blocking issues. Two actionable suggestions and observations below.

---

Commit-by-Commit Review

Commit 1: f913fd8 — Direct collaborators config flag

Correctness — Looks good

The flag is wired end-to-end with no gaps:

• config.go:62-69 → field definition with clear description
• config.go:100,107 → added to both PAT and App auth field groups
• conf.gen.go:16 → generated struct field (DirectCollaboratorsOnly)
• connector.go:106,312,393 → stored on GitHub struct, set in both auth paths
• repository.go:66,211-213 → the actual affiliation toggle

The affiliation switch is straightforward and correct.

---

Commit 2: a0096a0 — Fix N+1 API calls for teams

Correctness — Looks good

Eliminates per-team GetTeamByID calls by using ListTeams() data directly. The orgID parameter is now passed explicitly to teamResource() since ListTeams() doesn't populate team.GetOrganization(). All 4 call sites are correctly updated with the right orgID source:

Call site	orgID source	Correct?
team.go:113	parseResourceToGitHub(parentID)	Yes — parentID is the org
repository.go:321	parseResourceToGitHub(resource.ParentResourceId)	Yes — repo's parent is org
org_role.go:264	parseResourceToGitHub(resource.ParentResourceId)	Yes — role's parent is org
team_test.go:31	githubOrganization.GetID()	Yes — direct from test fixture

The members_count/repos_count tradeoff is properly documented in comments at team.go:36-37. These are only stored in the team profile and not consumed elsewhere — purely informational.

---

Commit 3: 055c205 — Org→repo grant expansion for base permission

Correctness — Looks good, with suggestions

This closes the gap where users whose only repo access comes from the org's default base permission won't appear in ListCollaborators(Affiliation: "direct").

Entitlement ID format verified. The manually constructed IDs at repository.go:183,196:

fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleAdmin)   // → "org:<id>:admin"
fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleMember)  // → "org:<id>:member"

match the SDK's NewEntitlementID format (resourceType:resourceID:permission) used by NewAssignmentEntitlement at org.go:163 and NewPermissionEntitlement at org.go:171. The expansion will correctly resolve.

Session caching via getOrgBasePermission (repository.go:484-509) correctly avoids redundant Organizations.Get calls across repos in the same org.

Permission mapping at orgBasePermissionToRepoPermissions (repository.go:514-523) is correct per GitHub's permission model:

• "admin" → all 5 levels ✓
• "write" → pull, triage, push ✓ (maintain correctly excluded — GitHub's "write" base permission doesn't grant it)
• "read" → pull ✓
• "none" / default → nil ✓

---

Commit 4: 6c99675 — CI fix: remove stale nolint directives, regenerate metadata

The //nolint:staticcheck comments were removed from GetTeamByID calls at repository.go:400,459 in the Grant/Revoke paths. These calls are still present (needed to look up team slug for provisioning), the directives were just stale.

Note on baton_capabilities.json: The api-key resource type was removed during regeneration. This type is conditionally registered (only when syncSecrets=true, see connector.go:124-126), so the generator only captures unconditionally registered types. Worth confirming this is expected — if the capabilities file is consumed by ConductorOne to understand what the connector can do, the absence of api-key could affect secret sync workflows.

---

Commit 5: 4b7297b — Restrict org→repo grant expansion to user resource type

Correctness — Looks good

Adds ResourceTypeIds: []string{resourceTypeUser.Id} to the expandable grant annotations at repository.go:189,202. This correctly constrains the SDK to only expand org membership grants into user principals, preventing unintended expansion to teams or other resource types.

---

Suggestions

1. (Actionable) Upgrade log level for org base permission failure — repository.go:179

The current l.Debug(...) means a failure to fetch the org's base permission will silently skip grant expansion for users whose only repo access comes from the org default permission. Since this directly impacts data completeness when the flag is enabled, Warn is more appropriate:

l.Warn("failed to fetch org base permission, skipping org expansion", zap.Error(err))

Fix this →

2. (Actionable) Missing test for directCollaboratorsOnly=true

The existing test at repository_test.go:28 only exercises directCollaboratorsOnly=false. A test case with true that verifies the ListCollaborators call uses Affiliation: "direct" and that org expansion grants are produced would strengthen coverage for the new code path.

---

Key Risk (reiterated from PR description)

The PR correctly flags that it's unclear whether GitHub's Affiliation: "direct" includes users whose only access is via the org's base permission. The GitHub API docs define "direct" as users added explicitly, which would likely exclude base-permission-only users. Commit 3 addresses this gap via the org→repo expansion, but this should still be validated with real GitHub data before enabling in production — particularly to confirm the expansion produces equivalent grants to what Affiliation: "all" would have returned.

---

Summary

Clean implementation across all five commits. The flag is properly opt-in with safe defaults. The N+1 fix is a clear performance win. The org→repo grant expansion correctly compensates for the semantic gap introduced by Affiliation: "direct", and the ResourceTypeIds filter in commit 5 properly scopes the expansion. Entitlement ID formats are verified to match the SDK's NewEntitlementID output. The two actionable suggestions (log level upgrade, additional test) are non-blocking.

LGTM.

[github-actions]

Claude finished @btipling's task in 2m 12s —— View job

---

PR Review: Add direct-collaborators-only config flag for repo grant sync perf

• Gather context and read changed files
• Analyze PR diff against origin/main
• Review code for correctness, security, and best practices
• Post review feedback

---

Overall Assessment

Clean, well-scoped PR across 6 commits. The flag is properly opt-in with safe defaults — when disabled, behavior is completely unchanged (no affiliation change, no org expansion, no team N+1 skip). All layers are correctly wired. No blocking issues found. Two actionable suggestions below.

---

Commit-by-Commit Review

1. f913fd8 — Config flag + affiliation toggle

The flag is wired end-to-end with no gaps:

• config.go:62-69 → field definition
• config.go:100,107 → added to both PAT and App auth groups
• conf.gen.go:16 → generated struct field
• connector.go:106,312,393 → stored on GitHub struct, set in both auth paths
• repository.go:66,211-213 → the affiliation toggle

Correct.

2. a0096a0 — Fix N+1 API calls for teams

Eliminates per-team GetTeamByID calls. The orgID parameter is now passed explicitly to teamResource() since ListTeams() doesn't populate team.GetOrganization(). All 4 call sites updated correctly. (Later gated behind the flag in commit 6.)

3. 055c205 — Org→repo grant expansion for base permission

This is the most substantial change. When the flag is enabled, users whose only repo access comes from the org's default base permission won't appear in ListCollaborators(Affiliation: "direct"). This commit closes that gap via expandable grants.

Entitlement ID format verified. The manually constructed IDs at repository.go:183,196:

fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleAdmin)   // → "org:<id>:admin"
fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleMember)  // → "org:<id>:member"

match the SDK's NewEntitlementID format used by NewAssignmentEntitlement at org.go:163 and NewPermissionEntitlement at org.go:171. The expansion will correctly resolve.

Session caching via getOrgBasePermission (repository.go:484-509) correctly avoids redundant Organizations.Get calls across repos in the same org.

Permission mapping at orgBasePermissionToRepoPermissions (repository.go:514-523) is correct per GitHub's permission model:

• "admin" → all 5 levels ✓
• "write" → pull, triage, push ✓ (maintain correctly excluded)
• "read" → pull ✓
• "none" / default → nil ✓

4. 6c99675 — CI fix: remove stale nolint directives, regenerate metadata

Housekeeping commit. Note: the api-key resource type was removed from baton_capabilities.json during regeneration. This type is conditionally registered (only when syncSecrets=true, see connector.go:124-126), so the generator only captures unconditionally registered types. Worth confirming this is expected if the capabilities file is consumed by ConductorOne.

5. 4b7297b — Restrict org→repo grant expansion to user resource type

Adds ResourceTypeIds: []string{resourceTypeUser.Id} to the expandable grant annotations at repository.go:189,202. Correctly constrains the SDK to only expand into user principals.

6. 4839a26 — Gate team N+1 optimization behind flag

Restores GetTeamByID for the default path so members_count and repos_count remain populated. The N+1 skip only applies when directCollaboratorsOnly=true. This is the right tradeoff — the flag becomes a single toggle for all three optimizations, and default behavior is 100% preserved.

---

Suggestions

1. Upgrade log level for org base permission failure — repository.go:179

The current l.Debug(...) means a failure to fetch the org's base permission will silently skip grant expansion for users whose only repo access comes from the org default permission. Since this directly impacts data completeness when the flag is enabled, Warn is more appropriate:

l.Warn("failed to fetch org base permission, skipping org expansion", zap.Error(err))

Fix this →

2. Missing test for directCollaboratorsOnly=true

The existing tests at repository_test.go:28 and team_test.go:28 only exercise the default (false) behavior. A test case with true that verifies:

• ListCollaborators uses Affiliation: "direct"
• Org expansion grants are produced
• Team List() skips GetTeamByID

would exercise all three new code paths. The mock server could validate the query parameters.

---

Key Risk (reiterated from PR description)

The GitHub API docs define "direct" as users added explicitly, which would likely exclude base-permission-only users. Commit 3 addresses this via org→repo expansion, but this should be validated with real GitHub data before enabling in production — particularly to confirm the expansion produces equivalent grants.

---

Summary

Clean implementation across all six commits. The flag is properly opt-in with safe defaults. When enabled, it bundles three optimizations: direct-only collaborator filtering, org→repo grant expansion, and team detail skip. The entitlement ID formats are verified, session caching is correct, and the permission mapping matches GitHub's model. The two suggestions (log level upgrade, additional test) are non-blocking.

LGTM.

[github-actions]

Claude finished @btipling's task in 2m 5s —— View job

---

PR Review: Add direct-collaborators-only config flag for repo grant sync perf

• Gather context and read changed files
• Analyze PR diff against origin/main
• Review code for correctness, security, and best practices
• Post review feedback

---

Overall Assessment

Clean, well-scoped PR across 7 commits. The flag is properly opt-in with safe defaults — when disabled, behavior is completely unchanged. All layers are correctly wired end-to-end. No blocking issues found. Two actionable suggestions below.

---

Commit-by-Commit Review

1. f913fd8 — Config flag + affiliation toggle

The flag is wired end-to-end with no gaps:

• config.go:62-69 → field definition with clear description
• config.go:100,107 → added to both PAT and App auth groups
• conf.gen.go:16 → generated struct field (DirectCollaboratorsOnly)
• connector.go:106,312,393 → stored on GitHub struct, set in both auth paths
• repository.go:66,211-213 → the affiliation toggle

Correct.

2. a0096a0 — Fix N+1 API calls for teams

Eliminates per-team GetTeamByID calls by using ListTeams() data directly. The orgID parameter is now passed explicitly to teamResource() since ListTeams() doesn't populate team.GetOrganization(). All 4 call sites updated correctly:

Call site	orgID source	Correct?
team.go:113	parseResourceToGitHub(parentID)	✓ parentID is the org
repository.go:319	parseResourceToGitHub(resource.ParentResourceId)	✓ repo's parent is org
org_role.go:264	parseResourceToGitHub(resource.ParentResourceId)	✓ role's parent is org
team_test.go:31	githubOrganization.GetID()	✓ direct from test fixture

The members_count/repos_count tradeoff is properly documented in comments at team.go:36-37. These fields are only stored in the team profile and not consumed elsewhere — purely informational.

3. 055c205 — Org→repo grant expansion for base permission

This is the most substantial change. When directCollaboratorsOnly=true, users whose only repo access comes from the org's default base permission won't appear in ListCollaborators(Affiliation: "direct"). This commit closes that gap via expandable grants.

Entitlement ID format verified. The manually constructed IDs at repository.go:183,196:

fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleAdmin)   // → "org:<id>:admin"
fmt.Sprintf("%s:%s:%s", resourceTypeOrg.Id, orgResID, orgRoleMember)  // → "org:<id>:member"

match the SDK's NewEntitlementID format at vendor/.../entitlement.go:47-48:

func NewEntitlementID(resource *v2.Resource, permission string) string {
    return fmt.Sprintf("%s:%s:%s", resource.GetId().GetResourceType(), resource.GetId().GetResource(), permission)
}

and the entitlements created in org.go:163 (NewAssignmentEntitlement(resource, orgRoleMember)) and org.go:171 (NewPermissionEntitlement(resource, orgRoleAdmin)). The expansion will correctly resolve.

Session caching via getOrgBasePermission (repository.go:484-509) correctly avoids redundant Organizations.Get calls across repos in the same org.

Permission mapping at orgBasePermissionToRepoPermissions (repository.go:514-523) is correct per GitHub's permission model:

• "admin" → all 5 levels ✓
• "write" → pull, triage, push ✓ (maintain correctly excluded — GitHub's "write" base permission doesn't grant it)
• "read" → pull ✓
• "none" / default → nil ✓

4. 6c99675 — CI fix: remove stale nolint directives, regenerate metadata

Housekeeping. The api-key resource type was removed from baton_capabilities.json during regeneration. This type is conditionally registered (only when syncSecrets=true, see connector.go:124-126), so the generator only captures unconditionally registered types. Worth confirming this is expected — if the capabilities file is consumed by ConductorOne to understand what the connector can do, the absence of api-key could affect secret sync workflows.

5. 4b7297b — Restrict org→repo grant expansion to user resource type

Adds ResourceTypeIds: []string{resourceTypeUser.Id} to the expandable grant annotations at repository.go:189,202. Correctly constrains the SDK to only expand into user principals.

6. 4839a26 — Gate team N+1 optimization behind flag

Restores GetTeamByID for the default path so members_count and repos_count remain populated. The N+1 skip only applies when directCollaboratorsOnly=true. This is the right tradeoff — default behavior is 100% preserved, and the flag becomes a single toggle for all three optimizations.

7. 2738712 — CI fix: add nolintlint to GetTeamByID nolint directives

Adds nolintlint to the //nolint:staticcheck directives at repository.go:400,458 and team.go:113. These are in the Grant/Revoke paths (repository) and List path (team) where GetTeamByID is still called and necessary.

---

Suggestions

1. (Actionable) Upgrade log level for org base permission failure — repository.go:179

The current l.Debug(...) means a failure to fetch the org's base permission will silently skip grant expansion for users whose only repo access comes from the org default permission. Since this directly impacts data completeness when the flag is enabled, Warn is more appropriate:

l.Warn("failed to fetch org base permission, skipping org expansion", zap.Error(err))

Fix this →

2. (Actionable) Missing test for directCollaboratorsOnly=true

The existing tests at repository_test.go:28 and team_test.go:28 only exercise the default (false) behavior. A test case with true that verifies:

• ListCollaborators uses Affiliation: "direct"
• Org expansion grants are produced
• Team List() skips GetTeamByID

would exercise all three new code paths. The mock server could validate the query parameters.

---

Key Risk (reiterated from PR description)

The GitHub API docs define "direct" as users added explicitly, which would likely exclude base-permission-only users. Commit 3 addresses this via org→repo expansion, but this should be validated with real GitHub data before enabling in production — particularly to confirm the expansion produces equivalent grants to what Affiliation: "all" would have returned.

---

Summary

Clean implementation across all 7 commits. The flag is properly opt-in with safe defaults. When enabled, it bundles three optimizations: direct-only collaborator filtering, org→repo grant expansion, and team detail skip. The entitlement ID formats are verified to match the SDK's NewEntitlementID output. Session caching is correct. Permission mapping matches GitHub's model. The two suggestions (log level upgrade, additional test) are non-blocking.

LGTM.