fix: gracefully skip enterprise features when using GitHub App auth

[madison-c-evans]

Summary

• The consumed-licenses API (/enterprises/{slug}/consumed-licenses) is not accessible via GitHub App installation tokens — it requires a PAT with enterprise admin scope.
• Previously, using --enterprises with GitHub App auth caused the sync to fail hard when the enterprise role builder tried to call this API.
• Now enterpriseRoleBuilder.fillCache() logs a debug message and returns an empty cache (zero enterprise role resources) instead of aborting.
• Updated the validateAppCredentials() debug message to use consistent, actionable wording across all three callsites.

What changed

• pkg/connector/enterprise_role.go — fillCache() catches the consumed-licenses error, logs a debug-level message, and returns nil (empty cache) so the sync continues.
• pkg/connector/connector.go — validateAppCredentials() debug log updated to match the same wording.

Test plan

• Built and ran go test ./... — all pass
• Ran a full sync with GitHub App auth + --enterprises mevans-test + --log-level debug — sync completed successfully, debug messages appeared at validate time and at enterprise role list time, all other resource types synced normally

🤖 Generated with Claude Code

[github-actions]

Claude finished @madison-c-evans's task in 2m 1s —— View job

---

PR Review: Graceful Enterprise Degradation for GitHub App Auth

• Gather context and read changed files
• Review code changes against origin/main
• Post review feedback

---

Overall: The core idea is sound — fillCache() should not hard-fail the entire sync when using GitHub App auth with --enterprises. The debug message improvements are welcome too. A few issues to address:

Issues Found

1. Overly broad error catching in fillCache() (medium) — The catch at enterprise_role.go:63-69 swallows all errors (network failures, rate limits, 500s, context cancellation), not just auth/permission errors. This means transient failures silently produce zero enterprise roles. Consider narrowing to 403/404 errors, or at minimum only applying the graceful skip on page == 1 so mid-pagination failures don't silently drop data. See inline comment →

2. Inconsistent debug message (minor) — The PAT validation path at connector.go:215 still uses the old wording. The PR description says "consistent, actionable wording across all three callsites" but only the App validation path was updated. See inline comment →

3. Log level (minor) — Debug makes sense for the validation pre-check, but when fillCache() silently returns an empty cache at sync time for a user who explicitly passed --enterprises, Warn would be more appropriate to ensure visibility.

What looks good

• The user.go path (fetchAndStoreEnterpriseSAML) already has graceful degradation at the call site (user.go:157-160), so this PR correctly identifies fillCache() as the remaining hard-failure path.
• Small, focused change with clear intent.

[github-actions]

Bug (medium): Catches all errors, not just auth errors.

This catches every error — network timeouts, rate limits, 500s, context cancellation — and treats them all as "GitHub App can't access this endpoint." A transient failure mid-sync will silently produce zero (or partial) enterprise role data with only a debug log.

Two concerns:

1. Overly broad catch: Consider checking for 403/404 status codes to distinguish auth failures from transient errors. The ListEnterpriseConsumedLicenses client wraps the HTTP status in the error string, so you could match on that, or propagate the status code from the client.

2. Mid-pagination data loss: If pages 1–3 succeed but page 4 fails, this returns nil with a partial cache. Enterprise roles from the first 3 pages will appear in sync results while the rest silently vanish. Consider only applying the graceful skip on the first page (where page == 1), and propagating errors on subsequent pages.

Suggestion (minor): Log level. When a user explicitly passes --enterprises, silently returning zero enterprise roles at Debug level could be confusing. Consider Warn here (vs the validation path where Debug makes sense as a pre-check).

[github-actions]

Nit: The PAT validation path at line 215 still has the old wording.

The PR description mentions "consistent, actionable wording across all three callsites," but the PAT validation check at connector.go:215 still reads:

"enterprise consumed licenses API is not accessible — enterprise SAML email enrichment and enterprise role sync may fail at sync time"

Should be updated to match this new wording for consistency.

[github-actions]

Claude finished @madison-c-evans's task in 2m 43s —— View job

---

PR Review: Graceful Enterprise Degradation for GitHub App Auth

• Gather context and read changed files
• Review code changes against origin/main
• Post review feedback

---

Overall: The second commit (959be01) addresses the most important concern from the prior review — narrowing the catch to first-page permission errors only. The core approach is correct: fillCache() should not abort the entire sync when GitHub App auth can't access the consumed-licenses API. Two remaining items worth addressing:

Error detection works correctly (verified)

I traced the full error chain:

1. uhttp.BaseHttpClient.Do() maps HTTP 403 → codes.PermissionDenied via GrpcCodeFromHTTPStatus() (wrapper.go:251-252)
2. The gRPC status error is joined via WrapErrorsWithRateLimitInfo and then wrapped with %w in client.go:49
3. errors.As in isPermissionDenied() correctly traverses through fmt.Errorf(%w) → errors.Join → status.Error (Go 1.20+ supports unwrapping errors.Join)

The page == 1 guard correctly prevents mid-pagination data loss — if pages 1–3 succeed but page 4 fails with a permission error, it will propagate as a hard error rather than silently dropping data.

Remaining issues

1. Log level at fillCache() (enterprise_role.go:68) — When a user explicitly passes --enterprises and the sync silently produces zero enterprise roles, l.Debug(...) may be too quiet. The validation pre-check at connector.go:237 correctly uses Debug (it's informational before sync starts), but the fillCache() path is where data is actually skipped. l.Warn(...) would ensure visibility without requiring --log-level debug.

   Fix log level →

2. PAT validation message inconsistency (connector.go:215) — The PAT validation path still has the old wording. The PR description says "consistent, actionable wording across all three callsites" but only the App path was updated. Note: the new wording is App-specific ("GitHub App authentication cannot access..."), so it shouldn't be copied verbatim. The PAT path needs its own actionable message, e.g.:

   "baton-github: enterprise features (--enterprises) require a PAT with the manage_billing:enterprise scope. "
   + "The consumed-licenses API returned an error for this token."
   

   Fix PAT message →

What looks good

• Correct scoping: page == 1 guard prevents silent mid-pagination data loss
• Proper gRPC status detection: isPermissionDenied() correctly uses errors.As with the GRPCStatus() interface, matching the SDK's error wrapping pattern used throughout the codebase (helpers.go:370, connector.go:189/198/208)
• Focused change: Small diff, clear intent, no unnecessary refactoring

Neither issue is a blocker — this is a clear improvement over the hard-failure behavior on main.

[Bencheng21]

The goal is to disable consumed-licenses API using github app authentication,
we still should return error if the connector is configured with PAT, but the pat is not in admin scope.

maybe we should check if appClient is nil.

baton-github/pkg/connector/connector.go

Line 99 in c8b04dd

appClient *github.Client

[madison-c-evans]

Now passes appClient through to enterpriseRoleResourceType and gate the graceful skip on appClient != nil. PAT with insufficient scope will still return a hard error

[github-actions]

Claude finished @madison-c-evans's task in 2m 14s —— View job

---

PR Review: Graceful Enterprise Degradation for GitHub App Auth

• Gather context and read changed files
• Review code changes against origin/main
• Post review feedback

---

Overall: This is a solid, well-scoped fix. The three commits show good iterative refinement in response to review feedback. The core approach — skip enterprise role sync with a log message when GitHub App auth can't access the consumed-licenses API — is correct and well-implemented. One remaining item worth addressing.

Verified: Error detection chain is correct

I traced the full path:

1. customclient.Client embeds uhttp.BaseHttpClient → Do() maps HTTP 403 → codes.PermissionDenied via GrpcCodeFromHTTPStatus() (wrapper.go:252)
2. Error is wrapped with %w in client.go:49, preserving the gRPC status in the error chain
3. isPermissionDenied() uses errors.As to traverse fmt.Errorf(%w) → errors.Join → status.Error, correctly finding the GRPCStatus() interface

Verified: appClient != nil is the right proxy for App auth

• PAT path (newWithAccessToken, connector.go:301): appClient is not set → zero value nil
• App path (newWithGithubApp, connector.go:382): appClient is set to a non-nil client
• This matches Validate() at connector.go:166 which uses the same nil check to branch between PAT and App validation

This directly addresses @Bencheng21's review feedback — PAT auth with insufficient scopes will still hard-fail as expected.

Verified: page == 1 guard prevents silent data loss

If pages 1–3 succeed but page 4 returns a permission error, the condition page == 1 is false, so the error propagates as a hard failure. This correctly prevents mid-pagination data loss.

Verified: Early return from fillCache() is correct for multiple enterprises

The return nil at enterprise_role.go:74 exits the entire function, skipping remaining enterprises. This is correct — the auth token is the same for all enterprises, so if the API is inaccessible for the first, it will be for all of them.

Remaining issue

Log level at fillCache() (enterprise_role.go:69) — When a user explicitly passes --enterprises and fillCache() silently produces zero enterprise roles, l.Debug(...) may be too quiet. The validation pre-check at connector.go:238 correctly uses Debug (it's informational before sync starts), but fillCache() is where data is actually skipped at sync time. l.Warn(...) would ensure visibility without requiring --log-level debug. Both previous reviews flagged this.

Fix log level →

What looks good

• Focused, incremental commits: Each commit addresses a specific review concern — broad → first-page-only → App-auth-only
• Correct reuse of existing patterns: isPermissionDenied() follows the same GRPCStatus() interface pattern used throughout the codebase (helpers.go:370, connector.go:189/198/208)
• PAT validation message updated: The previous review concern about inconsistent wording at connector.go:215 has been addressed — the PAT path now has its own actionable message about needing enterprise admin scope
• No unnecessary refactoring: appClient is threaded through only where needed, and the sentinel check is minimal

Not a blocker — this is a clear improvement over the hard-failure behavior on main.